lkml.org 
[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
    From
    Date
    On Fri, 2010-07-09 at 16:11 +0100, Will Deacon wrote:
    >
    > 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:

    > 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?

    Ah yes.. Silly me..

    We used to only call ->enable() [ now called ->add() ] inside
    perf_disable()/perf_enable() regions.

    But due to patch 9/13 that is no longer the case. I did move it inside
    several ->enable methods, but not for ARM (and SH) as it wasn't evident
    they needed it (that'll teach me to assume things).

    Once we get context per pmu we can move it outside again.

    So yes, your patch looks good and I added the SH bit as well. I'll push
    it out to the git tree as and worry about rebasing the whole series once
    I get back.

    Thanks!

    (new HEAD 0a20dff8474e5738f086e9acc38649f56938c0ad)

    ---
    arch/arm/kernel/perf_event.c | 3 +++
    arch/sh/kernel/perf_event.c | 10 ++++++++--
    2 files changed, 11 insertions(+), 2 deletions(-)

    Index: linux-2.6/arch/arm/kernel/perf_event.c
    ===================================================================
    --- linux-2.6.orig/arch/arm/kernel/perf_event.c
    +++ linux-2.6/arch/arm/kernel/perf_event.c
    @@ -304,6 +304,8 @@ armpmu_add(struct perf_event *event, int
    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
    perf_event_update_userpage(event);

    out:
    + perf_pmu_enable(event->pmu);
    return err;
    }

    Index: linux-2.6/arch/sh/kernel/perf_event.c
    ===================================================================
    --- linux-2.6.orig/arch/sh/kernel/perf_event.c
    +++ linux-2.6/arch/sh/kernel/perf_event.c
    @@ -256,11 +256,14 @@ static int sh_pmu_add(struct perf_event
    struct cpu_hw_events *cpuc = &__get_cpu_var(cpu_hw_events);
    struct hw_perf_event *hwc = &event->hw;
    int idx = hwc->idx;
    + int ret = -EAGAIN;
    +
    + perf_pmu_disable(event->pmu);

    if (__test_and_set_bit(idx, cpuc->used_mask)) {
    idx = find_first_zero_bit(cpuc->used_mask, sh_pmu->num_events);
    if (idx == sh_pmu->num_events)
    - return -EAGAIN;
    + goto ret;

    __set_bit(idx, cpuc->used_mask);
    hwc->idx = idx;
    @@ -273,8 +276,11 @@ static int sh_pmu_add(struct perf_event
    sh_pmu_start(event, PERF_EF_RELOAD);

    perf_event_update_userpage(event);
    + ret = 0;

    - return 0;
    +out:
    + perf_pmu_enable(event->pmu);
    + return ret;
    }

    static void sh_pmu_read(struct perf_event *event)


    \
     
     \ /
      Last update: 2010-07-09 17:55    [W:4.245 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site