Binder: add TF_UPDATE_TXN to replace outdated txn

When the target process is busy, incoming oneway transactions are
queued in the async_todo list. If the clients continue sending extra
oneway transactions while the target process is frozen, this queue can
become too large to accommodate new transactions. That's why binder
driver introduced ONEWAY_SPAM_DETECTION to detect this situation. It's
helpful to debug the async binder buffer exhausting issue, but the
issue itself isn't solved directly.

In real cases applications are designed to send oneway transactions
repeatedly, delivering updated inforamtion to the target process.
Typical examples are Wi-Fi signal strength and some real time sensor
data. Even if the apps might only care about the lastet information,
all outdated oneway transactions are still accumulated there until the
frozen process is thawed later. For this kind of situations, there's
no existing method to skip those outdated transactions and deliver the
latest one only.

This patch introduces a new transaction flag TF_UPDATE_TXN. To use it,
use apps can set this new flag along with TF_ONE_WAY. When such an
oneway transaction is to be queued into the async_todo list of a frozen
process, binder driver will check if any previous pending transactions
can be superseded by comparing their code, flags and target node. If
such an outdated pending transaction is found, the latest transaction
will supersede that outdated one. This effectively prevents the async
binder buffer running out and saves unnecessary binder read workloads.

Acked-by: Todd Kjos <tkjos@google.com>
Signed-off-by: Li Li <dualli@google.com>
Link: https://lore.kernel.org/r/20220526220018.3334775-2-dualli@chromium.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 362c0deb65f1138e4640dd5278ee39ba1ec905e1..d4f84f25c30b0d053f911babdee4877d3a838e52 100644 (file)
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -2626,6 +2626,56 @@ static int binder_fixup_parent(struct list_head *pf_head,
        return binder_add_fixup(pf_head, buffer_offset, bp->buffer, 0);
 }
 
+/**
+ * binder_can_update_transaction() - Can a txn be superseded by an updated one?
+ * @t1: the pending async txn in the frozen process
+ * @t2: the new async txn to supersede the outdated pending one
+ *
+ * Return:  true if t2 can supersede t1
+ *          false if t2 can not supersede t1
+ */
+static bool binder_can_update_transaction(struct binder_transaction *t1,
+                                         struct binder_transaction *t2)
+{
+       if ((t1->flags & t2->flags & (TF_ONE_WAY | TF_UPDATE_TXN)) !=
+           (TF_ONE_WAY | TF_UPDATE_TXN) || !t1->to_proc || !t2->to_proc)
+               return false;
+       if (t1->to_proc->tsk == t2->to_proc->tsk && t1->code == t2->code &&
+           t1->flags == t2->flags && t1->buffer->pid == t2->buffer->pid &&
+           t1->buffer->target_node->ptr == t2->buffer->target_node->ptr &&
+           t1->buffer->target_node->cookie == t2->buffer->target_node->cookie)
+               return true;
+       return false;
+}
+
+/**
+ * binder_find_outdated_transaction_ilocked() - Find the outdated transaction
+ * @t:          new async transaction
+ * @target_list: list to find outdated transaction
+ *
+ * Return: the outdated transaction if found
+ *         NULL if no outdated transacton can be found
+ *
+ * Requires the proc->inner_lock to be held.
+ */
+static struct binder_transaction *
+binder_find_outdated_transaction_ilocked(struct binder_transaction *t,
+                                        struct list_head *target_list)
+{
+       struct binder_work *w;
+
+       list_for_each_entry(w, target_list, entry) {
+               struct binder_transaction *t_queued;
+
+               if (w->type != BINDER_WORK_TRANSACTION)
+                       continue;
+               t_queued = container_of(w, struct binder_transaction, work);
+               if (binder_can_update_transaction(t_queued, t))
+                       return t_queued;
+       }
+       return NULL;
+}
+
 /**
  * binder_proc_transaction() - sends a transaction to a process and wakes it up
  * @t:         transaction to send
@@ -2651,6 +2701,7 @@ static int binder_proc_transaction(struct binder_transaction *t,
        struct binder_node *node = t->buffer->target_node;
        bool oneway = !!(t->flags & TF_ONE_WAY);
        bool pending_async = false;
+       struct binder_transaction *t_outdated = NULL;
 
        BUG_ON(!node);
        binder_node_lock(node);
@@ -2678,12 +2729,24 @@ static int binder_proc_transaction(struct binder_transaction *t,
        if (!thread && !pending_async)
                thread = binder_select_thread_ilocked(proc);
 
-       if (thread)
+       if (thread) {
                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 {
+               if ((t->flags & TF_UPDATE_TXN) && proc->is_frozen) {
+                       t_outdated = binder_find_outdated_transaction_ilocked(t,
+                                                                             &node->async_todo);
+                       if (t_outdated) {
+                               binder_debug(BINDER_DEBUG_TRANSACTION,
+                                            "txn %d supersedes %d\n",
+                                            t->debug_id, t_outdated->debug_id);
+                               list_del_init(&t_outdated->work.entry);
+                               proc->outstanding_txns--;
+                       }
+               }
                binder_enqueue_work_ilocked(&t->work, &node->async_todo);
+       }
 
        if (!pending_async)
                binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync */);
@@ -2692,6 +2755,22 @@ static int binder_proc_transaction(struct binder_transaction *t,
        binder_inner_proc_unlock(proc);
        binder_node_unlock(node);
 
+       /*
+        * To reduce potential contention, free the outdated transaction and
+        * buffer after releasing the locks.
+        */
+       if (t_outdated) {
+               struct binder_buffer *buffer = t_outdated->buffer;
+
+               t_outdated->buffer = NULL;
+               buffer->transaction = NULL;
+               trace_binder_transaction_update_buffer_release(buffer);
+               binder_transaction_buffer_release(proc, NULL, buffer, 0, 0);
+               binder_alloc_free_buf(&proc->alloc, buffer);
+               kfree(t_outdated);
+               binder_stats_deleted(BINDER_STAT_TRANSACTION);
+       }
+
        return 0;
 }
 

diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h
index 8eeccdc64724b12b18e63a3ebfff8edf0c733ecb..8cc07e6a4273e7f4ed888bd19798af0aa1aa2535 100644 (file)
--- a/drivers/android/binder_trace.h
+++ b/drivers/android/binder_trace.h
@@ -311,6 +311,10 @@ DEFINE_EVENT(binder_buffer_class, binder_transaction_failed_buffer_release,
        TP_PROTO(struct binder_buffer *buffer),
        TP_ARGS(buffer));
 
+DEFINE_EVENT(binder_buffer_class, binder_transaction_update_buffer_release,
+            TP_PROTO(struct binder_buffer *buffer),
+            TP_ARGS(buffer));
+
 TRACE_EVENT(binder_update_page_range,
        TP_PROTO(struct binder_alloc *alloc, bool allocate,
                 void __user *start, void __user *end),

diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
index 986333cf5bbe044f76af7c5764a4e17449de3605..e72e4de8f452089d41d36f7c49e12509e38789ba 100644 (file)
--- a/include/uapi/linux/android/binder.h
+++ b/include/uapi/linux/android/binder.h
@@ -287,6 +287,7 @@ enum transaction_flags {
        TF_STATUS_CODE  = 0x08, /* contents are a 32-bit status code */
        TF_ACCEPT_FDS   = 0x10, /* allow replies with file descriptors */
        TF_CLEAR_BUF    = 0x20, /* clear buffer on txn complete */
+       TF_UPDATE_TXN   = 0x40, /* update the outdated pending async txn */
 };
 
 struct binder_transaction_data {