public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf tools: allow building for ARM
@ 2009-12-11  9:20 Jamie Iles
  2009-12-11  9:21 ` [PATCH 2/2] perf tools: allow cross compiling Jamie Iles
  2009-12-11 10:23 ` [PATCH 1/2] perf tools: allow " Ingo Molnar
  0 siblings, 2 replies; 20+ messages in thread
From: Jamie Iles @ 2009-12-11  9:20 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jamie Iles, Russell King, Peter Zijlstra

Add definitions of rmb() and cpu_relax() and include the ARM unistd.h
header. The kernel uses different definitions for rmb() depending on
the arch revision and whether the system is SMP or not. The lowest common
denominator is a compiler memory barrier so use that.

Signed-off-by: Jamie Iles <jamie.iles@picochip.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/perf.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 454d5d5..4b8eac6 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -59,6 +59,12 @@
 #define cpu_relax()	asm volatile ("hint @pause" ::: "memory")
 #endif
 
+#ifdef __arm__
+#include "../../arch/arm/include/asm/unistd.h"
+#define rmb()		asm volatile("":::"memory")
+#define cpu_relax()	asm volatile("":::"memory")
+#endif
+
 #include <time.h>
 #include <unistd.h>
 #include <sys/types.h>
-- 
1.6.5.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 2/2] perf tools: allow cross compiling
  2009-12-11  9:20 [PATCH 1/2] perf tools: allow building for ARM Jamie Iles
@ 2009-12-11  9:21 ` Jamie Iles
  2009-12-11 10:27   ` [tip:perf/urgent] perf tools: Allow " tip-bot for Jamie Iles
  2009-12-11 12:20   ` [PATCH] perf tools: allow building for ARM (patch v2) Jamie Iles
  2009-12-11 10:23 ` [PATCH 1/2] perf tools: allow " Ingo Molnar
  1 sibling, 2 replies; 20+ messages in thread
From: Jamie Iles @ 2009-12-11  9:21 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jamie Iles, Peter Zijlstra

For embedded platforms, we want to be able to build the perf
tools on a build machine to run on a different arch. This patch
allows $CROSS_COMPILE to set the cross compiler. Additionally, if
NO_LIBPERL is set, then don't use perl include paths as they will
be for the host arch.

Signed-off-by: Jamie Iles <jamie.iles@picochip.com>
CC: Peter Zijlstra <peterz@infradead.org>
---
 tools/perf/Makefile |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 23ec660..e2ee3b5 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -237,8 +237,8 @@ lib = lib
 
 export prefix bindir sharedir sysconfdir
 
-CC = gcc
-AR = ar
+CC = $(CROSS_COMPILE)gcc
+AR = $(CROSS_COMPILE)ar
 RM = rm -f
 TAR = tar
 FIND = find
@@ -492,8 +492,10 @@ else
 	LIB_OBJS += util/probe-finder.o
 endif
 
+ifndef NO_LIBPERL
 PERL_EMBED_LDOPTS = `perl -MExtUtils::Embed -e ldopts 2>/dev/null`
 PERL_EMBED_CCOPTS = `perl -MExtUtils::Embed -e ccopts 2>/dev/null`
+endif
 
 ifneq ($(shell sh -c "(echo '\#include <EXTERN.h>'; echo '\#include <perl.h>'; echo 'int main(void) { perl_alloc(); return 0; }') | $(CC) -x c - $(PERL_EMBED_CCOPTS) -o /dev/null $(PERL_EMBED_LDOPTS) > /dev/null 2>&1 && echo y"), y)
 	BASIC_CFLAGS += -DNO_LIBPERL
-- 
1.6.5.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11  9:20 [PATCH 1/2] perf tools: allow building for ARM Jamie Iles
  2009-12-11  9:21 ` [PATCH 2/2] perf tools: allow cross compiling Jamie Iles
@ 2009-12-11 10:23 ` Ingo Molnar
  2009-12-11 10:30   ` Jamie Iles
  2009-12-11 10:38   ` David Miller
  1 sibling, 2 replies; 20+ messages in thread
From: Ingo Molnar @ 2009-12-11 10:23 UTC (permalink / raw)
  To: Jamie Iles; +Cc: linux-kernel, Russell King, Peter Zijlstra


* Jamie Iles <jamie.iles@picochip.com> wrote:

> +#ifdef __arm__
> +#include "../../arch/arm/include/asm/unistd.h"
> +#define rmb()		asm volatile("":::"memory")
> +#define cpu_relax()	asm volatile("":::"memory")
> +#endif

cpu_relax() looks fine, but rmb() seems not to match the one that can be 
found in arch/arm/:

arch/arm/include/asm/system.h:#define rmb()		dmb()
arch/arm/include/asm/system.h:#define rmb()	do { if (arch_is_coherent()) dmb(); else barrier(); } while (0)
arch/arm/include/asm/system.h:#define smp_rmb()	rmb()

arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("dmb" : : : "memory")
arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")

	Ingo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tip:perf/urgent] perf tools: Allow cross compiling
  2009-12-11  9:21 ` [PATCH 2/2] perf tools: allow cross compiling Jamie Iles
@ 2009-12-11 10:27   ` tip-bot for Jamie Iles
  2009-12-11 12:20   ` [PATCH] perf tools: allow building for ARM (patch v2) Jamie Iles
  1 sibling, 0 replies; 20+ messages in thread
From: tip-bot for Jamie Iles @ 2009-12-11 10:27 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, peterz, jamie.iles, tglx, mingo

Commit-ID:  cc835752ae3634acd2d487fdf5152f6075f45aef
Gitweb:     http://git.kernel.org/tip/cc835752ae3634acd2d487fdf5152f6075f45aef
Author:     Jamie Iles <jamie.iles@picochip.com>
AuthorDate: Fri, 11 Dec 2009 09:21:00 +0000
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 11 Dec 2009 11:24:13 +0100

perf tools: Allow cross compiling

For embedded platforms, we want to be able to build the perf
tools on a build machine to run on a different arch. This patch
allows $CROSS_COMPILE to set the cross compiler.

Additionally, if NO_LIBPERL is set, then don't use perl include
paths as they will be for the host arch.

Signed-off-by: Jamie Iles <jamie.iles@picochip.com>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <1260523260-15694-2-git-send-email-jamie.iles@picochip.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/Makefile |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 23ec660..e2ee3b5 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -237,8 +237,8 @@ lib = lib
 
 export prefix bindir sharedir sysconfdir
 
-CC = gcc
-AR = ar
+CC = $(CROSS_COMPILE)gcc
+AR = $(CROSS_COMPILE)ar
 RM = rm -f
 TAR = tar
 FIND = find
@@ -492,8 +492,10 @@ else
 	LIB_OBJS += util/probe-finder.o
 endif
 
+ifndef NO_LIBPERL
 PERL_EMBED_LDOPTS = `perl -MExtUtils::Embed -e ldopts 2>/dev/null`
 PERL_EMBED_CCOPTS = `perl -MExtUtils::Embed -e ccopts 2>/dev/null`
+endif
 
 ifneq ($(shell sh -c "(echo '\#include <EXTERN.h>'; echo '\#include <perl.h>'; echo 'int main(void) { perl_alloc(); return 0; }') | $(CC) -x c - $(PERL_EMBED_CCOPTS) -o /dev/null $(PERL_EMBED_LDOPTS) > /dev/null 2>&1 && echo y"), y)
 	BASIC_CFLAGS += -DNO_LIBPERL

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:23 ` [PATCH 1/2] perf tools: allow " Ingo Molnar
@ 2009-12-11 10:30   ` Jamie Iles
  2009-12-11 10:38     ` Ingo Molnar
       [not found]     ` <000101ca7a50$b3e2ad70$1ba80850$@deacon@arm.com>
  2009-12-11 10:38   ` David Miller
  1 sibling, 2 replies; 20+ messages in thread
From: Jamie Iles @ 2009-12-11 10:30 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jamie Iles, linux-kernel, Russell King, Peter Zijlstra

On Fri, Dec 11, 2009 at 11:23:16AM +0100, Ingo Molnar wrote:
> cpu_relax() looks fine, but rmb() seems not to match the one that can be 
> found in arch/arm/:
> 
> arch/arm/include/asm/system.h:#define rmb()		dmb()
> arch/arm/include/asm/system.h:#define rmb()	do { if (arch_is_coherent()) dmb(); else barrier(); } while (0)
> arch/arm/include/asm/system.h:#define smp_rmb()	rmb()
> 
> arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("dmb" : : : "memory")
> arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
> arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
> arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
The implementation of the barriers depend on the CPU arch revision which is
defined in the kernel config. As the perf tools don't use the kernel config,
we don't know here what arch revision we're building for. Perhaps we need a
LINUX_ARM_ARCH parameter when building for ARM so we can pick the correct one.

Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:23 ` [PATCH 1/2] perf tools: allow " Ingo Molnar
  2009-12-11 10:30   ` Jamie Iles
@ 2009-12-11 10:38   ` David Miller
  2009-12-11 10:41     ` Ingo Molnar
  2009-12-11 21:19     ` Russell King - ARM Linux
  1 sibling, 2 replies; 20+ messages in thread
From: David Miller @ 2009-12-11 10:38 UTC (permalink / raw)
  To: mingo; +Cc: jamie.iles, linux-kernel, linux, peterz

From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 11 Dec 2009 11:23:16 +0100

> 
> * Jamie Iles <jamie.iles@picochip.com> wrote:
> 
>> +#ifdef __arm__
>> +#include "../../arch/arm/include/asm/unistd.h"
>> +#define rmb()		asm volatile("":::"memory")
>> +#define cpu_relax()	asm volatile("":::"memory")
>> +#endif
> 
> cpu_relax() looks fine, but rmb() seems not to match the one that can be 
> found in arch/arm/:

I think he did it this way so it can compile in the meantime,
and that doing it right requires runtime cpu detection to
select which barrier instruction is even available on the
current ARM cpu.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:30   ` Jamie Iles
@ 2009-12-11 10:38     ` Ingo Molnar
  2009-12-11 11:01       ` Jamie Iles
       [not found]     ` <000101ca7a50$b3e2ad70$1ba80850$@deacon@arm.com>
  1 sibling, 1 reply; 20+ messages in thread
From: Ingo Molnar @ 2009-12-11 10:38 UTC (permalink / raw)
  To: Jamie Iles; +Cc: linux-kernel, Russell King, Peter Zijlstra


* Jamie Iles <jamie.iles@picochip.com> wrote:

> On Fri, Dec 11, 2009 at 11:23:16AM +0100, Ingo Molnar wrote:
> > cpu_relax() looks fine, but rmb() seems not to match the one that can be 
> > found in arch/arm/:
> > 
> > arch/arm/include/asm/system.h:#define rmb()		dmb()
> > arch/arm/include/asm/system.h:#define rmb()	do { if (arch_is_coherent()) dmb(); else barrier(); } while (0)
> > arch/arm/include/asm/system.h:#define smp_rmb()	rmb()
> > 
> > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("dmb" : : : "memory")
> > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
> > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
> > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
>
> The implementation of the barriers depend on the CPU arch revision 
> which is defined in the kernel config. As the perf tools don't use the 
> kernel config, we don't know here what arch revision we're building 
> for. Perhaps we need a LINUX_ARM_ARCH parameter when building for ARM 
> so we can pick the correct one.

rmb() is used in two places in perf:

 tools/perf/builtin-record.c:    rmb();
 tools/perf/builtin-top.c:       rmb();

to interact with the shared kernel/user ring-buffer. Getting a barrier 
wrong there may cause hickups in recording.

Could you tell me a bit more about this ARM instruction - is the 'DMB' 
instruction used on all SMP ARM cores? Can it be used unconditionally, 
or is the instruction undefined on certain versions? To get the ball 
rolling we could use it unconditionally in the initial patch, but this 
needs to be solved i suspect.

	Ingo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:38   ` David Miller
@ 2009-12-11 10:41     ` Ingo Molnar
  2009-12-11 11:48       ` Jamie Iles
  2009-12-11 21:19     ` Russell King - ARM Linux
  1 sibling, 1 reply; 20+ messages in thread
From: Ingo Molnar @ 2009-12-11 10:41 UTC (permalink / raw)
  To: David Miller; +Cc: jamie.iles, linux-kernel, linux, peterz


* David Miller <davem@davemloft.net> wrote:

> From: Ingo Molnar <mingo@elte.hu>
> Date: Fri, 11 Dec 2009 11:23:16 +0100
> 
> > 
> > * Jamie Iles <jamie.iles@picochip.com> wrote:
> > 
> >> +#ifdef __arm__
> >> +#include "../../arch/arm/include/asm/unistd.h"
> >> +#define rmb()		asm volatile("":::"memory")
> >> +#define cpu_relax()	asm volatile("":::"memory")
> >> +#endif
> > 
> > cpu_relax() looks fine, but rmb() seems not to match the one that can be 
> > found in arch/arm/:
> 
> I think he did it this way so it can compile in the meantime, and that 
> doing it right requires runtime cpu detection to select which barrier 
> instruction is even available on the current ARM cpu.

Yeah. We can merge a quick patch for it if runtime detection is 
difficult - but if then such a patch should err on the side of using the 
barrier instruction unconditionally - even if this causes perf to 
segfault on certain (older? UP configured?) ARM cores.

	Ingo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:38     ` Ingo Molnar
@ 2009-12-11 11:01       ` Jamie Iles
  2009-12-11 11:26         ` Mikael Pettersson
  0 siblings, 1 reply; 20+ messages in thread
From: Jamie Iles @ 2009-12-11 11:01 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: Jamie Iles, linux-kernel, Russell King, Peter Zijlstra

On Fri, Dec 11, 2009 at 11:38:48AM +0100, Ingo Molnar wrote:
> 
> * Jamie Iles <jamie.iles@picochip.com> wrote:
> 
> > On Fri, Dec 11, 2009 at 11:23:16AM +0100, Ingo Molnar wrote:
> > > cpu_relax() looks fine, but rmb() seems not to match the one that can be 
> > > found in arch/arm/:
> > > 
> > > arch/arm/include/asm/system.h:#define rmb()		dmb()
> > > arch/arm/include/asm/system.h:#define rmb()	do { if (arch_is_coherent()) dmb(); else barrier(); } while (0)
> > > arch/arm/include/asm/system.h:#define smp_rmb()	rmb()
> > > 
> > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("dmb" : : : "memory")
> > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
> > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
> > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
> >
> > The implementation of the barriers depend on the CPU arch revision 
> > which is defined in the kernel config. As the perf tools don't use the 
> > kernel config, we don't know here what arch revision we're building 
> > for. Perhaps we need a LINUX_ARM_ARCH parameter when building for ARM 
> > so we can pick the correct one.
> 
> rmb() is used in two places in perf:
> 
>  tools/perf/builtin-record.c:    rmb();
>  tools/perf/builtin-top.c:       rmb();
> 
> to interact with the shared kernel/user ring-buffer. Getting a barrier 
> wrong there may cause hickups in recording.
> 
> Could you tell me a bit more about this ARM instruction - is the 'DMB' 
> instruction used on all SMP ARM cores? Can it be used unconditionally, 
> or is the instruction undefined on certain versions? To get the ball 
> rolling we could use it unconditionally in the initial patch, but this 
> needs to be solved i suspect.
There are a few cases we need to deal with:
	- v7 SMP: DMB instruction
	- v6 SMP: MCR coprocessor instruction
	- v5 and earlier no instructions for barriers.

Looking at the TRM for a v7 core (cortex A9) the MCR instruction that v6 uses
is deprecated but still present. I suspect we could use this to cover the v6
and v7 cores but we wouldn't be able to do soft perf events on v5 or earlier
(which don't have hardware counters).

Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
       [not found]     ` <000101ca7a50$b3e2ad70$1ba80850$@deacon@arm.com>
@ 2009-12-11 11:02       ` Ingo Molnar
  2009-12-11 11:08         ` Pekka Enberg
  0 siblings, 1 reply; 20+ messages in thread
From: Ingo Molnar @ 2009-12-11 11:02 UTC (permalink / raw)
  To: Will Deacon
  Cc: 'Jamie Iles', linux-kernel, Russell King, Peter Zijlstra


* Will Deacon <will.deacon@arm.com> wrote:

> > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org]
> > On Behalf Of Jamie Iles
> > Sent: 11 December 2009 10:31
> >
> > The implementation of the barriers depend on the CPU arch revision 
> > which is defined in the kernel config. As the perf tools don't use 
> > the kernel config, we don't know here what arch revision we're 
> > building for. Perhaps we need a LINUX_ARM_ARCH parameter when 
> > building for ARM so we can pick the correct one.
> 
> Hi Jamie, Ingo,
> 
> Surely a better way to proceed with this would be to build the perf 
> tool as a side effect of building the kernel? That way the relevant 
> definitions from system.h could be included directly and there would 
> be no need to duplicate the architectural conditionals in perf.h.

Might make sense.

> I'm also working on perf-events for ARM and am using:
> 
> #define rmb()           asm volatile("mcr p15, 0, %0, c7, c10, 5" :: "r" (0) : "memory")
> 
> This will work on v6 and v7 [although the dmb instruction is preferred 
> here] cores.

Note that the codepath where it's used isnt very performance sensitive 
(we call it about once per batch of event processing), so we could use 
the broadest instruction that works on as many cores as possible - to 
keep things simple.

	Ingo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 11:02       ` Ingo Molnar
@ 2009-12-11 11:08         ` Pekka Enberg
  0 siblings, 0 replies; 20+ messages in thread
From: Pekka Enberg @ 2009-12-11 11:08 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Will Deacon, Jamie Iles, linux-kernel, Russell King,
	Peter Zijlstra

On Fri, Dec 11, 2009 at 1:02 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Will Deacon <will.deacon@arm.com> wrote:
>
>> > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-owner@vger.kernel.org]
>> > On Behalf Of Jamie Iles
>> > Sent: 11 December 2009 10:31
>> >
>> > The implementation of the barriers depend on the CPU arch revision
>> > which is defined in the kernel config. As the perf tools don't use
>> > the kernel config, we don't know here what arch revision we're
>> > building for. Perhaps we need a LINUX_ARM_ARCH parameter when
>> > building for ARM so we can pick the correct one.
>>
>> Hi Jamie, Ingo,
>>
>> Surely a better way to proceed with this would be to build the perf
>> tool as a side effect of building the kernel? That way the relevant
>> definitions from system.h could be included directly and there would
>> be no need to duplicate the architectural conditionals in perf.h.
>
> Might make sense.
>
>> I'm also working on perf-events for ARM and am using:
>>
>> #define rmb()           asm volatile("mcr p15, 0, %0, c7, c10, 5" :: "r" (0) : "memory")
>>
>> This will work on v6 and v7 [although the dmb instruction is preferred
>> here] cores.
>
> Note that the codepath where it's used isnt very performance sensitive
> (we call it about once per batch of event processing), so we could use
> the broadest instruction that works on as many cores as possible - to
> keep things simple.

How plausible is it to reuse the bits in
arch/arm/include/asm/cputype.h and do an version of rmb() that has
if-else for the v6 and v7 cases?

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 11:01       ` Jamie Iles
@ 2009-12-11 11:26         ` Mikael Pettersson
  2009-12-11 21:22           ` Russell King - ARM Linux
  0 siblings, 1 reply; 20+ messages in thread
From: Mikael Pettersson @ 2009-12-11 11:26 UTC (permalink / raw)
  To: Jamie Iles; +Cc: Ingo Molnar, linux-kernel, Russell King, Peter Zijlstra

Jamie Iles writes:
 > On Fri, Dec 11, 2009 at 11:38:48AM +0100, Ingo Molnar wrote:
 > > 
 > > * Jamie Iles <jamie.iles@picochip.com> wrote:
 > > 
 > > > On Fri, Dec 11, 2009 at 11:23:16AM +0100, Ingo Molnar wrote:
 > > > > cpu_relax() looks fine, but rmb() seems not to match the one that can be 
 > > > > found in arch/arm/:
 > > > > 
 > > > > arch/arm/include/asm/system.h:#define rmb()		dmb()
 > > > > arch/arm/include/asm/system.h:#define rmb()	do { if (arch_is_coherent()) dmb(); else barrier(); } while (0)
 > > > > arch/arm/include/asm/system.h:#define smp_rmb()	rmb()
 > > > > 
 > > > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("dmb" : : : "memory")
 > > > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("mcr p15, 0, %0, c7, c10, 5" \
 > > > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
 > > > > arch/arm/include/asm/system.h:#define dmb() __asm__ __volatile__ ("" : : : "memory")
 > > >
 > > > The implementation of the barriers depend on the CPU arch revision 
 > > > which is defined in the kernel config. As the perf tools don't use the 
 > > > kernel config, we don't know here what arch revision we're building 
 > > > for. Perhaps we need a LINUX_ARM_ARCH parameter when building for ARM 
 > > > so we can pick the correct one.
 > > 
 > > rmb() is used in two places in perf:
 > > 
 > >  tools/perf/builtin-record.c:    rmb();
 > >  tools/perf/builtin-top.c:       rmb();
 > > 
 > > to interact with the shared kernel/user ring-buffer. Getting a barrier 
 > > wrong there may cause hickups in recording.
 > > 
 > > Could you tell me a bit more about this ARM instruction - is the 'DMB' 
 > > instruction used on all SMP ARM cores? Can it be used unconditionally, 
 > > or is the instruction undefined on certain versions? To get the ball 
 > > rolling we could use it unconditionally in the initial patch, but this 
 > > needs to be solved i suspect.
 > There are a few cases we need to deal with:
 > 	- v7 SMP: DMB instruction
 > 	- v6 SMP: MCR coprocessor instruction
 > 	- v5 and earlier no instructions for barriers.
 > 
 > Looking at the TRM for a v7 core (cortex A9) the MCR instruction that v6 uses
 > is deprecated but still present. I suspect we could use this to cover the v6
 > and v7 cores but we wouldn't be able to do soft perf events on v5 or earlier
 > (which don't have hardware counters).

The correct solution is to invoke a kernel-exported CPU-specific helper
function in the ARM kernel helper page.

I see a __kuser_memory_barrier entry there which maps to smp_dmb.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:41     ` Ingo Molnar
@ 2009-12-11 11:48       ` Jamie Iles
  2009-12-11 12:48         ` Ingo Molnar
  0 siblings, 1 reply; 20+ messages in thread
From: Jamie Iles @ 2009-12-11 11:48 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: David Miller, jamie.iles, linux-kernel, linux, peterz

On Fri, Dec 11, 2009 at 11:41:26AM +0100, Ingo Molnar wrote:
> > I think he did it this way so it can compile in the meantime, and that 
> > doing it right requires runtime cpu detection to select which barrier 
> > instruction is even available on the current ARM cpu.
> 
> Yeah. We can merge a quick patch for it if runtime detection is 
> difficult - but if then such a patch should err on the side of using the 
> barrier instruction unconditionally - even if this causes perf to 
> segfault on certain (older? UP configured?) ARM cores.
Ok, unless anyone has any objections, I'll post a revised patch that uses the
MCR instruction so that we get the correct behaviour on v6/v7 SMP and UP
systems and an illegal instruction on v5 or earlier. I've had a quick look at
runtime detection and the ID registers are only accessible from privileged
modes so for long term, it might be better to define the rmb() at build time
from the kernel config.

Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] perf tools: allow building for ARM (patch v2)
  2009-12-11  9:21 ` [PATCH 2/2] perf tools: allow cross compiling Jamie Iles
  2009-12-11 10:27   ` [tip:perf/urgent] perf tools: Allow " tip-bot for Jamie Iles
@ 2009-12-11 12:20   ` Jamie Iles
  2009-12-11 12:54     ` [tip:perf/urgent] perf tools: Allow building for ARM tip-bot for Jamie Iles
  1 sibling, 1 reply; 20+ messages in thread
From: Jamie Iles @ 2009-12-11 12:20 UTC (permalink / raw)
  To: linux-kernel; +Cc: Jamie Iles, Russell King, Peter Zijlstra, Mikael Pettersson

Add definitions of rmb() and cpu_relax() and include the ARM unistd.h
header. The __kuser_memory_barrier helper in the helper page is used to
provide the correct memory barrier depending on the CPU type.

Signed-off-by: Jamie Iles <jamie.iles@picochip.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mikael Pettersson <mikpe@it.uu.se>
---
 tools/perf/perf.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 454d5d5..75f941b 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -59,6 +59,18 @@
 #define cpu_relax()	asm volatile ("hint @pause" ::: "memory")
 #endif
 
+#ifdef __arm__
+#include "../../arch/arm/include/asm/unistd.h"
+/*
+ * Use the __kuser_memory_barrier helper in the CPU helper page. See
+ * arch/arm/kernel/entry-armv.S in the kernel source for details.
+ */
+#define rmb()		asm volatile("mov r0, #0xffff0fff; mov lr, pc;" \
+				     "sub pc, r0, #95" ::: "r0", "lr", "cc", \
+				     "memory")
+#define cpu_relax()	asm volatile("":::"memory")
+#endif
+
 #include <time.h>
 #include <unistd.h>
 #include <sys/types.h>
-- 
1.6.5.4


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 11:48       ` Jamie Iles
@ 2009-12-11 12:48         ` Ingo Molnar
  0 siblings, 0 replies; 20+ messages in thread
From: Ingo Molnar @ 2009-12-11 12:48 UTC (permalink / raw)
  To: Jamie Iles; +Cc: David Miller, linux-kernel, linux, peterz


* Jamie Iles <jamie.iles@picochip.com> wrote:

> On Fri, Dec 11, 2009 at 11:41:26AM +0100, Ingo Molnar wrote:
> > > I think he did it this way so it can compile in the meantime, and that 
> > > doing it right requires runtime cpu detection to select which barrier 
> > > instruction is even available on the current ARM cpu.
> > 
> > Yeah. We can merge a quick patch for it if runtime detection is 
> > difficult - but if then such a patch should err on the side of using the 
> > barrier instruction unconditionally - even if this causes perf to 
> > segfault on certain (older? UP configured?) ARM cores.
>
> Ok, unless anyone has any objections, I'll post a revised patch that 
> uses the MCR instruction so that we get the correct behaviour on v6/v7 
> SMP and UP systems and an illegal instruction on v5 or earlier. I've 
> had a quick look at runtime detection and the ID registers are only 
> accessible from privileged modes so for long term, it might be better 
> to define the rmb() at build time from the kernel config.

Sounds good to me.

	Ingo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [tip:perf/urgent] perf tools: Allow building for ARM
  2009-12-11 12:20   ` [PATCH] perf tools: allow building for ARM (patch v2) Jamie Iles
@ 2009-12-11 12:54     ` tip-bot for Jamie Iles
  2009-12-11 13:04       ` Jamie Iles
  0 siblings, 1 reply; 20+ messages in thread
From: tip-bot for Jamie Iles @ 2009-12-11 12:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, peterz, jamie.iles, mikpe, linux, tglx,
	mingo

Commit-ID:  58e9f94138c1d9c47f6a63632ca7a78fc6dcc15f
Gitweb:     http://git.kernel.org/tip/58e9f94138c1d9c47f6a63632ca7a78fc6dcc15f
Author:     Jamie Iles <jamie.iles@picochip.com>
AuthorDate: Fri, 11 Dec 2009 12:20:09 +0000
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 11 Dec 2009 13:50:21 +0100

perf tools: Allow building for ARM

Add definitions of rmb() and cpu_relax() and include the ARM
unistd.h header. The __kuser_memory_barrier helper in the helper
page is used to provide the correct memory barrier depending on
the CPU type.

[ The rmb() will work on v6 and v7, segfault on v5. Dynamic
  detection to add v5 support will be added later. ]

Signed-off-by: Jamie Iles <jamie.iles@picochip.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mikael Pettersson <mikpe@it.uu.se>
LKML-Reference: <1260534009-5394-1-git-send-email-jamie.iles@picochip.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/perf.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 454d5d5..75f941b 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -59,6 +59,18 @@
 #define cpu_relax()	asm volatile ("hint @pause" ::: "memory")
 #endif
 
+#ifdef __arm__
+#include "../../arch/arm/include/asm/unistd.h"
+/*
+ * Use the __kuser_memory_barrier helper in the CPU helper page. See
+ * arch/arm/kernel/entry-armv.S in the kernel source for details.
+ */
+#define rmb()		asm volatile("mov r0, #0xffff0fff; mov lr, pc;" \
+				     "sub pc, r0, #95" ::: "r0", "lr", "cc", \
+				     "memory")
+#define cpu_relax()	asm volatile("":::"memory")
+#endif
+
 #include <time.h>
 #include <unistd.h>
 #include <sys/types.h>

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [tip:perf/urgent] perf tools: Allow building for ARM
  2009-12-11 12:54     ` [tip:perf/urgent] perf tools: Allow building for ARM tip-bot for Jamie Iles
@ 2009-12-11 13:04       ` Jamie Iles
  2009-12-11 13:26         ` Ingo Molnar
  0 siblings, 1 reply; 20+ messages in thread
From: Jamie Iles @ 2009-12-11 13:04 UTC (permalink / raw)
  To: mingo, hpa, linux-kernel, peterz, jamie.iles, mikpe, linux, tglx,
	mingo
  Cc: linux-tip-commits

On Fri, Dec 11, 2009 at 12:54:33PM +0000, tip-bot for Jamie Iles wrote:
> Commit-ID:  58e9f94138c1d9c47f6a63632ca7a78fc6dcc15f
> Gitweb:     http://git.kernel.org/tip/58e9f94138c1d9c47f6a63632ca7a78fc6dcc15f
> Author:     Jamie Iles <jamie.iles@picochip.com>
> AuthorDate: Fri, 11 Dec 2009 12:20:09 +0000
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Fri, 11 Dec 2009 13:50:21 +0100
> 
> perf tools: Allow building for ARM
> 
> Add definitions of rmb() and cpu_relax() and include the ARM
> unistd.h header. The __kuser_memory_barrier helper in the helper
> page is used to provide the correct memory barrier depending on
> the CPU type.
> 
> [ The rmb() will work on v6 and v7, segfault on v5. Dynamic
>   detection to add v5 support will be added later. ]
Sorry Ingo, my comment probably wasn't clear enough. The helper that Mikael
suggested should work for _all_ ARM processors including v5 and earlier. It
will just be a branch to a nop.

Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [tip:perf/urgent] perf tools: Allow building for ARM
  2009-12-11 13:04       ` Jamie Iles
@ 2009-12-11 13:26         ` Ingo Molnar
  0 siblings, 0 replies; 20+ messages in thread
From: Ingo Molnar @ 2009-12-11 13:26 UTC (permalink / raw)
  To: Jamie Iles
  Cc: mingo, hpa, linux-kernel, peterz, mikpe, linux, tglx,
	linux-tip-commits


* Jamie Iles <jamie.iles@picochip.com> wrote:

> On Fri, Dec 11, 2009 at 12:54:33PM +0000, tip-bot for Jamie Iles wrote:
> > Commit-ID:  58e9f94138c1d9c47f6a63632ca7a78fc6dcc15f
> > Gitweb:     http://git.kernel.org/tip/58e9f94138c1d9c47f6a63632ca7a78fc6dcc15f
> > Author:     Jamie Iles <jamie.iles@picochip.com>
> > AuthorDate: Fri, 11 Dec 2009 12:20:09 +0000
> > Committer:  Ingo Molnar <mingo@elte.hu>
> > CommitDate: Fri, 11 Dec 2009 13:50:21 +0100
> > 
> > perf tools: Allow building for ARM
> > 
> > Add definitions of rmb() and cpu_relax() and include the ARM
> > unistd.h header. The __kuser_memory_barrier helper in the helper
> > page is used to provide the correct memory barrier depending on
> > the CPU type.
> > 
> > [ The rmb() will work on v6 and v7, segfault on v5. Dynamic
> >   detection to add v5 support will be added later. ]
>
> Sorry Ingo, my comment probably wasn't clear enough. The helper that 
> Mikael suggested should work for _all_ ARM processors including v5 and 
> earlier. It will just be a branch to a nop.

oh - good! Too late to fix the comment, i already pushed it out - but as 
long as the _code_ is fine it's a good tradeoff ;-)

	Ingo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 10:38   ` David Miller
  2009-12-11 10:41     ` Ingo Molnar
@ 2009-12-11 21:19     ` Russell King - ARM Linux
  1 sibling, 0 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2009-12-11 21:19 UTC (permalink / raw)
  To: David Miller; +Cc: mingo, jamie.iles, linux-kernel, peterz

On Fri, Dec 11, 2009 at 02:38:08AM -0800, David Miller wrote:
> From: Ingo Molnar <mingo@elte.hu>
> Date: Fri, 11 Dec 2009 11:23:16 +0100
> 
> > 
> > * Jamie Iles <jamie.iles@picochip.com> wrote:
> > 
> >> +#ifdef __arm__
> >> +#include "../../arch/arm/include/asm/unistd.h"
> >> +#define rmb()		asm volatile("":::"memory")
> >> +#define cpu_relax()	asm volatile("":::"memory")
> >> +#endif
> > 
> > cpu_relax() looks fine, but rmb() seems not to match the one that can be 
> > found in arch/arm/:
> 
> I think he did it this way so it can compile in the meantime,
> and that doing it right requires runtime cpu detection to
> select which barrier instruction is even available on the
> current ARM cpu.

We provide a way for userspace to be independent of the CPU for these
operations by providing code snippets up in the vector page for userspace
to call.  The kernel places the correct code there according to the CPU
it's built for.

Look for __kuser_memory_barrier in arch/arm/kernel/entry-armv.S

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 1/2] perf tools: allow building for ARM
  2009-12-11 11:26         ` Mikael Pettersson
@ 2009-12-11 21:22           ` Russell King - ARM Linux
  0 siblings, 0 replies; 20+ messages in thread
From: Russell King - ARM Linux @ 2009-12-11 21:22 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Jamie Iles, Ingo Molnar, linux-kernel, Peter Zijlstra

On Fri, Dec 11, 2009 at 12:26:57PM +0100, Mikael Pettersson wrote:
> The correct solution is to invoke a kernel-exported CPU-specific helper
> function in the ARM kernel helper page.
> 
> I see a __kuser_memory_barrier entry there which maps to smp_dmb.

That is indeed the correct way to do this - otherwise user programs will
be tied to particular architecture versions, and either an undefined
instruction fault will happen, or worse, the desired effect will not
happen.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2009-12-11 21:22 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-11  9:20 [PATCH 1/2] perf tools: allow building for ARM Jamie Iles
2009-12-11  9:21 ` [PATCH 2/2] perf tools: allow cross compiling Jamie Iles
2009-12-11 10:27   ` [tip:perf/urgent] perf tools: Allow " tip-bot for Jamie Iles
2009-12-11 12:20   ` [PATCH] perf tools: allow building for ARM (patch v2) Jamie Iles
2009-12-11 12:54     ` [tip:perf/urgent] perf tools: Allow building for ARM tip-bot for Jamie Iles
2009-12-11 13:04       ` Jamie Iles
2009-12-11 13:26         ` Ingo Molnar
2009-12-11 10:23 ` [PATCH 1/2] perf tools: allow " Ingo Molnar
2009-12-11 10:30   ` Jamie Iles
2009-12-11 10:38     ` Ingo Molnar
2009-12-11 11:01       ` Jamie Iles
2009-12-11 11:26         ` Mikael Pettersson
2009-12-11 21:22           ` Russell King - ARM Linux
     [not found]     ` <000101ca7a50$b3e2ad70$1ba80850$@deacon@arm.com>
2009-12-11 11:02       ` Ingo Molnar
2009-12-11 11:08         ` Pekka Enberg
2009-12-11 10:38   ` David Miller
2009-12-11 10:41     ` Ingo Molnar
2009-12-11 11:48       ` Jamie Iles
2009-12-11 12:48         ` Ingo Molnar
2009-12-11 21:19     ` Russell King - ARM Linux

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox