Re: [PATCH 2/2] perf tools: Make Power7 events available for perf

From: Michael Ellerman
Date: Wed Jun 19 2013 - 21:41:36 EST


On Wed, 2013-06-19 at 17:15 +0800, Runzhen Wang wrote:
> Power7 supports over 530 different perf events but only a small
> subset of these can be specified by name, for the remaining
> events, we must specify them by their raw code:
>
> perf stat -e r2003c <application>
>
> This patch makes all the POWER7 events available in sysfs.
> So we can instead specify these as:
>
> perf stat -e 'cpu/PM_CMPLU_STALL_DFU/' <application>
>
> where PM_CMPLU_STALL_DFU is the r2003c in previous example.
>
> Before this patch is applied, the size of power7-pmu.o is:
>
> $ size arch/powerpc/perf/power7-pmu.o
> text data bss dec hex filename
> 3073 2720 0 5793 16a1 arch/powerpc/perf/power7-pmu.o
>
> and after the patch is applied, it is:
>
> $ size arch/powerpc/perf/power7-pmu.o
> text data bss dec hex filename
> 14451 31112 0 45563 b1fb arch/powerpc/perf/power7-pmu.o

OK so that's ~38K. Which is not terrible.

Can you measure the runtime overhead as well. I suspect it will be more.


You'll notice below that each event name is repeated four times, which
for 530 events is a bit ugly.

I think we should be able to do something better using the C
preprocessor, this is exactly the sort of thing it's good at.

What I mean is something like we do with arch/powerpc/include/asm/systbl.h,
where we define the list of syscalls once, and then include it in
multiple places, using different macro definitions to get different
outputs.

So perhaps you'd define the list of events like:

#define EVENT(PM_CMPLU_STALL_FXU, 0x20014)
#define EVENT(PM_CMPLU_STALL_DIV, 0x40014)

etc.

Then you do something approximately like:

#define EVENT(_name, _code) POWER_EVENT_ATTR(_name, _code)
#include "event-list.h"
#undef EVENT

#define EVENT(_name, _code) POWER_EVENT_PTR(_name)

static struct attribute *power7_events_attr[] = {
#include "event-list.h"
};


You will obviously need to rework the POWER_EVENT macros to make that
work, but it should be possible.

The end result will be we have a single list of the events which we can
check for accuracy once. And we can be sure that there are no mixups
between events.


> diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
> index d1821b8..55e2404 100644
> --- a/arch/powerpc/perf/power7-pmu.c
> +++ b/arch/powerpc/perf/power7-pmu.c
> @@ -53,37 +53,544 @@
> /*
> * Power7 event codes.
> */

...

> +#define PME_PM_MRK_DERAT_MISS_64K 0x2d05c
> +#define PME_PM_INST_PTEG_FROM_DL2L3_MOD 0x4e054
> +#define PME_PM_L2_ST_MISS 0x26082
> +#define PME_PM_MRK_PTEG_FROM_L21_SHR 0x4d056
> +#undef LWSYNC
^
What is this doing here?

That is not your macro to undefine. Please be more careful.

> +#define PME_PM_LWSYNC 0xd094



cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/