[PATCH 2/7] perf: New helper function for pmu name

[PATCH 2/7] perf: New helper function for pmu name

[Matt Fleming]

Introduce perf_pmu_name() helper function that returns the name of the
pmu. This gives us a generic way to get the name of a pmu regardless of
how an architecture identifies it internally, e.g. ARM uses an id
whereas SH currently uses a string.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
 arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
 arch/arm/oprofile/common.c   |   22 +---------------------
 arch/sh/kernel/perf_event.c  |   14 ++++++++++++++
 include/linux/perf_event.h   |    1 +
 kernel/perf_event.c          |    5 +++++
 5 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index ef3bc33..3bff24d 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -123,6 +123,29 @@ armpmu_get_max_events(void)
 }
 EXPORT_SYMBOL_GPL(armpmu_get_max_events);
 
+const char *perf_pmu_name(void)
+{
+	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
+
+	switch (id) {
+	case ARM_PERF_PMU_ID_XSCALE1:
+		return "arm/xscale1";
+	case ARM_PERF_PMU_ID_XSCALE2:
+		return "arm/xscale2";
+	case ARM_PERF_PMU_ID_V6:
+		return "arm/armv6";
+	case ARM_PERF_PMU_ID_V6MP:
+		return "arm/mpcore";
+	case ARM_PERF_PMU_ID_CA8:
+		return "arm/armv7";
+	case ARM_PERF_PMU_ID_CA9:
+		return "arm/armv7-ca9";
+	default:
+		return NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(perf_pmu_name);
+
 int perf_num_counters(void)
 {
 	return armpmu_get_max_events();
diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 23f18a0..cb224ee 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -155,26 +155,6 @@ static void op_perf_stop(void)
 }
 
 
-static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
-{
-	switch (id) {
-	case ARM_PERF_PMU_ID_XSCALE1:
-		return "arm/xscale1";
-	case ARM_PERF_PMU_ID_XSCALE2:
-		return "arm/xscale2";
-	case ARM_PERF_PMU_ID_V6:
-		return "arm/armv6";
-	case ARM_PERF_PMU_ID_V6MP:
-		return "arm/mpcore";
-	case ARM_PERF_PMU_ID_CA8:
-		return "arm/armv7";
-	case ARM_PERF_PMU_ID_CA9:
-		return "arm/armv7-ca9";
-	default:
-		return NULL;
-	}
-}
-
 static int op_arm_create_files(struct super_block *sb, struct dentry *root)
 {
 	unsigned int i;
@@ -391,7 +371,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	ops->start		= op_arm_start;
 	ops->stop		= op_arm_stop;
 	ops->shutdown		= op_arm_stop;
-	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
+	ops->cpu_type		= perf_pmu_name();
 
 	if (!ops->cpu_type)
 		ret = -ENODEV;
diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
index 2cb9ad5..e065a1d 100644
--- a/arch/sh/kernel/perf_event.c
+++ b/arch/sh/kernel/perf_event.c
@@ -59,6 +59,20 @@ static inline int sh_pmu_initialized(void)
 	return !!sh_pmu;
 }
 
+const char *perf_pmu_name(void)
+{
+	if (!sh_pmu)
+		return NULL;
+
+	if (!strcmp(sh_pmu->name, "SH7750"))
+		return "sh/sh7750";
+	if (!strcmp(sh_pmu->name, "SH-4A"))
+		return "sh/sh4a";
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(perf_pmu_name);
+
 int perf_num_counters(void)
 {
 	if (!sh_pmu)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1a02192..33f08da 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -850,6 +850,7 @@ extern int perf_max_events;
 extern const struct pmu *hw_perf_event_init(struct perf_event *event);
 
 extern int perf_num_counters(void);
+extern const char *perf_pmu_name(void);
 extern void perf_event_task_sched_in(struct task_struct *task);
 extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
 extern void perf_event_task_tick(struct task_struct *task);
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index db5b560..fc51268 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
 
 void __weak perf_event_print_debug(void)	{ }
 
+extern __weak const char *perf_pmu_name(void)
+{
+	return "pmu";
+}
+
 static DEFINE_PER_CPU(int, perf_disable_count);
 
 void perf_disable(void)
-- 
1.7.1

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Mon, Oct 04, 2010 at 09:44:20PM +0100, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally, e.g. ARM uses an id
> whereas SH currently uses a string.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>

This looks fine too. Later we can see about switching the SH case over to
CPU subtype IDs, but it doesn't matter for now.

Acked-by: Paul Mundt <lethal@linux-sh.org>

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Mon, Oct 04, 2010 at 09:44:20PM +0100, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally, e.g. ARM uses an id
> whereas SH currently uses a string.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>

This looks fine too. Later we can see about switching the SH case over to
CPU subtype IDs, but it doesn't matter for now.

Acked-by: Paul Mundt <lethal@linux-sh.org>

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 04.10.10 16:44:20, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally, e.g. ARM uses an id
> whereas SH currently uses a string.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
>  arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
>  arch/arm/oprofile/common.c   |   22 +---------------------
>  arch/sh/kernel/perf_event.c  |   14 ++++++++++++++
>  include/linux/perf_event.h   |    1 +
>  kernel/perf_event.c          |    5 +++++
>  5 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index ef3bc33..3bff24d 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -123,6 +123,29 @@ armpmu_get_max_events(void)
>  }
>  EXPORT_SYMBOL_GPL(armpmu_get_max_events);
>  
> +const char *perf_pmu_name(void)
> +{
> +	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
> +
> +	switch (id) {
> +	case ARM_PERF_PMU_ID_XSCALE1:
> +		return "arm/xscale1";
> +	case ARM_PERF_PMU_ID_XSCALE2:
> +		return "arm/xscale2";
> +	case ARM_PERF_PMU_ID_V6:
> +		return "arm/armv6";
> +	case ARM_PERF_PMU_ID_V6MP:
> +		return "arm/mpcore";
> +	case ARM_PERF_PMU_ID_CA8:
> +		return "arm/armv7";
> +	case ARM_PERF_PMU_ID_CA9:
> +		return "arm/armv7-ca9";
> +	default:
> +		return NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);
> +
>  int perf_num_counters(void)
>  {
>  	return armpmu_get_max_events();
> diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> index 23f18a0..cb224ee 100644
> --- a/arch/arm/oprofile/common.c
> +++ b/arch/arm/oprofile/common.c
> @@ -155,26 +155,6 @@ static void op_perf_stop(void)
>  }
>  
>  
> -static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
> -{
> -	switch (id) {
> -	case ARM_PERF_PMU_ID_XSCALE1:
> -		return "arm/xscale1";
> -	case ARM_PERF_PMU_ID_XSCALE2:
> -		return "arm/xscale2";
> -	case ARM_PERF_PMU_ID_V6:
> -		return "arm/armv6";
> -	case ARM_PERF_PMU_ID_V6MP:
> -		return "arm/mpcore";
> -	case ARM_PERF_PMU_ID_CA8:
> -		return "arm/armv7";
> -	case ARM_PERF_PMU_ID_CA9:
> -		return "arm/armv7-ca9";
> -	default:
> -		return NULL;
> -	}
> -}

I don't like moving this oprofile names out to perf. These strings
don't have much to do with the pmu names and are hints for the
oprofile daemon to detect the pmu type with no other use. Of course
this is more human readable than an enum or so, but it is of no worth
other than oprofile. So it should be kept inside oprofile.

> -
>  static int op_arm_create_files(struct super_block *sb, struct dentry *root)
>  {
>  	unsigned int i;
> @@ -391,7 +371,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
>  	ops->start		= op_arm_start;
>  	ops->stop		= op_arm_stop;
>  	ops->shutdown		= op_arm_stop;
> -	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
> +	ops->cpu_type		= perf_pmu_name();
>  
>  	if (!ops->cpu_type)
>  		ret = -ENODEV;
> diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> index 2cb9ad5..e065a1d 100644
> --- a/arch/sh/kernel/perf_event.c
> +++ b/arch/sh/kernel/perf_event.c
> @@ -59,6 +59,20 @@ static inline int sh_pmu_initialized(void)
>  	return !!sh_pmu;
>  }
>  
> +const char *perf_pmu_name(void)
> +{
> +	if (!sh_pmu)
> +		return NULL;
> +
> +	if (!strcmp(sh_pmu->name, "SH7750"))
> +		return "sh/sh7750";
> +	if (!strcmp(sh_pmu->name, "SH-4A"))
> +		return "sh/sh4a";
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);

I rather want use here the solution we discussed earlier, simply
including <asm/perf_event.h> and then access sh_pmu->name directly
from oprofile.

-Robert

> +
>  int perf_num_counters(void)
>  {
>  	if (!sh_pmu)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1a02192..33f08da 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -850,6 +850,7 @@ extern int perf_max_events;
>  extern const struct pmu *hw_perf_event_init(struct perf_event *event);
>  
>  extern int perf_num_counters(void);
> +extern const char *perf_pmu_name(void);
>  extern void perf_event_task_sched_in(struct task_struct *task);
>  extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
>  extern void perf_event_task_tick(struct task_struct *task);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index db5b560..fc51268 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
>  
>  void __weak perf_event_print_debug(void)	{ }
>  
> +extern __weak const char *perf_pmu_name(void)
> +{
> +	return "pmu";
> +}
> +
>  static DEFINE_PER_CPU(int, perf_disable_count);
>  
>  void perf_disable(void)
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 04.10.10 16:44:20, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally, e.g. ARM uses an id
> whereas SH currently uses a string.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
>  arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
>  arch/arm/oprofile/common.c   |   22 +---------------------
>  arch/sh/kernel/perf_event.c  |   14 ++++++++++++++
>  include/linux/perf_event.h   |    1 +
>  kernel/perf_event.c          |    5 +++++
>  5 files changed, 44 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index ef3bc33..3bff24d 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -123,6 +123,29 @@ armpmu_get_max_events(void)
>  }
>  EXPORT_SYMBOL_GPL(armpmu_get_max_events);
>  
> +const char *perf_pmu_name(void)
> +{
> +	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
> +
> +	switch (id) {
> +	case ARM_PERF_PMU_ID_XSCALE1:
> +		return "arm/xscale1";
> +	case ARM_PERF_PMU_ID_XSCALE2:
> +		return "arm/xscale2";
> +	case ARM_PERF_PMU_ID_V6:
> +		return "arm/armv6";
> +	case ARM_PERF_PMU_ID_V6MP:
> +		return "arm/mpcore";
> +	case ARM_PERF_PMU_ID_CA8:
> +		return "arm/armv7";
> +	case ARM_PERF_PMU_ID_CA9:
> +		return "arm/armv7-ca9";
> +	default:
> +		return NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);
> +
>  int perf_num_counters(void)
>  {
>  	return armpmu_get_max_events();
> diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> index 23f18a0..cb224ee 100644
> --- a/arch/arm/oprofile/common.c
> +++ b/arch/arm/oprofile/common.c
> @@ -155,26 +155,6 @@ static void op_perf_stop(void)
>  }
>  
>  
> -static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
> -{
> -	switch (id) {
> -	case ARM_PERF_PMU_ID_XSCALE1:
> -		return "arm/xscale1";
> -	case ARM_PERF_PMU_ID_XSCALE2:
> -		return "arm/xscale2";
> -	case ARM_PERF_PMU_ID_V6:
> -		return "arm/armv6";
> -	case ARM_PERF_PMU_ID_V6MP:
> -		return "arm/mpcore";
> -	case ARM_PERF_PMU_ID_CA8:
> -		return "arm/armv7";
> -	case ARM_PERF_PMU_ID_CA9:
> -		return "arm/armv7-ca9";
> -	default:
> -		return NULL;
> -	}
> -}

I don't like moving this oprofile names out to perf. These strings
don't have much to do with the pmu names and are hints for the
oprofile daemon to detect the pmu type with no other use. Of course
this is more human readable than an enum or so, but it is of no worth
other than oprofile. So it should be kept inside oprofile.

> -
>  static int op_arm_create_files(struct super_block *sb, struct dentry *root)
>  {
>  	unsigned int i;
> @@ -391,7 +371,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
>  	ops->start		= op_arm_start;
>  	ops->stop		= op_arm_stop;
>  	ops->shutdown		= op_arm_stop;
> -	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
> +	ops->cpu_type		= perf_pmu_name();
>  
>  	if (!ops->cpu_type)
>  		ret = -ENODEV;
> diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> index 2cb9ad5..e065a1d 100644
> --- a/arch/sh/kernel/perf_event.c
> +++ b/arch/sh/kernel/perf_event.c
> @@ -59,6 +59,20 @@ static inline int sh_pmu_initialized(void)
>  	return !!sh_pmu;
>  }
>  
> +const char *perf_pmu_name(void)
> +{
> +	if (!sh_pmu)
> +		return NULL;
> +
> +	if (!strcmp(sh_pmu->name, "SH7750"))
> +		return "sh/sh7750";
> +	if (!strcmp(sh_pmu->name, "SH-4A"))
> +		return "sh/sh4a";
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);

I rather want use here the solution we discussed earlier, simply
including <asm/perf_event.h> and then access sh_pmu->name directly
from oprofile.

-Robert

> +
>  int perf_num_counters(void)
>  {
>  	if (!sh_pmu)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1a02192..33f08da 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -850,6 +850,7 @@ extern int perf_max_events;
>  extern const struct pmu *hw_perf_event_init(struct perf_event *event);
>  
>  extern int perf_num_counters(void);
> +extern const char *perf_pmu_name(void);
>  extern void perf_event_task_sched_in(struct task_struct *task);
>  extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
>  extern void perf_event_task_tick(struct task_struct *task);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index db5b560..fc51268 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
>  
>  void __weak perf_event_print_debug(void)	{ }
>  
> +extern __weak const char *perf_pmu_name(void)
> +{
> +	return "pmu";
> +}
> +
>  static DEFINE_PER_CPU(int, perf_disable_count);
>  
>  void perf_disable(void)
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> On 04.10.10 16:44:20, Matt Fleming wrote:
> > Introduce perf_pmu_name() helper function that returns the name of the
> > pmu. This gives us a generic way to get the name of a pmu regardless of
> > how an architecture identifies it internally, e.g. ARM uses an id
> > whereas SH currently uses a string.
> 
> I rather want use here the solution we discussed earlier, simply
> including <asm/perf_event.h> and then access sh_pmu->name directly
> from oprofile.
> 
No. Exposing sh_pmu generically is unacceptable. This is already
centrally managed through perf, and if oprofile needs any additional
information then it needs to get that through the perf layer. We already
have the situation that effectively every architecture with perf support
implements a name string already, so making this part of the perf API
hardly seems like that big of a stretch. If the perf people are violently
opposed to this, then of course we can look at alternatives.

sh_pmu is created for use solely by the perf events code, we are not
going to have oprofile poking around in data structures it has no place
knowing anything about when 99% of everything else it is doing is already
abstracted cleanly through the perf events interfaces. Likewise for
special oprofile-specific APIs.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> On 04.10.10 16:44:20, Matt Fleming wrote:
> > Introduce perf_pmu_name() helper function that returns the name of the
> > pmu. This gives us a generic way to get the name of a pmu regardless of
> > how an architecture identifies it internally, e.g. ARM uses an id
> > whereas SH currently uses a string.
> 
> I rather want use here the solution we discussed earlier, simply
> including <asm/perf_event.h> and then access sh_pmu->name directly
> from oprofile.
> 
No. Exposing sh_pmu generically is unacceptable. This is already
centrally managed through perf, and if oprofile needs any additional
information then it needs to get that through the perf layer. We already
have the situation that effectively every architecture with perf support
implements a name string already, so making this part of the perf API
hardly seems like that big of a stretch. If the perf people are violently
opposed to this, then of course we can look at alternatives.

sh_pmu is created for use solely by the perf events code, we are not
going to have oprofile poking around in data structures it has no place
knowing anything about when 99% of everything else it is doing is already
abstracted cleanly through the perf events interfaces. Likewise for
special oprofile-specific APIs.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

(cc'ing Peter)

On 06.10.10 08:39:50, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> > On 04.10.10 16:44:20, Matt Fleming wrote:
> > > Introduce perf_pmu_name() helper function that returns the name of the
> > > pmu. This gives us a generic way to get the name of a pmu regardless of
> > > how an architecture identifies it internally, e.g. ARM uses an id
> > > whereas SH currently uses a string.
> > 
> > I rather want use here the solution we discussed earlier, simply
> > including <asm/perf_event.h> and then access sh_pmu->name directly
> > from oprofile.
> > 
> No. Exposing sh_pmu generically is unacceptable. This is already

I don't want to be the perf_pmu_name() discussion a show stopper for
this patch set. We don't have a generic perf interface available now
that allows us to detect the pmu type for sh cpus. I also don't see a
name string as a final solution for pmu detection. So it will be more
than just adding perf_pmu_name().

Until then, as we currently would only need the name for SH, I don't
see something wrong here to include architectural interfaces from
<asm/perf_event.h> directly by architectural code until a general
solution is available.  If you run a 'git grep include..asm/perf' this
is common for other architectures.

> centrally managed through perf, and if oprofile needs any additional
> information then it needs to get that through the perf layer. We already
> have the situation that effectively every architecture with perf support
> implements a name string already, so making this part of the perf API
> hardly seems like that big of a stretch. If the perf people are violently
> opposed to this, then of course we can look at alternatives.

I am not against adding the pmu name to the perf API. But the oprofile
cpu_type strings are oprofile centric esp. for the userland. So these
strings will remain part of oprofile. Also I don't think we want to
polute the perf pmu names with it.

> sh_pmu is created for use solely by the perf events code, we are not
> going to have oprofile poking around in data structures it has no place
> knowing anything about when 99% of everything else it is doing is already
> abstracted cleanly through the perf events interfaces. Likewise for
> special oprofile-specific APIs.

So, I am also fine with implementing a generic perf_pmu_name() for sh
and then derive the oprofile cpu_type string from it in the oprofile
code.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

(cc'ing Peter)

On 06.10.10 08:39:50, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 02:27:36PM +0200, Robert Richter wrote:
> > On 04.10.10 16:44:20, Matt Fleming wrote:
> > > Introduce perf_pmu_name() helper function that returns the name of the
> > > pmu. This gives us a generic way to get the name of a pmu regardless of
> > > how an architecture identifies it internally, e.g. ARM uses an id
> > > whereas SH currently uses a string.
> > 
> > I rather want use here the solution we discussed earlier, simply
> > including <asm/perf_event.h> and then access sh_pmu->name directly
> > from oprofile.
> > 
> No. Exposing sh_pmu generically is unacceptable. This is already

I don't want to be the perf_pmu_name() discussion a show stopper for
this patch set. We don't have a generic perf interface available now
that allows us to detect the pmu type for sh cpus. I also don't see a
name string as a final solution for pmu detection. So it will be more
than just adding perf_pmu_name().

Until then, as we currently would only need the name for SH, I don't
see something wrong here to include architectural interfaces from
<asm/perf_event.h> directly by architectural code until a general
solution is available.  If you run a 'git grep include..asm/perf' this
is common for other architectures.

> centrally managed through perf, and if oprofile needs any additional
> information then it needs to get that through the perf layer. We already
> have the situation that effectively every architecture with perf support
> implements a name string already, so making this part of the perf API
> hardly seems like that big of a stretch. If the perf people are violently
> opposed to this, then of course we can look at alternatives.

I am not against adding the pmu name to the perf API. But the oprofile
cpu_type strings are oprofile centric esp. for the userland. So these
strings will remain part of oprofile. Also I don't think we want to
polute the perf pmu names with it.

> sh_pmu is created for use solely by the perf events code, we are not
> going to have oprofile poking around in data structures it has no place
> knowing anything about when 99% of everything else it is doing is already
> abstracted cleanly through the perf events interfaces. Likewise for
> special oprofile-specific APIs.

So, I am also fine with implementing a generic perf_pmu_name() for sh
and then derive the oprofile cpu_type string from it in the oprofile
code.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> I don't want to be the perf_pmu_name() discussion a show stopper for
> this patch set. We don't have a generic perf interface available now
> that allows us to detect the pmu type for sh cpus. I also don't see a
> name string as a final solution for pmu detection. So it will be more
> than just adding perf_pmu_name().
> 
Whether you see it as a show stopper or not is not the issue, I will not
expose sh_pmu generically. Nor do I imagine that the x86 people or any
other architecture wishing to tie in to the oprofile wrapper are
interested in doing the equivalent there. We have a registration
interface to do, and I will not provide oprofile-specific workarounds
for it hidden in the arch code.

> Until then, as we currently would only need the name for SH, I don't
> see something wrong here to include architectural interfaces from
> <asm/perf_event.h> directly by architectural code until a general
> solution is available.  If you run a 'git grep include..asm/perf' this
> is common for other architectures.
> 
This is nonsense. Every architecture that needs to support the oprofile
userspace tools needs to support the string interface. At the same time,
everyone supporting hardware perf events in the kernel today already has
a string to pmu mapping somewhere in their data structures. This can
easily be exposed generically, and platforms that have special mangling
they need to do before exposing the string to oprofile can do so if the
need to.

> I am not against adding the pmu name to the perf API. But the oprofile
> cpu_type strings are oprofile centric esp. for the userland. So these
> strings will remain part of oprofile. Also I don't think we want to
> polute the perf pmu names with it.
> 
And those architectures that have opted to use different strings for perf
events are free to mangle them however they want for the oprofile case.
It doesn't change the fact that strings are still being managed by all of
the architectures. The perf PMU names aren't presently locked in to an
ABI, whereas the oprofile strings are, so it seems fairly straightforward
to develop standard mangling rules for preventing an oprofile-facing
string, or to simply reuse the strings verbatim.

> > sh_pmu is created for use solely by the perf events code, we are not
> > going to have oprofile poking around in data structures it has no place
> > knowing anything about when 99% of everything else it is doing is already
> > abstracted cleanly through the perf events interfaces. Likewise for
> > special oprofile-specific APIs.
> 
> So, I am also fine with implementing a generic perf_pmu_name() for sh
> and then derive the oprofile cpu_type string from it in the oprofile
> code.
> 
Again, this is unacceptable.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 10:30:41PM +0900, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > I am not against adding the pmu name to the perf API. But the oprofile
> > cpu_type strings are oprofile centric esp. for the userland. So these
> > strings will remain part of oprofile. Also I don't think we want to
> > polute the perf pmu names with it.
> > 
> And those architectures that have opted to use different strings for perf
> events are free to mangle them however they want for the oprofile case.
> It doesn't change the fact that strings are still being managed by all of
> the architectures. The perf PMU names aren't presently locked in to an
> ABI, whereas the oprofile strings are, so it seems fairly straightforward
> to develop standard mangling rules for preventing an oprofile-facing
> string, or to simply reuse the strings verbatim.
> 
So to add a bit of context here, I was just looking at the oprofile
tools. The naming format here is one of:

	<arch>/<pmu>

if there were a generic perf to oprofile pmu name mangler that did this
it would cover almost all of the ARM cases already, the SH strings I'm
happy to convert to work this way, and a good chunk of the PowerPC PMUs
would work fine, too. PowerPC already has an oprofile CPU string in its
CPU spec, so this would be even more trivial to wire up there if such a
generic interface were to exist.

This would just leave x86 as the odd one out, but I suppose if x86 were
to move to the oprofile perf wrapper in the future then a bit of id to
name mangling as an override wouldn't be too much work.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 10:30:41PM +0900, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > I am not against adding the pmu name to the perf API. But the oprofile
> > cpu_type strings are oprofile centric esp. for the userland. So these
> > strings will remain part of oprofile. Also I don't think we want to
> > polute the perf pmu names with it.
> > 
> And those architectures that have opted to use different strings for perf
> events are free to mangle them however they want for the oprofile case.
> It doesn't change the fact that strings are still being managed by all of
> the architectures. The perf PMU names aren't presently locked in to an
> ABI, whereas the oprofile strings are, so it seems fairly straightforward
> to develop standard mangling rules for preventing an oprofile-facing
> string, or to simply reuse the strings verbatim.
> 
So to add a bit of context here, I was just looking at the oprofile
tools. The naming format here is one of:

	<arch>/<pmu>

if there were a generic perf to oprofile pmu name mangler that did this
it would cover almost all of the ARM cases already, the SH strings I'm
happy to convert to work this way, and a good chunk of the PowerPC PMUs
would work fine, too. PowerPC already has an oprofile CPU string in its
CPU spec, so this would be even more trivial to wire up there if such a
generic interface were to exist.

This would just leave x86 as the odd one out, but I suppose if x86 were
to move to the oprofile perf wrapper in the future then a bit of id to
name mangling as an override wouldn't be too much work.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 06.10.10 10:13:47, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 10:30:41PM +0900, Paul Mundt wrote:
> > On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > > I am not against adding the pmu name to the perf API. But the oprofile
> > > cpu_type strings are oprofile centric esp. for the userland. So these
> > > strings will remain part of oprofile. Also I don't think we want to
> > > polute the perf pmu names with it.
> > > 
> > And those architectures that have opted to use different strings for perf
> > events are free to mangle them however they want for the oprofile case.
> > It doesn't change the fact that strings are still being managed by all of
> > the architectures. The perf PMU names aren't presently locked in to an
> > ABI, whereas the oprofile strings are, so it seems fairly straightforward
> > to develop standard mangling rules for preventing an oprofile-facing
> > string, or to simply reuse the strings verbatim.
> > 
> So to add a bit of context here, I was just looking at the oprofile
> tools. The naming format here is one of:
> 
> 	<arch>/<pmu>

Do you really want the oprofile name scheme move to perf?

> 
> if there were a generic perf to oprofile pmu name mangler that did this

Or, do you mean here to derive an oprofile name from the pmu string in
a generic way? I was suggesting this for SH when commenting on version
3 of this patch set. We dropped this idea to keep changes simple for
this initial patch set, because it was much easier to implement it
with strcmp() and the pmu strings are not expected to be changed for
sh in the near future.

So, can't we use op_name_from_perf_name() from [PATCH -v3 6/6] for SH
here and implement a perf_pmu_name() function for sh that is part of
perf's generic interface? 

Later we can add a generic function for sh ...

> it would cover almost all of the ARM cases already, the SH strings I'm
> happy to convert to work this way, and a good chunk of the PowerPC PMUs
> would work fine, too. PowerPC already has an oprofile CPU string in its
> CPU spec, so this would be even more trivial to wire up there if such a
> generic interface were to exist.
> 
> This would just leave x86 as the odd one out, but I suppose if x86 were
> to move to the oprofile perf wrapper in the future then a bit of id to
> name mangling as an override wouldn't be too much work.

... and also other architectures on top of these patches.

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 06.10.10 10:13:47, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 10:30:41PM +0900, Paul Mundt wrote:
> > On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > > I am not against adding the pmu name to the perf API. But the oprofile
> > > cpu_type strings are oprofile centric esp. for the userland. So these
> > > strings will remain part of oprofile. Also I don't think we want to
> > > polute the perf pmu names with it.
> > > 
> > And those architectures that have opted to use different strings for perf
> > events are free to mangle them however they want for the oprofile case.
> > It doesn't change the fact that strings are still being managed by all of
> > the architectures. The perf PMU names aren't presently locked in to an
> > ABI, whereas the oprofile strings are, so it seems fairly straightforward
> > to develop standard mangling rules for preventing an oprofile-facing
> > string, or to simply reuse the strings verbatim.
> > 
> So to add a bit of context here, I was just looking at the oprofile
> tools. The naming format here is one of:
> 
> 	<arch>/<pmu>

Do you really want the oprofile name scheme move to perf?

> 
> if there were a generic perf to oprofile pmu name mangler that did this

Or, do you mean here to derive an oprofile name from the pmu string in
a generic way? I was suggesting this for SH when commenting on version
3 of this patch set. We dropped this idea to keep changes simple for
this initial patch set, because it was much easier to implement it
with strcmp() and the pmu strings are not expected to be changed for
sh in the near future.

So, can't we use op_name_from_perf_name() from [PATCH -v3 6/6] for SH
here and implement a perf_pmu_name() function for sh that is part of
perf's generic interface? 

Later we can add a generic function for sh ...

> it would cover almost all of the ARM cases already, the SH strings I'm
> happy to convert to work this way, and a good chunk of the PowerPC PMUs
> would work fine, too. PowerPC already has an oprofile CPU string in its
> CPU spec, so this would be even more trivial to wire up there if such a
> generic interface were to exist.
> 
> This would just leave x86 as the odd one out, but I suppose if x86 were
> to move to the oprofile perf wrapper in the future then a bit of id to
> name mangling as an override wouldn't be too much work.

... and also other architectures on top of these patches.

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 05:37:10PM +0200, Robert Richter wrote:
> On 06.10.10 10:13:47, Paul Mundt wrote:
> > if there were a generic perf to oprofile pmu name mangler that did this
> 
> Or, do you mean here to derive an oprofile name from the pmu string in
> a generic way? I was suggesting this for SH when commenting on version
> 3 of this patch set. We dropped this idea to keep changes simple for
> this initial patch set, because it was much easier to implement it
> with strcmp() and the pmu strings are not expected to be changed for
> sh in the near future.
> 
Yes, this is what I meant, so it seems we basically have a consensus
after all.

> So, can't we use op_name_from_perf_name() from [PATCH -v3 6/6] for SH
> here and implement a perf_pmu_name() function for sh that is part of
> perf's generic interface? 
> 
As long as perf_pmu_name() is generically provided and we just have
architecture overrides as necessary, then this is fine. If
perf_pmu_name() is __weak then we can provide an override in the SH code
that returns the PMU name without issue, and we don't have to expose
sh_pmu generically, so this is the solution I prefer.

That is assuming that perf folks are ok with a generic perf_pmu_name()
anyways!

Once that is provided, we can simply have the oprofile wrapper provide a
generic op_name_from_perf_name() that does the arch/perf_pmu_name()
string contstruction for the oprofile case, and then start killing off
the special cases.

> Later we can add a generic function for sh ...
> 
> > it would cover almost all of the ARM cases already, the SH strings I'm
> > happy to convert to work this way, and a good chunk of the PowerPC PMUs
> > would work fine, too. PowerPC already has an oprofile CPU string in its
> > CPU spec, so this would be even more trivial to wire up there if such a
> > generic interface were to exist.
> > 
> > This would just leave x86 as the odd one out, but I suppose if x86 were
> > to move to the oprofile perf wrapper in the future then a bit of id to
> > name mangling as an override wouldn't be too much work.
> 
> ... and also other architectures on top of these patches.
> 
That sounds like a plan to me.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 05:37:10PM +0200, Robert Richter wrote:
> On 06.10.10 10:13:47, Paul Mundt wrote:
> > if there were a generic perf to oprofile pmu name mangler that did this
> 
> Or, do you mean here to derive an oprofile name from the pmu string in
> a generic way? I was suggesting this for SH when commenting on version
> 3 of this patch set. We dropped this idea to keep changes simple for
> this initial patch set, because it was much easier to implement it
> with strcmp() and the pmu strings are not expected to be changed for
> sh in the near future.
> 
Yes, this is what I meant, so it seems we basically have a consensus
after all.

> So, can't we use op_name_from_perf_name() from [PATCH -v3 6/6] for SH
> here and implement a perf_pmu_name() function for sh that is part of
> perf's generic interface? 
> 
As long as perf_pmu_name() is generically provided and we just have
architecture overrides as necessary, then this is fine. If
perf_pmu_name() is __weak then we can provide an override in the SH code
that returns the PMU name without issue, and we don't have to expose
sh_pmu generically, so this is the solution I prefer.

That is assuming that perf folks are ok with a generic perf_pmu_name()
anyways!

Once that is provided, we can simply have the oprofile wrapper provide a
generic op_name_from_perf_name() that does the arch/perf_pmu_name()
string contstruction for the oprofile case, and then start killing off
the special cases.

> Later we can add a generic function for sh ...
> 
> > it would cover almost all of the ARM cases already, the SH strings I'm
> > happy to convert to work this way, and a good chunk of the PowerPC PMUs
> > would work fine, too. PowerPC already has an oprofile CPU string in its
> > CPU spec, so this would be even more trivial to wire up there if such a
> > generic interface were to exist.
> > 
> > This would just leave x86 as the odd one out, but I suppose if x86 were
> > to move to the oprofile perf wrapper in the future then a bit of id to
> > name mangling as an override wouldn't be too much work.
> 
> ... and also other architectures on top of these patches.
> 
That sounds like a plan to me.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 06.10.10 09:30:41, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > So, I am also fine with implementing a generic perf_pmu_name() for sh
> > and then derive the oprofile cpu_type string from it in the oprofile
> > code.
> > 
> Again, this is unacceptable.

Paul,

maybe I misunderstood you, but isn't this you preferred solution? We
make perf_pmu_name() part of the generic i/f and then derive the
oprofile name from the pmu name provided by perf.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 06.10.10 09:30:41, Paul Mundt wrote:
> On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > So, I am also fine with implementing a generic perf_pmu_name() for sh
> > and then derive the oprofile cpu_type string from it in the oprofile
> > code.
> > 
> Again, this is unacceptable.

Paul,

maybe I misunderstood you, but isn't this you preferred solution? We
make perf_pmu_name() part of the generic i/f and then derive the
oprofile name from the pmu name provided by perf.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 05:50:26PM +0200, Robert Richter wrote:
> On 06.10.10 09:30:41, Paul Mundt wrote:
> > On Wed, Oct 06, 2010 at 03:18:25PM +0200, Robert Richter wrote:
> > > So, I am also fine with implementing a generic perf_pmu_name() for sh
> > > and then derive the oprofile cpu_type string from it in the oprofile
> > > code.
> > > 
> > Again, this is unacceptable.
> 
> Paul,
> 
> maybe I misunderstood you, but isn't this you preferred solution? We
> make perf_pmu_name() part of the generic i/f and then derive the
> oprofile name from the pmu name provided by perf.
> 
Perhaps we've misunderstood each other. I thought what you meant is that
we would carry a special API deviation in the architecture code or expose
the sh_pmu structure to generic code, both of which I have no interest
in. My preferred solution is that we provide a perf_pmu_name() in the
generic perf code as a __weak and then override it in the sh code, where
we have sh_pmu visibility. This way we aren't providing any special APIs
and sh_pmu remains constrained to the sh perf events code.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 04.10.10 16:44:20, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally, e.g. ARM uses an id
> whereas SH currently uses a string.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
>  arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
>  arch/arm/oprofile/common.c   |   22 +---------------------
>  arch/sh/kernel/perf_event.c  |   14 ++++++++++++++
>  include/linux/perf_event.h   |    1 +
>  kernel/perf_event.c          |    5 +++++
>  5 files changed, 44 insertions(+), 21 deletions(-)
> 

Matt,

as an outcome of the discussion we have had in this thread I think we
agree on the following:

We extend the generic perf interface with perf_pmu_name() implenting a
default with the __weak attribute.  On sh we override this function
and will use it for perf/oprofile string translation. This will be
implemented in the oprofile function op_name_from_perf_name().

Later we implement a generic oprofile function that derives the
cpu_type from perf_pmu_name() in a generic way and add other
architectures. Thus, we will drop the ARM changes with this patch set.

Paul,

please correct me if I am wrong with the above.

Peter,

please ack the perf_pmu_name() interface introduced below.

See also my comments below. If there are any other objections, please
let me know.

Thanks,

-Robert

> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index ef3bc33..3bff24d 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -123,6 +123,29 @@ armpmu_get_max_events(void)
>  }
>  EXPORT_SYMBOL_GPL(armpmu_get_max_events);
>  
> +const char *perf_pmu_name(void)
> +{
> +	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
> +
> +	switch (id) {
> +	case ARM_PERF_PMU_ID_XSCALE1:
> +		return "arm/xscale1";
> +	case ARM_PERF_PMU_ID_XSCALE2:
> +		return "arm/xscale2";
> +	case ARM_PERF_PMU_ID_V6:
> +		return "arm/armv6";
> +	case ARM_PERF_PMU_ID_V6MP:
> +		return "arm/mpcore";
> +	case ARM_PERF_PMU_ID_CA8:
> +		return "arm/armv7";
> +	case ARM_PERF_PMU_ID_CA9:
> +		return "arm/armv7-ca9";
> +	default:
> +		return NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);
> +
>  int perf_num_counters(void)
>  {
>  	return armpmu_get_max_events();
> diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> index 23f18a0..cb224ee 100644
> --- a/arch/arm/oprofile/common.c
> +++ b/arch/arm/oprofile/common.c
> @@ -155,26 +155,6 @@ static void op_perf_stop(void)
>  }
>  
>  
> -static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
> -{
> -	switch (id) {
> -	case ARM_PERF_PMU_ID_XSCALE1:
> -		return "arm/xscale1";
> -	case ARM_PERF_PMU_ID_XSCALE2:
> -		return "arm/xscale2";
> -	case ARM_PERF_PMU_ID_V6:
> -		return "arm/armv6";
> -	case ARM_PERF_PMU_ID_V6MP:
> -		return "arm/mpcore";
> -	case ARM_PERF_PMU_ID_CA8:
> -		return "arm/armv7";
> -	case ARM_PERF_PMU_ID_CA9:
> -		return "arm/armv7-ca9";
> -	default:
> -		return NULL;
> -	}
> -}
> -
>  static int op_arm_create_files(struct super_block *sb, struct dentry *root)
>  {
>  	unsigned int i;
> @@ -391,7 +371,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
>  	ops->start		= op_arm_start;
>  	ops->stop		= op_arm_stop;
>  	ops->shutdown		= op_arm_stop;
> -	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
> +	ops->cpu_type		= perf_pmu_name();

Please drop all arm changes in this patch. We will do the change after
a generic patch for perf/oprofile string translation is available.

>  
>  	if (!ops->cpu_type)
>  		ret = -ENODEV;
> diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> index 2cb9ad5..e065a1d 100644
> --- a/arch/sh/kernel/perf_event.c
> +++ b/arch/sh/kernel/perf_event.c
> @@ -59,6 +59,20 @@ static inline int sh_pmu_initialized(void)
>  	return !!sh_pmu;
>  }
>  
> +const char *perf_pmu_name(void)
> +{
> +	if (!sh_pmu)
> +		return NULL;
> +
> +	if (!strcmp(sh_pmu->name, "SH7750"))
> +		return "sh/sh7750";
> +	if (!strcmp(sh_pmu->name, "SH-4A"))
> +		return "sh/sh4a";
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);

The implementation should be moved to function
op_name_from_perf_name() in sh oprofile code. perf_pmu_name() should
simply return sh_pmu->name.

> +
>  int perf_num_counters(void)
>  {
>  	if (!sh_pmu)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1a02192..33f08da 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -850,6 +850,7 @@ extern int perf_max_events;
>  extern const struct pmu *hw_perf_event_init(struct perf_event *event);
>  
>  extern int perf_num_counters(void);
> +extern const char *perf_pmu_name(void);
>  extern void perf_event_task_sched_in(struct task_struct *task);
>  extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
>  extern void perf_event_task_tick(struct task_struct *task);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index db5b560..fc51268 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
>  
>  void __weak perf_event_print_debug(void)	{ }
>  
> +extern __weak const char *perf_pmu_name(void)
> +{
> +	return "pmu";
> +}
> +
>  static DEFINE_PER_CPU(int, perf_disable_count);
>  
>  void perf_disable(void)
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 04.10.10 16:44:20, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally, e.g. ARM uses an id
> whereas SH currently uses a string.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
>  arch/arm/kernel/perf_event.c |   23 +++++++++++++++++++++++
>  arch/arm/oprofile/common.c   |   22 +---------------------
>  arch/sh/kernel/perf_event.c  |   14 ++++++++++++++
>  include/linux/perf_event.h   |    1 +
>  kernel/perf_event.c          |    5 +++++
>  5 files changed, 44 insertions(+), 21 deletions(-)
> 

Matt,

as an outcome of the discussion we have had in this thread I think we
agree on the following:

We extend the generic perf interface with perf_pmu_name() implenting a
default with the __weak attribute.  On sh we override this function
and will use it for perf/oprofile string translation. This will be
implemented in the oprofile function op_name_from_perf_name().

Later we implement a generic oprofile function that derives the
cpu_type from perf_pmu_name() in a generic way and add other
architectures. Thus, we will drop the ARM changes with this patch set.

Paul,

please correct me if I am wrong with the above.

Peter,

please ack the perf_pmu_name() interface introduced below.

See also my comments below. If there are any other objections, please
let me know.

Thanks,

-Robert

> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index ef3bc33..3bff24d 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -123,6 +123,29 @@ armpmu_get_max_events(void)
>  }
>  EXPORT_SYMBOL_GPL(armpmu_get_max_events);
>  
> +const char *perf_pmu_name(void)
> +{
> +	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
> +
> +	switch (id) {
> +	case ARM_PERF_PMU_ID_XSCALE1:
> +		return "arm/xscale1";
> +	case ARM_PERF_PMU_ID_XSCALE2:
> +		return "arm/xscale2";
> +	case ARM_PERF_PMU_ID_V6:
> +		return "arm/armv6";
> +	case ARM_PERF_PMU_ID_V6MP:
> +		return "arm/mpcore";
> +	case ARM_PERF_PMU_ID_CA8:
> +		return "arm/armv7";
> +	case ARM_PERF_PMU_ID_CA9:
> +		return "arm/armv7-ca9";
> +	default:
> +		return NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);
> +
>  int perf_num_counters(void)
>  {
>  	return armpmu_get_max_events();
> diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> index 23f18a0..cb224ee 100644
> --- a/arch/arm/oprofile/common.c
> +++ b/arch/arm/oprofile/common.c
> @@ -155,26 +155,6 @@ static void op_perf_stop(void)
>  }
>  
>  
> -static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
> -{
> -	switch (id) {
> -	case ARM_PERF_PMU_ID_XSCALE1:
> -		return "arm/xscale1";
> -	case ARM_PERF_PMU_ID_XSCALE2:
> -		return "arm/xscale2";
> -	case ARM_PERF_PMU_ID_V6:
> -		return "arm/armv6";
> -	case ARM_PERF_PMU_ID_V6MP:
> -		return "arm/mpcore";
> -	case ARM_PERF_PMU_ID_CA8:
> -		return "arm/armv7";
> -	case ARM_PERF_PMU_ID_CA9:
> -		return "arm/armv7-ca9";
> -	default:
> -		return NULL;
> -	}
> -}
> -
>  static int op_arm_create_files(struct super_block *sb, struct dentry *root)
>  {
>  	unsigned int i;
> @@ -391,7 +371,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
>  	ops->start		= op_arm_start;
>  	ops->stop		= op_arm_stop;
>  	ops->shutdown		= op_arm_stop;
> -	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
> +	ops->cpu_type		= perf_pmu_name();

Please drop all arm changes in this patch. We will do the change after
a generic patch for perf/oprofile string translation is available.

>  
>  	if (!ops->cpu_type)
>  		ret = -ENODEV;
> diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> index 2cb9ad5..e065a1d 100644
> --- a/arch/sh/kernel/perf_event.c
> +++ b/arch/sh/kernel/perf_event.c
> @@ -59,6 +59,20 @@ static inline int sh_pmu_initialized(void)
>  	return !!sh_pmu;
>  }
>  
> +const char *perf_pmu_name(void)
> +{
> +	if (!sh_pmu)
> +		return NULL;
> +
> +	if (!strcmp(sh_pmu->name, "SH7750"))
> +		return "sh/sh7750";
> +	if (!strcmp(sh_pmu->name, "SH-4A"))
> +		return "sh/sh4a";
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);

The implementation should be moved to function
op_name_from_perf_name() in sh oprofile code. perf_pmu_name() should
simply return sh_pmu->name.

> +
>  int perf_num_counters(void)
>  {
>  	if (!sh_pmu)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1a02192..33f08da 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -850,6 +850,7 @@ extern int perf_max_events;
>  extern const struct pmu *hw_perf_event_init(struct perf_event *event);
>  
>  extern int perf_num_counters(void);
> +extern const char *perf_pmu_name(void);
>  extern void perf_event_task_sched_in(struct task_struct *task);
>  extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
>  extern void perf_event_task_tick(struct task_struct *task);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index db5b560..fc51268 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
>  
>  void __weak perf_event_print_debug(void)	{ }
>  
> +extern __weak const char *perf_pmu_name(void)
> +{
> +	return "pmu";
> +}
> +
>  static DEFINE_PER_CPU(int, perf_disable_count);
>  
>  void perf_disable(void)
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 08:15:48PM +0200, Robert Richter wrote:
> On 04.10.10 16:44:20, Matt Fleming wrote:
> > Introduce perf_pmu_name() helper function that returns the name of the
> > pmu. This gives us a generic way to get the name of a pmu regardless of
> > how an architecture identifies it internally, e.g. ARM uses an id
> > whereas SH currently uses a string.
> > 
> 
> Matt,
> 
> as an outcome of the discussion we have had in this thread I think we
> agree on the following:
> 
> We extend the generic perf interface with perf_pmu_name() implenting a
> default with the __weak attribute.  On sh we override this function
> and will use it for perf/oprofile string translation. This will be
> implemented in the oprofile function op_name_from_perf_name().
> 
> Later we implement a generic oprofile function that derives the
> cpu_type from perf_pmu_name() in a generic way and add other
> architectures. Thus, we will drop the ARM changes with this patch set.
> 
> Paul,
> 
> please correct me if I am wrong with the above.
> 
That all sounds fine to me, thanks for summarizing!

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Wed, Oct 06, 2010 at 08:15:48PM +0200, Robert Richter wrote:
> On 04.10.10 16:44:20, Matt Fleming wrote:
> > Introduce perf_pmu_name() helper function that returns the name of the
> > pmu. This gives us a generic way to get the name of a pmu regardless of
> > how an architecture identifies it internally, e.g. ARM uses an id
> > whereas SH currently uses a string.
> > 
> 
> Matt,
> 
> as an outcome of the discussion we have had in this thread I think we
> agree on the following:
> 
> We extend the generic perf interface with perf_pmu_name() implenting a
> default with the __weak attribute.  On sh we override this function
> and will use it for perf/oprofile string translation. This will be
> implemented in the oprofile function op_name_from_perf_name().
> 
> Later we implement a generic oprofile function that derives the
> cpu_type from perf_pmu_name() in a generic way and add other
> architectures. Thus, we will drop the ARM changes with this patch set.
> 
> Paul,
> 
> please correct me if I am wrong with the above.
> 
That all sounds fine to me, thanks for summarizing!

[PATCH V5 0/7] Generalise ARM perf-events backend for oprofile

[Matt Fleming]

The perf-events backend for OProfile that Will Deacon wrote in
8c1fc96f6fd1f361428ba805103af0d0eee65179 ("ARM: 6072/1: oprofile: use
perf-events framework as backend") is of use to more architectures
than just ARM. Move the code into drivers/oprofile/ so that SH can use
it instead of the nearly identical copy of its OProfile code.

The benefit of the backend is that it becomes necessary to only
maintain one copy of the PMU accessor functions for each architecture,
with bug fixes and new features benefiting both OProfile and perf.

Note that I haven't been able to test these patches on an ARM board to
see if I've caused any regressions. If anyone else could do that I'd
appreciate it. Though, I have been able to compile this version of the
series.

This patch series is based on,

    git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

These patches can also be found at,

    git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/sh-2.6.git perf-oprofile


Robert, I haven't rebased this work on your oprofile updates for
2.6.37. If you need me to do that, just shout. If you have any comments
I'll respin this ASAP but it's probably ready for merging now.


Changes from v4:
 - SH is the only arch that doesn't default perf_pmu_name()
 - Changed perf_nr_counters to num_counters
 
Changes from v3:
 - New perf_pmu_name() function
 - Sprinkled an EXPORT_SYMBOL_GPL()
 - Don't repeatedly call perf_num_counters()
 - Change name of oprofile_driver to "oprofile-perf"
 - Hard overwrite the oprofile_operations in oprofile_perf_init()
 - Dropped the patch that made oprofile depend on HW_PERF_EVENTS for ARM

Changes from v2:
 - Rebased against Robert's oprofile core branch
 - Moved even more of the ARM code into the generic oprofile code
 - Broke the patches up into more logical steps

Changes from v1:
 - Prefix the new functons with "oprofile_" instead of "op_".
 - Fix ARM compilation errors
 - Move all the oprofile-perf logic into oprofile_perf.c
 - Include cleanup patch from Will

Matt Fleming (7):
  perf: Add helper function to return number of counters
  perf: New helper function for pmu name
  oprofile: Make op_name_from_perf_id() global
  ARM: oprofile: Rename op_arm to oprofile_perf
  ARM: oprofile: Move non-ARM code into separate init/exit
  oprofile: Abstract the perf-events backend
  sh: oprofile: Use perf-events oprofile backend

 arch/arm/kernel/perf_event.c     |    6 +
 arch/arm/oprofile/Makefile       |    4 +
 arch/arm/oprofile/common.c       |  309 +-----------------------------------
 arch/sh/Kconfig                  |   13 ++
 arch/sh/kernel/perf_event.c      |   18 ++
 arch/sh/oprofile/Makefile        |    4 +
 arch/sh/oprofile/common.c        |  115 +++-----------
 arch/sh/oprofile/op_impl.h       |   33 ----
 drivers/oprofile/oprofile_perf.c |  326 ++++++++++++++++++++++++++++++++++++++
 include/linux/oprofile.h         |    7 +
 include/linux/perf_event.h       |    2 +
 kernel/perf_event.c              |    5 +
 12 files changed, 412 insertions(+), 430 deletions(-)
 delete mode 100644 arch/sh/oprofile/op_impl.h
 create mode 100644 drivers/oprofile/oprofile_perf.c

[PATCH 1/7] perf: Add helper function to return number of counters

[Matt Fleming]

The number of counters for the registered pmu is needed in a few places
so provide a helper function that returns this number.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Tested-by: Will Deacon <will.deacon@arm.com>
Acked-by: Paul Mundt <lethal@linux-sh.org>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 arch/arm/kernel/perf_event.c |    6 ++++++
 arch/arm/oprofile/common.c   |   31 ++++++++++++++++++-------------
 arch/sh/kernel/perf_event.c  |    9 +++++++++
 include/linux/perf_event.h   |    1 +
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index ecbb028..ef3bc33 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -123,6 +123,12 @@ armpmu_get_max_events(void)
 }
 EXPORT_SYMBOL_GPL(armpmu_get_max_events);
 
+int perf_num_counters(void)
+{
+	return armpmu_get_max_events();
+}
+EXPORT_SYMBOL_GPL(perf_num_counters);
+
 #define HW_OP_UNSUPPORTED		0xFFFF
 
 #define C(_x) \
diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index d660cb8..1e971a7 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -43,7 +43,7 @@ static DEFINE_MUTEX(op_arm_mutex);
 
 static struct op_counter_config *counter_config;
 static struct perf_event **perf_events[nr_cpumask_bits];
-static int perf_num_counters;
+static int num_counters;
 
 /*
  * Overflow callback for oprofile.
@@ -54,11 +54,11 @@ static void op_overflow_handler(struct perf_event *event, int unused,
 	int id;
 	u32 cpu = smp_processor_id();
 
-	for (id = 0; id < perf_num_counters; ++id)
+	for (id = 0; id < num_counters; ++id)
 		if (perf_events[cpu][id] == event)
 			break;
 
-	if (id != perf_num_counters)
+	if (id != num_counters)
 		oprofile_add_sample(regs, id);
 	else
 		pr_warning("oprofile: ignoring spurious overflow "
@@ -76,7 +76,7 @@ static void op_perf_setup(void)
 	u32 size = sizeof(struct perf_event_attr);
 	struct perf_event_attr *attr;
 
-	for (i = 0; i < perf_num_counters; ++i) {
+	for (i = 0; i < num_counters; ++i) {
 		attr = &counter_config[i].attr;
 		memset(attr, 0, size);
 		attr->type		= PERF_TYPE_RAW;
@@ -131,7 +131,7 @@ static int op_perf_start(void)
 	int cpu, event, ret = 0;
 
 	for_each_online_cpu(cpu) {
-		for (event = 0; event < perf_num_counters; ++event) {
+		for (event = 0; event < num_counters; ++event) {
 			ret = op_create_counter(cpu, event);
 			if (ret)
 				goto out;
@@ -150,7 +150,7 @@ static void op_perf_stop(void)
 	int cpu, event;
 
 	for_each_online_cpu(cpu)
-		for (event = 0; event < perf_num_counters; ++event)
+		for (event = 0; event < num_counters; ++event)
 			op_destroy_counter(cpu, event);
 }
 
@@ -179,7 +179,7 @@ static int op_arm_create_files(struct super_block *sb, struct dentry *root)
 {
 	unsigned int i;
 
-	for (i = 0; i < perf_num_counters; i++) {
+	for (i = 0; i < num_counters; i++) {
 		struct dentry *dir;
 		char buf[4];
 
@@ -353,14 +353,19 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 
 	memset(&perf_events, 0, sizeof(perf_events));
 
-	perf_num_counters = armpmu_get_max_events();
+	num_counters = perf_num_counters();
+	if (num_counters <= 0) {
+		pr_info("oprofile: no performance counters\n");
+		ret = -ENODEV;
+		goto out;
+	}
 
-	counter_config = kcalloc(perf_num_counters,
+	counter_config = kcalloc(num_counters,
 			sizeof(struct op_counter_config), GFP_KERNEL);
 
 	if (!counter_config) {
 		pr_info("oprofile: failed to allocate %d "
-				"counters\n", perf_num_counters);
+				"counters\n", num_counters);
 		ret = -ENOMEM;
 		goto out;
 	}
@@ -370,11 +375,11 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 		goto out;
 
 	for_each_possible_cpu(cpu) {
-		perf_events[cpu] = kcalloc(perf_num_counters,
+		perf_events[cpu] = kcalloc(num_counters,
 				sizeof(struct perf_event *), GFP_KERNEL);
 		if (!perf_events[cpu]) {
 			pr_info("oprofile: failed to allocate %d perf events "
-					"for cpu %d\n", perf_num_counters, cpu);
+					"for cpu %d\n", num_counters, cpu);
 			ret = -ENOMEM;
 			goto out;
 		}
@@ -409,7 +414,7 @@ void __exit oprofile_arch_exit(void)
 	struct perf_event *event;
 
 	for_each_possible_cpu(cpu) {
-		for (id = 0; id < perf_num_counters; ++id) {
+		for (id = 0; id < num_counters; ++id) {
 			event = perf_events[cpu][id];
 			if (event)
 				perf_event_release_kernel(event);
diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
index 7a3dc35..2cb9ad5 100644
--- a/arch/sh/kernel/perf_event.c
+++ b/arch/sh/kernel/perf_event.c
@@ -59,6 +59,15 @@ static inline int sh_pmu_initialized(void)
 	return !!sh_pmu;
 }
 
+int perf_num_counters(void)
+{
+	if (!sh_pmu)
+		return 0;
+
+	return sh_pmu->num_events;
+}
+EXPORT_SYMBOL_GPL(perf_num_counters);
+
 /*
  * Release the PMU if this is the last perf_event.
  */
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 716f99b..1a02192 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -849,6 +849,7 @@ extern int perf_max_events;
 
 extern const struct pmu *hw_perf_event_init(struct perf_event *event);
 
+extern int perf_num_counters(void);
 extern void perf_event_task_sched_in(struct task_struct *task);
 extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
 extern void perf_event_task_tick(struct task_struct *task);
-- 
1.7.1

[PATCH 1/7] perf: Add helper function to return number of counters

[Matt Fleming]

The number of counters for the registered pmu is needed in a few places
so provide a helper function that returns this number.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Tested-by: Will Deacon <will.deacon@arm.com>
Acked-by: Paul Mundt <lethal@linux-sh.org>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 arch/arm/kernel/perf_event.c |    6 ++++++
 arch/arm/oprofile/common.c   |   31 ++++++++++++++++++-------------
 arch/sh/kernel/perf_event.c  |    9 +++++++++
 include/linux/perf_event.h   |    1 +
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index ecbb028..ef3bc33 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -123,6 +123,12 @@ armpmu_get_max_events(void)
 }
 EXPORT_SYMBOL_GPL(armpmu_get_max_events);
 
+int perf_num_counters(void)
+{
+	return armpmu_get_max_events();
+}
+EXPORT_SYMBOL_GPL(perf_num_counters);
+
 #define HW_OP_UNSUPPORTED		0xFFFF
 
 #define C(_x) \
diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index d660cb8..1e971a7 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -43,7 +43,7 @@ static DEFINE_MUTEX(op_arm_mutex);
 
 static struct op_counter_config *counter_config;
 static struct perf_event **perf_events[nr_cpumask_bits];
-static int perf_num_counters;
+static int num_counters;
 
 /*
  * Overflow callback for oprofile.
@@ -54,11 +54,11 @@ static void op_overflow_handler(struct perf_event *event, int unused,
 	int id;
 	u32 cpu = smp_processor_id();
 
-	for (id = 0; id < perf_num_counters; ++id)
+	for (id = 0; id < num_counters; ++id)
 		if (perf_events[cpu][id] == event)
 			break;
 
-	if (id != perf_num_counters)
+	if (id != num_counters)
 		oprofile_add_sample(regs, id);
 	else
 		pr_warning("oprofile: ignoring spurious overflow "
@@ -76,7 +76,7 @@ static void op_perf_setup(void)
 	u32 size = sizeof(struct perf_event_attr);
 	struct perf_event_attr *attr;
 
-	for (i = 0; i < perf_num_counters; ++i) {
+	for (i = 0; i < num_counters; ++i) {
 		attr = &counter_config[i].attr;
 		memset(attr, 0, size);
 		attr->type		= PERF_TYPE_RAW;
@@ -131,7 +131,7 @@ static int op_perf_start(void)
 	int cpu, event, ret = 0;
 
 	for_each_online_cpu(cpu) {
-		for (event = 0; event < perf_num_counters; ++event) {
+		for (event = 0; event < num_counters; ++event) {
 			ret = op_create_counter(cpu, event);
 			if (ret)
 				goto out;
@@ -150,7 +150,7 @@ static void op_perf_stop(void)
 	int cpu, event;
 
 	for_each_online_cpu(cpu)
-		for (event = 0; event < perf_num_counters; ++event)
+		for (event = 0; event < num_counters; ++event)
 			op_destroy_counter(cpu, event);
 }
 
@@ -179,7 +179,7 @@ static int op_arm_create_files(struct super_block *sb, struct dentry *root)
 {
 	unsigned int i;
 
-	for (i = 0; i < perf_num_counters; i++) {
+	for (i = 0; i < num_counters; i++) {
 		struct dentry *dir;
 		char buf[4];
 
@@ -353,14 +353,19 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 
 	memset(&perf_events, 0, sizeof(perf_events));
 
-	perf_num_counters = armpmu_get_max_events();
+	num_counters = perf_num_counters();
+	if (num_counters <= 0) {
+		pr_info("oprofile: no performance counters\n");
+		ret = -ENODEV;
+		goto out;
+	}
 
-	counter_config = kcalloc(perf_num_counters,
+	counter_config = kcalloc(num_counters,
 			sizeof(struct op_counter_config), GFP_KERNEL);
 
 	if (!counter_config) {
 		pr_info("oprofile: failed to allocate %d "
-				"counters\n", perf_num_counters);
+				"counters\n", num_counters);
 		ret = -ENOMEM;
 		goto out;
 	}
@@ -370,11 +375,11 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 		goto out;
 
 	for_each_possible_cpu(cpu) {
-		perf_events[cpu] = kcalloc(perf_num_counters,
+		perf_events[cpu] = kcalloc(num_counters,
 				sizeof(struct perf_event *), GFP_KERNEL);
 		if (!perf_events[cpu]) {
 			pr_info("oprofile: failed to allocate %d perf events "
-					"for cpu %d\n", perf_num_counters, cpu);
+					"for cpu %d\n", num_counters, cpu);
 			ret = -ENOMEM;
 			goto out;
 		}
@@ -409,7 +414,7 @@ void __exit oprofile_arch_exit(void)
 	struct perf_event *event;
 
 	for_each_possible_cpu(cpu) {
-		for (id = 0; id < perf_num_counters; ++id) {
+		for (id = 0; id < num_counters; ++id) {
 			event = perf_events[cpu][id];
 			if (event)
 				perf_event_release_kernel(event);
diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
index 7a3dc35..2cb9ad5 100644
--- a/arch/sh/kernel/perf_event.c
+++ b/arch/sh/kernel/perf_event.c
@@ -59,6 +59,15 @@ static inline int sh_pmu_initialized(void)
 	return !!sh_pmu;
 }
 
+int perf_num_counters(void)
+{
+	if (!sh_pmu)
+		return 0;
+
+	return sh_pmu->num_events;
+}
+EXPORT_SYMBOL_GPL(perf_num_counters);
+
 /*
  * Release the PMU if this is the last perf_event.
  */
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 716f99b..1a02192 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -849,6 +849,7 @@ extern int perf_max_events;
 
 extern const struct pmu *hw_perf_event_init(struct perf_event *event);
 
+extern int perf_num_counters(void);
 extern void perf_event_task_sched_in(struct task_struct *task);
 extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
 extern void perf_event_task_tick(struct task_struct *task);
-- 
1.7.1

[PATCH 2/7] perf: New helper function for pmu name

[Matt Fleming]

Introduce perf_pmu_name() helper function that returns the name of the
pmu. This gives us a generic way to get the name of a pmu regardless of
how an architecture identifies it internally.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
 arch/sh/kernel/perf_event.c |    9 +++++++++
 include/linux/perf_event.h  |    1 +
 kernel/perf_event.c         |    5 +++++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
index 2cb9ad5..55fe89b 100644
--- a/arch/sh/kernel/perf_event.c
+++ b/arch/sh/kernel/perf_event.c
@@ -59,6 +59,15 @@ static inline int sh_pmu_initialized(void)
 	return !!sh_pmu;
 }
 
+const char *perf_pmu_name(void)
+{
+	if (!sh_pmu)
+		return NULL;
+
+	return sh_pmu->name;
+}
+EXPORT_SYMBOL_GPL(perf_pmu_name);
+
 int perf_num_counters(void)
 {
 	if (!sh_pmu)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1a02192..33f08da 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -850,6 +850,7 @@ extern int perf_max_events;
 extern const struct pmu *hw_perf_event_init(struct perf_event *event);
 
 extern int perf_num_counters(void);
+extern const char *perf_pmu_name(void);
 extern void perf_event_task_sched_in(struct task_struct *task);
 extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
 extern void perf_event_task_tick(struct task_struct *task);
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index db5b560..fc51268 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
 
 void __weak perf_event_print_debug(void)	{ }
 
+extern __weak const char *perf_pmu_name(void)
+{
+	return "pmu";
+}
+
 static DEFINE_PER_CPU(int, perf_disable_count);
 
 void perf_disable(void)
-- 
1.7.1

[PATCH 2/7] perf: New helper function for pmu name

[Matt Fleming]

Introduce perf_pmu_name() helper function that returns the name of the
pmu. This gives us a generic way to get the name of a pmu regardless of
how an architecture identifies it internally.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
 arch/sh/kernel/perf_event.c |    9 +++++++++
 include/linux/perf_event.h  |    1 +
 kernel/perf_event.c         |    5 +++++
 3 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
index 2cb9ad5..55fe89b 100644
--- a/arch/sh/kernel/perf_event.c
+++ b/arch/sh/kernel/perf_event.c
@@ -59,6 +59,15 @@ static inline int sh_pmu_initialized(void)
 	return !!sh_pmu;
 }
 
+const char *perf_pmu_name(void)
+{
+	if (!sh_pmu)
+		return NULL;
+
+	return sh_pmu->name;
+}
+EXPORT_SYMBOL_GPL(perf_pmu_name);
+
 int perf_num_counters(void)
 {
 	if (!sh_pmu)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 1a02192..33f08da 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -850,6 +850,7 @@ extern int perf_max_events;
 extern const struct pmu *hw_perf_event_init(struct perf_event *event);
 
 extern int perf_num_counters(void);
+extern const char *perf_pmu_name(void);
 extern void perf_event_task_sched_in(struct task_struct *task);
 extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
 extern void perf_event_task_tick(struct task_struct *task);
diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index db5b560..fc51268 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
 
 void __weak perf_event_print_debug(void)	{ }
 
+extern __weak const char *perf_pmu_name(void)
+{
+	return "pmu";
+}
+
 static DEFINE_PER_CPU(int, perf_disable_count);
 
 void perf_disable(void)
-- 
1.7.1

[PATCH 3/7] oprofile: Make op_name_from_perf_id() global

[Matt Fleming]

Make op_name_from_perf_id() global so that we have a way for each
architecture to construct an oprofile name for op->cpu_type. We need to
remove the argument from the function prototype so that we can hide all
implementation details inside the function.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
 arch/arm/oprofile/common.c |    6 ++++--
 include/linux/oprofile.h   |    4 ++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 1e971a7..4f67cfa 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -155,8 +155,10 @@ static void op_perf_stop(void)
 }
 
 
-static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
+char *op_name_from_perf_id(void)
 {
+	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
+
 	switch (id) {
 	case ARM_PERF_PMU_ID_XSCALE1:
 		return "arm/xscale1";
@@ -391,7 +393,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	ops->start		= op_arm_start;
 	ops->stop		= op_arm_stop;
 	ops->shutdown		= op_arm_stop;
-	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
+	ops->cpu_type		= op_name_from_perf_id();
 
 	if (!ops->cpu_type)
 		ret = -ENODEV;
diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index 5171639..1574d4a 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -185,4 +185,8 @@ int oprofile_add_data(struct op_entry *entry, unsigned long val);
 int oprofile_add_data64(struct op_entry *entry, u64 val);
 int oprofile_write_commit(struct op_entry *entry);
 
+#ifdef CONFIG_PERF_EVENTS
+char *op_name_from_perf_id(void);
+#endif /* CONFIG_PERF_EVENTS */
+
 #endif /* OPROFILE_H */
-- 
1.7.1

[PATCH 3/7] oprofile: Make op_name_from_perf_id() global

[Matt Fleming]

Make op_name_from_perf_id() global so that we have a way for each
architecture to construct an oprofile name for op->cpu_type. We need to
remove the argument from the function prototype so that we can hide all
implementation details inside the function.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
---
 arch/arm/oprofile/common.c |    6 ++++--
 include/linux/oprofile.h   |    4 ++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 1e971a7..4f67cfa 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -155,8 +155,10 @@ static void op_perf_stop(void)
 }
 
 
-static char *op_name_from_perf_id(enum arm_perf_pmu_ids id)
+char *op_name_from_perf_id(void)
 {
+	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
+
 	switch (id) {
 	case ARM_PERF_PMU_ID_XSCALE1:
 		return "arm/xscale1";
@@ -391,7 +393,7 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	ops->start		= op_arm_start;
 	ops->stop		= op_arm_stop;
 	ops->shutdown		= op_arm_stop;
-	ops->cpu_type		= op_name_from_perf_id(armpmu_get_pmu_id());
+	ops->cpu_type		= op_name_from_perf_id();
 
 	if (!ops->cpu_type)
 		ret = -ENODEV;
diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index 5171639..1574d4a 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -185,4 +185,8 @@ int oprofile_add_data(struct op_entry *entry, unsigned long val);
 int oprofile_add_data64(struct op_entry *entry, u64 val);
 int oprofile_write_commit(struct op_entry *entry);
 
+#ifdef CONFIG_PERF_EVENTS
+char *op_name_from_perf_id(void);
+#endif /* CONFIG_PERF_EVENTS */
+
 #endif /* OPROFILE_H */
-- 
1.7.1

[PATCH 4/7] ARM: oprofile: Rename op_arm to oprofile_perf

[Matt Fleming]

In preparation for moving the generic functions out of this file, give
the functions more general names (e.g. remove "arm" from the names).

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Tested-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/oprofile/common.c |   58 ++++++++++++++++++++++----------------------
 1 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 4f67cfa..fd6e323 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -38,8 +38,8 @@ struct op_counter_config {
 	struct perf_event_attr attr;
 };
 
-static int op_arm_enabled;
-static DEFINE_MUTEX(op_arm_mutex);
+static int oprofile_perf_enabled;
+static DEFINE_MUTEX(oprofile_perf_mutex);
 
 static struct op_counter_config *counter_config;
 static struct perf_event **perf_events[nr_cpumask_bits];
@@ -66,7 +66,7 @@ static void op_overflow_handler(struct perf_event *event, int unused,
 }
 
 /*
- * Called by op_arm_setup to create perf attributes to mirror the oprofile
+ * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile
  * settings in counter_config. Attributes are created as `pinned' events and
  * so are permanently scheduled on the PMU.
  */
@@ -123,7 +123,7 @@ static void op_destroy_counter(int cpu, int event)
 }
 
 /*
- * Called by op_arm_start to create active perf events based on the
+ * Called by oprofile_perf_start to create active perf events based on the
  * perviously configured attributes.
  */
 static int op_perf_start(void)
@@ -143,7 +143,7 @@ out:
 }
 
 /*
- * Called by op_arm_stop at the end of a profiling run.
+ * Called by oprofile_perf_stop at the end of a profiling run.
  */
 static void op_perf_stop(void)
 {
@@ -177,7 +177,7 @@ char *op_name_from_perf_id(void)
 	}
 }
 
-static int op_arm_create_files(struct super_block *sb, struct dentry *root)
+static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root)
 {
 	unsigned int i;
 
@@ -198,7 +198,7 @@ static int op_arm_create_files(struct super_block *sb, struct dentry *root)
 	return 0;
 }
 
-static int op_arm_setup(void)
+static int oprofile_perf_setup(void)
 {
 	spin_lock(&oprofilefs_lock);
 	op_perf_setup();
@@ -206,54 +206,54 @@ static int op_arm_setup(void)
 	return 0;
 }
 
-static int op_arm_start(void)
+static int oprofile_perf_start(void)
 {
 	int ret = -EBUSY;
 
-	mutex_lock(&op_arm_mutex);
-	if (!op_arm_enabled) {
+	mutex_lock(&oprofile_perf_mutex);
+	if (!oprofile_perf_enabled) {
 		ret = 0;
 		op_perf_start();
-		op_arm_enabled = 1;
+		oprofile_perf_enabled = 1;
 	}
-	mutex_unlock(&op_arm_mutex);
+	mutex_unlock(&oprofile_perf_mutex);
 	return ret;
 }
 
-static void op_arm_stop(void)
+static void oprofile_perf_stop(void)
 {
-	mutex_lock(&op_arm_mutex);
-	if (op_arm_enabled)
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled)
 		op_perf_stop();
-	op_arm_enabled = 0;
-	mutex_unlock(&op_arm_mutex);
+	oprofile_perf_enabled = 0;
+	mutex_unlock(&oprofile_perf_mutex);
 }
 
 #ifdef CONFIG_PM
-static int op_arm_suspend(struct platform_device *dev, pm_message_t state)
+static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state)
 {
-	mutex_lock(&op_arm_mutex);
-	if (op_arm_enabled)
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled)
 		op_perf_stop();
-	mutex_unlock(&op_arm_mutex);
+	mutex_unlock(&oprofile_perf_mutex);
 	return 0;
 }
 
-static int op_arm_resume(struct platform_device *dev)
+static int oprofile_perf_resume(struct platform_device *dev)
 {
-	mutex_lock(&op_arm_mutex);
-	if (op_arm_enabled && op_perf_start())
-		op_arm_enabled = 0;
-	mutex_unlock(&op_arm_mutex);
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled && op_perf_start())
+		oprofile_perf_enabled = 0;
+	mutex_unlock(&oprofile_perf_mutex);
 	return 0;
 }
 
 static struct platform_driver oprofile_driver = {
 	.driver		= {
-		.name		= "arm-oprofile",
+		.name		= "oprofile-perf",
 	},
-	.resume		= op_arm_resume,
-	.suspend	= op_arm_suspend,
+	.resume		= oprofile_perf_resume,
+	.suspend	= oprofile_perf_suspend,
 };
 
 static struct platform_device *oprofile_pdev;
-- 
1.7.1

[PATCH 5/7] ARM: oprofile: Move non-ARM code into separate init/exit

[Matt Fleming]

In preparation for moving the majority of this oprofile code into an
architecture-neutral place separate the architecture-independent code
into oprofile_perf_init() and oprofile_perf_exit().

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Tested-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/oprofile/common.c |   27 +++++++++++++++++++--------
 1 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index fd6e323..8718311 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -349,7 +349,7 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
 		tail = user_backtrace(tail);
 }
 
-int __init oprofile_arch_init(struct oprofile_operations *ops)
+int __init oprofile_perf_init(struct oprofile_operations *ops)
 {
 	int cpu, ret = 0;
 
@@ -387,12 +387,11 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 		}
 	}
 
-	ops->backtrace		= arm_backtrace;
-	ops->create_files	= op_arm_create_files;
-	ops->setup		= op_arm_setup;
-	ops->start		= op_arm_start;
-	ops->stop		= op_arm_stop;
-	ops->shutdown		= op_arm_stop;
+	ops->create_files	= oprofile_perf_create_files;
+	ops->setup		= oprofile_perf_setup;
+	ops->start		= oprofile_perf_start;
+	ops->stop		= oprofile_perf_stop;
+	ops->shutdown		= oprofile_perf_stop;
 	ops->cpu_type		= op_name_from_perf_id();
 
 	if (!ops->cpu_type)
@@ -410,7 +409,14 @@ out:
 	return ret;
 }
 
-void __exit oprofile_arch_exit(void)
+int __init oprofile_arch_init(struct oprofile_operations *ops)
+{
+	ops->backtrace		= arm_backtrace;
+
+	return oprofile_perf_init(ops);
+}
+
+void __exit oprofile_perf_exit(void)
 {
 	int cpu, id;
 	struct perf_event *event;
@@ -428,6 +434,11 @@ void __exit oprofile_arch_exit(void)
 	kfree(counter_config);
 	exit_driverfs();
 }
+
+void __exit oprofile_arch_exit(void)
+{
+	oprofile_perf_exit();
+}
 #else
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
-- 
1.7.1

[PATCH 6/7] oprofile: Abstract the perf-events backend

[Matt Fleming]

Move the perf-events backend from arch/arm/oprofile into
drivers/oprofile so that the code can be shared between architectures.

This allows each architecture to maintain only a single copy of the PMU
accessor functions instead of one for both perf and OProfile. It also
becomes possible for other architectures to delete much of their
OProfile code in favour of the common code now available in
drivers/oprofile/oprofile_perf.c.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Tested-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/oprofile/Makefile       |    4 +
 arch/arm/oprofile/common.c       |  319 -------------------------------------
 drivers/oprofile/oprofile_perf.c |  326 ++++++++++++++++++++++++++++++++++++++
 include/linux/oprofile.h         |    3 +
 4 files changed, 333 insertions(+), 319 deletions(-)
 create mode 100644 drivers/oprofile/oprofile_perf.c

diff --git a/arch/arm/oprofile/Makefile b/arch/arm/oprofile/Makefile
index e666eaf..b2215c6 100644
--- a/arch/arm/oprofile/Makefile
+++ b/arch/arm/oprofile/Makefile
@@ -6,4 +6,8 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprofilefs.o oprofile_stats.o \
 		timer_int.o )
 
+ifeq ($(CONFIG_HW_PERF_EVENTS),y)
+DRIVER_OBJS += $(addprefix ../../../drivers/oprofile/, oprofile_perf.o)
+endif
+
 oprofile-y				:= $(DRIVER_OBJS) common.o
diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 8718311..8aa9744 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -25,136 +25,6 @@
 #include <asm/ptrace.h>
 
 #ifdef CONFIG_HW_PERF_EVENTS
-/*
- * Per performance monitor configuration as set via oprofilefs.
- */
-struct op_counter_config {
-	unsigned long count;
-	unsigned long enabled;
-	unsigned long event;
-	unsigned long unit_mask;
-	unsigned long kernel;
-	unsigned long user;
-	struct perf_event_attr attr;
-};
-
-static int oprofile_perf_enabled;
-static DEFINE_MUTEX(oprofile_perf_mutex);
-
-static struct op_counter_config *counter_config;
-static struct perf_event **perf_events[nr_cpumask_bits];
-static int num_counters;
-
-/*
- * Overflow callback for oprofile.
- */
-static void op_overflow_handler(struct perf_event *event, int unused,
-			struct perf_sample_data *data, struct pt_regs *regs)
-{
-	int id;
-	u32 cpu = smp_processor_id();
-
-	for (id = 0; id < num_counters; ++id)
-		if (perf_events[cpu][id] == event)
-			break;
-
-	if (id != num_counters)
-		oprofile_add_sample(regs, id);
-	else
-		pr_warning("oprofile: ignoring spurious overflow "
-				"on cpu %u\n", cpu);
-}
-
-/*
- * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile
- * settings in counter_config. Attributes are created as `pinned' events and
- * so are permanently scheduled on the PMU.
- */
-static void op_perf_setup(void)
-{
-	int i;
-	u32 size = sizeof(struct perf_event_attr);
-	struct perf_event_attr *attr;
-
-	for (i = 0; i < num_counters; ++i) {
-		attr = &counter_config[i].attr;
-		memset(attr, 0, size);
-		attr->type		= PERF_TYPE_RAW;
-		attr->size		= size;
-		attr->config		= counter_config[i].event;
-		attr->sample_period	= counter_config[i].count;
-		attr->pinned		= 1;
-	}
-}
-
-static int op_create_counter(int cpu, int event)
-{
-	int ret = 0;
-	struct perf_event *pevent;
-
-	if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL))
-		return ret;
-
-	pevent = perf_event_create_kernel_counter(&counter_config[event].attr,
-						  cpu, -1,
-						  op_overflow_handler);
-
-	if (IS_ERR(pevent)) {
-		ret = PTR_ERR(pevent);
-	} else if (pevent->state != PERF_EVENT_STATE_ACTIVE) {
-		pr_warning("oprofile: failed to enable event %d "
-				"on CPU %d\n", event, cpu);
-		ret = -EBUSY;
-	} else {
-		perf_events[cpu][event] = pevent;
-	}
-
-	return ret;
-}
-
-static void op_destroy_counter(int cpu, int event)
-{
-	struct perf_event *pevent = perf_events[cpu][event];
-
-	if (pevent) {
-		perf_event_release_kernel(pevent);
-		perf_events[cpu][event] = NULL;
-	}
-}
-
-/*
- * Called by oprofile_perf_start to create active perf events based on the
- * perviously configured attributes.
- */
-static int op_perf_start(void)
-{
-	int cpu, event, ret = 0;
-
-	for_each_online_cpu(cpu) {
-		for (event = 0; event < num_counters; ++event) {
-			ret = op_create_counter(cpu, event);
-			if (ret)
-				goto out;
-		}
-	}
-
-out:
-	return ret;
-}
-
-/*
- * Called by oprofile_perf_stop at the end of a profiling run.
- */
-static void op_perf_stop(void)
-{
-	int cpu, event;
-
-	for_each_online_cpu(cpu)
-		for (event = 0; event < num_counters; ++event)
-			op_destroy_counter(cpu, event);
-}
-
-
 char *op_name_from_perf_id(void)
 {
 	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
@@ -177,116 +47,6 @@ char *op_name_from_perf_id(void)
 	}
 }
 
-static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root)
-{
-	unsigned int i;
-
-	for (i = 0; i < num_counters; i++) {
-		struct dentry *dir;
-		char buf[4];
-
-		snprintf(buf, sizeof buf, "%d", i);
-		dir = oprofilefs_mkdir(sb, root, buf);
-		oprofilefs_create_ulong(sb, dir, "enabled", &counter_config[i].enabled);
-		oprofilefs_create_ulong(sb, dir, "event", &counter_config[i].event);
-		oprofilefs_create_ulong(sb, dir, "count", &counter_config[i].count);
-		oprofilefs_create_ulong(sb, dir, "unit_mask", &counter_config[i].unit_mask);
-		oprofilefs_create_ulong(sb, dir, "kernel", &counter_config[i].kernel);
-		oprofilefs_create_ulong(sb, dir, "user", &counter_config[i].user);
-	}
-
-	return 0;
-}
-
-static int oprofile_perf_setup(void)
-{
-	spin_lock(&oprofilefs_lock);
-	op_perf_setup();
-	spin_unlock(&oprofilefs_lock);
-	return 0;
-}
-
-static int oprofile_perf_start(void)
-{
-	int ret = -EBUSY;
-
-	mutex_lock(&oprofile_perf_mutex);
-	if (!oprofile_perf_enabled) {
-		ret = 0;
-		op_perf_start();
-		oprofile_perf_enabled = 1;
-	}
-	mutex_unlock(&oprofile_perf_mutex);
-	return ret;
-}
-
-static void oprofile_perf_stop(void)
-{
-	mutex_lock(&oprofile_perf_mutex);
-	if (oprofile_perf_enabled)
-		op_perf_stop();
-	oprofile_perf_enabled = 0;
-	mutex_unlock(&oprofile_perf_mutex);
-}
-
-#ifdef CONFIG_PM
-static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state)
-{
-	mutex_lock(&oprofile_perf_mutex);
-	if (oprofile_perf_enabled)
-		op_perf_stop();
-	mutex_unlock(&oprofile_perf_mutex);
-	return 0;
-}
-
-static int oprofile_perf_resume(struct platform_device *dev)
-{
-	mutex_lock(&oprofile_perf_mutex);
-	if (oprofile_perf_enabled && op_perf_start())
-		oprofile_perf_enabled = 0;
-	mutex_unlock(&oprofile_perf_mutex);
-	return 0;
-}
-
-static struct platform_driver oprofile_driver = {
-	.driver		= {
-		.name		= "oprofile-perf",
-	},
-	.resume		= oprofile_perf_resume,
-	.suspend	= oprofile_perf_suspend,
-};
-
-static struct platform_device *oprofile_pdev;
-
-static int __init init_driverfs(void)
-{
-	int ret;
-
-	ret = platform_driver_register(&oprofile_driver);
-	if (ret)
-		goto out;
-
-	oprofile_pdev =	platform_device_register_simple(
-				oprofile_driver.driver.name, 0, NULL, 0);
-	if (IS_ERR(oprofile_pdev)) {
-		ret = PTR_ERR(oprofile_pdev);
-		platform_driver_unregister(&oprofile_driver);
-	}
-
-out:
-	return ret;
-}
-
-static void __exit exit_driverfs(void)
-{
-	platform_device_unregister(oprofile_pdev);
-	platform_driver_unregister(&oprofile_driver);
-}
-#else
-static int __init init_driverfs(void) { return 0; }
-#define exit_driverfs() do { } while (0)
-#endif /* CONFIG_PM */
-
 static int report_trace(struct stackframe *frame, void *d)
 {
 	unsigned int *depth = d;
@@ -349,66 +109,6 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
 		tail = user_backtrace(tail);
 }
 
-int __init oprofile_perf_init(struct oprofile_operations *ops)
-{
-	int cpu, ret = 0;
-
-	memset(&perf_events, 0, sizeof(perf_events));
-
-	num_counters = perf_num_counters();
-	if (num_counters <= 0) {
-		pr_info("oprofile: no performance counters\n");
-		ret = -ENODEV;
-		goto out;
-	}
-
-	counter_config = kcalloc(num_counters,
-			sizeof(struct op_counter_config), GFP_KERNEL);
-
-	if (!counter_config) {
-		pr_info("oprofile: failed to allocate %d "
-				"counters\n", num_counters);
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	ret = init_driverfs();
-	if (ret)
-		goto out;
-
-	for_each_possible_cpu(cpu) {
-		perf_events[cpu] = kcalloc(num_counters,
-				sizeof(struct perf_event *), GFP_KERNEL);
-		if (!perf_events[cpu]) {
-			pr_info("oprofile: failed to allocate %d perf events "
-					"for cpu %d\n", num_counters, cpu);
-			ret = -ENOMEM;
-			goto out;
-		}
-	}
-
-	ops->create_files	= oprofile_perf_create_files;
-	ops->setup		= oprofile_perf_setup;
-	ops->start		= oprofile_perf_start;
-	ops->stop		= oprofile_perf_stop;
-	ops->shutdown		= oprofile_perf_stop;
-	ops->cpu_type		= op_name_from_perf_id();
-
-	if (!ops->cpu_type)
-		ret = -ENODEV;
-	else
-		pr_info("oprofile: using %s\n", ops->cpu_type);
-
-out:
-	if (ret) {
-		for_each_possible_cpu(cpu)
-			kfree(perf_events[cpu]);
-		kfree(counter_config);
-	}
-
-	return ret;
-}
-
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
 	ops->backtrace		= arm_backtrace;
@@ -416,25 +116,6 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return oprofile_perf_init(ops);
 }
 
-void __exit oprofile_perf_exit(void)
-{
-	int cpu, id;
-	struct perf_event *event;
-
-	for_each_possible_cpu(cpu) {
-		for (id = 0; id < num_counters; ++id) {
-			event = perf_events[cpu][id];
-			if (event)
-				perf_event_release_kernel(event);
-		}
-
-		kfree(perf_events[cpu]);
-	}
-
-	kfree(counter_config);
-	exit_driverfs();
-}
-
 void __exit oprofile_arch_exit(void)
 {
 	oprofile_perf_exit();
diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c
new file mode 100644
index 0000000..ebb40cb
--- /dev/null
+++ b/drivers/oprofile/oprofile_perf.c
@@ -0,0 +1,326 @@
+/*
+ * Copyright 2010 ARM Ltd.
+ *
+ * Perf-events backend for OProfile.
+ */
+#include <linux/perf_event.h>
+#include <linux/oprofile.h>
+#include <linux/slab.h>
+
+/*
+ * Per performance monitor configuration as set via oprofilefs.
+ */
+struct op_counter_config {
+	unsigned long count;
+	unsigned long enabled;
+	unsigned long event;
+	unsigned long unit_mask;
+	unsigned long kernel;
+	unsigned long user;
+	struct perf_event_attr attr;
+};
+
+static int oprofile_perf_enabled;
+static DEFINE_MUTEX(oprofile_perf_mutex);
+
+static struct op_counter_config *counter_config;
+static struct perf_event **perf_events[nr_cpumask_bits];
+static int num_counters;
+
+/*
+ * Overflow callback for oprofile.
+ */
+static void op_overflow_handler(struct perf_event *event, int unused,
+			struct perf_sample_data *data, struct pt_regs *regs)
+{
+	int id;
+	u32 cpu = smp_processor_id();
+
+	for (id = 0; id < num_counters; ++id)
+		if (perf_events[cpu][id] == event)
+			break;
+
+	if (id != num_counters)
+		oprofile_add_sample(regs, id);
+	else
+		pr_warning("oprofile: ignoring spurious overflow "
+				"on cpu %u\n", cpu);
+}
+
+/*
+ * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile
+ * settings in counter_config. Attributes are created as `pinned' events and
+ * so are permanently scheduled on the PMU.
+ */
+static void op_perf_setup(void)
+{
+	int i;
+	u32 size = sizeof(struct perf_event_attr);
+	struct perf_event_attr *attr;
+
+	for (i = 0; i < num_counters; ++i) {
+		attr = &counter_config[i].attr;
+		memset(attr, 0, size);
+		attr->type		= PERF_TYPE_RAW;
+		attr->size		= size;
+		attr->config		= counter_config[i].event;
+		attr->sample_period	= counter_config[i].count;
+		attr->pinned		= 1;
+	}
+}
+
+static int op_create_counter(int cpu, int event)
+{
+	int ret = 0;
+	struct perf_event *pevent;
+
+	if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL))
+		return ret;
+
+	pevent = perf_event_create_kernel_counter(&counter_config[event].attr,
+						  cpu, -1,
+						  op_overflow_handler);
+
+	if (IS_ERR(pevent)) {
+		ret = PTR_ERR(pevent);
+	} else if (pevent->state != PERF_EVENT_STATE_ACTIVE) {
+		pr_warning("oprofile: failed to enable event %d "
+				"on CPU %d\n", event, cpu);
+		ret = -EBUSY;
+	} else {
+		perf_events[cpu][event] = pevent;
+	}
+
+	return ret;
+}
+
+static void op_destroy_counter(int cpu, int event)
+{
+	struct perf_event *pevent = perf_events[cpu][event];
+
+	if (pevent) {
+		perf_event_release_kernel(pevent);
+		perf_events[cpu][event] = NULL;
+	}
+}
+
+/*
+ * Called by oprofile_perf_start to create active perf events based on the
+ * perviously configured attributes.
+ */
+static int op_perf_start(void)
+{
+	int cpu, event, ret = 0;
+
+	for_each_online_cpu(cpu) {
+		for (event = 0; event < num_counters; ++event) {
+			ret = op_create_counter(cpu, event);
+			if (ret)
+				goto out;
+		}
+	}
+
+out:
+	return ret;
+}
+
+/*
+ * Called by oprofile_perf_stop at the end of a profiling run.
+ */
+static void op_perf_stop(void)
+{
+	int cpu, event;
+
+	for_each_online_cpu(cpu)
+		for (event = 0; event < num_counters; ++event)
+			op_destroy_counter(cpu, event);
+}
+
+static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root)
+{
+	unsigned int i;
+
+	for (i = 0; i < num_counters; i++) {
+		struct dentry *dir;
+		char buf[4];
+
+		snprintf(buf, sizeof buf, "%d", i);
+		dir = oprofilefs_mkdir(sb, root, buf);
+		oprofilefs_create_ulong(sb, dir, "enabled", &counter_config[i].enabled);
+		oprofilefs_create_ulong(sb, dir, "event", &counter_config[i].event);
+		oprofilefs_create_ulong(sb, dir, "count", &counter_config[i].count);
+		oprofilefs_create_ulong(sb, dir, "unit_mask", &counter_config[i].unit_mask);
+		oprofilefs_create_ulong(sb, dir, "kernel", &counter_config[i].kernel);
+		oprofilefs_create_ulong(sb, dir, "user", &counter_config[i].user);
+	}
+
+	return 0;
+}
+
+static int oprofile_perf_setup(void)
+{
+	spin_lock(&oprofilefs_lock);
+	op_perf_setup();
+	spin_unlock(&oprofilefs_lock);
+	return 0;
+}
+
+static int oprofile_perf_start(void)
+{
+	int ret = -EBUSY;
+
+	mutex_lock(&oprofile_perf_mutex);
+	if (!oprofile_perf_enabled) {
+		ret = 0;
+		op_perf_start();
+		oprofile_perf_enabled = 1;
+	}
+	mutex_unlock(&oprofile_perf_mutex);
+	return ret;
+}
+
+static void oprofile_perf_stop(void)
+{
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled)
+		op_perf_stop();
+	oprofile_perf_enabled = 0;
+	mutex_unlock(&oprofile_perf_mutex);
+}
+
+#ifdef CONFIG_PM
+static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state)
+{
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled)
+		op_perf_stop();
+	mutex_unlock(&oprofile_perf_mutex);
+	return 0;
+}
+
+static int oprofile_perf_resume(struct platform_device *dev)
+{
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled && op_perf_start())
+		oprofile_perf_enabled = 0;
+	mutex_unlock(&oprofile_perf_mutex);
+	return 0;
+}
+
+static struct platform_driver oprofile_driver = {
+	.driver		= {
+		.name		= "oprofile-perf",
+	},
+	.resume		= oprofile_perf_resume,
+	.suspend	= oprofile_perf_suspend,
+};
+
+static struct platform_device *oprofile_pdev;
+
+static int __init init_driverfs(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&oprofile_driver);
+	if (ret)
+		goto out;
+
+	oprofile_pdev =	platform_device_register_simple(
+				oprofile_driver.driver.name, 0, NULL, 0);
+	if (IS_ERR(oprofile_pdev)) {
+		ret = PTR_ERR(oprofile_pdev);
+		platform_driver_unregister(&oprofile_driver);
+	}
+
+out:
+	return ret;
+}
+
+static void __exit exit_driverfs(void)
+{
+	platform_device_unregister(oprofile_pdev);
+	platform_driver_unregister(&oprofile_driver);
+}
+#else
+static int __init init_driverfs(void) { return 0; }
+#define exit_driverfs() do { } while (0)
+#endif /* CONFIG_PM */
+
+int __init oprofile_perf_init(struct oprofile_operations *ops)
+{
+	int cpu, ret = 0;
+
+	memset(&perf_events, 0, sizeof(perf_events));
+
+	num_counters = perf_num_counters();
+	if (num_counters <= 0) {
+		pr_info("oprofile: no performance counters\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
+	counter_config = kcalloc(num_counters,
+			sizeof(struct op_counter_config), GFP_KERNEL);
+
+	if (!counter_config) {
+		pr_info("oprofile: failed to allocate %d "
+				"counters\n", num_counters);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = init_driverfs();
+	if (ret)
+		goto out;
+
+	for_each_possible_cpu(cpu) {
+		perf_events[cpu] = kcalloc(num_counters,
+				sizeof(struct perf_event *), GFP_KERNEL);
+		if (!perf_events[cpu]) {
+			pr_info("oprofile: failed to allocate %d perf events "
+					"for cpu %d\n", num_counters, cpu);
+			ret = -ENOMEM;
+			goto out;
+		}
+	}
+
+	ops->create_files	= oprofile_perf_create_files;
+	ops->setup		= oprofile_perf_setup;
+	ops->start		= oprofile_perf_start;
+	ops->stop		= oprofile_perf_stop;
+	ops->shutdown		= oprofile_perf_stop;
+	ops->cpu_type		= op_name_from_perf_id();
+
+	if (!ops->cpu_type)
+		ret = -ENODEV;
+	else
+		pr_info("oprofile: using %s\n", ops->cpu_type);
+
+out:
+	if (ret) {
+		for_each_possible_cpu(cpu)
+			kfree(perf_events[cpu]);
+		kfree(counter_config);
+	}
+
+	return ret;
+}
+
+void __exit oprofile_perf_exit(void)
+{
+	int cpu, id;
+	struct perf_event *event;
+
+	for_each_possible_cpu(cpu) {
+		for (id = 0; id < num_counters; ++id) {
+			event = perf_events[cpu][id];
+			if (event)
+				perf_event_release_kernel(event);
+		}
+
+		kfree(perf_events[cpu]);
+	}
+
+	kfree(counter_config);
+	exit_driverfs();
+}
diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index 1574d4a..d67a833 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -15,6 +15,7 @@
 
 #include <linux/types.h>
 #include <linux/spinlock.h>
+#include <linux/init.h>
 #include <asm/atomic.h>
  
 /* Each escaped entry is prefixed by ESCAPE_CODE
@@ -186,6 +187,8 @@ int oprofile_add_data64(struct op_entry *entry, u64 val);
 int oprofile_write_commit(struct op_entry *entry);
 
 #ifdef CONFIG_PERF_EVENTS
+int __init oprofile_perf_init(struct oprofile_operations *ops);
+void __exit oprofile_perf_exit(void);
 char *op_name_from_perf_id(void);
 #endif /* CONFIG_PERF_EVENTS */
 
-- 
1.7.1

[PATCH 6/7] oprofile: Abstract the perf-events backend

[Matt Fleming]

Move the perf-events backend from arch/arm/oprofile into
drivers/oprofile so that the code can be shared between architectures.

This allows each architecture to maintain only a single copy of the PMU
accessor functions instead of one for both perf and OProfile. It also
becomes possible for other architectures to delete much of their
OProfile code in favour of the common code now available in
drivers/oprofile/oprofile_perf.c.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Tested-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/oprofile/Makefile       |    4 +
 arch/arm/oprofile/common.c       |  319 -------------------------------------
 drivers/oprofile/oprofile_perf.c |  326 ++++++++++++++++++++++++++++++++++++++
 include/linux/oprofile.h         |    3 +
 4 files changed, 333 insertions(+), 319 deletions(-)
 create mode 100644 drivers/oprofile/oprofile_perf.c

diff --git a/arch/arm/oprofile/Makefile b/arch/arm/oprofile/Makefile
index e666eaf..b2215c6 100644
--- a/arch/arm/oprofile/Makefile
+++ b/arch/arm/oprofile/Makefile
@@ -6,4 +6,8 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprofilefs.o oprofile_stats.o \
 		timer_int.o )
 
+ifeq ($(CONFIG_HW_PERF_EVENTS),y)
+DRIVER_OBJS += $(addprefix ../../../drivers/oprofile/, oprofile_perf.o)
+endif
+
 oprofile-y				:= $(DRIVER_OBJS) common.o
diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 8718311..8aa9744 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -25,136 +25,6 @@
 #include <asm/ptrace.h>
 
 #ifdef CONFIG_HW_PERF_EVENTS
-/*
- * Per performance monitor configuration as set via oprofilefs.
- */
-struct op_counter_config {
-	unsigned long count;
-	unsigned long enabled;
-	unsigned long event;
-	unsigned long unit_mask;
-	unsigned long kernel;
-	unsigned long user;
-	struct perf_event_attr attr;
-};
-
-static int oprofile_perf_enabled;
-static DEFINE_MUTEX(oprofile_perf_mutex);
-
-static struct op_counter_config *counter_config;
-static struct perf_event **perf_events[nr_cpumask_bits];
-static int num_counters;
-
-/*
- * Overflow callback for oprofile.
- */
-static void op_overflow_handler(struct perf_event *event, int unused,
-			struct perf_sample_data *data, struct pt_regs *regs)
-{
-	int id;
-	u32 cpu = smp_processor_id();
-
-	for (id = 0; id < num_counters; ++id)
-		if (perf_events[cpu][id] == event)
-			break;
-
-	if (id != num_counters)
-		oprofile_add_sample(regs, id);
-	else
-		pr_warning("oprofile: ignoring spurious overflow "
-				"on cpu %u\n", cpu);
-}
-
-/*
- * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile
- * settings in counter_config. Attributes are created as `pinned' events and
- * so are permanently scheduled on the PMU.
- */
-static void op_perf_setup(void)
-{
-	int i;
-	u32 size = sizeof(struct perf_event_attr);
-	struct perf_event_attr *attr;
-
-	for (i = 0; i < num_counters; ++i) {
-		attr = &counter_config[i].attr;
-		memset(attr, 0, size);
-		attr->type		= PERF_TYPE_RAW;
-		attr->size		= size;
-		attr->config		= counter_config[i].event;
-		attr->sample_period	= counter_config[i].count;
-		attr->pinned		= 1;
-	}
-}
-
-static int op_create_counter(int cpu, int event)
-{
-	int ret = 0;
-	struct perf_event *pevent;
-
-	if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL))
-		return ret;
-
-	pevent = perf_event_create_kernel_counter(&counter_config[event].attr,
-						  cpu, -1,
-						  op_overflow_handler);
-
-	if (IS_ERR(pevent)) {
-		ret = PTR_ERR(pevent);
-	} else if (pevent->state != PERF_EVENT_STATE_ACTIVE) {
-		pr_warning("oprofile: failed to enable event %d "
-				"on CPU %d\n", event, cpu);
-		ret = -EBUSY;
-	} else {
-		perf_events[cpu][event] = pevent;
-	}
-
-	return ret;
-}
-
-static void op_destroy_counter(int cpu, int event)
-{
-	struct perf_event *pevent = perf_events[cpu][event];
-
-	if (pevent) {
-		perf_event_release_kernel(pevent);
-		perf_events[cpu][event] = NULL;
-	}
-}
-
-/*
- * Called by oprofile_perf_start to create active perf events based on the
- * perviously configured attributes.
- */
-static int op_perf_start(void)
-{
-	int cpu, event, ret = 0;
-
-	for_each_online_cpu(cpu) {
-		for (event = 0; event < num_counters; ++event) {
-			ret = op_create_counter(cpu, event);
-			if (ret)
-				goto out;
-		}
-	}
-
-out:
-	return ret;
-}
-
-/*
- * Called by oprofile_perf_stop at the end of a profiling run.
- */
-static void op_perf_stop(void)
-{
-	int cpu, event;
-
-	for_each_online_cpu(cpu)
-		for (event = 0; event < num_counters; ++event)
-			op_destroy_counter(cpu, event);
-}
-
-
 char *op_name_from_perf_id(void)
 {
 	enum arm_perf_pmu_ids id = armpmu_get_pmu_id();
@@ -177,116 +47,6 @@ char *op_name_from_perf_id(void)
 	}
 }
 
-static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root)
-{
-	unsigned int i;
-
-	for (i = 0; i < num_counters; i++) {
-		struct dentry *dir;
-		char buf[4];
-
-		snprintf(buf, sizeof buf, "%d", i);
-		dir = oprofilefs_mkdir(sb, root, buf);
-		oprofilefs_create_ulong(sb, dir, "enabled", &counter_config[i].enabled);
-		oprofilefs_create_ulong(sb, dir, "event", &counter_config[i].event);
-		oprofilefs_create_ulong(sb, dir, "count", &counter_config[i].count);
-		oprofilefs_create_ulong(sb, dir, "unit_mask", &counter_config[i].unit_mask);
-		oprofilefs_create_ulong(sb, dir, "kernel", &counter_config[i].kernel);
-		oprofilefs_create_ulong(sb, dir, "user", &counter_config[i].user);
-	}
-
-	return 0;
-}
-
-static int oprofile_perf_setup(void)
-{
-	spin_lock(&oprofilefs_lock);
-	op_perf_setup();
-	spin_unlock(&oprofilefs_lock);
-	return 0;
-}
-
-static int oprofile_perf_start(void)
-{
-	int ret = -EBUSY;
-
-	mutex_lock(&oprofile_perf_mutex);
-	if (!oprofile_perf_enabled) {
-		ret = 0;
-		op_perf_start();
-		oprofile_perf_enabled = 1;
-	}
-	mutex_unlock(&oprofile_perf_mutex);
-	return ret;
-}
-
-static void oprofile_perf_stop(void)
-{
-	mutex_lock(&oprofile_perf_mutex);
-	if (oprofile_perf_enabled)
-		op_perf_stop();
-	oprofile_perf_enabled = 0;
-	mutex_unlock(&oprofile_perf_mutex);
-}
-
-#ifdef CONFIG_PM
-static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state)
-{
-	mutex_lock(&oprofile_perf_mutex);
-	if (oprofile_perf_enabled)
-		op_perf_stop();
-	mutex_unlock(&oprofile_perf_mutex);
-	return 0;
-}
-
-static int oprofile_perf_resume(struct platform_device *dev)
-{
-	mutex_lock(&oprofile_perf_mutex);
-	if (oprofile_perf_enabled && op_perf_start())
-		oprofile_perf_enabled = 0;
-	mutex_unlock(&oprofile_perf_mutex);
-	return 0;
-}
-
-static struct platform_driver oprofile_driver = {
-	.driver		= {
-		.name		= "oprofile-perf",
-	},
-	.resume		= oprofile_perf_resume,
-	.suspend	= oprofile_perf_suspend,
-};
-
-static struct platform_device *oprofile_pdev;
-
-static int __init init_driverfs(void)
-{
-	int ret;
-
-	ret = platform_driver_register(&oprofile_driver);
-	if (ret)
-		goto out;
-
-	oprofile_pdev =	platform_device_register_simple(
-				oprofile_driver.driver.name, 0, NULL, 0);
-	if (IS_ERR(oprofile_pdev)) {
-		ret = PTR_ERR(oprofile_pdev);
-		platform_driver_unregister(&oprofile_driver);
-	}
-
-out:
-	return ret;
-}
-
-static void __exit exit_driverfs(void)
-{
-	platform_device_unregister(oprofile_pdev);
-	platform_driver_unregister(&oprofile_driver);
-}
-#else
-static int __init init_driverfs(void) { return 0; }
-#define exit_driverfs() do { } while (0)
-#endif /* CONFIG_PM */
-
 static int report_trace(struct stackframe *frame, void *d)
 {
 	unsigned int *depth = d;
@@ -349,66 +109,6 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
 		tail = user_backtrace(tail);
 }
 
-int __init oprofile_perf_init(struct oprofile_operations *ops)
-{
-	int cpu, ret = 0;
-
-	memset(&perf_events, 0, sizeof(perf_events));
-
-	num_counters = perf_num_counters();
-	if (num_counters <= 0) {
-		pr_info("oprofile: no performance counters\n");
-		ret = -ENODEV;
-		goto out;
-	}
-
-	counter_config = kcalloc(num_counters,
-			sizeof(struct op_counter_config), GFP_KERNEL);
-
-	if (!counter_config) {
-		pr_info("oprofile: failed to allocate %d "
-				"counters\n", num_counters);
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	ret = init_driverfs();
-	if (ret)
-		goto out;
-
-	for_each_possible_cpu(cpu) {
-		perf_events[cpu] = kcalloc(num_counters,
-				sizeof(struct perf_event *), GFP_KERNEL);
-		if (!perf_events[cpu]) {
-			pr_info("oprofile: failed to allocate %d perf events "
-					"for cpu %d\n", num_counters, cpu);
-			ret = -ENOMEM;
-			goto out;
-		}
-	}
-
-	ops->create_files	= oprofile_perf_create_files;
-	ops->setup		= oprofile_perf_setup;
-	ops->start		= oprofile_perf_start;
-	ops->stop		= oprofile_perf_stop;
-	ops->shutdown		= oprofile_perf_stop;
-	ops->cpu_type		= op_name_from_perf_id();
-
-	if (!ops->cpu_type)
-		ret = -ENODEV;
-	else
-		pr_info("oprofile: using %s\n", ops->cpu_type);
-
-out:
-	if (ret) {
-		for_each_possible_cpu(cpu)
-			kfree(perf_events[cpu]);
-		kfree(counter_config);
-	}
-
-	return ret;
-}
-
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
 	ops->backtrace		= arm_backtrace;
@@ -416,25 +116,6 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
 	return oprofile_perf_init(ops);
 }
 
-void __exit oprofile_perf_exit(void)
-{
-	int cpu, id;
-	struct perf_event *event;
-
-	for_each_possible_cpu(cpu) {
-		for (id = 0; id < num_counters; ++id) {
-			event = perf_events[cpu][id];
-			if (event)
-				perf_event_release_kernel(event);
-		}
-
-		kfree(perf_events[cpu]);
-	}
-
-	kfree(counter_config);
-	exit_driverfs();
-}
-
 void __exit oprofile_arch_exit(void)
 {
 	oprofile_perf_exit();
diff --git a/drivers/oprofile/oprofile_perf.c b/drivers/oprofile/oprofile_perf.c
new file mode 100644
index 0000000..ebb40cb
--- /dev/null
+++ b/drivers/oprofile/oprofile_perf.c
@@ -0,0 +1,326 @@
+/*
+ * Copyright 2010 ARM Ltd.
+ *
+ * Perf-events backend for OProfile.
+ */
+#include <linux/perf_event.h>
+#include <linux/oprofile.h>
+#include <linux/slab.h>
+
+/*
+ * Per performance monitor configuration as set via oprofilefs.
+ */
+struct op_counter_config {
+	unsigned long count;
+	unsigned long enabled;
+	unsigned long event;
+	unsigned long unit_mask;
+	unsigned long kernel;
+	unsigned long user;
+	struct perf_event_attr attr;
+};
+
+static int oprofile_perf_enabled;
+static DEFINE_MUTEX(oprofile_perf_mutex);
+
+static struct op_counter_config *counter_config;
+static struct perf_event **perf_events[nr_cpumask_bits];
+static int num_counters;
+
+/*
+ * Overflow callback for oprofile.
+ */
+static void op_overflow_handler(struct perf_event *event, int unused,
+			struct perf_sample_data *data, struct pt_regs *regs)
+{
+	int id;
+	u32 cpu = smp_processor_id();
+
+	for (id = 0; id < num_counters; ++id)
+		if (perf_events[cpu][id] == event)
+			break;
+
+	if (id != num_counters)
+		oprofile_add_sample(regs, id);
+	else
+		pr_warning("oprofile: ignoring spurious overflow "
+				"on cpu %u\n", cpu);
+}
+
+/*
+ * Called by oprofile_perf_setup to create perf attributes to mirror the oprofile
+ * settings in counter_config. Attributes are created as `pinned' events and
+ * so are permanently scheduled on the PMU.
+ */
+static void op_perf_setup(void)
+{
+	int i;
+	u32 size = sizeof(struct perf_event_attr);
+	struct perf_event_attr *attr;
+
+	for (i = 0; i < num_counters; ++i) {
+		attr = &counter_config[i].attr;
+		memset(attr, 0, size);
+		attr->type		= PERF_TYPE_RAW;
+		attr->size		= size;
+		attr->config		= counter_config[i].event;
+		attr->sample_period	= counter_config[i].count;
+		attr->pinned		= 1;
+	}
+}
+
+static int op_create_counter(int cpu, int event)
+{
+	int ret = 0;
+	struct perf_event *pevent;
+
+	if (!counter_config[event].enabled || (perf_events[cpu][event] != NULL))
+		return ret;
+
+	pevent = perf_event_create_kernel_counter(&counter_config[event].attr,
+						  cpu, -1,
+						  op_overflow_handler);
+
+	if (IS_ERR(pevent)) {
+		ret = PTR_ERR(pevent);
+	} else if (pevent->state != PERF_EVENT_STATE_ACTIVE) {
+		pr_warning("oprofile: failed to enable event %d "
+				"on CPU %d\n", event, cpu);
+		ret = -EBUSY;
+	} else {
+		perf_events[cpu][event] = pevent;
+	}
+
+	return ret;
+}
+
+static void op_destroy_counter(int cpu, int event)
+{
+	struct perf_event *pevent = perf_events[cpu][event];
+
+	if (pevent) {
+		perf_event_release_kernel(pevent);
+		perf_events[cpu][event] = NULL;
+	}
+}
+
+/*
+ * Called by oprofile_perf_start to create active perf events based on the
+ * perviously configured attributes.
+ */
+static int op_perf_start(void)
+{
+	int cpu, event, ret = 0;
+
+	for_each_online_cpu(cpu) {
+		for (event = 0; event < num_counters; ++event) {
+			ret = op_create_counter(cpu, event);
+			if (ret)
+				goto out;
+		}
+	}
+
+out:
+	return ret;
+}
+
+/*
+ * Called by oprofile_perf_stop at the end of a profiling run.
+ */
+static void op_perf_stop(void)
+{
+	int cpu, event;
+
+	for_each_online_cpu(cpu)
+		for (event = 0; event < num_counters; ++event)
+			op_destroy_counter(cpu, event);
+}
+
+static int oprofile_perf_create_files(struct super_block *sb, struct dentry *root)
+{
+	unsigned int i;
+
+	for (i = 0; i < num_counters; i++) {
+		struct dentry *dir;
+		char buf[4];
+
+		snprintf(buf, sizeof buf, "%d", i);
+		dir = oprofilefs_mkdir(sb, root, buf);
+		oprofilefs_create_ulong(sb, dir, "enabled", &counter_config[i].enabled);
+		oprofilefs_create_ulong(sb, dir, "event", &counter_config[i].event);
+		oprofilefs_create_ulong(sb, dir, "count", &counter_config[i].count);
+		oprofilefs_create_ulong(sb, dir, "unit_mask", &counter_config[i].unit_mask);
+		oprofilefs_create_ulong(sb, dir, "kernel", &counter_config[i].kernel);
+		oprofilefs_create_ulong(sb, dir, "user", &counter_config[i].user);
+	}
+
+	return 0;
+}
+
+static int oprofile_perf_setup(void)
+{
+	spin_lock(&oprofilefs_lock);
+	op_perf_setup();
+	spin_unlock(&oprofilefs_lock);
+	return 0;
+}
+
+static int oprofile_perf_start(void)
+{
+	int ret = -EBUSY;
+
+	mutex_lock(&oprofile_perf_mutex);
+	if (!oprofile_perf_enabled) {
+		ret = 0;
+		op_perf_start();
+		oprofile_perf_enabled = 1;
+	}
+	mutex_unlock(&oprofile_perf_mutex);
+	return ret;
+}
+
+static void oprofile_perf_stop(void)
+{
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled)
+		op_perf_stop();
+	oprofile_perf_enabled = 0;
+	mutex_unlock(&oprofile_perf_mutex);
+}
+
+#ifdef CONFIG_PM
+static int oprofile_perf_suspend(struct platform_device *dev, pm_message_t state)
+{
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled)
+		op_perf_stop();
+	mutex_unlock(&oprofile_perf_mutex);
+	return 0;
+}
+
+static int oprofile_perf_resume(struct platform_device *dev)
+{
+	mutex_lock(&oprofile_perf_mutex);
+	if (oprofile_perf_enabled && op_perf_start())
+		oprofile_perf_enabled = 0;
+	mutex_unlock(&oprofile_perf_mutex);
+	return 0;
+}
+
+static struct platform_driver oprofile_driver = {
+	.driver		= {
+		.name		= "oprofile-perf",
+	},
+	.resume		= oprofile_perf_resume,
+	.suspend	= oprofile_perf_suspend,
+};
+
+static struct platform_device *oprofile_pdev;
+
+static int __init init_driverfs(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&oprofile_driver);
+	if (ret)
+		goto out;
+
+	oprofile_pdev =	platform_device_register_simple(
+				oprofile_driver.driver.name, 0, NULL, 0);
+	if (IS_ERR(oprofile_pdev)) {
+		ret = PTR_ERR(oprofile_pdev);
+		platform_driver_unregister(&oprofile_driver);
+	}
+
+out:
+	return ret;
+}
+
+static void __exit exit_driverfs(void)
+{
+	platform_device_unregister(oprofile_pdev);
+	platform_driver_unregister(&oprofile_driver);
+}
+#else
+static int __init init_driverfs(void) { return 0; }
+#define exit_driverfs() do { } while (0)
+#endif /* CONFIG_PM */
+
+int __init oprofile_perf_init(struct oprofile_operations *ops)
+{
+	int cpu, ret = 0;
+
+	memset(&perf_events, 0, sizeof(perf_events));
+
+	num_counters = perf_num_counters();
+	if (num_counters <= 0) {
+		pr_info("oprofile: no performance counters\n");
+		ret = -ENODEV;
+		goto out;
+	}
+
+	counter_config = kcalloc(num_counters,
+			sizeof(struct op_counter_config), GFP_KERNEL);
+
+	if (!counter_config) {
+		pr_info("oprofile: failed to allocate %d "
+				"counters\n", num_counters);
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	ret = init_driverfs();
+	if (ret)
+		goto out;
+
+	for_each_possible_cpu(cpu) {
+		perf_events[cpu] = kcalloc(num_counters,
+				sizeof(struct perf_event *), GFP_KERNEL);
+		if (!perf_events[cpu]) {
+			pr_info("oprofile: failed to allocate %d perf events "
+					"for cpu %d\n", num_counters, cpu);
+			ret = -ENOMEM;
+			goto out;
+		}
+	}
+
+	ops->create_files	= oprofile_perf_create_files;
+	ops->setup		= oprofile_perf_setup;
+	ops->start		= oprofile_perf_start;
+	ops->stop		= oprofile_perf_stop;
+	ops->shutdown		= oprofile_perf_stop;
+	ops->cpu_type		= op_name_from_perf_id();
+
+	if (!ops->cpu_type)
+		ret = -ENODEV;
+	else
+		pr_info("oprofile: using %s\n", ops->cpu_type);
+
+out:
+	if (ret) {
+		for_each_possible_cpu(cpu)
+			kfree(perf_events[cpu]);
+		kfree(counter_config);
+	}
+
+	return ret;
+}
+
+void __exit oprofile_perf_exit(void)
+{
+	int cpu, id;
+	struct perf_event *event;
+
+	for_each_possible_cpu(cpu) {
+		for (id = 0; id < num_counters; ++id) {
+			event = perf_events[cpu][id];
+			if (event)
+				perf_event_release_kernel(event);
+		}
+
+		kfree(perf_events[cpu]);
+	}
+
+	kfree(counter_config);
+	exit_driverfs();
+}
diff --git a/include/linux/oprofile.h b/include/linux/oprofile.h
index 1574d4a..d67a833 100644
--- a/include/linux/oprofile.h
+++ b/include/linux/oprofile.h
@@ -15,6 +15,7 @@
 
 #include <linux/types.h>
 #include <linux/spinlock.h>
+#include <linux/init.h>
 #include <asm/atomic.h>
  
 /* Each escaped entry is prefixed by ESCAPE_CODE
@@ -186,6 +187,8 @@ int oprofile_add_data64(struct op_entry *entry, u64 val);
 int oprofile_write_commit(struct op_entry *entry);
 
 #ifdef CONFIG_PERF_EVENTS
+int __init oprofile_perf_init(struct oprofile_operations *ops);
+void __exit oprofile_perf_exit(void);
 char *op_name_from_perf_id(void);
 #endif /* CONFIG_PERF_EVENTS */
 
-- 
1.7.1

[PATCH 7/7] sh: oprofile: Use perf-events oprofile backend

[Matt Fleming]

Now that we've got a generic perf-events based oprofile backend we might
as well make use of it seeing as SH doesn't do anything special with its
oprofile backend. Also introduce a new CONFIG_HW_PERF_EVENTS symbol so
that we can fallback to using the timer interrupt for oprofile if the
CPU doesn't support perf events.

Also, to avoid a section mismatch warning we need to annotate
oprofile_arch_exit() with an __exit marker.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Acked-by: Paul Mundt <lethal@linux-sh.org>
---
 arch/sh/Kconfig            |   13 +++++
 arch/sh/oprofile/Makefile  |    4 ++
 arch/sh/oprofile/common.c  |  115 +++++++++-----------------------------------
 arch/sh/oprofile/op_impl.h |   33 -------------
 4 files changed, 40 insertions(+), 125 deletions(-)
 delete mode 100644 arch/sh/oprofile/op_impl.h

diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 33990fa..35b6c3f 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -249,6 +249,11 @@ config ARCH_SHMOBILE
 	select PM
 	select PM_RUNTIME
 
+config CPU_HAS_PMU
+       depends on CPU_SH4 || CPU_SH4A
+       default y
+       bool
+
 if SUPERH32
 
 choice
@@ -738,6 +743,14 @@ config GUSA_RB
 	  LLSC, this should be more efficient than the other alternative of
 	  disabling interrupts around the atomic sequence.
 
+config HW_PERF_EVENTS
+	bool "Enable hardware performance counter support for perf events"
+	depends on PERF_EVENTS && CPU_HAS_PMU
+	default y
+	help
+	  Enable hardware performance counter support for perf events. If
+	  disabled, perf events will use software events only.
+
 source "drivers/sh/Kconfig"
 
 endmenu
diff --git a/arch/sh/oprofile/Makefile b/arch/sh/oprofile/Makefile
index 4886c5c..e85aae7 100644
--- a/arch/sh/oprofile/Makefile
+++ b/arch/sh/oprofile/Makefile
@@ -6,4 +6,8 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprofilefs.o oprofile_stats.o \
 		timer_int.o )
 
+ifeq ($(CONFIG_HW_PERF_EVENTS),y)
+DRIVER_OBJS += $(addprefix ../../../drivers/oprofile/, oprofile_perf.o)
+endif
+
 oprofile-y	:= $(DRIVER_OBJS) common.o backtrace.o
diff --git a/arch/sh/oprofile/common.c b/arch/sh/oprofile/common.c
index ac60493..e10d893 100644
--- a/arch/sh/oprofile/common.c
+++ b/arch/sh/oprofile/common.c
@@ -17,114 +17,45 @@
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/smp.h>
+#include <linux/perf_event.h>
 #include <asm/processor.h>
-#include "op_impl.h"
-
-static struct op_sh_model *model;
-
-static struct op_counter_config ctr[20];
 
+#ifdef CONFIG_HW_PERF_EVENTS
 extern void sh_backtrace(struct pt_regs * const regs, unsigned int depth);
 
-static int op_sh_setup(void)
-{
-	/* Pre-compute the values to stuff in the hardware registers.  */
-	model->reg_setup(ctr);
-
-	/* Configure the registers on all cpus.  */
-	on_each_cpu(model->cpu_setup, NULL, 1);
-
-        return 0;
-}
-
-static int op_sh_create_files(struct super_block *sb, struct dentry *root)
+char *op_name_from_perf_id(void)
 {
-	int i, ret = 0;
+	const char *pmu;
+	char buf[20];
+	int size;
 
-	for (i = 0; i < model->num_counters; i++) {
-		struct dentry *dir;
-		char buf[4];
+	pmu = perf_pmu_name();
+	if (!pmu)
+		return NULL;
 
-		snprintf(buf, sizeof(buf), "%d", i);
-		dir = oprofilefs_mkdir(sb, root, buf);
+	size = snprintf(buf, sizeof(buf), "sh/%s", pmu);
+	if (size > -1 && size < sizeof(buf))
+		return buf;
 
-		ret |= oprofilefs_create_ulong(sb, dir, "enabled", &ctr[i].enabled);
-		ret |= oprofilefs_create_ulong(sb, dir, "event", &ctr[i].event);
-		ret |= oprofilefs_create_ulong(sb, dir, "kernel", &ctr[i].kernel);
-		ret |= oprofilefs_create_ulong(sb, dir, "user", &ctr[i].user);
-
-		if (model->create_files)
-			ret |= model->create_files(sb, dir);
-		else
-			ret |= oprofilefs_create_ulong(sb, dir, "count", &ctr[i].count);
-
-		/* Dummy entries */
-		ret |= oprofilefs_create_ulong(sb, dir, "unit_mask", &ctr[i].unit_mask);
-	}
-
-	return ret;
+	return NULL;
 }
 
-static int op_sh_start(void)
+int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
-	/* Enable performance monitoring for all counters.  */
-	on_each_cpu(model->cpu_start, NULL, 1);
+	ops->backtrace = sh_backtrace;
 
-	return 0;
+	return oprofile_perf_init(ops);
 }
 
-static void op_sh_stop(void)
+void __exit oprofile_arch_exit(void)
 {
-	/* Disable performance monitoring for all counters.  */
-	on_each_cpu(model->cpu_stop, NULL, 1);
+	oprofile_perf_exit();
 }
-
+#else
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
-	struct op_sh_model *lmodel = NULL;
-	int ret;
-
-	/*
-	 * Always assign the backtrace op. If the counter initialization
-	 * fails, we fall back to the timer which will still make use of
-	 * this.
-	 */
-	ops->backtrace = sh_backtrace;
-
-	/*
-	 * XXX
-	 *
-	 * All of the SH7750/SH-4A counters have been converted to perf,
-	 * this infrastructure hook is left for other users until they've
-	 * had a chance to convert over, at which point all of this
-	 * will be deleted.
-	 */
-
-	if (!lmodel)
-		return -ENODEV;
-	if (!(current_cpu_data.flags & CPU_HAS_PERF_COUNTER))
-		return -ENODEV;
-
-	ret = lmodel->init();
-	if (unlikely(ret != 0))
-		return ret;
-
-	model = lmodel;
-
-	ops->setup		= op_sh_setup;
-	ops->create_files	= op_sh_create_files;
-	ops->start		= op_sh_start;
-	ops->stop		= op_sh_stop;
-	ops->cpu_type		= lmodel->cpu_type;
-
-	printk(KERN_INFO "oprofile: using %s performance monitoring.\n",
-	       lmodel->cpu_type);
-
-	return 0;
-}
-
-void oprofile_arch_exit(void)
-{
-	if (model && model->exit)
-		model->exit();
+	pr_info("oprofile: hardware counters not available\n");
+	return -ENODEV;
 }
+void __exit oprofile_arch_exit(void) {}
+#endif /* CONFIG_HW_PERF_EVENTS */
diff --git a/arch/sh/oprofile/op_impl.h b/arch/sh/oprofile/op_impl.h
deleted file mode 100644
index 1244479..0000000
--- a/arch/sh/oprofile/op_impl.h
+++ /dev/null
@@ -1,33 +0,0 @@
-#ifndef __OP_IMPL_H
-#define __OP_IMPL_H
-
-/* Per-counter configuration as set via oprofilefs.  */
-struct op_counter_config {
-	unsigned long enabled;
-	unsigned long event;
-
-	unsigned long count;
-
-	/* Dummy values for userspace tool compliance */
-	unsigned long kernel;
-	unsigned long user;
-	unsigned long unit_mask;
-};
-
-/* Per-architecture configury and hooks.  */
-struct op_sh_model {
-	void (*reg_setup)(struct op_counter_config *);
-	int (*create_files)(struct super_block *sb, struct dentry *dir);
-	void (*cpu_setup)(void *dummy);
-	int (*init)(void);
-	void (*exit)(void);
-	void (*cpu_start)(void *args);
-	void (*cpu_stop)(void *args);
-	char *cpu_type;
-	unsigned char num_counters;
-};
-
-/* arch/sh/oprofile/common.c */
-extern void sh_backtrace(struct pt_regs * const regs, unsigned int depth);
-
-#endif /* __OP_IMPL_H */
-- 
1.7.1

[PATCH 7/7] sh: oprofile: Use perf-events oprofile backend

[Matt Fleming]

Now that we've got a generic perf-events based oprofile backend we might
as well make use of it seeing as SH doesn't do anything special with its
oprofile backend. Also introduce a new CONFIG_HW_PERF_EVENTS symbol so
that we can fallback to using the timer interrupt for oprofile if the
CPU doesn't support perf events.

Also, to avoid a section mismatch warning we need to annotate
oprofile_arch_exit() with an __exit marker.

Signed-off-by: Matt Fleming <matt@console-pimps.org>
Acked-by: Paul Mundt <lethal@linux-sh.org>
---
 arch/sh/Kconfig            |   13 +++++
 arch/sh/oprofile/Makefile  |    4 ++
 arch/sh/oprofile/common.c  |  115 +++++++++-----------------------------------
 arch/sh/oprofile/op_impl.h |   33 -------------
 4 files changed, 40 insertions(+), 125 deletions(-)
 delete mode 100644 arch/sh/oprofile/op_impl.h

diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 33990fa..35b6c3f 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -249,6 +249,11 @@ config ARCH_SHMOBILE
 	select PM
 	select PM_RUNTIME
 
+config CPU_HAS_PMU
+       depends on CPU_SH4 || CPU_SH4A
+       default y
+       bool
+
 if SUPERH32
 
 choice
@@ -738,6 +743,14 @@ config GUSA_RB
 	  LLSC, this should be more efficient than the other alternative of
 	  disabling interrupts around the atomic sequence.
 
+config HW_PERF_EVENTS
+	bool "Enable hardware performance counter support for perf events"
+	depends on PERF_EVENTS && CPU_HAS_PMU
+	default y
+	help
+	  Enable hardware performance counter support for perf events. If
+	  disabled, perf events will use software events only.
+
 source "drivers/sh/Kconfig"
 
 endmenu
diff --git a/arch/sh/oprofile/Makefile b/arch/sh/oprofile/Makefile
index 4886c5c..e85aae7 100644
--- a/arch/sh/oprofile/Makefile
+++ b/arch/sh/oprofile/Makefile
@@ -6,4 +6,8 @@ DRIVER_OBJS = $(addprefix ../../../drivers/oprofile/, \
 		oprofilefs.o oprofile_stats.o \
 		timer_int.o )
 
+ifeq ($(CONFIG_HW_PERF_EVENTS),y)
+DRIVER_OBJS += $(addprefix ../../../drivers/oprofile/, oprofile_perf.o)
+endif
+
 oprofile-y	:= $(DRIVER_OBJS) common.o backtrace.o
diff --git a/arch/sh/oprofile/common.c b/arch/sh/oprofile/common.c
index ac60493..e10d893 100644
--- a/arch/sh/oprofile/common.c
+++ b/arch/sh/oprofile/common.c
@@ -17,114 +17,45 @@
 #include <linux/init.h>
 #include <linux/errno.h>
 #include <linux/smp.h>
+#include <linux/perf_event.h>
 #include <asm/processor.h>
-#include "op_impl.h"
-
-static struct op_sh_model *model;
-
-static struct op_counter_config ctr[20];
 
+#ifdef CONFIG_HW_PERF_EVENTS
 extern void sh_backtrace(struct pt_regs * const regs, unsigned int depth);
 
-static int op_sh_setup(void)
-{
-	/* Pre-compute the values to stuff in the hardware registers.  */
-	model->reg_setup(ctr);
-
-	/* Configure the registers on all cpus.  */
-	on_each_cpu(model->cpu_setup, NULL, 1);
-
-        return 0;
-}
-
-static int op_sh_create_files(struct super_block *sb, struct dentry *root)
+char *op_name_from_perf_id(void)
 {
-	int i, ret = 0;
+	const char *pmu;
+	char buf[20];
+	int size;
 
-	for (i = 0; i < model->num_counters; i++) {
-		struct dentry *dir;
-		char buf[4];
+	pmu = perf_pmu_name();
+	if (!pmu)
+		return NULL;
 
-		snprintf(buf, sizeof(buf), "%d", i);
-		dir = oprofilefs_mkdir(sb, root, buf);
+	size = snprintf(buf, sizeof(buf), "sh/%s", pmu);
+	if (size > -1 && size < sizeof(buf))
+		return buf;
 
-		ret |= oprofilefs_create_ulong(sb, dir, "enabled", &ctr[i].enabled);
-		ret |= oprofilefs_create_ulong(sb, dir, "event", &ctr[i].event);
-		ret |= oprofilefs_create_ulong(sb, dir, "kernel", &ctr[i].kernel);
-		ret |= oprofilefs_create_ulong(sb, dir, "user", &ctr[i].user);
-
-		if (model->create_files)
-			ret |= model->create_files(sb, dir);
-		else
-			ret |= oprofilefs_create_ulong(sb, dir, "count", &ctr[i].count);
-
-		/* Dummy entries */
-		ret |= oprofilefs_create_ulong(sb, dir, "unit_mask", &ctr[i].unit_mask);
-	}
-
-	return ret;
+	return NULL;
 }
 
-static int op_sh_start(void)
+int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
-	/* Enable performance monitoring for all counters.  */
-	on_each_cpu(model->cpu_start, NULL, 1);
+	ops->backtrace = sh_backtrace;
 
-	return 0;
+	return oprofile_perf_init(ops);
 }
 
-static void op_sh_stop(void)
+void __exit oprofile_arch_exit(void)
 {
-	/* Disable performance monitoring for all counters.  */
-	on_each_cpu(model->cpu_stop, NULL, 1);
+	oprofile_perf_exit();
 }
-
+#else
 int __init oprofile_arch_init(struct oprofile_operations *ops)
 {
-	struct op_sh_model *lmodel = NULL;
-	int ret;
-
-	/*
-	 * Always assign the backtrace op. If the counter initialization
-	 * fails, we fall back to the timer which will still make use of
-	 * this.
-	 */
-	ops->backtrace = sh_backtrace;
-
-	/*
-	 * XXX
-	 *
-	 * All of the SH7750/SH-4A counters have been converted to perf,
-	 * this infrastructure hook is left for other users until they've
-	 * had a chance to convert over, at which point all of this
-	 * will be deleted.
-	 */
-
-	if (!lmodel)
-		return -ENODEV;
-	if (!(current_cpu_data.flags & CPU_HAS_PERF_COUNTER))
-		return -ENODEV;
-
-	ret = lmodel->init();
-	if (unlikely(ret != 0))
-		return ret;
-
-	model = lmodel;
-
-	ops->setup		= op_sh_setup;
-	ops->create_files	= op_sh_create_files;
-	ops->start		= op_sh_start;
-	ops->stop		= op_sh_stop;
-	ops->cpu_type		= lmodel->cpu_type;
-
-	printk(KERN_INFO "oprofile: using %s performance monitoring.\n",
-	       lmodel->cpu_type);
-
-	return 0;
-}
-
-void oprofile_arch_exit(void)
-{
-	if (model && model->exit)
-		model->exit();
+	pr_info("oprofile: hardware counters not available\n");
+	return -ENODEV;
 }
+void __exit oprofile_arch_exit(void) {}
+#endif /* CONFIG_HW_PERF_EVENTS */
diff --git a/arch/sh/oprofile/op_impl.h b/arch/sh/oprofile/op_impl.h
deleted file mode 100644
index 1244479..0000000
--- a/arch/sh/oprofile/op_impl.h
+++ /dev/null
@@ -1,33 +0,0 @@
-#ifndef __OP_IMPL_H
-#define __OP_IMPL_H
-
-/* Per-counter configuration as set via oprofilefs.  */
-struct op_counter_config {
-	unsigned long enabled;
-	unsigned long event;
-
-	unsigned long count;
-
-	/* Dummy values for userspace tool compliance */
-	unsigned long kernel;
-	unsigned long user;
-	unsigned long unit_mask;
-};
-
-/* Per-architecture configury and hooks.  */
-struct op_sh_model {
-	void (*reg_setup)(struct op_counter_config *);
-	int (*create_files)(struct super_block *sb, struct dentry *dir);
-	void (*cpu_setup)(void *dummy);
-	int (*init)(void);
-	void (*exit)(void);
-	void (*cpu_start)(void *args);
-	void (*cpu_stop)(void *args);
-	char *cpu_type;
-	unsigned char num_counters;
-};
-
-/* arch/sh/oprofile/common.c */
-extern void sh_backtrace(struct pt_regs * const regs, unsigned int depth);
-
-#endif /* __OP_IMPL_H */
-- 
1.7.1

Re: [PATCH 5/7] ARM: oprofile: Move non-ARM code into separate init/exit

[Robert Richter]

On 08.10.10 20:46:20, Matt Fleming wrote:
> In preparation for moving the majority of this oprofile code into an
> architecture-neutral place separate the architecture-independent code
> into oprofile_perf_init() and oprofile_perf_exit().
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> Tested-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm/oprofile/common.c |   27 +++++++++++++++++++--------
>  1 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> index fd6e323..8718311 100644
> --- a/arch/arm/oprofile/common.c
> +++ b/arch/arm/oprofile/common.c
> @@ -349,7 +349,7 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
>  		tail = user_backtrace(tail);
>  }
>  
> -int __init oprofile_arch_init(struct oprofile_operations *ops)
> +int __init oprofile_perf_init(struct oprofile_operations *ops)
>  {
>  	int cpu, ret = 0;
>  
> @@ -387,12 +387,11 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
>  		}
>  	}
>  
> -	ops->backtrace		= arm_backtrace;
> -	ops->create_files	= op_arm_create_files;
> -	ops->setup		= op_arm_setup;
> -	ops->start		= op_arm_start;
> -	ops->stop		= op_arm_stop;
> -	ops->shutdown		= op_arm_stop;
> +	ops->create_files	= oprofile_perf_create_files;
> +	ops->setup		= oprofile_perf_setup;
> +	ops->start		= oprofile_perf_start;
> +	ops->stop		= oprofile_perf_stop;
> +	ops->shutdown		= oprofile_perf_stop;

There is a compile breakage caused by patch #4. Will move the renames
to patch #4 when applying the patch.

-Robert

>  	ops->cpu_type		= op_name_from_perf_id();
>  
>  	if (!ops->cpu_type)
> @@ -410,7 +409,14 @@ out:
>  	return ret;
>  }
>  
> -void __exit oprofile_arch_exit(void)
> +int __init oprofile_arch_init(struct oprofile_operations *ops)
> +{
> +	ops->backtrace		= arm_backtrace;
> +
> +	return oprofile_perf_init(ops);
> +}
> +
> +void __exit oprofile_perf_exit(void)
>  {
>  	int cpu, id;
>  	struct perf_event *event;
> @@ -428,6 +434,11 @@ void __exit oprofile_arch_exit(void)
>  	kfree(counter_config);
>  	exit_driverfs();
>  }
> +
> +void __exit oprofile_arch_exit(void)
> +{
> +	oprofile_perf_exit();
> +}
>  #else
>  int __init oprofile_arch_init(struct oprofile_operations *ops)
>  {
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 5/7] ARM: oprofile: Move non-ARM code into separate init/exit

[Robert Richter]

On 09.10.10 03:26:47, Robert Richter wrote:
> There is a compile breakage caused by patch #4. Will move the renames
> to patch #4 when applying the patch.

(For some reason I did not receive your mail 0/7, so I reply here.)

Matt,

on the first glance you patches are looking good (the above is the
only thing I found). Will do some further testing and waiting for more
replies. If all goes well I am going to apply them.

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 5/7] ARM: oprofile: Move non-ARM code into separate init/exit

[Matt Fleming]

On Sat, Oct 09, 2010 at 03:26:47AM +0200, Robert Richter wrote:
> On 08.10.10 20:46:20, Matt Fleming wrote:
> > In preparation for moving the majority of this oprofile code into an
> > architecture-neutral place separate the architecture-independent code
> > into oprofile_perf_init() and oprofile_perf_exit().
> > 
> > Signed-off-by: Matt Fleming <matt@console-pimps.org>
> > Tested-by: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm/oprofile/common.c |   27 +++++++++++++++++++--------
> >  1 files changed, 19 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> > index fd6e323..8718311 100644
> > --- a/arch/arm/oprofile/common.c
> > +++ b/arch/arm/oprofile/common.c
> > @@ -349,7 +349,7 @@ static void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
> >  		tail = user_backtrace(tail);
> >  }
> >  
> > -int __init oprofile_arch_init(struct oprofile_operations *ops)
> > +int __init oprofile_perf_init(struct oprofile_operations *ops)
> >  {
> >  	int cpu, ret = 0;
> >  
> > @@ -387,12 +387,11 @@ int __init oprofile_arch_init(struct oprofile_operations *ops)
> >  		}
> >  	}
> >  
> > -	ops->backtrace		= arm_backtrace;
> > -	ops->create_files	= op_arm_create_files;
> > -	ops->setup		= op_arm_setup;
> > -	ops->start		= op_arm_start;
> > -	ops->stop		= op_arm_stop;
> > -	ops->shutdown		= op_arm_stop;
> > +	ops->create_files	= oprofile_perf_create_files;
> > +	ops->setup		= oprofile_perf_setup;
> > +	ops->start		= oprofile_perf_start;
> > +	ops->stop		= oprofile_perf_stop;
> > +	ops->shutdown		= oprofile_perf_stop;
> 
> There is a compile breakage caused by patch #4. Will move the renames
> to patch #4 when applying the patch.

Sorry, I don't know how this happened. These renames were originally in
patch #4 but seem to have moved into #5 when I was reshuffling stuff.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 08.10.10 20:46:17, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>

Peter, I need your ack here.

Thanks,

-Robert

> ---
>  arch/sh/kernel/perf_event.c |    9 +++++++++
>  include/linux/perf_event.h  |    1 +
>  kernel/perf_event.c         |    5 +++++
>  3 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> index 2cb9ad5..55fe89b 100644
> --- a/arch/sh/kernel/perf_event.c
> +++ b/arch/sh/kernel/perf_event.c
> @@ -59,6 +59,15 @@ static inline int sh_pmu_initialized(void)
>  	return !!sh_pmu;
>  }
>  
> +const char *perf_pmu_name(void)
> +{
> +	if (!sh_pmu)
> +		return NULL;
> +
> +	return sh_pmu->name;
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);
> +
>  int perf_num_counters(void)
>  {
>  	if (!sh_pmu)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1a02192..33f08da 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -850,6 +850,7 @@ extern int perf_max_events;
>  extern const struct pmu *hw_perf_event_init(struct perf_event *event);
>  
>  extern int perf_num_counters(void);
> +extern const char *perf_pmu_name(void);
>  extern void perf_event_task_sched_in(struct task_struct *task);
>  extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
>  extern void perf_event_task_tick(struct task_struct *task);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index db5b560..fc51268 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
>  
>  void __weak perf_event_print_debug(void)	{ }
>  
> +extern __weak const char *perf_pmu_name(void)
> +{
> +	return "pmu";
> +}
> +
>  static DEFINE_PER_CPU(int, perf_disable_count);
>  
>  void perf_disable(void)
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Robert Richter]

On 08.10.10 20:46:17, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>

Peter, I need your ack here.

Thanks,

-Robert

> ---
>  arch/sh/kernel/perf_event.c |    9 +++++++++
>  include/linux/perf_event.h  |    1 +
>  kernel/perf_event.c         |    5 +++++
>  3 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> index 2cb9ad5..55fe89b 100644
> --- a/arch/sh/kernel/perf_event.c
> +++ b/arch/sh/kernel/perf_event.c
> @@ -59,6 +59,15 @@ static inline int sh_pmu_initialized(void)
>  	return !!sh_pmu;
>  }
>  
> +const char *perf_pmu_name(void)
> +{
> +	if (!sh_pmu)
> +		return NULL;
> +
> +	return sh_pmu->name;
> +}
> +EXPORT_SYMBOL_GPL(perf_pmu_name);
> +
>  int perf_num_counters(void)
>  {
>  	if (!sh_pmu)
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 1a02192..33f08da 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -850,6 +850,7 @@ extern int perf_max_events;
>  extern const struct pmu *hw_perf_event_init(struct perf_event *event);
>  
>  extern int perf_num_counters(void);
> +extern const char *perf_pmu_name(void);
>  extern void perf_event_task_sched_in(struct task_struct *task);
>  extern void perf_event_task_sched_out(struct task_struct *task, struct task_struct *next);
>  extern void perf_event_task_tick(struct task_struct *task);
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index db5b560..fc51268 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -85,6 +85,11 @@ void __weak hw_perf_enable(void)		{ barrier(); }
>  
>  void __weak perf_event_print_debug(void)	{ }
>  
> +extern __weak const char *perf_pmu_name(void)
> +{
> +	return "pmu";
> +}
> +
>  static DEFINE_PER_CPU(int, perf_disable_count);
>  
>  void perf_disable(void)
> -- 
> 1.7.1
> 
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 2/7] perf: New helper function for pmu name

[Peter Zijlstra]

On Mon, 2010-10-11 at 11:18 +0200, Robert Richter wrote:
> On 08.10.10 20:46:17, Matt Fleming wrote:
> > Introduce perf_pmu_name() helper function that returns the name of the
> > pmu. This gives us a generic way to get the name of a pmu regardless of
> > how an architecture identifies it internally.
> > 
> > Signed-off-by: Matt Fleming <matt@console-pimps.org>
> 
> Peter, I need your ack here.

Sure,

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH 2/7] perf: New helper function for pmu name

[Peter Zijlstra]

On Mon, 2010-10-11 at 11:18 +0200, Robert Richter wrote:
> On 08.10.10 20:46:17, Matt Fleming wrote:
> > Introduce perf_pmu_name() helper function that returns the name of the
> > pmu. This gives us a generic way to get the name of a pmu regardless of
> > how an architecture identifies it internally.
> > 
> > Signed-off-by: Matt Fleming <matt@console-pimps.org>
> 
> Peter, I need your ack here.

Sure,

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH 7/7] sh: oprofile: Use perf-events oprofile backend

[Robert Richter]

On 08.10.10 20:46:22, Matt Fleming wrote:
> Now that we've got a generic perf-events based oprofile backend we might
> as well make use of it seeing as SH doesn't do anything special with its
> oprofile backend. Also introduce a new CONFIG_HW_PERF_EVENTS symbol so
> that we can fallback to using the timer interrupt for oprofile if the
> CPU doesn't support perf events.
> 
> Also, to avoid a section mismatch warning we need to annotate
> oprofile_arch_exit() with an __exit marker.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> Acked-by: Paul Mundt <lethal@linux-sh.org>
> ---
>  arch/sh/Kconfig            |   13 +++++
>  arch/sh/oprofile/Makefile  |    4 ++
>  arch/sh/oprofile/common.c  |  115 +++++++++-----------------------------------
>  arch/sh/oprofile/op_impl.h |   33 -------------
>  4 files changed, 40 insertions(+), 125 deletions(-)
>  delete mode 100644 arch/sh/oprofile/op_impl.h

> -static int op_sh_create_files(struct super_block *sb, struct dentry *root)
> +char *op_name_from_perf_id(void)
>  {
> -	int i, ret = 0;
> +	const char *pmu;
> +	char buf[20];
> +	int size;
>  
> -	for (i = 0; i < model->num_counters; i++) {
> -		struct dentry *dir;
> -		char buf[4];
> +	pmu = perf_pmu_name();
> +	if (!pmu)
> +		return NULL;
>  
> -		snprintf(buf, sizeof(buf), "%d", i);
> -		dir = oprofilefs_mkdir(sb, root, buf);
> +	size = snprintf(buf, sizeof(buf), "sh/%s", pmu);

Matt and Paul,

are those (upper case) cpu_type strings already supported by the
oprofile userland?

-Robert

> +	if (size > -1 && size < sizeof(buf))
> +		return buf;
>  
> -		ret |= oprofilefs_create_ulong(sb, dir, "enabled", &ctr[i].enabled);
> -		ret |= oprofilefs_create_ulong(sb, dir, "event", &ctr[i].event);
> -		ret |= oprofilefs_create_ulong(sb, dir, "kernel", &ctr[i].kernel);
> -		ret |= oprofilefs_create_ulong(sb, dir, "user", &ctr[i].user);
> -
> -		if (model->create_files)
> -			ret |= model->create_files(sb, dir);
> -		else
> -			ret |= oprofilefs_create_ulong(sb, dir, "count", &ctr[i].count);
> -
> -		/* Dummy entries */
> -		ret |= oprofilefs_create_ulong(sb, dir, "unit_mask", &ctr[i].unit_mask);
> -	}
> -
> -	return ret;
> +	return NULL;
>  }

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH 7/7] sh: oprofile: Use perf-events oprofile backend

[Paul Mundt]

On Mon, Oct 11, 2010 at 01:06:54PM +0200, Robert Richter wrote:
> Matt and Paul,
> 
> are those (upper case) cpu_type strings already supported by the
> oprofile userland?
> 
I've only ever used out-of-tree patches with it due to the generally
volatile nature of support. Now that we have a more consistent interface
to go through, it probably makes sense to get those bits merged, too.
We're certainly not tied down to any ABI at this point.

Re: [PATCH 2/7] perf: New helper function for pmu name

[Paul Mundt]

On Sat, Oct 09, 2010 at 01:46:17AM +0100, Matt Fleming wrote:
> Introduce perf_pmu_name() helper function that returns the name of the
> pmu. This gives us a generic way to get the name of a pmu regardless of
> how an architecture identifies it internally.
> 
> Signed-off-by: Matt Fleming <matt@console-pimps.org>
> ---
>  arch/sh/kernel/perf_event.c |    9 +++++++++
>  include/linux/perf_event.h  |    1 +
>  kernel/perf_event.c         |    5 +++++
>  3 files changed, 15 insertions(+), 0 deletions(-)
> 
This part is obviously fine with me, too.

Acked-by: Paul Mundt <lethal@linux-sh.org>

Re: [PATCH V5 0/7] Generalise ARM perf-events backend for oprofile

[Robert Richter]

On 08.10.10 20:46:15, Matt Fleming wrote:
> The perf-events backend for OProfile that Will Deacon wrote in
> 8c1fc96f6fd1f361428ba805103af0d0eee65179 ("ARM: 6072/1: oprofile: use
> perf-events framework as backend") is of use to more architectures
> than just ARM. Move the code into drivers/oprofile/ so that SH can use
> it instead of the nearly identical copy of its OProfile code.
> 
> The benefit of the backend is that it becomes necessary to only
> maintain one copy of the PMU accessor functions for each architecture,
> with bug fixes and new features benefiting both OProfile and perf.

> Matt Fleming (7):
>   perf: Add helper function to return number of counters
>   perf: New helper function for pmu name
>   oprofile: Make op_name_from_perf_id() global
>   ARM: oprofile: Rename op_arm to oprofile_perf
>   ARM: oprofile: Move non-ARM code into separate init/exit
>   oprofile: Abstract the perf-events backend
>   sh: oprofile: Use perf-events oprofile backend
> 
>  arch/arm/kernel/perf_event.c     |    6 +
>  arch/arm/oprofile/Makefile       |    4 +
>  arch/arm/oprofile/common.c       |  309 +-----------------------------------
>  arch/sh/Kconfig                  |   13 ++
>  arch/sh/kernel/perf_event.c      |   18 ++
>  arch/sh/oprofile/Makefile        |    4 +
>  arch/sh/oprofile/common.c        |  115 +++-----------
>  arch/sh/oprofile/op_impl.h       |   33 ----
>  drivers/oprofile/oprofile_perf.c |  326 ++++++++++++++++++++++++++++++++++++++
>  include/linux/oprofile.h         |    7 +
>  include/linux/perf_event.h       |    2 +
>  kernel/perf_event.c              |    5 +
>  12 files changed, 412 insertions(+), 430 deletions(-)
>  delete mode 100644 arch/sh/oprofile/op_impl.h
>  create mode 100644 drivers/oprofile/oprofile_perf.c

Thanks Matt, I applied your patches to the oprofile tree and merged
them with the core branch. As I already wrote, I modified patch #4 to
fix the compile breakage.

The patches are available for testing in

 git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core

Thanks again,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

Re: [PATCH V5 0/7] Generalise ARM perf-events backend for oprofile

[Matt Fleming]

On Mon, Oct 11, 2010 at 08:59:23PM +0200, Robert Richter wrote:
> 
> Thanks Matt, I applied your patches to the oprofile tree and merged
> them with the core branch. As I already wrote, I modified patch #4 to
> fix the compile breakage.
> 
> The patches are available for testing in
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> Thanks again,
> 
> -Robert

Thanks Robert, I've just read through the merge and it looks good!

Re: [PATCH V5 0/7] Generalise ARM perf-events backend for oprofile

[Matt Fleming]

On Mon, Oct 11, 2010 at 08:59:23PM +0200, Robert Richter wrote:
> 
> Thanks Matt, I applied your patches to the oprofile tree and merged
> them with the core branch. As I already wrote, I modified patch #4 to
> fix the compile breakage.
> 
> The patches are available for testing in
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git core
> 
> Thanks again,
> 
> -Robert

Thanks Robert, I've just read through the merge and it looks good!