android/android_kernel_asus_sm8350
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "ANDROID: binder: fix sleeping from invalid function caused by RT inheritance"

Browse Source 
This reverts commit aefd2d632e.

This patch introduced a race condition reported by Pete Zhang
<pete.zhang@nxp.com> where a thread already selected to handle a
transaction could be selected again by a different client. The signature
was triggering of the WARN_ON() in binder_enqueue_thread_work_ilocked():

static void
binder_enqueue_thread_work_ilocked(...)
{
	WARN_ON(!list_empty(&thread->waiting_thread_node));
	...
}

which indicates that a thread is unexpectedly enqueued
as a waiting thread when it has been selected and work is
being assigned to it.

There was a 2nd issue introduced by the same patch that could,
at least in theory, cause async transactions to hang.

Bug: 151861772
Change-Id: I0c2bdb4c4a0d57caae1551bcbb4c31a8e09e024b
Signed-off-by: Todd Kjos <tkjos@google.com>
This commit is contained in:
Todd Kjos 2020-03-27 10:02:15 -07:00 committed by Alistair Delva
parent 4061772e28
commit 553f33f495
1 changed files with 9 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
drivers/android/binder.c
View File

@ -2911,50 +2911,19 @@ static bool binder_proc_transaction(struct binder_transaction *t,
struct binder_priority node_prio;
bool oneway = !!(t->flags & TF_ONE_WAY);
bool pending_async = false;
bool retry = false;
BUG_ON(!node);
set_thread_prio:
binder_node_lock(node);
node_prio.prio = node->min_priority;
node_prio.sched_policy = node->sched_policy;
if (thread) {
/*
* Priority must be set outside of lock, but must be
* done before enqueuing the transaction.
*/
binder_transaction_priority(thread->task, t, node_prio,
node->inherit_rt);
}
retry_after_prio_restore:
binder_node_lock(node);
if (oneway) {
BUG_ON(!retry && thread);
BUG_ON(thread);
if (node->has_async_transaction) {
pending_async = true;
} else {
node->has_async_transaction = true;
}
if (thread && pending_async) {
/*
* The node state has changed since we selected
* the thread. Return the thread to the
* waiting_threads list. We have to drop
* the node lock to restore priority so we
* have to re-check the node state.
*/
binder_node_unlock(node);
binder_restore_priority(thread->task,
proc->default_priority);
binder_inner_proc_lock(proc);
list_add(&thread->waiting_thread_node,
&proc->waiting_threads);
binder_inner_proc_unlock(proc);
thread = NULL;
goto retry_after_prio_restore;
}
}
binder_inner_proc_lock(proc);
@ -2965,24 +2934,18 @@ static bool binder_proc_transaction(struct binder_transaction *t,
return false;
}
if (!thread && !pending_async) {
if (!thread && !pending_async)
thread = binder_select_thread_ilocked(proc);
if (thread) {
if (oneway)
node->has_async_transaction = false;
binder_inner_proc_unlock(proc);
binder_node_unlock(node);
retry = true;
goto set_thread_prio;
}
}
if (thread)
if (thread) {
binder_transaction_priority(thread->task, t, node_prio,
node->inherit_rt);
binder_enqueue_thread_work_ilocked(thread, &t->work);
else if (!pending_async)
} else if (!pending_async) {
binder_enqueue_work_ilocked(&t->work, &proc->todo);
else
} else {
binder_enqueue_work_ilocked(&t->work, &node->async_todo);
}
if (!pending_async)
binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync */);