Messages in this thread | | | Date | Fri, 21 Jan 2011 14:03:23 +0100 | From | Ingo Molnar <> | Subject | Re: Q: perf_install_in_context/perf_event_enable are racy? |
| |
* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Thu, 2011-01-20 at 20:30 +0100, Oleg Nesterov wrote: > > On 01/19, Oleg Nesterov wrote: > > > > > > Also. I believe there are more problems in perf_install_event(), but > > > I need to recheck. > > > > Help! I can't believe it can be so trivially wrong, but otoh I can't > > understand how this can be correct. > > > > So, ignoring details and !task case, __perf_install_in_context() does: > > > > if (cpuctx->task_ctx || ctx->task != current) > > return; > > > > cpuctx->task_ctx = ctx; > > event_sched_in(event); > > > > Stupid question, what if this task has already passed > > perf_event_exit_task() and thus it doesn't have ->perf_event_ctxp[] ? > > Given that perf_event_context_sched_out() does nothing if !ctx, who > > will event_sched_out() this event? > > > > OK, even if I am right this is trivial, we just need the additional > > check. > > Indeed (or do the cleanup from put_ctx(), but that's too complex a > change I think). > > > But, it seems, there is another problem. Forget about the exiting, > > I can't understand why we can trust current in the code above. > > With __ARCH_WANT_INTERRUPTS_ON_CTXSW schedule() does: > > > > // sets cpuctx->task_ctx = NULL > > perf_event_task_sched_out(); > > > > // enables irqs > > prepare_lock_switch(); > > > > > > // updates current_task > > switch_to(); > > > > What if IPI comes in the window before switch_to() ? > > > > (the same questions for __perf_event_enable). > > Ingo, do you have any insights in that, I think you wrote all that > initially?
Not sure. Can an IPI come there - we have irqs disabled usually, dont we?
Thanks,
Ingo
| |