Skip to content

Commit 5bfbcd1

Browse files
committed
tracing/timerlat: Add interface_lock around clearing of kthread in stop_kthread()
The timerlat interface will get and put the task that is part of the "kthread" field of the osn_var to keep it around until all references are released. But here's a race in the "stop_kthread()" code that will call put_task_struct() on the kthread if it is not a kernel thread. This can race with the releasing of the references to that task struct and the put_task_struct() can be called twice when it should have been called just once. Take the interface_lock() in stop_kthread() to synchronize this change. But to do so, the function stop_per_cpu_kthreads() needs to change the loop from for_each_online_cpu() to for_each_possible_cpu() and remove the cpu_read_lock(), as the interface_lock can not be taken while the cpu locks are held. The only side effect of this change is that it may do some extra work, as the per_cpu variables of the offline CPUs would not be set anyway, and would simply be skipped in the loop. Remove unneeded "return;" in stop_kthread(). Cc: [email protected] Cc: Masami Hiramatsu <[email protected]> Cc: Mathieu Desnoyers <[email protected]> Cc: Tomas Glozar <[email protected]> Cc: John Kacur <[email protected]> Cc: "Luis Claudio R. Goncalves" <[email protected]> Link: https://lore.kernel.org/[email protected] Fixes: e88ed22 ("tracing/timerlat: Add user-space interface") Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent e6a5348 commit 5bfbcd1

File tree

1 file changed

+6
-7
lines changed

1 file changed

+6
-7
lines changed

kernel/trace/trace_osnoise.c

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1953,8 +1953,12 @@ static void stop_kthread(unsigned int cpu)
19531953
{
19541954
struct task_struct *kthread;
19551955

1956+
mutex_lock(&interface_lock);
19561957
kthread = per_cpu(per_cpu_osnoise_var, cpu).kthread;
19571958
if (kthread) {
1959+
per_cpu(per_cpu_osnoise_var, cpu).kthread = NULL;
1960+
mutex_unlock(&interface_lock);
1961+
19581962
if (cpumask_test_and_clear_cpu(cpu, &kthread_cpumask) &&
19591963
!WARN_ON(!test_bit(OSN_WORKLOAD, &osnoise_options))) {
19601964
kthread_stop(kthread);
@@ -1967,16 +1971,15 @@ static void stop_kthread(unsigned int cpu)
19671971
kill_pid(kthread->thread_pid, SIGKILL, 1);
19681972
put_task_struct(kthread);
19691973
}
1970-
per_cpu(per_cpu_osnoise_var, cpu).kthread = NULL;
19711974
} else {
1975+
mutex_unlock(&interface_lock);
19721976
/* if no workload, just return */
19731977
if (!test_bit(OSN_WORKLOAD, &osnoise_options)) {
19741978
/*
19751979
* This is set in the osnoise tracer case.
19761980
*/
19771981
per_cpu(per_cpu_osnoise_var, cpu).sampling = false;
19781982
barrier();
1979-
return;
19801983
}
19811984
}
19821985
}
@@ -1991,12 +1994,8 @@ static void stop_per_cpu_kthreads(void)
19911994
{
19921995
int cpu;
19931996

1994-
cpus_read_lock();
1995-
1996-
for_each_online_cpu(cpu)
1997+
for_each_possible_cpu(cpu)
19971998
stop_kthread(cpu);
1998-
1999-
cpus_read_unlock();
20001999
}
20012000

20022001
/*

0 commit comments

Comments
 (0)