Message-ID: <20231101-rust-binder-v1-16-08ba9197f637@google.com>
Date:   Wed, 01 Nov 2023 18:01:46 +0000
From:   Alice Ryhl <aliceryhl@...gle.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Arve Hjønnevåg" <arve@...roid.com>,
        Todd Kjos <tkjos@...roid.com>,
        Martijn Coenen <maco@...roid.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Christian Brauner <brauner@...nel.org>,
        Carlos Llamas <cmllamas@...gle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Wedson Almeida Filho <wedsonaf@...il.com>
Cc:     linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
        Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
        "Björn Roy Baron" <bjorn3_gh@...tonmail.com>,
        Benno Lossin <benno.lossin@...ton.me>,
        Andreas Hindborg <a.hindborg@...sung.com>,
        Matt Gilbride <mattgilbride@...gle.com>,
        Jeffrey Vander Stoep <jeffv@...gle.com>,
        Matthew Maurer <mmaurer@...gle.com>,
        Alice Ryhl <aliceryhl@...gle.com>
Subject: [PATCH RFC 16/20] rust_binder: add TF_UPDATE_TXN support

When a process is frozen, incoming oneway transactions are held in a
queue until the process is unfrozen. If many oneway transactions are
sent, then the process could run out of space for them. This patch adds
a flag that avoids this by replacing previous oneway transactions in the
queue to avoid having transactions of the same type build up. This can
be useful when only the most recent transaction is necessary.

Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
---
 drivers/android/defs.rs        |  8 +++++++-
 drivers/android/node.rs        | 19 +++++++++++++++++++
 drivers/android/transaction.rs | 26 ++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/android/defs.rs b/drivers/android/defs.rs
index 30659bd26bff..b1a54f85b365 100644
--- a/drivers/android/defs.rs
+++ b/drivers/android/defs.rs
@@ -58,7 +58,13 @@ macro_rules! pub_no_prefix {
 pub(crate) const FLAT_BINDER_FLAG_ACCEPTS_FDS: u32 = kernel::bindings::FLAT_BINDER_FLAG_ACCEPTS_FDS;
 pub(crate) const FLAT_BINDER_FLAG_TXN_SECURITY_CTX: u32 =
     kernel::bindings::FLAT_BINDER_FLAG_TXN_SECURITY_CTX;
-pub_no_prefix!(transaction_flags_, TF_ONE_WAY, TF_ACCEPT_FDS, TF_CLEAR_BUF);
+pub_no_prefix!(
+    transaction_flags_,
+    TF_ONE_WAY,
+    TF_ACCEPT_FDS,
+    TF_CLEAR_BUF,
+    TF_UPDATE_TXN
+);
 
 pub(crate) use bindings::{
     BINDER_TYPE_BINDER, BINDER_TYPE_FD, BINDER_TYPE_FDA, BINDER_TYPE_HANDLE, BINDER_TYPE_PTR,
diff --git a/drivers/android/node.rs b/drivers/android/node.rs
index 7ed494bf9f7c..2c056bd7582e 100644
--- a/drivers/android/node.rs
+++ b/drivers/android/node.rs
@@ -298,6 +298,25 @@ pub(crate) fn pending_oneway_finished(&self) {
             }
         }
     }
+
+    /// Finds an outdated transaction that the given transaction can replace.
+    ///
+    /// If one is found, it is removed from the list and returned.
+    pub(crate) fn take_outdated_transaction(
+        &self,
+        new: &Transaction,
+        guard: &mut Guard<'_, ProcessInner, SpinLockBackend>,
+    ) -> Option<DLArc<Transaction>> {
+        let inner = self.inner.access_mut(guard);
+        let mut cursor_opt = inner.oneway_todo.cursor_front();
+        while let Some(cursor) = cursor_opt {
+            if new.can_replace(&cursor.current()) {
+                return Some(cursor.remove());
+            }
+            cursor_opt = cursor.next();
+        }
+        None
+    }
 }
 
 impl DeliverToRead for Node {
diff --git a/drivers/android/transaction.rs b/drivers/android/transaction.rs
index 96f63684b1a3..7028c504ef8c 100644
--- a/drivers/android/transaction.rs
+++ b/drivers/android/transaction.rs
@@ -201,6 +201,9 @@ fn drop_outstanding_txn(&self) {
     ///
     /// Not used for replies.
     pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
+        // Defined before `process_inner` so that the destructor runs after releasing the lock.
+        let mut _t_outdated = None;
+
         let oneway = self.flags & TF_ONE_WAY != 0;
         let process = self.to.clone();
         let mut process_inner = process.inner.lock();
@@ -211,6 +214,10 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
             if let Some(target_node) = self.target_node.clone() {
                 if process_inner.is_frozen {
                     process_inner.async_recv = true;
+                    if self.flags & TF_UPDATE_TXN != 0 {
+                        _t_outdated =
+                            target_node.take_outdated_transaction(&self, &mut process_inner);
+                    }
                 }
                 match target_node.submit_oneway(self, &mut process_inner) {
                     Ok(()) => return Ok(()),
@@ -251,6 +258,25 @@ pub(crate) fn submit(self: DLArc<Self>) -> BinderResult {
         }
     }
 
+    /// Check whether one oneway transaction can supersede another.
+    pub(crate) fn can_replace(&self, old: &Transaction) -> bool {
+        if self.from.process.task.pid() != old.from.process.task.pid() {
+            return false;
+        }
+
+        if self.flags & old.flags & (TF_ONE_WAY | TF_UPDATE_TXN) != (TF_ONE_WAY | TF_UPDATE_TXN) {
+            return false;
+        }
+
+        let target_node_match = match (self.target_node.as_ref(), old.target_node.as_ref()) {
+            (None, None) => true,
+            (Some(tn1), Some(tn2)) => Arc::ptr_eq(tn1, tn2),
+            _ => false,
+        };
+
+        self.code == old.code && self.flags == old.flags && target_node_match
+    }
+
     fn prepare_file_list(&self) -> Result<TranslatedFds> {
         let mut alloc = self.allocation.lock().take().ok_or(ESRCH)?;
 

-- 
2.42.0.820.g83a721a137-goog

Powered by blists - more mailing lists