[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[jgq516 at gmail.com]

From: Xiao Jiang <jgq516@gmail.com>

Add related name for A5/A7/A15 which are consistent with the OProfile
user ABI.

Signed-off-by: Xiao Jiang <jgq516@gmail.com>
---
 arch/arm/oprofile/common.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
index 99c63d4b..ec10db1 100644
--- a/arch/arm/oprofile/common.c
+++ b/arch/arm/oprofile/common.c
@@ -37,8 +37,11 @@ static struct op_perf_name {
 	{ "xscale1",		"arm/xscale2"	},
 	{ "v6",			"arm/armv6"	},
 	{ "v6mpcore",		"arm/mpcore"	},
+	{ "ARMv7 Cortex-A5",	"arm/armv7-ca5"	},
+	{ "ARMv7 Cortex-A7",	"arm/armv7-ca7"	},
 	{ "ARMv7 Cortex-A8",	"arm/armv7"	},
 	{ "ARMv7 Cortex-A9",	"arm/armv7-ca9"	},
+	{ "ARMv7 Cortex-A15",	"arm/armv7-ca15" },
 };
 
 char *op_name_from_perf_id(void)
-- 
1.7.3

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Will Deacon]

On Fri, Nov 02, 2012 at 10:43:36AM +0000, jgq516 at gmail.com wrote:
> From: Xiao Jiang <jgq516@gmail.com>
> 
> Add related name for A5/A7/A15 which are consistent with the OProfile
> user ABI.
> 
> Signed-off-by: Xiao Jiang <jgq516@gmail.com>
> ---
>  arch/arm/oprofile/common.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> index 99c63d4b..ec10db1 100644
> --- a/arch/arm/oprofile/common.c
> +++ b/arch/arm/oprofile/common.c
> @@ -37,8 +37,11 @@ static struct op_perf_name {
>  	{ "xscale1",		"arm/xscale2"	},
>  	{ "v6",			"arm/armv6"	},
>  	{ "v6mpcore",		"arm/mpcore"	},
> +	{ "ARMv7 Cortex-A5",	"arm/armv7-ca5"	},
> +	{ "ARMv7 Cortex-A7",	"arm/armv7-ca7"	},
>  	{ "ARMv7 Cortex-A8",	"arm/armv7"	},
>  	{ "ARMv7 Cortex-A9",	"arm/armv7-ca9"	},
> +	{ "ARMv7 Cortex-A15",	"arm/armv7-ca15" },
>  };
>  
>  char *op_name_from_perf_id(void)

I'd rather not go down this route now that we have the operf tool as part of
oprofile, which can use the perf syscall directly and doesn't need this
string translation.

Will

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Robert Richter]

Will,

On 05.11.12 11:31:03, Will Deacon wrote:
> > diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> > index 99c63d4b..ec10db1 100644
> > --- a/arch/arm/oprofile/common.c
> > +++ b/arch/arm/oprofile/common.c
> > @@ -37,8 +37,11 @@ static struct op_perf_name {
> >  	{ "xscale1",		"arm/xscale2"	},
> >  	{ "v6",			"arm/armv6"	},
> >  	{ "v6mpcore",		"arm/mpcore"	},
> > +	{ "ARMv7 Cortex-A5",	"arm/armv7-ca5"	},
> > +	{ "ARMv7 Cortex-A7",	"arm/armv7-ca7"	},
> >  	{ "ARMv7 Cortex-A8",	"arm/armv7"	},
> >  	{ "ARMv7 Cortex-A9",	"arm/armv7-ca9"	},
> > +	{ "ARMv7 Cortex-A15",	"arm/armv7-ca15" },
> >  };

> I'd rather not go down this route now that we have the operf tool as part of
> oprofile, which can use the perf syscall directly and doesn't need this
> string translation.

since this is just an update of cpu detection I would be willing to
include this into kernel code anyway.

We could further move the cpu detection to userspace if perf_event
exists. We let the kernel enable oprofile with cpu_type="unknown".
User space then could either bind mount the file (user could do this
manually) or we implement to write to cpu_type. Doing so oprofile
could use in-kernel perf_events if it exists always as fallback.

Any thoughts?

-Robert

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Will Deacon]

On Tue, Nov 20, 2012 at 12:17:47PM +0000, Robert Richter wrote:
> Will,
> 
> On 05.11.12 11:31:03, Will Deacon wrote:
> > > diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> > > index 99c63d4b..ec10db1 100644
> > > --- a/arch/arm/oprofile/common.c
> > > +++ b/arch/arm/oprofile/common.c
> > > @@ -37,8 +37,11 @@ static struct op_perf_name {
> > >  	{ "xscale1",		"arm/xscale2"	},
> > >  	{ "v6",			"arm/armv6"	},
> > >  	{ "v6mpcore",		"arm/mpcore"	},
> > > +	{ "ARMv7 Cortex-A5",	"arm/armv7-ca5"	},
> > > +	{ "ARMv7 Cortex-A7",	"arm/armv7-ca7"	},
> > >  	{ "ARMv7 Cortex-A8",	"arm/armv7"	},
> > >  	{ "ARMv7 Cortex-A9",	"arm/armv7-ca9"	},
> > > +	{ "ARMv7 Cortex-A15",	"arm/armv7-ca15" },
> > >  };
> 
> > I'd rather not go down this route now that we have the operf tool as part of
> > oprofile, which can use the perf syscall directly and doesn't need this
> > string translation.
> 
> since this is just an update of cpu detection I would be willing to
> include this into kernel code anyway.

Perhaps, but one day we might like to remove this compatibility layer as
tools move over to the perf system call, so adding new CPUs here is actively
going against that.

> We could further move the cpu detection to userspace if perf_event
> exists. We let the kernel enable oprofile with cpu_type="unknown".
> User space then could either bind mount the file (user could do this
> manually) or we implement to write to cpu_type. Doing so oprofile
> could use in-kernel perf_events if it exists always as fallback.

Not sure I follow you... operf already does the CPU detection from
userspace, so I guess that code could simply be re-used. What does the bind
mount involve?

Cheers,

Will

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Robert Richter]

On 20.11.12 15:57:17, Will Deacon wrote:
> On Tue, Nov 20, 2012 at 12:17:47PM +0000, Robert Richter wrote:
> > Will,
> > 
> > On 05.11.12 11:31:03, Will Deacon wrote:
> > > > diff --git a/arch/arm/oprofile/common.c b/arch/arm/oprofile/common.c
> > > > index 99c63d4b..ec10db1 100644
> > > > --- a/arch/arm/oprofile/common.c
> > > > +++ b/arch/arm/oprofile/common.c
> > > > @@ -37,8 +37,11 @@ static struct op_perf_name {
> > > >  	{ "xscale1",		"arm/xscale2"	},
> > > >  	{ "v6",			"arm/armv6"	},
> > > >  	{ "v6mpcore",		"arm/mpcore"	},
> > > > +	{ "ARMv7 Cortex-A5",	"arm/armv7-ca5"	},
> > > > +	{ "ARMv7 Cortex-A7",	"arm/armv7-ca7"	},
> > > >  	{ "ARMv7 Cortex-A8",	"arm/armv7"	},
> > > >  	{ "ARMv7 Cortex-A9",	"arm/armv7-ca9"	},
> > > > +	{ "ARMv7 Cortex-A15",	"arm/armv7-ca15" },
> > > >  };
> > 
> > > I'd rather not go down this route now that we have the operf tool as part of
> > > oprofile, which can use the perf syscall directly and doesn't need this
> > > string translation.
> > 
> > since this is just an update of cpu detection I would be willing to
> > include this into kernel code anyway.
> 
> Perhaps, but one day we might like to remove this compatibility layer as
> tools move over to the perf system call, so adding new CPUs here is actively
> going against that.

This would help people to use oprofile as they did before with legacy
oprofile tools. There is not much effort to keep oprofile kernel
support for these tools if in-kernel perf_event support exists for new
hardware. As this is not much effort to maintain, we could keep
supporting this.  Forcing users to use operf since this is the only
way to connect to newer hardware might not be what they want.

> > We could further move the cpu detection to userspace if perf_event
> > exists. We let the kernel enable oprofile with cpu_type="unknown".
> > User space then could either bind mount the file (user could do this
> > manually) or we implement to write to cpu_type. Doing so oprofile
> > could use in-kernel perf_events if it exists always as fallback.
> 
> Not sure I follow you... operf already does the CPU detection from
> userspace, so I guess that code could simply be re-used. What does the bind
> mount involve?

I am thinking of the following:

 # cat /root/cpu_type
 arm/armv7-ca5
 # cat /dev/oprofile/cpu_type
 unknown
 # mount --bind /root/cpu_type /dev/oprofile/cpu_type
 # cat /dev/oprofile/cpu_type
 arm/armv7-ca5

>From here legacy oprofile tools work as expected using oprofilefs. (I
think. Did not test it.) We need to change the kernel for this a bit
to return 'unknown'. The mount could be done by the oprofile tools
using existing cpu detection code. This is only one way to setup
cpu_type from userland, there could be other ways too.

-Robert

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Will Deacon]

On Tue, Nov 20, 2012 at 04:31:58PM +0000, Robert Richter wrote:
> On 20.11.12 15:57:17, Will Deacon wrote:
> > On Tue, Nov 20, 2012 at 12:17:47PM +0000, Robert Richter wrote:
> > > 
> > > since this is just an update of cpu detection I would be willing to
> > > include this into kernel code anyway.
> > 
> > Perhaps, but one day we might like to remove this compatibility layer as
> > tools move over to the perf system call, so adding new CPUs here is actively
> > going against that.
> 
> This would help people to use oprofile as they did before with legacy
> oprofile tools. There is not much effort to keep oprofile kernel
> support for these tools if in-kernel perf_event support exists for new
> hardware. As this is not much effort to maintain, we could keep
> supporting this.  Forcing users to use operf since this is the only
> way to connect to newer hardware might not be what they want.

For arch/arm/, yes, it's not a lot of work but I'm thinking more of things
like heterogeneous multi-core (big/little) and arm64, which we definitely
don't want to deal with in the old tools.

I think newer hardware really will require operf.

> > > We could further move the cpu detection to userspace if perf_event
> > > exists. We let the kernel enable oprofile with cpu_type="unknown".
> > > User space then could either bind mount the file (user could do this
> > > manually) or we implement to write to cpu_type. Doing so oprofile
> > > could use in-kernel perf_events if it exists always as fallback.
> > 
> > Not sure I follow you... operf already does the CPU detection from
> > userspace, so I guess that code could simply be re-used. What does the bind
> > mount involve?
> 
> I am thinking of the following:
> 
>  # cat /root/cpu_type
>  arm/armv7-ca5
>  # cat /dev/oprofile/cpu_type
>  unknown
>  # mount --bind /root/cpu_type /dev/oprofile/cpu_type
>  # cat /dev/oprofile/cpu_type
>  arm/armv7-ca5
> 
> From here legacy oprofile tools work as expected using oprofilefs. (I
> think. Did not test it.) We need to change the kernel for this a bit
> to return 'unknown'. The mount could be done by the oprofile tools
> using existing cpu detection code. This is only one way to setup
> cpu_type from userland, there could be other ways too.

Ok, this is functionally equivalent to the patch that was submitted at the
start of this thread: it solves the problem of mapping a single ARM core to
a oprofile's CPU ID string. Technically, I don't mind doing that in the
kernel (at least, it means you don't need to do your trick above) but going
forward it *will* fall apart and people will have to move to newer tools.

So the question is: do we want to migrate users now or later?

Cheers,

Will

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Robert Richter]

On 20.11.12 16:55:17, Will Deacon wrote:
> On Tue, Nov 20, 2012 at 04:31:58PM +0000, Robert Richter wrote:
> > I am thinking of the following:
> > 
> >  # cat /root/cpu_type
> >  arm/armv7-ca5
> >  # cat /dev/oprofile/cpu_type
> >  unknown
> >  # mount --bind /root/cpu_type /dev/oprofile/cpu_type
> >  # cat /dev/oprofile/cpu_type
> >  arm/armv7-ca5
> > 
> > From here legacy oprofile tools work as expected using oprofilefs. (I
> > think. Did not test it.) We need to change the kernel for this a bit
> > to return 'unknown'. The mount could be done by the oprofile tools
> > using existing cpu detection code. This is only one way to setup
> > cpu_type from userland, there could be other ways too.
> 
> Ok, this is functionally equivalent to the patch that was submitted at the
> start of this thread: it solves the problem of mapping a single ARM core to
> a oprofile's CPU ID string. Technically, I don't mind doing that in the
> kernel (at least, it means you don't need to do your trick above)

The advantage of a solution where userland updates cpu_type is that we
never need to update the kernel anymore. This means, cpu detection can
be part of the tools.

-Robert

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Will Deacon]

On Tue, Nov 20, 2012 at 05:06:01PM +0000, Robert Richter wrote:
> On 20.11.12 16:55:17, Will Deacon wrote:
> > On Tue, Nov 20, 2012 at 04:31:58PM +0000, Robert Richter wrote:
> > > I am thinking of the following:
> > > 
> > >  # cat /root/cpu_type
> > >  arm/armv7-ca5
> > >  # cat /dev/oprofile/cpu_type
> > >  unknown
> > >  # mount --bind /root/cpu_type /dev/oprofile/cpu_type
> > >  # cat /dev/oprofile/cpu_type
> > >  arm/armv7-ca5
> > > 
> > > From here legacy oprofile tools work as expected using oprofilefs. (I
> > > think. Did not test it.) We need to change the kernel for this a bit
> > > to return 'unknown'. The mount could be done by the oprofile tools
> > > using existing cpu detection code. This is only one way to setup
> > > cpu_type from userland, there could be other ways too.
> > 
> > Ok, this is functionally equivalent to the patch that was submitted at the
> > start of this thread: it solves the problem of mapping a single ARM core to
> > a oprofile's CPU ID string. Technically, I don't mind doing that in the
> > kernel (at least, it means you don't need to do your trick above)
> 
> The advantage of a solution where userland updates cpu_type is that we
> never need to update the kernel anymore. This means, cpu detection can
> be part of the tools.

True, but the kernel-side perf code still needs updating to support the new
CPU so I don't think you win much.

Will

[PATCH 1/1] ARM: oprofile: add A5/A7/A15 entries in op_perf_name

[Robert Richter]

On 20.11.12 17:08:17, Will Deacon wrote:
> > The advantage of a solution where userland updates cpu_type is that we
> > never need to update the kernel anymore. This means, cpu detection can
> > be part of the tools.
> 
> True, but the kernel-side perf code still needs updating to support the new
> CPU so I don't think you win much.

You also need perf support to use operf. If oprofile can update
cpu_type, the oprofile kernel interface would work for new hardware
without changes. We have support for both then, operf and opcontrol.

-Robert