Re: [PATCH v2 7/9] tracing/osnoise: Split out per-cpu variable resets
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-10 02:36:57
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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?
quoted hunk ↗ jump to hunk
+ + tlat_var_reset(); + osnoise_data.tainted = false; +}
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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/20261010022338.2507589-1-crwood@redhat.com?part=7