FROMLIST: binder: refactor binder_pop_transaction

(from https://lkml.org/lkml/2017/6/29/754)

binder_pop_transaction needs to be split into 2 pieces to
to allow the proc lock to be held on entry to dequeue the
transaction stack, but no lock when kfree'ing the transaction.

Split into binder_pop_transaction_locked and binder_free_transaction
(the actual locks are still to be added).

Change-Id: I4b3d21d42ad54031f52cb94ab2b0152812747c38
Signed-off-by: Todd Kjos <tkjos@google.com>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 7e4471007159c1dee1bfb84a4e3fe0a01a31e9c8..3325bed0ceadf1a2b35734ef4638b7eddaf5b3ba 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -768,14 +768,16 @@ static int binder_dec_ref(struct binder_ref *ref, int strong)
 static void binder_pop_transaction(struct binder_thread *target_thread,
                                   struct binder_transaction *t)
 {
-       if (target_thread) {
-               BUG_ON(target_thread->transaction_stack != t);
-               BUG_ON(target_thread->transaction_stack->from != target_thread);
-               target_thread->transaction_stack =
-                       target_thread->transaction_stack->from_parent;
-               t->from = NULL;
-       }
-       t->need_reply = 0;
+       BUG_ON(!target_thread);
+       BUG_ON(target_thread->transaction_stack != t);
+       BUG_ON(target_thread->transaction_stack->from != target_thread);
+       target_thread->transaction_stack =
+               target_thread->transaction_stack->from_parent;
+       t->from = NULL;
+}
+
+static void binder_free_transaction(struct binder_transaction *t)
+{
        if (t->buffer)
                t->buffer->transaction = NULL;
        kfree(t);
@@ -808,6 +810,7 @@ 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);
+                               binder_free_transaction(t);
                        } else {
                                pr_err("reply failed, target thread, %d:%d, has error code %d already\n",
                                        target_thread->proc->pid,
@@ -822,7 +825,7 @@ static void binder_send_failed_reply(struct binder_transaction *t,
                             "send failed reply for transaction %d, target dead\n",
                             t->debug_id);
 
-               binder_pop_transaction(target_thread, t);
+               binder_free_transaction(t);
                if (next == NULL) {
                        binder_debug(BINDER_DEBUG_DEAD_BINDER,
                                     "reply failed, no target thread at root\n");
@@ -1806,6 +1809,7 @@ static void binder_transaction(struct binder_proc *proc,
        if (reply) {
                BUG_ON(t->buffer->async_transaction != 0);
                binder_pop_transaction(target_thread, in_reply_to);
+               binder_free_transaction(in_reply_to);
        } else if (!(t->flags & TF_ONE_WAY)) {
                BUG_ON(t->buffer->async_transaction != 0);
                t->need_reply = 1;
@@ -2635,9 +2639,7 @@ retry:
                        t->to_thread = thread;
                        thread->transaction_stack = t;
                } else {
-                       t->buffer->transaction = NULL;
-                       kfree(t);
-                       binder_stats_deleted(BINDER_STAT_TRANSACTION);
+                       binder_free_transaction(t);
                }
                break;
        }
@@ -2680,9 +2682,7 @@ static void binder_release_work(struct list_head *list)
                                binder_debug(BINDER_DEBUG_DEAD_TRANSACTION,
                                        "undelivered transaction %d\n",
                                        t->debug_id);
-                               t->buffer->transaction = NULL;
-                               kfree(t);
-                               binder_stats_deleted(BINDER_STAT_TRANSACTION);
+                               binder_free_transaction(t);
                        }
                } break;
                case BINDER_WORK_TRANSACTION_COMPLETE: {