Re: [PATCH v7 06/10] tracing/osnoise: Reset cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] tracing/osnoise: CPU hotplug race in timerlat_fd_open() leaves
  stale fdd
--

commit 25ee38a6983ff50cb9227e203eed4ba8758cefa4
Author: Crystal Wood <crwood@redhat.com>

tracing/osnoise: Reset cleanup

This commit reorganizes per-CPU and global state resets in the osnoise and
timerlat tracers when stopping kthreads, detaching file descriptors, or
restarting the tracer.

> diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c
> index e61f57230eefb..8db0697b69e4a 100644
> --- a/kernel/trace/trace_osnoise.c
> +++ b/kernel/trace/trace_osnoise.c
[ ... ]
> @@ -408,6 +380,39 @@ static struct osnoise_data {
>  #endif
>  };
>  
> +static inline void osn_reset_cpu(int cpu)
> +{
> +	struct osnoise_variables *osn = cpu_osn_var(cpu);
> +
> +	WARN_ON_ONCE(osn->kthread);
> +	WARN_ON_ONCE(osn->fdd);
> +
> +	osn->sampling = false;
> +	osn->pid = 0;
> +	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_ONCE(osn->kthread);
> +		WARN_ON_ONCE(osn->fdd);
> +		WARN_ON_ONCE(osn->sampling);
> +		memset(&osn->zero, 0, sizeof(osn->zero));
> +	}
> +
> +	tlat_var_reset();
> +	osnoise_data.tainted = false;
> +}

[Severity: Low]
Can osn_var_reset() hit WARN_ON_ONCE(osn->fdd) if timerlat_fd_open() races
with CPU hotplug takedown?

timerlat_fd_open() disables migration without holding cpus_read_lock() or
checking whether the CPU is still active under osn_var->lock.  If a task
pinned to CPU0 enters timerlat_fd_open() just as CPU0 is being offlined,
balance_push() skips the task while migration is disabled, allowing
osnoise_cpu_die() to run before timerlat_fd_open() attaches osn_var->fdd:

CPU0 (user task)                  CPU0 (cpuhp/0)
timerlat_fd_open()
  migrate_disable()
                                  sched_cpu_deactivate()
                                  ...
                                  osnoise_cpu_die()
                                    stop_kthread(0)
                                  sched_cpu_wait_empty()
                                    balance_hotplug_wait()
  mutex_lock(&osn_var->lock)
  osn_var->fdd = fdd
  tlat->thread = current
  mutex_unlock(&osn_var->lock)
  migrate_enable()
                                  <takedown completes>

When the tracer is later stopped and restarted, stop_per_cpu_kthreads()
only iterates over online CPUs and misses offline CPU0, so osn_var_reset()
iterating over for_each_possible_cpu() will trigger WARN_ON_ONCE(osn->fdd)
with stale fdd and tlat->thread state left attached on the offline CPU.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113064103.3041704-1-crwood@redhat.com?part=6