2012-11-07 19:17:14

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: [PATCH 1/4] perf/powerpc: Use uapi/unistd.h to fix build error


>From b8beef080260c1625c8f801105504a82005295e5 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <[email protected]>
Date: Wed, 31 Oct 2012 11:21:28 -0700
Subject: [PATCH 1/4] perf/powerpc: Use uapi/unistd.h to fix build error

Use the 'unistd.h' from arch/powerpc/include/uapi to build the perf tool.

Signed-off-by: Sukadev Bhattiprolu <[email protected]>
---
tools/perf/perf.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 054182e..f4952da 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -26,7 +26,7 @@ void get_term_dimensions(struct winsize *ws);
#endif

#ifdef __powerpc__
-#include "../../arch/powerpc/include/asm/unistd.h"
+#include "../../arch/powerpc/include/uapi/asm/unistd.h"
#define rmb() asm volatile ("sync" ::: "memory")
#define cpu_relax() asm volatile ("" ::: "memory");
#define CPUINFO_PROC "cpu"
--
1.7.1


2012-11-07 19:17:39

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: [PATCH 2/4] perf/Power7: Use macros to identify perf events


>From 8a0dbd8f3fce2834292efa50c15ca64d4f6a6536 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <[email protected]>
Date: Wed, 7 Nov 2012 09:36:14 -0800
Subject: [PATCH 2/4] perf/Power7: Use macros to identify perf events

Define and use macros to identify perf events codes This would make it
easier and more readable when these event codes need to be used in more
than one place.

Signed-off-by: Sukadev Bhattiprolu <[email protected]>
---
arch/powerpc/perf/power7-pmu.c | 25 +++++++++++++++++--------
1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 441af08..256db4f 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -295,15 +295,24 @@ static void power7_disable_pmc(unsigned int pmc, unsigned long mmcr[])
mmcr[1] &= ~(0xffUL << MMCR1_PMCSEL_SH(pmc));
}

+#define PM_CYC 0x1e
+#define PM_GCT_NOSLOT_CYC 0x100f8
+#define PM_CMPLU_STALL 0x4000a
+#define PM_INST_CMPL 0x2
+#define PM_LD_REF_L1 0xc880
+#define PM_LD_MISS_L1 0x400f0
+#define PM_BRU_FIN 0x10068
+#define PM_BRU_MPRED 0x400f6
+
static int power7_generic_events[] = {
- [PERF_COUNT_HW_CPU_CYCLES] = 0x1e,
- [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = 0x100f8, /* GCT_NOSLOT_CYC */
- [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = 0x4000a, /* CMPLU_STALL */
- [PERF_COUNT_HW_INSTRUCTIONS] = 2,
- [PERF_COUNT_HW_CACHE_REFERENCES] = 0xc880, /* LD_REF_L1_LSU*/
- [PERF_COUNT_HW_CACHE_MISSES] = 0x400f0, /* LD_MISS_L1 */
- [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = 0x10068, /* BRU_FIN */
- [PERF_COUNT_HW_BRANCH_MISSES] = 0x400f6, /* BR_MPRED */
+ [PERF_COUNT_HW_CPU_CYCLES] = PM_CYC,
+ [PERF_COUNT_HW_STALLED_CYCLES_FRONTEND] = PM_GCT_NOSLOT_CYC,
+ [PERF_COUNT_HW_STALLED_CYCLES_BACKEND] = PM_CMPLU_STALL,
+ [PERF_COUNT_HW_INSTRUCTIONS] = PM_INST_CMPL,
+ [PERF_COUNT_HW_CACHE_REFERENCES] = PM_LD_REF_L1,
+ [PERF_COUNT_HW_CACHE_MISSES] = PM_LD_MISS_L1,
+ [PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = PM_BRU_FIN,
+ [PERF_COUNT_HW_BRANCH_MISSES] = PM_BRU_MPRED,
};

#define C(x) PERF_COUNT_HW_CACHE_##x
--
1.7.1

2012-11-07 19:18:23

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs


>From d05d1ce6d55bf339eee6230ded9f5dd1351f60e5 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <[email protected]>
Date: Tue, 6 Nov 2012 14:07:36 -0800
Subject: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs

Make the perf events supported by POWER7 available via sysfs.

$ ls /sys/bus/event_source/devices/cpu/events
branch-instructions
branch-misses
cache-misses
cache-references
cpu-cycles
instructions
stalled-cycles-backend
stalled-cycles-frontend

$ cat /sys/bus/event_source/devices/cpu/events/cache-misses
event=0x03

This patch is based on commits that implement this functionality on x86.
Eg:
commit a47473939db20e3961b200eb00acf5fcf084d755
Author: Jiri Olsa <[email protected]>
Date: Wed Oct 10 14:53:11 2012 +0200

perf/x86: Make hardware event translations available in sysfs

Signed-off-by: Sukadev Bhattiprolu <[email protected]>
---
arch/powerpc/include/asm/perf_event_server.h | 21 +++++++++++++++++
arch/powerpc/perf/core-book3s.c | 12 +++++++++
arch/powerpc/perf/power7-pmu.c | 32 ++++++++++++++++++++++++++
3 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index 9710be3..ad84f73 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -11,6 +11,8 @@

#include <linux/types.h>
#include <asm/hw_irq.h>
+#include <linux/sysfs.h>
+#include <linux/device.h>

#define MAX_HWEVENTS 8
#define MAX_EVENT_ALTERNATIVES 8
@@ -35,6 +37,7 @@ struct power_pmu {
void (*disable_pmc)(unsigned int pmc, unsigned long mmcr[]);
int (*limited_pmc_event)(u64 event_id);
u32 flags;
+ const struct attribute_group **attr_groups;
int n_generic;
int *generic_events;
int (*cache_events)[PERF_COUNT_HW_CACHE_MAX]
@@ -109,3 +112,21 @@ extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
* If an event_id is not subject to the constraint expressed by a particular
* field, then it will have 0 in both the mask and value for that field.
*/
+
+
+struct perf_pmu_events_attr {
+ struct device_attribute attr;
+ u64 id;
+};
+
+extern ssize_t power_events_sysfs_show(struct device *dev,
+ struct device_attribute *attr, char *page);
+
+#define EVENT_VAR(_id) event_attr_##_id
+#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
+
+#define EVENT_ATTR(_name, _id) \
+ static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
+ .attr = __ATTR(_name, 0444, power_events_sysfs_show, NULL),\
+ .id = PM_##_id, \
+ };
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index aa2465e..19b23bd 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1305,6 +1305,16 @@ static int power_pmu_event_idx(struct perf_event *event)
return event->hw.idx;
}

+ssize_t power_events_sysfs_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ struct perf_pmu_events_attr *pmu_attr;
+
+ pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
+
+ return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
+}
+
struct pmu power_pmu = {
.pmu_enable = power_pmu_enable,
.pmu_disable = power_pmu_disable,
@@ -1537,6 +1547,8 @@ int __cpuinit register_power_pmu(struct power_pmu *pmu)
pr_info("%s performance monitor hardware support registered\n",
pmu->name);

+ power_pmu.attr_groups = ppmu->attr_groups;
+
#ifdef MSR_HV
/*
* Use FCHV to ignore kernel events if MSR.HV is set.
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 256db4f..31c61ab 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -360,6 +360,37 @@ static int power7_cache_events[C(MAX)][C(OP_MAX)][C(RESULT_MAX)] = {
},
};

+EVENT_ATTR(cpu-cycles, CYC);
+EVENT_ATTR(stalled-cycles-frontend, GCT_NOSLOT_CYC);
+EVENT_ATTR(stalled-cycles-backend, CMPLU_STALL);
+EVENT_ATTR(instructions, INST_CMPL);
+EVENT_ATTR(cache-references, LD_REF_L1);
+EVENT_ATTR(cache-misses, LD_MISS_L1);
+EVENT_ATTR(branch-instructions, BRU_FIN);
+EVENT_ATTR(branch-misses, BRU_MPRED);
+
+static struct attribute *power7_events_attr[] = {
+ EVENT_PTR(CYC),
+ EVENT_PTR(GCT_NOSLOT_CYC),
+ EVENT_PTR(CMPLU_STALL),
+ EVENT_PTR(INST_CMPL),
+ EVENT_PTR(LD_REF_L1),
+ EVENT_PTR(LD_MISS_L1),
+ EVENT_PTR(BRU_FIN),
+ EVENT_PTR(BRU_MPRED),
+ NULL,
+};
+
+static struct attribute_group power7_pmu_events_group = {
+ .name = "events",
+ .attrs = power7_events_attr,
+};
+
+static const struct attribute_group *power7_pmu_attr_groups[] = {
+ &power7_pmu_events_group,
+ NULL,
+};
+
static struct power_pmu power7_pmu = {
.name = "POWER7",
.n_counter = 6,
@@ -371,6 +402,7 @@ static struct power_pmu power7_pmu = {
.get_alternatives = power7_get_alternatives,
.disable_pmc = power7_disable_pmc,
.flags = PPMU_ALT_SIPR,
+ .attr_groups = power7_pmu_attr_groups,
.n_generic = ARRAY_SIZE(power7_generic_events),
.generic_events = power7_generic_events,
.cache_events = &power7_cache_events,
--
1.7.1

2012-11-07 19:19:09

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: [PATCH 4/4] perf: Create a sysfs entry for Power event format


>From bafc551c31ce23c1cba0b75d23de6c46aba90f26 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <[email protected]>
Date: Tue, 6 Nov 2012 16:30:28 -0800
Subject: [PATCH 4/4] perf: Create a sysfs entry for Power event format

Create a sysfs entry, '/sys/bus/event_source/devices/cpu/format/event'
which describes the format of a POWER cpu.

$ cat /sys/bus/event_source/devices/cpu/format/event
config:0-20

The format of the event is the same for all POWER cpus, so bulk of this
change is in the code common to POWER cpus.

Signed-off-by: Sukadev Bhattiprolu <[email protected]>
---
arch/powerpc/include/asm/perf_event_server.h | 8 ++++++++
arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++++++
arch/powerpc/perf/power7-pmu.c | 1 +
3 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
index ad84f73..20a49bf 100644
--- a/arch/powerpc/include/asm/perf_event_server.h
+++ b/arch/powerpc/include/asm/perf_event_server.h
@@ -130,3 +130,11 @@ extern ssize_t power_events_sysfs_show(struct device *dev,
.attr = __ATTR(_name, 0444, power_events_sysfs_show, NULL),\
.id = PM_##_id, \
};
+
+/*
+ * Format of a perf event is the same on all POWER cpus. Declare a
+ * common sysfs attribute group that individual POWER cpus can share.
+ */
+extern struct attribute_group power_pmu_format_group;
+
+
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 19b23bd..388e2a1 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1315,6 +1315,25 @@ ssize_t power_events_sysfs_show(struct device *dev,
return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
}

+static ssize_t power_config_sysfs_show(struct device *dev,
+ struct device_attribute *attr, char *page)
+{
+ return sprintf(page, "config:0-20\n");
+}
+
+static struct device_attribute config_dev_attr = \
+ __ATTR(event, 0444, power_config_sysfs_show, NULL);
+
+static struct attribute *power_pmu_format_attr[] = {
+ &config_dev_attr.attr,
+ NULL,
+};
+
+struct attribute_group power_pmu_format_group = {
+ .name = "format",
+ .attrs = power_pmu_format_attr,
+};
+
struct pmu power_pmu = {
.pmu_enable = power_pmu_enable,
.pmu_disable = power_pmu_disable,
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index 31c61ab..aa9f588 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -387,6 +387,7 @@ static struct attribute_group power7_pmu_events_group = {
};

static const struct attribute_group *power7_pmu_attr_groups[] = {
+ &power_pmu_format_group,
&power7_pmu_events_group,
NULL,
};
--
1.7.1

2012-11-14 10:25:55

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs

On Wed, Nov 07, 2012 at 11:19:28AM -0800, Sukadev Bhattiprolu wrote:

SNIP

> +struct perf_pmu_events_attr {
> + struct device_attribute attr;
> + u64 id;
> +};
> +
> +extern ssize_t power_events_sysfs_show(struct device *dev,
> + struct device_attribute *attr, char *page);
> +
> +#define EVENT_VAR(_id) event_attr_##_id
> +#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
> +
> +#define EVENT_ATTR(_name, _id) \
> + static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
> + .attr = __ATTR(_name, 0444, power_events_sysfs_show, NULL),\
> + .id = PM_##_id, \
> + };

this is duplicating the x86 code, perhaps it could be moved
to include/linux/perf_event.h and shared globaly


> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index aa2465e..19b23bd 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1305,6 +1305,16 @@ static int power_pmu_event_idx(struct perf_event *event)
> return event->hw.idx;
> }
>
> +ssize_t power_events_sysfs_show(struct device *dev,
> + struct device_attribute *attr, char *page)
> +{
> + struct perf_pmu_events_attr *pmu_attr;
> +
> + pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
> +
> + return sprintf(page, "event=0x%02llx\n", pmu_attr->id);

whitespace issues


jirka

2012-11-14 10:28:12

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 4/4] perf: Create a sysfs entry for Power event format

On Wed, Nov 07, 2012 at 11:19:52AM -0800, Sukadev Bhattiprolu wrote:
>
> From bafc551c31ce23c1cba0b75d23de6c46aba90f26 Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <[email protected]>
> Date: Tue, 6 Nov 2012 16:30:28 -0800
> Subject: [PATCH 4/4] perf: Create a sysfs entry for Power event format
>
> Create a sysfs entry, '/sys/bus/event_source/devices/cpu/format/event'
> which describes the format of a POWER cpu.
>
> $ cat /sys/bus/event_source/devices/cpu/format/event
> config:0-20
>
> The format of the event is the same for all POWER cpus, so bulk of this
> change is in the code common to POWER cpus.
>
> Signed-off-by: Sukadev Bhattiprolu <[email protected]>
> ---
> arch/powerpc/include/asm/perf_event_server.h | 8 ++++++++
> arch/powerpc/perf/core-book3s.c | 19 +++++++++++++++++++
> arch/powerpc/perf/power7-pmu.c | 1 +
> 3 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index ad84f73..20a49bf 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -130,3 +130,11 @@ extern ssize_t power_events_sysfs_show(struct device *dev,
> .attr = __ATTR(_name, 0444, power_events_sysfs_show, NULL),\
> .id = PM_##_id, \
> };
> +
> +/*
> + * Format of a perf event is the same on all POWER cpus. Declare a
> + * common sysfs attribute group that individual POWER cpus can share.
> + */
> +extern struct attribute_group power_pmu_format_group;
> +
> +
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 19b23bd..388e2a1 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1315,6 +1315,25 @@ ssize_t power_events_sysfs_show(struct device *dev,
> return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
> }
>
> +static ssize_t power_config_sysfs_show(struct device *dev,
> + struct device_attribute *attr, char *page)
> +{
> + return sprintf(page, "config:0-20\n");
> +}
> +
> +static struct device_attribute config_dev_attr = \
> + __ATTR(event, 0444, power_config_sysfs_show, NULL);

there's PMU_FORMAT_ATTR in include/linux/perf_event.h macro doing this

jirka

2012-11-14 18:26:48

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: Re: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs

Jiri Olsa [[email protected]] wrote:
| On Wed, Nov 07, 2012 at 11:19:28AM -0800, Sukadev Bhattiprolu wrote:
|
| SNIP
|
| > +struct perf_pmu_events_attr {
| > + struct device_attribute attr;
| > + u64 id;
| > +};
| > +
| > +extern ssize_t power_events_sysfs_show(struct device *dev,
| > + struct device_attribute *attr, char *page);
| > +
| > +#define EVENT_VAR(_id) event_attr_##_id
| > +#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
| > +
| > +#define EVENT_ATTR(_name, _id) \
| > + static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
| > + .attr = __ATTR(_name, 0444, power_events_sysfs_show, NULL),\
| > + .id = PM_##_id, \
| > + };
|
| this is duplicating the x86 code, perhaps it could be moved
| to include/linux/perf_event.h and shared globaly

Ok.

Can we remove the assumption that the event id is a generic event that
has PERF_COUNT_HW_ prefix and also let the architectures pass in a "show"
function ? This would allow architectures to display any arch specific
events that don't yet have a generic counterpart.

IOW, can we do something like this (untested) and make PERF_EVENT_ATTR global:



diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 4428fd1..25298f7 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1354,12 +1354,15 @@ static ssize_t events_sysfs_show(struct device *dev, struct device_attribute *at
#define EVENT_VAR(_id) event_attr_##_id
#define EVENT_PTR(_id) &event_attr_##_id.attr.attr

-#define EVENT_ATTR(_name, _id) \
+#define PERF_EVENT_ATTR(_name, _id, _show) \
static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
- .attr = __ATTR(_name, 0444, events_sysfs_show, NULL), \
- .id = PERF_COUNT_HW_##_id, \
+ .attr = __ATTR(_name, 0444, _show, NULL), \
+ .id = _id, \
};

+#define EVENT_ATTR(_name, _id) \
+ PERF_EVENT_ATTR(_name, PERF_COUNT_HW_##_id, events_sysfs_show)
+
EVENT_ATTR(cpu-cycles, CPU_CYCLES );
EVENT_ATTR(instructions, INSTRUCTIONS );
EVENT_ATTR(cache-references, CACHE_REFERENCES );

|
|
| > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
| > index aa2465e..19b23bd 100644
| > --- a/arch/powerpc/perf/core-book3s.c
| > +++ b/arch/powerpc/perf/core-book3s.c
| > @@ -1305,6 +1305,16 @@ static int power_pmu_event_idx(struct perf_event *event)
| > return event->hw.idx;
| > }
| >
| > +ssize_t power_events_sysfs_show(struct device *dev,
| > + struct device_attribute *attr, char *page)
| > +{
| > + struct perf_pmu_events_attr *pmu_attr;
| > +
| > + pmu_attr = container_of(attr, struct perf_pmu_events_attr, attr);
| > +
| > + return sprintf(page, "event=0x%02llx\n", pmu_attr->id);
|
| whitespace issues

Will fix. Thanks for the review.

Sukadev

2012-11-16 12:52:15

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs

On Wed, Nov 14, 2012 at 10:20:45AM -0800, Sukadev Bhattiprolu wrote:
> Jiri Olsa [[email protected]] wrote:
> | On Wed, Nov 07, 2012 at 11:19:28AM -0800, Sukadev Bhattiprolu wrote:
> |
> | SNIP
> |
> | > +struct perf_pmu_events_attr {
> | > + struct device_attribute attr;
> | > + u64 id;
> | > +};
> | > +
> | > +extern ssize_t power_events_sysfs_show(struct device *dev,
> | > + struct device_attribute *attr, char *page);
> | > +
> | > +#define EVENT_VAR(_id) event_attr_##_id
> | > +#define EVENT_PTR(_id) &event_attr_##_id.attr.attr
> | > +
> | > +#define EVENT_ATTR(_name, _id) \
> | > + static struct perf_pmu_events_attr EVENT_VAR(_id) = { \
> | > + .attr = __ATTR(_name, 0444, power_events_sysfs_show, NULL),\
> | > + .id = PM_##_id, \
> | > + };
> |
> | this is duplicating the x86 code, perhaps it could be moved
> | to include/linux/perf_event.h and shared globaly
>
> Ok.
>
> Can we remove the assumption that the event id is a generic event that
> has PERF_COUNT_HW_ prefix and also let the architectures pass in a "show"
> function ? This would allow architectures to display any arch specific
> events that don't yet have a generic counterpart.
>
> IOW, can we do something like this (untested) and make PERF_EVENT_ATTR global:

hm, then you probably can use following:

http://www.spinics.net/lists/kernel/msg1434233.html
http://www.spinics.net/lists/kernel/msg1434235.html
http://www.spinics.net/lists/kernel/msg1434226.html

jirka

2012-11-16 19:34:38

by Sukadev Bhattiprolu

[permalink] [raw]
Subject: Re: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs

Jiri Olsa [[email protected]] wrote:
| >
| > Can we remove the assumption that the event id is a generic event that
| > has PERF_COUNT_HW_ prefix and also let the architectures pass in a "show"
| > function ? This would allow architectures to display any arch specific
| > events that don't yet have a generic counterpart.
| >
| > IOW, can we do something like this (untested) and make PERF_EVENT_ATTR global:
|
| hm, then you probably can use following:
|
| http://www.spinics.net/lists/kernel/msg1434233.html

For now, power events can simply be u64 - so am hoping to have something
like this and replace the raw codes:

#define PM_CYC 0x1e
#define PM_GCT_NOSLOT_CYC 0x100f8

EVENT_ATTR_STR() is interesting, but would require new/extra macros like

#define PM_CYC_STR "0x1e"
#define PM_GCT_NOSLOT_CYC_STR "0x100f8"

I went down the path we discussed earlier - to have x86 and Power share
the EVENT_ATTR() macros. This is making the x86 code less concise

EVENT_ATTR(cpu-cycles, CPU_CYCLES)

becomes

EVENT_ATTR(cpu-cycles, PERF_COUNT_HW_CPU_CYCLES, events_sysfs_show)

with each EVENT_ATTR() explcitly adding events_sysfs_show or needing another
wrapper around the wrappers.

Still tweaking it, but would it be flexible to make 'struct pmu_events_attr'
common and let architectures define the EVENT_ATTR() as needed ? Would
duplicate some code but hopefully not a lot.

Sukadev

2012-11-19 20:34:37

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH 3/4] perf/POWER7: Make event translations available in sysfs

On Fri, Nov 16, 2012 at 11:35:37AM -0800, Sukadev Bhattiprolu wrote:
> Jiri Olsa [[email protected]] wrote:
> | >
> | > Can we remove the assumption that the event id is a generic event that
> | > has PERF_COUNT_HW_ prefix and also let the architectures pass in a "show"
> | > function ? This would allow architectures to display any arch specific
> | > events that don't yet have a generic counterpart.
> | >
> | > IOW, can we do something like this (untested) and make PERF_EVENT_ATTR global:
> |
> | hm, then you probably can use following:
> |
> | http://www.spinics.net/lists/kernel/msg1434233.html
>
> For now, power events can simply be u64 - so am hoping to have something
> like this and replace the raw codes:
>
> #define PM_CYC 0x1e
> #define PM_GCT_NOSLOT_CYC 0x100f8
>
> EVENT_ATTR_STR() is interesting, but would require new/extra macros like
>
> #define PM_CYC_STR "0x1e"
> #define PM_GCT_NOSLOT_CYC_STR "0x100f8"
>
> I went down the path we discussed earlier - to have x86 and Power share
> the EVENT_ATTR() macros. This is making the x86 code less concise
>
> EVENT_ATTR(cpu-cycles, CPU_CYCLES)
>
> becomes
>
> EVENT_ATTR(cpu-cycles, PERF_COUNT_HW_CPU_CYCLES, events_sysfs_show)
>
> with each EVENT_ATTR() explcitly adding events_sysfs_show or needing another
> wrapper around the wrappers.
>
> Still tweaking it, but would it be flexible to make 'struct pmu_events_attr'
> common and let architectures define the EVENT_ATTR() as needed ? Would
> duplicate some code but hopefully not a lot.

sounds good, let's see ;)

jirka

2012-11-20 10:43:56

by suzuki

[permalink] [raw]
Subject: Re: [PATCH 1/4] perf/powerpc: Use uapi/unistd.h to fix build error

On 11/08/2012 12:48 AM, Sukadev Bhattiprolu wrote:
>
> From b8beef080260c1625c8f801105504a82005295e5 Mon Sep 17 00:00:00 2001
> From: Sukadev Bhattiprolu <[email protected]>
> Date: Wed, 31 Oct 2012 11:21:28 -0700
> Subject: [PATCH 1/4] perf/powerpc: Use uapi/unistd.h to fix build error
>
> Use the 'unistd.h' from arch/powerpc/include/uapi to build the perf tool.
>
> Signed-off-by: Sukadev Bhattiprolu <[email protected]>
Without this patch, I couldn't build perf on powerpc, with 3.7.0-rc2

Tested-by: Suzuki K. Poulose <[email protected]>

Thanks
Suzuki
> ---
> tools/perf/perf.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 054182e..f4952da 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -26,7 +26,7 @@ void get_term_dimensions(struct winsize *ws);
> #endif
>
> #ifdef __powerpc__
> -#include "../../arch/powerpc/include/asm/unistd.h"
> +#include "../../arch/powerpc/include/uapi/asm/unistd.h"
> #define rmb() asm volatile ("sync" ::: "memory")
> #define cpu_relax() asm volatile ("" ::: "memory");
> #define CPUINFO_PROC "cpu"
>