Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but 
unregister_trace_sched_switch does not wait for in-flight probes to complete 
before ftrace_ignore_pid is reset, permanently disabling tracing for a CPU.
--

commit 03992a852b272ad537b862229061fb0d87062720
Author: Shengming Hu <[email protected]>

[RFC PATCH 2/4] ftrace: Centralize task filter state updates

This commit moves probe registration and cache refresh logic into a centralized 
helper ftrace_task_filters_changed() to prepare for task comm filtering. It 
updates shared state when a filter changes and refreshes cached results for 
running tasks when PID filters are cleared.

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 825d4474876cc..05775047837d2 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c

[ ... ]

> @@ -8677,11 +8687,31 @@ void ftrace_pid_follow_fork(struct trace_array *tr, 
> bool enable)
>       }
>  }
>  
> +static void ftrace_task_filters_changed(struct trace_array *tr,
> +                                     bool was_enabled)
> +{
> +     bool enabled = ftrace_task_filters_active(tr);
> +     int cpu;
> +
> +     if (!was_enabled && enabled)
> +             
> register_trace_sched_switch(ftrace_filter_task_sched_switch_probe, tr);
> +     else if (was_enabled && !enabled) {
> +             
> unregister_trace_sched_switch(ftrace_filter_task_sched_switch_probe, tr);
> +             for_each_possible_cpu(cpu)
> +                     per_cpu_ptr(tr->array_buffer.data, 
> cpu)->ftrace_ignore_pid =
> +                             FTRACE_PID_TRACE;
> +             return;
> +     }

[Severity: High]
This is a pre-existing issue, but does unregister_trace_sched_switch wait for 
in-flight probes to complete before ftrace_ignore_pid is reset in the loop 
here?

If a concurrent sched_switch probe evaluates the old PID list and decides to 
ignore a task, while another CPU clears the filters, unregisters the probe, 
and resets ftrace_ignore_pid, we might have a race condition. If the in-flight 
probe writes FTRACE_PID_IGNORE after this reset loop completes, the state 
becomes corrupted and could permanently disable tracing for a CPU.

Can we safely reset ftrace_ignore_pid here without an RCU synchronization 
barrier first?

[ ... ]

-- 
Sashiko AI review · 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to