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 <[email protected]> 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/[email protected]?part=11
