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    [from the cache]
©2003-2011 Jasper Spaans