The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Carlos Llamas" <cmllamas@google.com>, <jahnavimn@google.com>
Cc: "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>, "Gary Guo" <gary@garyguo.net>,
	"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 21:27:37 +0100	[thread overview]
Message-ID: <DJV6AD2E0WK3.BFCKB6AM6A79@garyguo.net> (raw)
In-Reply-To: <alFMOYlB24vuOxP2@google.com>

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.

>
>> +
>> +    /// 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.

would do. Unfortunately this has to be integer bit-or with casts (and cannot use
DebugMasks due to lack of const trait impl).

Best,
Gary

  reply	other threads:[~2026-07-10 20:27 UTC|newest]

Thread overview: 23+ 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 [this message]
2026-07-10 22:12       ` Carlos Llamas
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

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=DJV6AD2E0WK3.BFCKB6AM6A79@garyguo.net \
    --to=gary@garyguo.net \
    --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=cmllamas@google.com \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --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