Messages in this thread Patch in this message | | | Subject | Re: Q: perf_install_in_context/perf_event_enable are racy? | From | Peter Zijlstra <> | Date | Wed, 26 Jan 2011 20:33:03 +0100 |
| |
On Wed, 2011-01-26 at 20:05 +0100, Peter Zijlstra wrote: > On Wed, 2011-01-26 at 19:49 +0100, Oleg Nesterov wrote: > > On 01/26, Oleg Nesterov wrote: > > > > > > Please see the untested patch below. It doesn't change perf_event_enable(), > > > only perf_install_in_context(). > > > > Forgot to mention... Also, it doesn't try to fix the race with do_exit(), > > this needs another change. > > > > And, damn, can't resist. This is mostly cosmetic issue, but I feel > > discomfort every time I look at task_oncpu_function_call(). It _looks_ > > obviously wrong, even if the problem doesn't exist in practice. I'll > > send the pedantic fix to keep the maintainers busy ;) > > I've been trying to sit down and work my way through it today, your last > suggestion very nearly seemed to make sense, but I kept getting > distracted. > > FWIW I think perf_event_enable() has the very same issue...
Wouldn't something like the below cure things too?
--- kernel/sched.c | 23 ++++++++++++++++++++++- 1 files changed, 22 insertions(+), 1 deletions(-)
diff --git a/kernel/sched.c b/kernel/sched.c index 18d38e4..7eadbcf 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -2265,6 +2265,22 @@ void kick_process(struct task_struct *p) EXPORT_SYMBOL_GPL(kick_process); #endif /* CONFIG_SMP */ +struct task_function_call { + struct task_struct *p; + void (*func)(void *info); + void *info; +}; + +void task_function_trampoline(void *data) +{ + struct task_function_call *tfc = data; + + if (this_rq()->curr != tfc->p) + return; + + tfc->func(tfc->data); +} + /** * task_oncpu_function_call - call a function on the cpu on which a task runs * @p: the task to evaluate @@ -2278,11 +2294,16 @@ void task_oncpu_function_call(struct task_struct *p, void (*func) (void *info), void *info) { int cpu; + struct task_function_call data = { + .p = p, + .func = func, + .info = info, + }; preempt_disable(); cpu = task_cpu(p); if (task_curr(p)) - smp_call_function_single(cpu, func, info, 1); + smp_call_function_single(cpu, task_function_trampoline, &data, 1); preempt_enable(); }
| |