Re: perf_disable()

[Frederic Weisbecker <fweisbec@gmail.com>]

On Fri, Jun 11, 2010 at 06:29:44PM +0200, Peter Zijlstra wrote:
> Hi,
> 
> I've been going over perf_disable() usage in kernel/perf_event.c and
> wondered if we actually need it at all.
> 
> Currently the only thing we seem to require it for is around pmu::enable
> calls (and for that powerpc at least does it itself, on x86 we rely on
> it to call ->enable_all and reprogram the pmu state).
> 
> But I can't really find any NMI races wrt data structures or the like as
> seems implied by some comments. 



I suspect the problem is also on per context integrity. When you adjust
the period, enable or disable a counter, this counter becomes async with
the rest of the group or the rest of the counters in the same context, for
a small bunch of time.

The longer you run your events, the higher is going to be this jitter.

Take an example, when you adjust a period, you:

perf_disable()
perf_event_stop()
left_period = 0
perf_event_start()
perf_enable()

During all this time, the given event is paused, but the whole rest of
the events running on the cpu continue to count.

The problem is the same on context switch.

And I think this high resolution of synchronisation per context is
sensitive, especially with perf start kind of workflows.

(Although software events are not touched by perf_enable()/perf_disable().


> 
> There is a fun little recursion issue with perf_adjust_period(), where
> if we fully removed perf_disable() we could end up calling pmu::stop()
> twice and such.
> 
> But aside from that it looks to me its mostly about optimizing hardware
> writes.
> 
> If nobody else known about/can find anything, I'm going to mostly remove
> perf_disable() for now and later think about how to optimize the
> hardware writes again.


Not sure that's a good idea IMHO.