Messages in this thread Patch in this message | | | Subject | Re: [RFC][PATCH 00/13] perf pmu interface changes -v3 | From | Will Deacon <> | Date | Fri, 09 Jul 2010 16:11:42 +0100 |
| |
Hi Peter,
On Fri, 2010-07-09 at 09:21 +0100, Peter Zijlstra wrote: > The patches are boot tested on x86_64 and compile tested on: powerpc, > ppc-fsl, > sparc and arm and SH (by courtesy of Matt Fleming). > > X86 -- appears to be fully functional. > FSL -- should, after the initial few fixes, at least compile again. > ARM -- is known to have grown some issues, Will was looking into that.
I've been looking at this and after fixing atomic64 operations and a sign-extension bug, I'm getting closer to finding the *real* cause of the problem! It appears that the PMU isn't being enabled for pinned events. The following patch fixes that, but I'm not sure that it's the correct thing to do:
diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c index c440ae1..a946a77 100644 --- a/arch/arm/kernel/perf_event.c +++ b/arch/arm/kernel/perf_event.c @@ -304,6 +304,8 @@ armpmu_add(struct perf_event *event, int flags) int idx; int err = 0; + perf_pmu_disable(event->pmu); + /* If we don't have a space for the counter then finish early. */ idx = armpmu->get_event_idx(cpuc, hwc); if (idx < 0) { @@ -328,6 +330,7 @@ armpmu_add(struct perf_event *event, int flags) perf_event_update_userpage(event); out: + perf_pmu_enable(event->pmu); return err; }
Because only pinned events seem to be broken, I'm worried that this is just hiding the problem... or is pmu->add(...) supposed to enable the PMU as well as installing the event? The function names aren't especially helpful here either: armpmu_{start,stop} call armpmu->{enable,disable} and armpmu_{enable,disable} call armpmu->{start,stop}! Any thoughts?
Will
| |