Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dyndbg: race between reset_ddebug_call_count() and ddebug_increment_call_count() --
commit 9bd6a53ec548939d0c4d27704bc80cf4e68b28ef Author: Jim Cromie <[email protected]> dyndbg: add +c flag to count pr_debug calls without printing This commit adds a +c flag to dynamic debug to count enabled pr_debug() invocations via a per-CPU counter without printing messages to the syslog. It also supports resetting the counter via the reset_stats command and reports the total in the /proc/dynamic_debug/control epilogue. > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index efc7a741008ed..027f7c31fe226 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c [ ... ] > @@ -71,6 +73,21 @@ struct flag_settings { > unsigned int mask; > }; > > +static DEFINE_PER_CPU(unsigned long, ddebug_call_count); > +void ddebug_increment_call_count(void) > +{ > + this_cpu_inc(ddebug_call_count); > +} > +EXPORT_SYMBOL(ddebug_increment_call_count); > + > +static void reset_ddebug_call_count(void) > +{ > + int cpu; > + > + for_each_possible_cpu(cpu) > + per_cpu(ddebug_call_count, cpu) = 0; > +} [Severity: Medium] Can reset_ddebug_call_count() race with concurrent calls to ddebug_increment_call_count() on remote CPUs and lose the reset? When userspace writes reset_stats to /proc/dynamic_debug/control, ddebug_proc_write() -> ddebug_exec_queries() -> reset_ddebug_call_count() directly stores 0 into each CPU's ddebug_call_count from the resetting CPU. Meanwhile, other CPUs executing +c callsites invoke DYNAMIC_DEBUG_BRANCH() -> DYNAMIC_DEBUG_COUNT() -> ddebug_increment_call_count(), which uses this_cpu_inc(ddebug_call_count). Because this_cpu_inc() only guarantees atomicity with respect to the local CPU rather than cross-CPU stores, a remote CPU performing a read-modify-write can load its old counter value N just before reset_ddebug_call_count() writes 0, and then store N + 1 afterward. That causes a data race and leaves the remote CPU's entire previous count intact instead of resetting it to zero. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=35
