[Top] [All Lists]

Re: [PATCH 3/4] MIPS/Perf-events: temporarily connect event to its pmu a

To: Deng-Cheng Zhu <>
Subject: Re: [PATCH 3/4] MIPS/Perf-events: temporarily connect event to its pmu at event init
From: Deng-Cheng Zhu <>
Date: Tue, 25 Oct 2011 13:36:51 +0800
Cc: <>, <>, Peter Zijlstra <>, Paul Mackerras <>, Ingo Molnar <>, Arnaldo Carvalho de Melo <>, David Daney <>, David Ahern <>, Will Deacon <>
In-reply-to: <>
References: <> <>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20110617 Lightning/1.0b2 Thunderbird/3.1.11
On 10/24/2011 06:56 PM, Deng-Cheng Zhu wrote:
When arch level event init is called, the event is not yet connected to
the PMU, thereby causing validate_group() to always do dummy work. On MIPS,
this is due to the following lines in validate_event() called by

if (event->pmu !=&pmu || event->state<= PERF_EVENT_STATE_OFF)
         return 1;

This patch fixes it.

Signed-off-by: Deng-Cheng Zhu<>
Cc: Peter Zijlstra<>
Cc: Paul Mackerras<>
Cc: Ingo Molnar<>
Cc: Arnaldo Carvalho de Melo<>
Cc: David Daney<>
  arch/mips/kernel/perf_event_mipsxx.c |   17 +++++++++++++----
  1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c 
index 1f654ca..c804fdd 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -548,6 +548,7 @@ static int __hw_perf_event_init(struct perf_event *event)
        struct perf_event_attr *attr =&event->attr;
        struct hw_perf_event *hwc =&event->hw;
        const struct mips_perf_event *pev;
+       struct pmu *tmp;
        int err;

        /* Returning MIPS event descriptor for generic perf event. */
@@ -611,11 +612,19 @@ static int __hw_perf_event_init(struct perf_event *event)

        err = 0;
-       if (event->group_leader != event) {
+       /*
+        * we temporarily connect event to its pmu such that
+        * validate_event() in validate_group() can classify
+        * it as a MIPS event by passing (event->pmu ==&pmu).
+        */
+       tmp = event->pmu;
+       event->pmu =&pmu;
+       if (event->group_leader != event)
                err = validate_group(event);
-               if (err)
-                       return -EINVAL;
-       }
+       event->pmu = tmp;

        event->destroy = hw_perf_event_destroy;

After looking at David Ahern's reply to my another patch (link
provided below), I started to think whether the PMU and event state
checks are redundant in validate_event() on MIPS (ARM may also have
the same consideration).

The related patch link:

_If_ the state fix goes to group checks instead of to the perf tool,
then the following line in validate_event() on MIPS seems redundant:

if (event->pmu !=&pmu || event->state <= PERF_EVENT_STATE_OFF)
        return 1;

This is because validate_event() is only called by validate_group()
which is called only by __hw_perf_event_init(), and the issue of the
pmu check is already addressed in this patch, and we don't want the
grouped events which are in PERF_EVENT_STATE_OFF (by "attr->disabled =
1") to be filtered out here.

Also, _if_ the state fix goes to group checks instead of to the perf
tool, then X86 may need a patch to allow collect_events() to do real
work in validate_group().

Any comments?


<Prev in Thread] Current Thread [Next in Thread>