The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Carlos Llamas <cmllamas@google.com>
To: Gary Guo <gary@garyguo.net>
Cc: jahnavimn@google.com,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Arve Hjønnevåg" <arve@android.com>,
	"Todd Kjos" <tkjos@android.com>,
	"Christian Brauner" <brauner@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v2 1/7] rust_binder: Add dynamic debug logging mask
Date: Fri, 10 Jul 2026 22:12:35 +0000	[thread overview]
Message-ID: <alFuU1Uek6t9v_tZ@google.com> (raw)
In-Reply-To: <DJV6AD2E0WK3.BFCKB6AM6A79@garyguo.net>

On Fri, Jul 10, 2026 at 09:27:37PM +0100, Gary Guo wrote:
> On Fri Jul 10, 2026 at 8:47 PM BST, Carlos Llamas wrote:
> > On Fri, Jul 10, 2026 at 02:32:52PM +0000, Jahnavi MN via B4 Relay wrote:
> >> From: Jahnavi MN <jahnavimn@google.com>
> >> 
> >> 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>
> >> ---
> >>  drivers/android/binder/debug.rs            | 80 ++++++++++++++++++++++++++++++
> >>  drivers/android/binder/process.rs          |  7 ++-
> >>  drivers/android/binder/rust_binder_main.rs |  2 +
> >>  drivers/android/binder/rust_binderfs.c     |  3 ++
> >>  rust/kernel/task.rs                        |  7 +++
> >>  5 files changed, 98 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/android/binder/debug.rs b/drivers/android/binder/debug.rs
> >> new file mode 100644
> >> index 000000000000..da48bce3df54
> >> --- /dev/null
> >> +++ b/drivers/android/binder/debug.rs
> >> @@ -0,0 +1,80 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +// Copyright (C) 2026 Google LLC.
> >> +
> >> +//! Binder debugging helpers.
> >> +
> >> +#![allow(dead_code)]
> >> +
> >> +kernel::impl_flags!(
> >> +    /// Represents multiple debug mask flags.
> >> +    #[derive(Debug, Clone, Default, Copy, PartialEq, Eq)]
> >> +    pub struct DebugMasks(u32);
> >
> > Where is this being used? I can't find it on subsequent patches.
> > Maybe leftover from previous implementation?
> 
> impl_flags macro requires a type for a single flag and a type for flag masks.

Oh I see. I was expecting the bitops to be peformed on this type
somewhere. I guess the reason it wasn't is because of the issue with
module parameters and their C macros.

> 
> >
> >> +
> >> +    /// Represents a single debug mask category.
> >> +    #[derive(Debug, Clone, Copy, PartialEq, Eq)]
> >> +    pub enum DebugMask {
> >> +        UserError = 1 << 0,
> >> +        FailedTransaction = 1 << 1,
> >> +        DeadTransaction = 1 << 2,
> >> +        OpenClose = 1 << 3,
> >> +        DeadBinder = 1 << 4,
> >> +        DeathNotification = 1 << 5,
> >> +        ReadWrite = 1 << 6,
> >> +        UserRefs = 1 << 7,
> >> +        Threads = 1 << 8,
> >> +        Transaction = 1 << 9,
> >> +        TransactionComplete = 1 << 10,
> >> +        FreeBuffer = 1 << 11,
> >> +        InternalRefs = 1 << 12,
> >> +        PriorityCap = 1 << 13,
> >> +        Spinlocks = 1 << 14,
> >> +    }
> >> +);
> >> diff --git a/drivers/android/binder/rust_binder_main.rs b/drivers/android/binder/rust_binder_main.rs
> >> index df47aba05133..995e6131bb77 100644
> >> --- a/drivers/android/binder/rust_binder_main.rs
> >> +++ b/drivers/android/binder/rust_binder_main.rs
> >> @@ -31,6 +31,8 @@
> >>  mod context;
> >>  mod deferred_close;
> >>  mod defs;
> >> +#[macro_use]
> >> +mod debug;
> >>  mod error;
> >>  mod netlink;
> >>  mod node;
> >> diff --git a/drivers/android/binder/rust_binderfs.c b/drivers/android/binder/rust_binderfs.c
> >> index ade1c4d92499..fa8b38465550 100644
> >> --- a/drivers/android/binder/rust_binderfs.c
> >> +++ b/drivers/android/binder/rust_binderfs.c
> >> @@ -51,6 +51,9 @@ DEFINE_SHOW_ATTRIBUTE(rust_binder_proc);
> >>  char *rust_binder_devices_param = CONFIG_ANDROID_BINDER_DEVICES;
> >>  module_param_named(rust_devices, rust_binder_devices_param, charp, 0444);
> >>  
> >> +u32 rust_binder_debug_mask = 7;
> >
> > hmm, I'm not sure about this magic number. Do you think we can
> > initialize on the rust end at init()? Or at least a comment about what
> > flags we are setting as default here?
> 
> It doesn't even need that, just a
> 
> static rust_binder_debug_mask: Atomic<u32> = Atomic::new((FLAG_A as u32) | (FLAG_B as u32) | ...);
> 
> and a `extern u32` on C side should do.

ha sure, reversing the declaration and extern would also work.

  reply	other threads:[~2026-07-10 22:12 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 14:32 [PATCH v2 0/7] rust_binder : Implement dynamic debug logging mask Jahnavi MN via B4 Relay
2026-07-10 14:32 ` [PATCH v2 1/7] rust_binder: Add " Jahnavi MN via B4 Relay
2026-07-10 14:59   ` Greg Kroah-Hartman
2026-07-10 16:37     ` Miguel Ojeda
2026-07-10 17:03       ` Miguel Ojeda
2026-07-10 16:51   ` Gary Guo
2026-07-10 19:47   ` Carlos Llamas
2026-07-10 20:27     ` Gary Guo
2026-07-10 22:12       ` Carlos Llamas [this message]
2026-07-10 14:32 ` [PATCH v2 2/7] rust_binder: Implement BINDER_DEBUG_USER_ERROR for freezer-related operation Jahnavi MN via B4 Relay
2026-07-10 19:55   ` Carlos Llamas
2026-07-10 14:32 ` [PATCH v2 3/7] rust_binder: Implement BINDER_DEBUG_USER_ERROR for refcounting and death notifications Jahnavi MN via B4 Relay
2026-07-10 19:56   ` Carlos Llamas
2026-07-10 19:58   ` Carlos Llamas
2026-07-10 14:32 ` [PATCH v2 4/7] rust_binder: Implement BINDER_DEBUG_USER_ERROR for transaction parsing failures Jahnavi MN via B4 Relay
2026-07-10 20:07   ` Carlos Llamas
2026-07-10 14:32 ` [PATCH v2 5/7] rust_binder: Implement BINDER_DEBUG_FAILED_TRANSACTION Jahnavi MN via B4 Relay
2026-07-10 20:15   ` Carlos Llamas
2026-07-10 14:32 ` [PATCH v2 6/7] rust_binder: Implement BINDER_DEBUG_DEATH_NOTIFICATION Jahnavi MN via B4 Relay
2026-07-10 20:20   ` Carlos Llamas
2026-07-10 14:32 ` [PATCH v2 7/7] rust_binder: Implement BINDER_DEBUG_DEAD_TRANSACTION Jahnavi MN via B4 Relay
2026-07-10 14:35 ` [PATCH v2 0/7] rust_binder : Implement dynamic debug logging mask Alice Ryhl
2026-07-10 15:01 ` Greg Kroah-Hartman
2026-07-11 10:28   ` Jahnavi Nitheesha Mandava
2026-07-11 10:42     ` Alice Ryhl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alFuU1Uek6t9v_tZ@google.com \
    --to=cmllamas@google.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=arve@android.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=brauner@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jahnavimn@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tkjos@android.com \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox