Re: [PATCH 1/7] rust_binder: Add dynamic debug logging mask

[Alice Ryhl <aliceryhl@google.com>]

On Fri, Jul 03, 2026 at 03:29:22PM +0000, Jahnavi MN wrote:
> Implement a dynamic debug logging mask (`debug_mask`) for the
> `rust_binder` module to allow dynamic runtime configuration of log
> levels. This enables parity with the legacy C driver's debug mask.
> 
> Since the Rust `module!` macro in the current kernel build does not yet
> support declaring module parameters directly in Rust, we define the
> `debug_mask` variable in a C companion file to expose it to the kernel
> runtime and import it using FFI with volatile reads.
> 
> To verify the setup, instrument process lifecycle events (open, flush,
> and release) in `process.rs` under the new `BINDER_DEBUG_OPEN_CLOSE`
> logging mask. These entry-point events are chosen for initial validation
> because they represent the start of the Binder lifecycle and occur
> at low frequency, allowing simple runtime verification of the dynamic
> toggle without log noise.
> 
> Signed-off-by: Jahnavi MN <jahnavimn@google.com>

> diff --git a/drivers/android/binder/process.rs b/drivers/android/binder/process.rs
> index 96b8440ceac6..c13a04a3afcb 100644
> --- a/drivers/android/binder/process.rs
> +++ b/drivers/android/binder/process.rs
> @@ -898,7 +898,6 @@ pub(crate) fn insert_or_update_handle(
>      }
>  
>      pub(crate) fn get_transaction_node(&self, handle: u32) -> BinderResult<NodeRef> {
> -        // When handle is zero, try to get the context manager.
>          if handle == 0 {
>              Ok(self.ctx.get_manager_node(true)?)
>          } else {

Removing this comment appears to be a spurious change.

> @@ -1314,6 +1313,11 @@ pub(crate) fn lock_with_nodes(&self) -> WithNodes<'_> {
>      }
>  
>      fn deferred_flush(&self) {
> +        binder_debug!(
> +            crate::debug::BINDER_DEBUG_OPEN_CLOSE,
> +            "binder_flush: process {}",
> +            self.pid_in_current_ns()
> +        );
>          let inner = self.inner.lock();
>          for thread in inner.threads.values() {
>              thread.exit_looper();
> @@ -1321,6 +1325,11 @@ fn deferred_flush(&self) {
>      }
>  
>      fn deferred_release(self: Arc<Self>) {
> +        binder_debug!(
> +            crate::debug::BINDER_DEBUG_OPEN_CLOSE,
> +            "binder_deferred_release: process {}",
> +            self.pid_in_current_ns()
> +        );

Since you included the pid in the binder_debug! macro itself, you can
remove it from these two println statements.

Also, I think it'd be nicer to word these as "flushing process" and
"releasing process".

> diff --git a/drivers/android/binder/thread.rs b/drivers/android/binder/thread.rs
> index 97d5f31e8fe3..c908dde5796a 100644
> --- a/drivers/android/binder/thread.rs
> +++ b/drivers/android/binder/thread.rs
> @@ -1222,6 +1222,7 @@ fn read_transaction_info(
>          info.buffers_size = td.buffers_size as usize;
>          // SAFETY: Above `read` call initializes all bytes, so this union read is ok.
>          info.target_handle = unsafe { td.transaction_data.target.handle };
> +        info.debug_id = super::next_debug_id();
>          Ok(())
>      }
>  
> diff --git a/drivers/android/binder/transaction.rs b/drivers/android/binder/transaction.rs
> index 1d9b66920a21..f9285eb93fc0 100644
> --- a/drivers/android/binder/transaction.rs
> +++ b/drivers/android/binder/transaction.rs
> @@ -42,6 +42,7 @@ pub(crate) struct TransactionInfo {
>      pub(crate) reply: u32,
>      pub(crate) oneway_spam_suspect: bool,
>      pub(crate) is_reply: bool,
> +    pub(crate) debug_id: usize,
>  }
>  
>  impl TransactionInfo {
> @@ -93,7 +94,7 @@ pub(crate) fn new(
>          from: &Arc<Thread>,
>          info: &mut TransactionInfo,
>      ) -> BinderResult<DLArc<Self>> {
> -        let debug_id = super::next_debug_id();
> +        let debug_id = info.debug_id;
>          let allow_fds = node_ref.node.flags & FLAT_BINDER_FLAG_ACCEPTS_FDS != 0;
>          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 };
> @@ -152,7 +153,7 @@ pub(crate) fn new_reply(
>          info: &mut TransactionInfo,
>          allow_fds: bool,
>      ) -> BinderResult<DLArc<Self>> {
> -        let debug_id = super::next_debug_id();
> +        let debug_id = info.debug_id;

It looks like this change to move debug_id so that it's present in
TransactionInfo is happening in the wrong patch. Ideally this change
would be part of the patch that actually needs to read debug_id from
TransactionInfo.

Alice