Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] tracing/osnoise: missing state reset when OSN_WORKLOAD is disabled --
commit 9927bbade704e7302f3a77ce1eeb45ccd749854e Author: Crystal Wood <[email protected]> tracing/osnoise: Split out per-cpu variable resets This commit resets per-cpu state when stopping a kthread or detaching a file descriptor, instead of assuming cleanup happens implicitly. It also adds debug checks to detect unclean states and no longer initializes the thread lazily. > diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c > index eaecd56492e39..ccd717db9a518 100644 > --- a/kernel/trace/trace_osnoise.c > +++ b/kernel/trace/trace_osnoise.c [ ... ] > @@ -409,6 +379,36 @@ static struct osnoise_data { > #endif > }; > > +static inline void osn_reset_cpu(int cpu) > +{ > + struct osnoise_variables *osn = cpu_osn_var(cpu); > + > + WARN_ON(osn->kthread); > + WARN_ON(osn->fdd); > + memset(&osn->zero, 0, sizeof(osn->zero)); > + tlat_reset_cpu(cpu); > +} > + > +/* > + * osn_var_reset - Reset non-per-cpu state, and check for stale data > + */ > +static inline void osn_var_reset(void) > +{ > + struct osnoise_variables *osn; > + int cpu; > + > + for_each_possible_cpu(cpu) { > + osn = cpu_osn_var(cpu); > + > + WARN_ON(osn->kthread); > + WARN_ON(osn->fdd); > + WARN_ON(osn->sampling); > + } [Severity: High] Since the explicit memset was removed from this function, could stale timestamps and counts remain if cleanup is somehow skipped during teardown? If osn->sampling is left true from a previous session, this code warns but does not clear the stale state. Does this mean delta_start timestamps for NMIs, IRQs, softirqs, and threads are retained, causing the next interrupt to compute a massive bogus duration and corrupt trace metrics? > + > + tlat_var_reset(); > + osnoise_data.tainted = false; > +} [ ... ] > @@ -2036,6 +2036,7 @@ static void stop_kthread(unsigned int cpu) > kthread_stop(osn_var->kthread); > put_task_struct(osn_var->kthread); > osn_var->kthread = NULL; > + osn_reset_cpu(cpu); > } > > mutex_unlock(&osn_var->lock); [Severity: High] When the osnoise tracer is used with the OSN_WORKLOAD option disabled, start_kthread() sets osn->sampling = true but does not spawn a kthread. When tracing stops, does this block bypass osn_reset_cpu() entirely because both osn_var->kthread and osn_var->fdd are NULL? If so, this leaves osn->sampling set to true and prevents the per-CPU state from being cleaned up, causing the stale timestamp issue in osn_var_reset() when the tracer is restarted. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
