[lkml]   [2010]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [RFC][PATCH 00/13] perf pmu interface changes -v3
    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_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?


     \ /
      Last update: 2010-07-09 17:15    [W:0.040 / U:41.340 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site