Re: [perfmon2] [PATCH] perf: fix the is_software_event() definition
Status: Beta
Brought to you by:
seranian
From: stephane e. <er...@go...> - 2010-01-18 12:57:56
|
On Mon, Jan 18, 2010 at 1:07 PM, Frederic Weisbecker <fwe...@gm...> wrote: > 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...@ch...> > > > > 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() > Then, we should use something that checks if the event is handled by the X86 PMU layer: int is_x86_hw_event(struct perf_event *event) { return event->pmu == x86_pmu; } |