Messages in this thread | | | Date | Mon, 18 Jan 2010 13:07:49 +0100 | From | Frederic Weisbecker <> | Subject | Re: [PATCH] perf: fix the is_software_event() definition |
| |
On Mon, Jan 18, 2010 at 12:53:36PM +0100, Peter Zijlstra wrote: > On Mon, 2010-01-18 at 12:13 +0100, Peter Zijlstra wrote: > > On Sun, 2010-01-17 at 15:12 +0100, Frederic Weisbecker wrote: > > > > > You need to also call pmu->disable() if it is a software event, > > > because a breakpoint needs to be unregistered in hardware level > > > too. > > > > breakpoint isn't a software pmu. But yeah, enable and disable need to > > match. > > That is, it shouldn't be a software pmu, because we assume software > events can always be scheduled, whereas that's definitely not so for the > breakpoint one. > > Which seems to suggest the following > > --- > Subject: perf: fix the is_software_event() definition > > When adding the breakpoint pmu Frederic forgot to exclude it from being > a software event. While we're at it, make it an inclusive expression. > > Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Agreed.
But then Stephane will need to update his patch and use something else than is_software_event() to guess if an event needs its pmu->enable/disable to be called.
A kind of helper that can tell: I am not handled by hw_perf_group_sched_in()
But I suck too much in naming to propose something sane :)
| |