Re: [RFC][BUG] Revert "sched, perf: Use a single callback into the scheduler"

[Peter Zijlstra <a.p.zijlstra@chello.nl>]

On Mon, 2012-05-21 at 10:06 +0200, Jiri Olsa wrote:
> hi,
> Arnaldo an I were hunting why test__PERF_RECORD stop work recently.
> It seems that following commit is the culprit:
> 
> sched, perf: Use a single callback into the scheduler
> commit cb04ff9ac424d0e689d9b612e9f73cb443ab4b7e
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date:   Tue May 8 18:56:04 2012 +0200
> 
> the reason seems to be following:
> 
>   - before commit this change was introduced, the process switch worked
>     like this (wrt to perf event schedule):
> 
>     schedule (prev, next)
>       - schedule out all perf events for prev
>       - switch to next
>       - schedule in all perf events for current (next)
> 
>   - after the commit, the process switch looks like:
> 
>     schedule (prev, next)
>       - schedule out all perf events for prev
>       - schedule in all perf events for (next)
>       - switch to next
> 
> The problem is, that after we schedule perf events in, the pmu is
> enabled and we can receive events even before we make the switch
> to next - so "current" still being prev process (event SAMPLE data
> are filled based on the value of the "current" process).
> 
> Thats exactly what we see for test__PERF_RECORD test. We receive
> SAMPLES with PID of the process that our tracee is scheduled from.
> Most of the time it's parent, sometimes idle (0).
> 
> I tried to keep the current code and add a new hook from finish_task_switch
> to enable related PMUs for task.. but I'm getting following warning:
> 
> WARNING: at arch/x86/kernel/cpu/perf_event.c:1054 x86_pmu_start+0x133/0x140()
> 
> which I haven't figured out yet... also I'm not sure if thats even
> right thing to do ;)
> 
> thoughts? 

Bah!, yeah I guess reverting is the right thing for now. Sad though.

So by having the two hooks we have a black-spot between them where we
receive no events at all, this black-spot covers the hand-over of
current and we thus don't receive the 'wrong' events.

I rather liked we could do away with both that black-spot and clean up
the code a little, but apparently people rely on it.