Evolution-X-Devices/kernel_google_wahoo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

binder: fix thread hangs waiting for proc work

Browse Source 
With fine-grained locking, there is an uncommon case where
a thread can be waiting on the per-proc wait queue while it
has per-thread work to do. If there isn't any proc work
to do, it may never be awakened so the binder transaction
is hung. This CL fixes the issue by checking if the thread
is waiting for the per-proc queue when waking a thread for
per-thread work.

There is another case fixed here that can also delay this
work in which no thread is awakened from the per-proc queue
because another thread is servicing the queue. If that
thread ends up doing per-thread work, it now wakes a thread
waiting on the per-proc queue.

Bug: 35135993
Change-Id: I18a0fc25abd97ad2eaf704630d1e6257a53f2215
Test: Camera CTS test had been a reliable reproducer. Verified fixed.
Signed-off-by: Todd Kjos <tkjos@google.com>
This commit is contained in:
Todd Kjos
2017-02-15 08:01:52 -08:00
committed by Thierry Strudel
parent c9c6590330
commit cfc2155f1b
1 changed files with 34 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
drivers/android/binder.c
View File

@@ -408,6 +408,7 @@ struct binder_thread {
int pid;
int looper; /* only modified by this thread */
bool looper_need_return; /* can be written by other thread */
bool waiting_for_proc_work; /* can be read by other thread */
struct binder_transaction *transaction_stack;
struct binder_worklist todo;
uint32_t return_error; /* Write failed, return error code in read buf */
@@ -1152,6 +1153,10 @@ static void binder_send_failed_reply(struct binder_transaction *t,
binder_pop_transaction(target_thread, t);
target_thread->return_error = error_code;
wake_up_interruptible(&target_thread->wait);
if (READ_ONCE(
target_thread->waiting_for_proc_work))
wake_up_interruptible_all(
&target_thread->proc->wait);
} else {
pr_err("reply failed, target thread, %d:%d, has error code %d already\n",
target_thread->proc->pid,
@@ -2148,6 +2153,15 @@ static void binder_transaction(struct binder_proc *proc,
binder_enqueue_work(tcomplete, &thread->todo, __LINE__);
if (target_wait) {
/*
* Handle rare case where thread work has been added but
* the thread is waiting on the proc workqueue. Make sure
* it wakes up.
*/
if (unlikely(target_thread &&
READ_ONCE(target_thread->waiting_for_proc_work)))
wake_up_interruptible_all(&target_proc->wait);
if (reply || !(t->flags & TF_ONE_WAY))
wake_up_interruptible_sync(target_wait);
else
@@ -2660,13 +2674,6 @@ static void binder_stat_br(struct binder_proc *proc,
}
}
static inline int binder_has_proc_work(struct binder_proc *proc,
struct binder_thread *thread)
{
return !binder_worklist_empty(&proc->todo) ||
READ_ONCE(thread->looper_need_return);
}
static inline int binder_has_thread_work(struct binder_thread *thread)
{
return !binder_worklist_empty(&thread->todo) ||
@@ -2674,6 +2681,13 @@ static inline int binder_has_thread_work(struct binder_thread *thread)
READ_ONCE(thread->looper_need_return);
}
static inline int binder_has_proc_work(struct binder_proc *proc,
struct binder_thread *thread)
{
return !binder_worklist_empty(&proc->todo) ||
binder_has_thread_work(thread);
}
static int binder_thread_read(struct binder_proc *proc,
struct binder_thread **threadp,
binder_uintptr_t binder_buffer, size_t size,
@@ -2720,6 +2734,14 @@ retry:
if (wait_for_proc_work)
atomic_inc(&proc->ready_threads);
else if (!binder_worklist_empty(&proc->todo) &&
atomic_read(&proc->ready_threads)) {
/*
* Need to kick the proc wait queue in case
* it was just unfrozen and has work to do
*/
wake_up_interruptible(&proc->wait);
}
binder_unlock(__func__);
@@ -2739,11 +2761,15 @@ retry:
ret = -EAGAIN;
}
} else {
WRITE_ONCE(thread->waiting_for_proc_work, true);
binder_put_thread(thread);
ret = wait_event_freezable_exclusive(proc->wait, binder_has_proc_work(proc, thread));
*threadp = thread = binder_get_thread(proc);
binder_lock(__func__);
if (!thread)
if (thread)
WRITE_ONCE(thread->waiting_for_proc_work,
false);
else
ret = -EINVAL;
}
} else {