* [PATCH v2 1/3] rust: error: implement ZeroableOption for Error
2026-10-13 12:37 [PATCH v2 0/3] rust_binder: consolidate transaction failure prints Alice Ryhl
@ 2026-10-13 12:37 ` Alice Ryhl
2026-10-13 12:44 ` Miguel Ojeda
2026-10-13 12:37 ` [PATCH v2 2/3] rust_binder: store Option<Error> in TransactionInfo Alice Ryhl
2026-10-13 12:37 ` [PATCH v2 3/3] rust_binder: consolidate transaction failure prints Alice Ryhl
2 siblings, 1 reply; 6+ messages in thread
From: Alice Ryhl @ 2026-10-13 12:37 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas, Arve Hjønnevåg,
Todd Kjos, Christian Brauner
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, rust-for-linux, linux-kernel, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] rust_binder: store Option<Error> in TransactionInfo
2026-10-13 12:37 [PATCH v2 0/3] rust_binder: consolidate transaction failure prints Alice Ryhl
2026-10-13 12:37 ` [PATCH v2 1/3] rust: error: implement ZeroableOption for Error Alice Ryhl
@ 2026-10-13 12:37 ` Alice Ryhl
2026-10-13 12:37 ` [PATCH v2 3/3] rust_binder: consolidate transaction failure prints Alice Ryhl
2 siblings, 0 replies; 6+ messages in thread
From: Alice Ryhl @ 2026-10-13 12:37 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas, Arve Hjønnevåg,
Todd Kjos, Christian Brauner
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, rust-for-linux, linux-kernel, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] rust_binder: consolidate transaction failure prints
2026-10-13 12:37 [PATCH v2 0/3] rust_binder: consolidate transaction failure prints Alice Ryhl
2026-10-13 12:37 ` [PATCH v2 1/3] rust: error: implement ZeroableOption for Error Alice Ryhl
2026-10-13 12:37 ` [PATCH v2 2/3] rust_binder: store Option<Error> in TransactionInfo Alice Ryhl
@ 2026-10-13 12:37 ` Alice Ryhl
2 siblings, 0 replies; 6+ messages in thread
From: Alice Ryhl @ 2026-10-13 12:37 UTC (permalink / raw)
To: Greg Kroah-Hartman, Carlos Llamas, Arve Hjønnevåg,
Todd Kjos, Christian Brauner
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Daniel Almeida, Tamir Duberstein, Alexandre Courbot,
Onur Özkan, rust-for-linux, linux-kernel, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread