[PATCH v2 0/3] rust_binder: consolidate transaction failure prints

[PATCH v2 0/3] rust_binder: consolidate transaction failure prints

[Alice Ryhl]

This series is split out from "Rate limited printing for Rust" [1] so it
can be routed through char-misc-next independently.

Patch 1 implements ZeroableOption for Error so that Option<Error> can be
stored in structs deriving Zeroable. Patch 2 updates TransactionInfo to
store Option<Error> instead of a raw i32 errno, and Patch 3 consolidates
the transaction failure prints into a single debug print.

Link: https://lore.kernel.org/r/20261007-pr-ratelimited-v6-0-f14accd4ea3b@google.com [1]

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Changes in v2:
- Rebase on char-misc-next.
- Expand SAFETY comment on ZeroableOption for Error.
- Pick up Acked-by tag.
- Link to v1: https://lore.kernel.org/r/20261007-binder-consolidate-txn-prints-v1-0-c1032ed3221f@google.com

---
Alice Ryhl (3):
      rust: error: implement ZeroableOption for Error
      rust_binder: store Option<Error> in TransactionInfo
      rust_binder: consolidate transaction failure prints

 drivers/android/binder/error.rs       |  4 --
 drivers/android/binder/thread.rs      | 78 +++++++++++++++--------------------
 drivers/android/binder/transaction.rs | 28 ++++---------
 rust/kernel/error.rs                  |  5 +++
 4 files changed, 48 insertions(+), 67 deletions(-)
---
base-commit: 79e5797d243a466138f641932f81dc69f796060c
change-id: 20261007-binder-consolidate-txn-prints-c12311ee8f49

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>

[PATCH v2 1/3] rust: error: implement ZeroableOption for Error

[Alice Ryhl]

Rust Binder wants to store an Option<Error> in TransactionInfo, which
derives Zeroable. Implement ZeroableOption for Error so that
Option<Error> implements Zeroable.

Acked-by: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/error.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index e52793f77196..9003a1b86da8 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -199,8 +199,13 @@ macro_rules! declare_err {
 ///
 /// The value is a valid `errno` (i.e. `>= -MAX_ERRNO && < 0`).
 #[derive(Clone, Copy, PartialEq, Eq)]
+#[repr(transparent)]
 pub struct Error(NonZeroI32);
 
+// SAFETY: `Error` is `repr(transparent)` over `NonZeroI32`, which is part of the option layout
+// optimization guarantee, so all zeroes is a valid representation for `None`.
+unsafe impl pin_init::ZeroableOption for Error {}
+
 impl Error {
     /// Creates an [`Error`] from a kernel error code.
     ///

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v2 2/3] rust_binder: store Option<Error> in TransactionInfo

[Alice Ryhl]

Store Option<Error> instead of a raw i32 errno in TransactionInfo so
that the Error value can be used directly when formatting transaction
failures without having to reconstruct it from the integer errno.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/thread.rs      | 6 +++---
 drivers/android/binder/transaction.rs | 8 ++++++--
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index e5ae3b0e56c5..5bb7ae2f7920 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -1333,8 +1333,8 @@ fn transaction(self: &Arc<Self>, cmd: u32, reader: &mut UserSliceReader) -> Resu
             if err.reply != BR_TRANSACTION_COMPLETE {
                 info.reply = err.reply;
                 info.error_line = Some(err.line);
-                if let Some(source) = &err.source {
-                    info.errno = source.to_errno();
+                if let Some(source) = err.source {
+                    info.errno = Some(source);
 
                     {
                         let mut inner = self.inner.lock();
@@ -1476,7 +1476,7 @@ fn reply_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResult {
             orig.from
                 .deliver_reply(Err(BR_FAILED_REPLY), &orig, Some(ee));
             info.reply = BR_FAILED_REPLY;
-            info.errno = param;
+            info.errno = err.source;
             info.error_line = Some(err.line);
             err.reply = BR_TRANSACTION_COMPLETE;
             err
diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
index e627522141b8..d934c6821b15 100644
--- a/drivers/android/binder/transaction.rs
+++ b/drivers/android/binder/transaction.rs
@@ -136,7 +136,11 @@ fn new(info: &TransactionInfo, ctx: &crate::Context) -> Self {
             offsets_size: info.offsets_size,
             return_error_line: if failed { info.error_line } else { None },
             return_error: if failed { info.reply } else { 0 },
-            return_error_param: if failed { info.errno } else { 0 },
+            return_error_param: if failed {
+                info.errno.map_or(0, |e| e.to_errno())
+            } else {
+                0
+            },
             context_name,
         }
     }
@@ -254,7 +258,7 @@ pub(crate) struct TransactionInfo {
     pub(crate) offsets_size: usize,
     pub(crate) buffers_size: usize,
     pub(crate) target_handle: u32,
-    pub(crate) errno: i32,
+    pub(crate) errno: Option<Error>,
     pub(crate) reply: u32,
     pub(crate) error_line: Option<ErrorLocation>,
     pub(crate) oneway_spam_suspect: bool,

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH v2 3/3] rust_binder: consolidate transaction failure prints

[Alice Ryhl]

When a transaction fails, it currently hits multiple print statements
meaning that a single failure can result in several lines in the kernel
log. This is unnecessary, so consolidate them into one print used for
all transaction failures.

Acked-by: Carlos Llamas <cmllamas@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 drivers/android/binder/error.rs       |  4 --
 drivers/android/binder/thread.rs      | 72 +++++++++++++++--------------------
 drivers/android/binder/transaction.rs | 20 ++--------
 3 files changed, 34 insertions(+), 62 deletions(-)

diff --git a/drivers/android/binder/error.rs b/drivers/android/binder/error.rs
index 41b07d687b30..89077c5baeaf 100644
--- a/drivers/android/binder/error.rs
+++ b/drivers/android/binder/error.rs
@@ -71,10 +71,6 @@ pub(crate) fn new_frozen_oneway() -> Self {
             line: ErrorLocation::caller(),
         }
     }
-
-    pub(crate) fn is_dead(&self) -> bool {
-        self.reply == BR_DEAD_REPLY
-    }
 }
 
 /// Convert an errno into a `BinderError` and store the errno used to construct it. The errno
diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
index 5bb7ae2f7920..bcb2038b7550 100644
--- a/drivers/android/binder/thread.rs
+++ b/drivers/android/binder/thread.rs
@@ -1065,17 +1065,7 @@ pub(crate) fn copy_transaction_data(
             size_of::<u64>(),
         );
         let secctx_off = aligned_data_size + offsets_size + buffers_size;
-        let mut alloc = match to_process.buffer_alloc(debug_id, len, info) {
-            Ok(alloc) => alloc,
-            Err(err) => {
-                pr_warn!(
-                    "Failed to allocate buffer. len:{}, is_oneway:{}",
-                    len,
-                    info.is_oneway(),
-                );
-                return Err(err);
-            }
-        };
+        let mut alloc = to_process.buffer_alloc(debug_id, len, info)?;
 
         trace_transaction_alloc_buf(debug_id, data_size, offsets_size, buffers_size);
 
@@ -1328,6 +1318,9 @@ fn transaction(self: &Arc<Self>, cmd: u32, reader: &mut UserSliceReader) -> Resu
             self.transaction_inner(&mut info)
         };
 
+        // This runs when return work is passed to the caller. This is not
+        // always the same as the transaction failing, as reply errors are
+        // delivered to the remote process.
         if let Err(err) = ret {
             self.push_return_work(err.reply);
             if err.reply != BR_TRANSACTION_COMPLETE {
@@ -1336,30 +1329,8 @@ fn transaction(self: &Arc<Self>, cmd: u32, reader: &mut UserSliceReader) -> Resu
                 if let Some(source) = err.source {
                     info.errno = Some(source);
 
-                    {
-                        let mut inner = self.inner.lock();
-                        inner.extended_error =
-                            ExtendedError::new(info.debug_id as u32, err.reply, source.to_errno());
-                    }
-
-                    binder_debug!(
-                        FailedTransaction,
-                        "transaction {} to {}:{} failed {:?}, code {} size {}-{} line {}",
-                        if info.is_reply {
-                            "reply"
-                        } else if info.is_oneway() {
-                            "async"
-                        } else {
-                            "call"
-                        },
-                        info.to_pid,
-                        info.to_tid,
-                        err,
-                        info.code,
-                        info.data_size,
-                        info.offsets_size,
-                        err.line
-                    );
+                    self.inner.lock().extended_error =
+                        ExtendedError::new(info.debug_id as u32, err.reply, source.to_errno());
                 }
             }
         }
@@ -1371,8 +1342,33 @@ fn transaction(self: &Arc<Self>, cmd: u32, reader: &mut UserSliceReader) -> Resu
             // useful in case the transaction failed with BR_TRANSACTION_PENDING_FROZEN.
             info.report_netlink(BR_ONEWAY_SPAM_SUSPECT, &self.process.ctx);
         }
+        // This runs when the transaction failed.
         if info.reply != 0 {
             info.report_netlink(info.reply, &self.process.ctx);
+            if let Some(errno) = info.errno {
+                let error_line: &dyn kernel::fmt::Display = match &info.error_line {
+                    Some(line) => line,
+                    None => &0,
+                };
+                binder_debug!(
+                    FailedTransaction,
+                    "transaction {} to {}:{} failed {:?}, code {} size {}-{} line {}",
+                    if info.is_reply {
+                        "reply"
+                    } else if info.is_oneway() {
+                        "async"
+                    } else {
+                        "call"
+                    },
+                    info.to_pid,
+                    info.to_tid,
+                    errno,
+                    info.code,
+                    info.data_size,
+                    info.offsets_size,
+                    error_line
+                );
+            }
         }
 
         Ok(())
@@ -1465,12 +1461,6 @@ fn reply_inner(self: &Arc<Self>, info: &mut TransactionInfo) -> BinderResult {
             // At this point we only return `BR_TRANSACTION_COMPLETE` to the caller, and we must let
             // the sender know that the transaction has completed (with an error in this case).
 
-            pr_warn!(
-                "{}:{} reply to {} failed: {err:?}",
-                info.from_pid,
-                info.from_tid,
-                info.to_pid
-            );
             let param = err.source.as_ref().map_or(0, |e| e.to_errno());
             let ee = ExtendedError::new(info.debug_id as u32, err.reply, param);
             orig.from
diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
index d934c6821b15..22558bdeef16 100644
--- a/drivers/android/binder/transaction.rs
+++ b/drivers/android/binder/transaction.rs
@@ -365,21 +365,13 @@ pub(crate) fn new(
         let txn_security_ctx = node_ref.node.flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX != 0;
         let mut txn_security_ctx_off = if txn_security_ctx { Some(0) } else { None };
         let to = node_ref.node.owner.clone();
-        let mut alloc = match from.copy_transaction_data(
+        let mut alloc = from.copy_transaction_data(
             to.clone(),
             info,
             info.debug_id,
             allow_fds,
             txn_security_ctx_off.as_mut(),
-        ) {
-            Ok(alloc) => alloc,
-            Err(err) => {
-                if !err.is_dead() {
-                    pr_warn!("Failure in copy_transaction_data: {:?}", err);
-                }
-                return Err(err);
-            }
-        };
+        )?;
         if info.is_oneway() {
             if from_parent.is_some() {
                 pr_warn!("Oneway transaction should not be in a transaction stack.");
@@ -420,13 +412,7 @@ pub(crate) fn new_reply(
         allow_fds: bool,
     ) -> BinderResult<DLArc<Self>> {
         let mut alloc =
-            match from.copy_transaction_data(to.clone(), info, info.debug_id, allow_fds, None) {
-                Ok(alloc) => alloc,
-                Err(err) => {
-                    pr_warn!("Failure in copy_transaction_data: {:?}", err);
-                    return Err(err);
-                }
-            };
+            from.copy_transaction_data(to.clone(), info, info.debug_id, allow_fds, None)?;
         if info.flags.contains(TransactionFlag::ClearBuf) {
             alloc.set_info_clear_on_drop();
         }

-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH v2 1/3] rust: error: implement ZeroableOption for Error

[Miguel Ojeda]

On Tue, Oct 13, 2026 at 2:38 PM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Rust Binder wants to store an Option<Error> in TransactionInfo, which
> derives Zeroable. Implement ZeroableOption for Error so that
> Option<Error> implements Zeroable.
>
> Acked-by: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>

Greg: if you don't plan to pick this series this cycle, then I am
happy to pick this one (#1) this cycle if that helps.

Cheers,
Miguel

Re: [PATCH v2 1/3] rust: error: implement ZeroableOption for Error

[Greg Kroah-Hartman]

On Tue, Oct 13, 2026 at 02:44:13PM +0200, Miguel Ojeda wrote:
> On Tue, Oct 13, 2026 at 2:38 PM Alice Ryhl <aliceryhl@google.com> wrote:
> >
> > Rust Binder wants to store an Option<Error> in TransactionInfo, which
> > derives Zeroable. Implement ZeroableOption for Error so that
> > Option<Error> implements Zeroable.
> >
> > Acked-by: Miguel Ojeda <ojeda@kernel.org>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> 
> Greg: if you don't plan to pick this series this cycle, then I am
> happy to pick this one (#1) this cycle if that helps.

Already grabbed it :)