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

2012-11-19 Thread Jiri Olsa 
On Fri, Nov 16, 2012 at 11:35:37AM -0800, Sukadev Bhattiprolu wrote:
 Jiri Olsa [jo...@redhat.com] 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_CYC0x1e
 #define PM_GCT_NOSLOT_CYC 0x100f8
 
 EVENT_ATTR_STR() is interesting, but would require new/extra macros like
 
 #define PM_CYC_STR0x1e
 #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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2012-11-16 Thread Jiri Olsa 
On Wed, Nov 14, 2012 at 10:20:45AM -0800, Sukadev Bhattiprolu wrote:
 Jiri Olsa [jo...@redhat.com] 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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2012-11-16 Thread Sukadev Bhattiprolu 
Jiri Olsa [jo...@redhat.com] 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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2012-11-14 Thread Jiri Olsa 
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2012-11-14 Thread Sukadev Bhattiprolu 
Jiri Olsa [jo...@redhat.com] 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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

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

2012-11-07 Thread Sukadev Bhattiprolu 

From d05d1ce6d55bf339eee6230ded9f5dd1351f60e5 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
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 jo...@redhat.com
Date:   Wed Oct 10 14:53:11 2012 +0200

perf/x86: Make hardware event translations available in sysfs

Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
---
 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