Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] tracing/osnoise: osnoise_hotplug_workfn() skips sampling when !OSN_WORKLOAD --
commit 4bedb2088cbb62deefeaad638901751b0c1e9f6d Author: Crystal Wood <[email protected]> tracing/osnoise: Start all eligible cpus from hotplug workfn This commit updates osnoise_hotplug_workfn() to iterate over all online CPUs in osnoise_cpumask and start their kthreads when scheduled from osnoise_cpu_init(), avoiding lost starts if multiple CPUs come online before the work item runs. > diff --git a/kernel/trace/trace_osnoise.c b/kernel/trace/trace_osnoise.c > index aa764ae13047e..29fa4150b26c0 100644 > --- a/kernel/trace/trace_osnoise.c > +++ b/kernel/trace/trace_osnoise.c > @@ -2123,7 +2123,7 @@ static int start_per_cpu_kthreads(void) > #ifdef CONFIG_HOTPLUG_CPU > static void osnoise_hotplug_workfn(struct work_struct *dummy) > { > - unsigned int cpu = smp_processor_id(); > + unsigned int cpu; > > guard(mutex)(&trace_types_lock); > > @@ -2133,13 +2133,13 @@ static void osnoise_hotplug_workfn(struct work_struct > *dummy) > guard(cpus_read_lock)(); > guard(mutex)(&interface_lock); > > - if (!cpu_online(cpu)) > + if (!test_bit(OSN_WORKLOAD, &osnoise_options)) > return; > > - if (!cpumask_test_cpu(cpu, &osnoise_cpumask)) > - return; > - > - start_kthread(cpu); > + for_each_online_cpu(cpu) { > + if (cpumask_test_cpu(cpu, &osnoise_cpumask)) > + start_kthread(cpu); > + } > } [Severity: Medium] Does returning early in osnoise_hotplug_workfn() whenever OSN_WORKLOAD is clear leave per-CPU sampling disabled on newly onlined CPUs when running the osnoise tracer with NO_OSNOISE_WORKLOAD? When timerlat is disabled (i.e. the osnoise tracer is active) and OSN_WORKLOAD is clear, start_kthread() is responsible for enabling sampling on the CPU without spawning a kthread: kernel/trace/trace_osnoise.c:start_kthread() { ... if (timerlat_enabled()) { snprintf(comm, 24, "timerlat/%d", cpu); main = timerlat_main; } else { /* if no workload, just return */ if (!test_bit(OSN_WORKLOAD, &osnoise_options)) { cpu_osn_var(cpu)->sampling = true; barrier(); return 0; } snprintf(comm, 24, "osnoise/%d", cpu); } ... } And when a CPU goes offline via osnoise_cpu_die() -> stop_kthread(), sampling is cleared on that CPU: kernel/trace/trace_osnoise.c:stop_kthread() { ... } else { /* if no workload, just return */ if (!test_bit(OSN_WORKLOAD, &osnoise_options)) { /* * This is set in the osnoise tracer case. */ cpu_osn_var(cpu)->sampling = false; barrier(); } } } In start_per_cpu_kthreads(), the early return for !OSN_WORKLOAD is gated on timerlat_enabled(): kernel/trace/trace_osnoise.c:start_per_cpu_kthreads() { ... if (!test_bit(OSN_WORKLOAD, &osnoise_options)) { if (timerlat_enabled()) return 0; } ... } Should osnoise_hotplug_workfn() likewise check timerlat_enabled() before returning early on !test_bit(OSN_WORKLOAD, &osnoise_options) (or let start_kthread() handle the !OSN_WORKLOAD case) so that cpu_osn_var(cpu)->sampling is set to true when a CPU in osnoise_cpumask comes online? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
