Re: [PATCH v3 4/5] rust: print: add dynamic debug support to pr_debug!

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Oct 01, 2026 at 05:59:39PM +0300, Zhi Wang wrote:
> +/// A dynamic debug callsite descriptor.
> +///
> +/// The [`crate::dynamic_debug!`] macro places descriptors at stable addresses in `__dyndbg`.
> +#[repr(transparent)]
> +pub struct Descriptor {
> +    /// The C descriptor, exposed for static-branch field addressing.
> +    pub inner: bindings::_ddebug,
> +}

Should this be #[repr(C, align(8))] to match how C adds __aligned(8) in 
DEFINE_DYNAMIC_DEBUG_METADATA_CLS?

> +impl Descriptor {
> +    /// Creates an initially disabled descriptor.
> +    pub const fn new(
> +        module: &'static CStr,
> +        file: &'static CStr,
> +        format: &'static CStr,
> +        line: u32,
> +    ) -> Self {
> +        // SAFETY: `_ddebug` contains raw pointers, integers and, with jump labels, a static key.
> +        // All accept zero; the pointers and bitfields are filled in before publication.
> +        let mut descriptor: bindings::_ddebug = unsafe { core::mem::zeroed() };
> +        descriptor.modname = as_char_ptr_in_const_context(module);
> +        descriptor.function = as_char_ptr_in_const_context(c"<unknown>");
> +        descriptor.filename = as_char_ptr_in_const_context(file);
> +        descriptor.format = as_char_ptr_in_const_context(format);
> +
> +        // C packs `lineno:18`, `class_id:6` and `flags:8` into an unsigned int. Bindgen's setters
> +        // are not const, so encode the target's bitfield ordering explicitly. Truncate line numbers
> +        // as the C bitfield assignment does.
> +        let line = line & ((1 << 18) - 1);
> +        let class = bindings::_DPRINTK_CLASS_DFLT;
> +        let flags = bindings::_DPRINTK_FLAGS_NONE;
> +        #[cfg(target_endian = "little")]
> +        let bits = (line | (class << 18) | (flags << 24)).to_le_bytes();
> +        #[cfg(target_endian = "big")]
> +        let bits = ((line << 14) | (class << 8) | flags).to_be_bytes();
> +        descriptor._bitfield_1 = bindings::__BindgenBitfieldUnit::new(bits);
> +
> +        #[cfg(CONFIG_JUMP_LABEL)]
> +        // An all-zero key is `STATIC_KEY_INIT_FALSE`.
> +        static_assert!(bindings::JUMP_TYPE_FALSE == 0);
> +
> +        Self { inner: descriptor }
> +    }

Two comments here:

1. Instead of hardcoding `c"<unknown>"` for `descriptor.function`, how
   about passing `$crate::c_str!(module_path!())` from `dynamic_debug!`
   (an idea from Thomas Weißschuh's RFC series [1])? Even though Rust
   doesn't have a `function_name!()` macro, recording `module_path!()`
   makes the `+f` prefix flag useful and allows `func` queries in
   `/proc/dynamic_debug/control` to filter by Rust module path within a
   multi-file crate.

   [1] https://lore.kernel.org/rust-for-linux/20261011-rust-dyndbg-v1-8-832e4039b71f@weissschuh.net/

2. The inline bitfield manipulation and `unsafe { core::mem::zeroed() }`
   is pretty ugly. Please use the same strategy as `FamilyFlags` in
   `rust/kernel/net/netlink.rs` to handle bitfields in `const` context:

   - Move the bitfield construction into a dedicated helper struct with a
     `const fn into_bitfield(self) -> bindings::__BindgenBitfieldUnit<[u8; 4]>`
     method.
   - Construct `bindings::_ddebug` directly using struct literal syntax
     with `..pin_init::zeroed()`, avoiding `unsafe { core::mem::zeroed() }`
     and field-by-field mutation.
   - Add a kunit test (like `test_family_flags_bitfield`) that verifies
     `into_bitfield()` against bindgen's generated `set_lineno`,
     `set_class_id`, and `set_flags` (or `bindings::_ddebug::new_bitfield_1`).

> +    /// Prints arguments with the crate's log prefix and the dynamic debug prefixes.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `descriptor` must point to an initialized descriptor valid for this call, whose string
> +    /// pointers refer to valid NUL-terminated C strings. Concurrent access must follow the C
> +    /// dynamic debug and jump label synchronization rules, without conflicting Rust accesses.
> +    pub unsafe fn print(descriptor: *mut Self, prefix: &CStr, args: fmt::Arguments<'_>) {
> +        // SAFETY: The descriptor is valid by the caller's guarantee. The helper uses the C string
> +        // `prefix` and consumes `args` through `%pA` synchronously, so their borrows remain valid.
> +        unsafe {
> +            bindings::ddebug_print(
> +                &raw mut (*descriptor).inner,
> +                prefix.as_char_ptr(),
> +                core::ptr::from_ref(&args).cast(),
> +            )
> +        };
> +    }

`Descriptor::print` (and `Descriptor::dev_print` in patch 5/5) should be
marked `#[inline]`, just like `Descriptor::enabled`.

> +macro_rules! dynamic_debug {
> +    ($descriptor:ident, ($($arg:tt)*), $body:block) => {{
> +        // Keep item names out of the user's scope, including implicit format captures.
> +        let (descriptor, enabled) = {
> +            // `used(compiler)` avoids `SHF_GNU_RETAIN`, which the module loader interprets as
> +            // `SHF_RO_AFTER_INIT`.
> +            #[used(compiler)]
> +            #[unsafe(link_section = "__dyndbg")]
> +            static mut DESCRIPTOR: $crate::print::dynamic_debug::Descriptor =
> +                $crate::print::dynamic_debug::Descriptor::new(
> +                    $crate::c_str!(env!("RUST_MODNAME")),
> +                    $crate::c_str!(file!()),
> +                    $crate::dynamic_debug_format!($($arg)*),
> +                    line!(),
> +                );

Is `#[used(compiler)]` needed here at all?

When the `pr_debug!` / `dev_dbg!` statement is reachable, `DESCRIPTOR`
is already referenced by `static_branch_unlikely!(DESCRIPTOR, ...)` and
by `&raw mut DESCRIPTOR` passed to `Descriptor::print`. And if the
callsite is optimized out (for example, inside an unused function or
dead branch), we *want* the static `DESCRIPTOR` to be deleted as well
rather than leaving an orphan entry in `__dyndbg`. This would match C's
`DEFINE_DYNAMIC_DEBUG_METADATA_CLS`, which does not use `__used`.

For instance, with `#[used(compiler)]` present, the uncalled `fn example`
in the `dev_dbg!` doctest still emits a dead `DESCRIPTOR` into `vmlinux`'s
`__dyndbg` section.

> +/// Stub for host doctests, matching [`crate::print_macro!`].
> +#[cfg(testlib)]
> +#[macro_export]
> +macro_rules! dynamic_debug {
> +    ($descriptor:ident, ($($arg:tt)*), $body:block) => {
> +        ()
> +    };
> +}

If `dynamic_debug!` drops `$body` under `#[cfg(testlib)]`, any variables
used only inside `pr_debug!` or `dev_dbg!` will trigger `unused_variables`
warnings under `testlib`, and `$body` won't be type-checked. How about:

    if false {
        let $descriptor: *mut $crate::print::dynamic_debug::Descriptor =
            ::core::ptr::null_mut();
        $body
    }

Alice