FROMLIST: binder: don't modify thread->looper from other threads

(from https://patchwork.kernel.org/patch/9817799/)

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>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index ae70670b3792e67ed3bce71203b0bf21f4780697..1961f30d14f1e10d5b7d4252dcba1e7563cb64a2 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -334,14 +334,14 @@ 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 {
        struct binder_proc *proc;
        struct rb_node rb_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 list_head todo;
        uint32_t return_error; /* Write failed, return error code in read buf */
@@ -2277,14 +2277,13 @@ static void binder_stat_br(struct binder_proc *proc,
 static int binder_has_proc_work(struct binder_proc *proc,
                                struct binder_thread *thread)
 {
-       return !list_empty(&proc->todo) ||
-               (thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
+       return !list_empty(&proc->todo) || thread->looper_need_return;
 }
 
 static int binder_has_thread_work(struct binder_thread *thread)
 {
        return !list_empty(&thread->todo) || thread->return_error != BR_OK ||
-               (thread->looper & BINDER_LOOPER_STATE_NEED_RETURN);
+               thread->looper_need_return;
 }
 
 static int binder_put_node_cmd(struct binder_proc *proc,
@@ -2413,8 +2412,7 @@ retry:
                                             entry);
                } else {
                        /* no data added */
-                       if (ptr - buffer == 4 &&
-                           !(thread->looper & BINDER_LOOPER_STATE_NEED_RETURN))
+                       if (ptr - buffer == 4 && !thread->looper_need_return)
                                goto retry;
                        break;
                }
@@ -2728,7 +2726,7 @@ static struct binder_thread *binder_get_thread(struct binder_proc *proc)
                INIT_LIST_HEAD(&thread->todo);
                rb_link_node(&thread->rb_node, parent, p);
                rb_insert_color(&thread->rb_node, &proc->threads);
-               thread->looper |= BINDER_LOOPER_STATE_NEED_RETURN;
+               thread->looper_need_return = true;
                thread->return_error = BR_OK;
                thread->return_error2 = BR_OK;
        }
@@ -2984,7 +2982,7 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
        ret = 0;
 err:
        if (thread)
-               thread->looper &= ~BINDER_LOOPER_STATE_NEED_RETURN;
+               thread->looper_need_return = false;
        binder_unlock(__func__);
        wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
        if (ret && ret != -ERESTARTSYS)
@@ -3139,7 +3137,7 @@ static void binder_deferred_flush(struct binder_proc *proc)
        for (n = rb_first(&proc->threads); n != NULL; n = rb_next(n)) {
                struct binder_thread *thread = rb_entry(n, struct binder_thread, rb_node);
 
-               thread->looper |= BINDER_LOOPER_STATE_NEED_RETURN;
+               thread->looper_need_return = true;
                if (thread->looper & BINDER_LOOPER_STATE_WAITING) {
                        wake_up_interruptible(&thread->wait);
                        wake_count++;
@@ -3400,7 +3398,9 @@ static void print_binder_thread(struct seq_file *m,
        size_t start_pos = m->count;
        size_t header_pos;
 
-       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,
+                       thread->looper_need_return);
        header_pos = m->count;
        t = thread->transaction_stack;
        while (t) {