binder: don't modify thread->looper from other threads
The looper member of struct binder_thread is a bitmask of control bits. All of the existing bits are modified by the affected thread except for BINDER_LOOPER_STATE_NEED_RETURN which can be modified in binder_deferred_flush() by another thread. To avoid adding a spinlock around all read-mod-writes to modify a bit, the BINDER_LOOPER_STATE_NEED_RETURN flag is replaced by a separate field in struct binder_thread. Bug: 33250092 32225111 Change-Id: Ia4cefbdbd683c6cb17c323ba7d278de5f2ca0745 Signed-off-by: Todd Kjos <tkjos@google.com>
This commit is contained in:
committed by
Thierry Strudel
parent
e3ede3cdf6
commit
c9c6590330
@@ -395,7 +395,6 @@ enum {
|
||||
BINDER_LOOPER_STATE_EXITED = 0x04,
|
||||
BINDER_LOOPER_STATE_INVALID = 0x08,
|
||||
BINDER_LOOPER_STATE_WAITING = 0x10,
|
||||
BINDER_LOOPER_STATE_NEED_RETURN = 0x20
|
||||
};
|
||||
|
||||
struct binder_thread {
|
||||
@@ -407,7 +406,8 @@ struct binder_thread {
|
||||
struct binder_seq_node active_node;
|
||||
|
||||
int pid;
|
||||
int looper;
|
||||
int looper; /* only modified by this thread */
|
||||
bool looper_need_return; /* can be written by other thread */
|
||||
struct binder_transaction *transaction_stack;
|
||||
struct binder_worklist todo;
|
||||
uint32_t return_error; /* Write failed, return error code in read buf */
|
||||
@@ -2664,14 +2664,14 @@ static inline int binder_has_proc_work(struct binder_proc *proc,
|
||||
struct binder_thread *thread)
|
||||
{
|
||||
return !binder_worklist_empty(&proc->todo) ||
|
||||
(thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
|
||||
READ_ONCE(thread->looper_need_return);
|
||||
}
|
||||
|
||||
static inline int binder_has_thread_work(struct binder_thread *thread)
|
||||
{
|
||||
return !binder_worklist_empty(&thread->todo) ||
|
||||
thread->return_error != BR_OK ||
|
||||
(thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
|
||||
READ_ONCE(thread->looper_need_return);
|
||||
}
|
||||
|
||||
static int binder_thread_read(struct binder_proc *proc,
|
||||
@@ -2802,10 +2802,8 @@ retry:
|
||||
binder_proc_unlock(thread->proc, __LINE__);
|
||||
/* no data added */
|
||||
if (ptr - buffer == 4 &&
|
||||
!(thread->looper &
|
||||
BINDER_LOOPER_STATE_NEED_RETURN)) {
|
||||
!READ_ONCE(thread->looper_need_return))
|
||||
goto retry;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -3253,7 +3251,7 @@ static struct binder_thread *binder_get_thread(struct binder_proc *proc)
|
||||
new_thread->pid = current->pid;
|
||||
init_waitqueue_head(&new_thread->wait);
|
||||
binder_init_worklist(&new_thread->todo);
|
||||
new_thread->looper |= BINDER_LOOPER_STATE_NEED_RETURN;
|
||||
WRITE_ONCE(new_thread->looper_need_return, true);
|
||||
new_thread->return_error = BR_OK;
|
||||
new_thread->return_error2 = BR_OK;
|
||||
INIT_LIST_HEAD(&new_thread->active_node.list_node);
|
||||
@@ -3623,7 +3621,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
|
||||
err:
|
||||
if (thread) {
|
||||
binder_proc_lock(thread->proc, __LINE__);
|
||||
thread->looper &= ~BINDER_LOOPER_STATE_NEED_RETURN;
|
||||
WRITE_ONCE(thread->looper_need_return, false);
|
||||
binder_proc_unlock(thread->proc, __LINE__);
|
||||
zombie_cleanup_check(proc);
|
||||
binder_put_thread(thread);
|
||||
@@ -3812,7 +3810,7 @@ static void binder_deferred_flush(struct binder_proc *proc)
|
||||
struct binder_thread *thread;
|
||||
|
||||
thread = rb_entry(n, struct binder_thread, rb_node);
|
||||
thread->looper |= BINDER_LOOPER_STATE_NEED_RETURN;
|
||||
WRITE_ONCE(thread->looper_need_return, true);
|
||||
if (thread->looper & BINDER_LOOPER_STATE_WAITING) {
|
||||
if (i < count)
|
||||
waits[i] = &thread->wait;
|
||||
@@ -4252,7 +4250,9 @@ static void _print_binder_thread(struct seq_file *m,
|
||||
|
||||
BUG_ON(!spin_is_locked(&thread->proc->proc_lock));
|
||||
|
||||
seq_printf(m, " thread %d: l %02x\n", thread->pid, thread->looper);
|
||||
seq_printf(m, " thread %d: l %02x need_return %d\n",
|
||||
thread->pid, thread->looper,
|
||||
READ_ONCE(thread->looper_need_return));
|
||||
header_pos = m->count;
|
||||
t = thread->transaction_stack;
|
||||
while (t) {
|
||||
|
||||
Reference in New Issue
Block a user