Re: [PATCH v2 7/9] tracing/osnoise: Split out per-cpu variable resets

[sashiko-bot@kernel.org]

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 <crwood@redhat.com>

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/20261010022338.2507589-1-crwood@redhat.com?part=7