Re: [PATCH RFC 11/11] rust: device: enable dynamic debug for dev_dbg!()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rust: dev_dbg!() unconditionally evaluates formatting arguments and device expression
--

commit 8da2aef728fa950b65c5e5a0eaa3ddd52cf5dbeb
Author: Thomas Weißschuh <linux@weissschuh.net>

rust: device: enable dynamic debug for dev_dbg!()

Wire up dynamic debug to also work for the dev_dbg!() macro.

> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 2d49063f4f7c8..db2c9adc5410e 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs

[ ... ]

> @@ -918,6 +918,15 @@ macro_rules! dev_dbg {
>      ($dev:expr, $($f:tt)*) => {
>          match (&$dev, $crate::prelude::fmt!($($f)*)) {
>              (dev, args) => {
> +                // TODO: Switch to cfg_select!() when that is usable.
> +                #[cfg(any(CONFIG_DYNAMIC_DEBUG, all(CONFIG_DYNAMIC_DEBUG_CORE, debug_assertions)))]
> +                $crate::dynamic_func_call!($crate::__format_string!($($f)*), || {
> +                    // SAFETY: klevel is null-terminated, uses one of the kernel constants.
> +                    unsafe { $crate::dev_printk!($crate::bindings::KERN_DEBUG, dev, args) }
> +                });

[Severity: Medium]
Does this unconditionally evaluate the formatting arguments and device
expression?

Since the match (&$dev, $crate::prelude::fmt!($($f)*)) block is placed
outside the dynamic debug conditional closure, the fmt! macro and $dev
expressions are evaluated before the closure is called.

This causes a performance regression by forcing runtime evaluation of all
arguments even when the dynamic debug branch is disabled, defeating the
zero-cost benefit of dynamic debug.

Could the match block be placed inside the conditionally executed closure
passed to dynamic_func_call!, and similarly placed inside the fallback
if cfg!(debug_assertions) block to properly defer evaluation?

> +
> +                #[cfg(not(any(CONFIG_DYNAMIC_DEBUG,
> +                          all(CONFIG_DYNAMIC_DEBUG_CORE, debug_assertions))))]
>                  if cfg!(debug_assertions) {
>                      // SAFETY: klevel is null-terminated, uses one of the kernel constants.
>                      unsafe { $crate::dev_printk!($crate::bindings::KERN_DEBUG, dev, args) }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011-rust-dyndbg-v1-0-832e4039b71f@weissschuh.net?part=11