2009-07-29 11:25:12

by Jens Axboe

[permalink] [raw]
Subject: [PATCH] basic perf support for sparc

Hi,

This adds the little necessary bits to get perf going at
least. Callgraph doesn't work, but basic profiling does.

Building the perf tool is somewhat involved on sparc64
though, since 64-bit versions of zlib/libelf/bfd aren't
directly available (at least on debian 5.x). But once you
get there, it runs :-). Would it be easier/functional
to build 32-bit userland perf instead?

$ perf record -f ls > /dev/null
[ perf record: Captured and wrote 0.004 MB perf.data (~194 samples) ]
$ perf report --sort comm,dso,symbol | head -n10
# Samples: 98
#
# Overhead Command Shared Object Symbol
# ........ ....... ............. ......
#
25.51% ls f7f62e00 [.] 0x000000f7f62e00
6.12% perf [kernel] [.] 0xfffff801007e4b38
6.12% ls [kernel] [k] sparc64_realfault_common
3.06% ls [kernel] [k] tg_shares_up
3.06% ls [kernel] [k] find_next_bit

$ perf record -f -g ls > /dev/null
[ perf record: Captured and wrote 0.005 MB perf.data (~223 samples) ]
$ perf report -g | head -n10
Warning: empty node in callchain tree
Warning: empty node in callchain tree
Warning: empty node in callchain tree
[...]

Patch is against latest -git.

Signed-off-by: Jens Axboe <[email protected]>

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 3f8b6a9..2f951a9 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -44,6 +44,7 @@ config SPARC64
select RTC_DRV_BQ4802
select RTC_DRV_SUN4V
select RTC_DRV_STARFIRE
+ select HAVE_PERF_COUNTERS

config ARCH_DEFCONFIG
string
diff --git a/arch/sparc/include/asm/unistd.h b/arch/sparc/include/asm/unistd.h
index b2c406d..706df66 100644
--- a/arch/sparc/include/asm/unistd.h
+++ b/arch/sparc/include/asm/unistd.h
@@ -395,8 +395,9 @@
#define __NR_preadv 324
#define __NR_pwritev 325
#define __NR_rt_tgsigqueueinfo 326
+#define __NR_perf_counter_open 327

-#define NR_SYSCALLS 327
+#define NR_SYSCALLS 328

#ifdef __32bit_syscall_numbers__
/* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
diff --git a/arch/sparc/kernel/systbls_64.S b/arch/sparc/kernel/systbls_64.S
index 6b3ee88..80ebf20 100644
--- a/arch/sparc/kernel/systbls_64.S
+++ b/arch/sparc/kernel/systbls_64.S
@@ -158,4 +158,4 @@ sys_call_table:
/*310*/ .word sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
.word sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
/*320*/ .word sys_dup3, sys_pipe2, sys_inotify_init1, sys_accept4, sys_preadv
- .word sys_pwritev, sys_rt_tgsigqueueinfo
+ .word sys_pwritev, sys_rt_tgsigqueueinfo, sys_perf_counter_open

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e5148e2..2abeb20 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -41,6 +41,12 @@
#define cpu_relax() asm volatile("" ::: "memory");
#endif

+#ifdef __sparc__
+#include "../../arch/sparc/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>

--
Jens Axboe


2009-07-29 19:28:15

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc

On Wed, Jul 29 2009, Jens Axboe wrote:
> Building the perf tool is somewhat involved on sparc64
> though, since 64-bit versions of zlib/libelf/bfd aren't
> directly available (at least on debian 5.x). But once you
> get there, it runs :-). Would it be easier/functional
> to build 32-bit userland perf instead?

Same is true on ppc64, btw. How are others handling this?

--
Jens Axboe

2009-08-01 01:18:10

by Anton Blanchard

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc


Hi,

> > Building the perf tool is somewhat involved on sparc64
> > though, since 64-bit versions of zlib/libelf/bfd aren't
> > directly available (at least on debian 5.x). But once you
> > get there, it runs :-). Would it be easier/functional
> > to build 32-bit userland perf instead?
>
> Same is true on ppc64, btw. How are others handling this?

The requirement for libz was removed, so up until recently we only needed
a 64bit version of elfutils which is easy to build.

It looks like we now have a requirement on binutils which is considerably
more painful to build. One option is to make the bfd requirement optional, all
you lose would be the ability to see c++ demangled names I think.

Anton

2009-09-04 04:36:48

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc

From: Jens Axboe <[email protected]>
Date: Mon, 17 Aug 2009 08:48:51 +0200

> sparc: add basic support for 'perf'
>
> This wires up the perf_counter_open() syscall so that basic
> software support for perf is working.
>
> Signed-off-by: Jens Axboe <[email protected]>

Does this build for you without adding an
arch/sparc/include/asm/perf_counter.h that looks
something like the following?

#ifndef _ASM_SPARC_PERF_COUNTER_H
#define _ASM_SPARC_PERF_COUNTER_H

#define PERF_COUNTER_INDEX_OFFSET 0

#endif

Or is this somehow now required in the -tip trees?

2009-09-04 05:03:09

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc


* David Miller <[email protected]> wrote:

> From: Jens Axboe <[email protected]>
> Date: Mon, 17 Aug 2009 08:48:51 +0200
>
> > sparc: add basic support for 'perf'
> >
> > This wires up the perf_counter_open() syscall so that basic
> > software support for perf is working.
> >
> > Signed-off-by: Jens Axboe <[email protected]>
>
> Does this build for you without adding an
> arch/sparc/include/asm/perf_counter.h that looks
> something like the following?
>
> #ifndef _ASM_SPARC_PERF_COUNTER_H
> #define _ASM_SPARC_PERF_COUNTER_H
>
> #define PERF_COUNTER_INDEX_OFFSET 0
>
> #endif
>
> Or is this somehow now required in the -tip trees?

This used to be required but i recently fixed this (and that fix is
upstream as well) via:

f738eb1: perf_counter: Fix the PARISC build

there's now a default define of 0 so you dont have to define it and
can leave out this chunk.

( That index is only interesting if the architecture has a way to
allow unprivileged user-space to access counter registers
directly. In that case the index reflects the offset from the
(constantly changing) dynamix index which we put into the mmap
header. With Sparc not having a hw-PMU implementation this index
is entirely uninteresting at this stage. )

Ingo

2009-09-04 05:08:54

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc

From: Ingo Molnar <[email protected]>
Date: Fri, 4 Sep 2009 07:02:56 +0200

>
> * David Miller <[email protected]> wrote:
>
>> Does this build for you without adding an
>> arch/sparc/include/asm/perf_counter.h that looks
>> something like the following?
>>
>> #ifndef _ASM_SPARC_PERF_COUNTER_H
>> #define _ASM_SPARC_PERF_COUNTER_H
>>
>> #define PERF_COUNTER_INDEX_OFFSET 0
>>
>> #endif
>>
>> Or is this somehow now required in the -tip trees?
>
> This used to be required but i recently fixed this (and that fix is
> upstream as well) via:
>
> f738eb1: perf_counter: Fix the PARISC build
>
> there's now a default define of 0 so you dont have to define it and
> can leave out this chunk.
>
> ( That index is only interesting if the architecture has a way to
> allow unprivileged user-space to access counter registers
> directly. In that case the index reflects the offset from the
> (constantly changing) dynamix index which we put into the mmap
> header. With Sparc not having a hw-PMU implementation this index
> is entirely uninteresting at this stage. )

But you still do need at least an empty perf_counter.h file
right? Jens must have left the file out of his submission
by accident, and that's what I'm trying to get to the bottom
of here :-)

I assume there was a similar change to deal with references to
set_perf_counter_pending() too or is at least a NOP definition
still needed?

2009-09-04 05:20:05

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc

On Thu, Sep 03 2009, David Miller wrote:
> From: Ingo Molnar <[email protected]>
> Date: Fri, 4 Sep 2009 07:02:56 +0200
>
> >
> > * David Miller <[email protected]> wrote:
> >
> >> Does this build for you without adding an
> >> arch/sparc/include/asm/perf_counter.h that looks
> >> something like the following?
> >>
> >> #ifndef _ASM_SPARC_PERF_COUNTER_H
> >> #define _ASM_SPARC_PERF_COUNTER_H
> >>
> >> #define PERF_COUNTER_INDEX_OFFSET 0
> >>
> >> #endif
> >>
> >> Or is this somehow now required in the -tip trees?
> >
> > This used to be required but i recently fixed this (and that fix is
> > upstream as well) via:
> >
> > f738eb1: perf_counter: Fix the PARISC build
> >
> > there's now a default define of 0 so you dont have to define it and
> > can leave out this chunk.
> >
> > ( That index is only interesting if the architecture has a way to
> > allow unprivileged user-space to access counter registers
> > directly. In that case the index reflects the offset from the
> > (constantly changing) dynamix index which we put into the mmap
> > header. With Sparc not having a hw-PMU implementation this index
> > is entirely uninteresting at this stage. )
>
> But you still do need at least an empty perf_counter.h file
> right? Jens must have left the file out of his submission
> by accident, and that's what I'm trying to get to the bottom
> of here :-)
>
> I assume there was a similar change to deal with references to
> set_perf_counter_pending() too or is at least a NOP definition
> still needed?

It wasn't required when I built and used it (and sent the patch), I used
the posted patch as-is. It's been a few weeks since I last updated and
ran that box, let me double check after morning coffee and send you a
fresh patch (if needed) :-)

--
Jens Axboe

2009-09-04 06:34:19

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc

On Fri, Sep 04 2009, Jens Axboe wrote:
> On Thu, Sep 03 2009, David Miller wrote:
> > From: Ingo Molnar <[email protected]>
> > Date: Fri, 4 Sep 2009 07:02:56 +0200
> >
> > >
> > > * David Miller <[email protected]> wrote:
> > >
> > >> Does this build for you without adding an
> > >> arch/sparc/include/asm/perf_counter.h that looks
> > >> something like the following?
> > >>
> > >> #ifndef _ASM_SPARC_PERF_COUNTER_H
> > >> #define _ASM_SPARC_PERF_COUNTER_H
> > >>
> > >> #define PERF_COUNTER_INDEX_OFFSET 0
> > >>
> > >> #endif
> > >>
> > >> Or is this somehow now required in the -tip trees?
> > >
> > > This used to be required but i recently fixed this (and that fix is
> > > upstream as well) via:
> > >
> > > f738eb1: perf_counter: Fix the PARISC build
> > >
> > > there's now a default define of 0 so you dont have to define it and
> > > can leave out this chunk.
> > >
> > > ( That index is only interesting if the architecture has a way to
> > > allow unprivileged user-space to access counter registers
> > > directly. In that case the index reflects the offset from the
> > > (constantly changing) dynamix index which we put into the mmap
> > > header. With Sparc not having a hw-PMU implementation this index
> > > is entirely uninteresting at this stage. )
> >
> > But you still do need at least an empty perf_counter.h file
> > right? Jens must have left the file out of his submission
> > by accident, and that's what I'm trying to get to the bottom
> > of here :-)
> >
> > I assume there was a similar change to deal with references to
> > set_perf_counter_pending() too or is at least a NOP definition
> > still needed?
>
> It wasn't required when I built and used it (and sent the patch), I used
> the posted patch as-is. It's been a few weeks since I last updated and
> ran that box, let me double check after morning coffee and send you a
> fresh patch (if needed) :-)

It seems I had a left-over arch/sparc/include/asm/perf_counter.h from
earlier experiments that was never checked in, so that is why it worked
for me. include/linux/sched.h does:

#ifdef CONFIG_PERF_COUNTERS
# include <asm/perf_counter.h>
#endif

so that's no way around an empty stub file. Updated patch below, this is
what I have been using.

commit 99d9c7f3326a73e2b9952c99d6c7a15bf761858f
Author: Jens Axboe <[email protected]>
Date: Fri Sep 4 08:31:22 2009 +0200

sparc: add basic support for 'perf'

This wires up the perf_counter_open() syscall so that basic
software support for perf is working.

Signed-off-by: Jens Axboe <[email protected]>

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 3f8b6a9..71a9a11 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -25,6 +25,7 @@ config SPARC
select ARCH_WANT_OPTIONAL_GPIOLIB
select RTC_CLASS
select RTC_DRV_M48T59
+ select HAVE_PERF_COUNTERS

config SPARC32
def_bool !64BIT
@@ -44,6 +45,7 @@ config SPARC64
select RTC_DRV_BQ4802
select RTC_DRV_SUN4V
select RTC_DRV_STARFIRE
+ select HAVE_PERF_COUNTERS

config ARCH_DEFCONFIG
string
diff --git a/arch/sparc/include/asm/perf_counter.h b/arch/sparc/include/asm/perf_counter.h
new file mode 100644
index 0000000..664b624
--- /dev/null
+++ b/arch/sparc/include/asm/perf_counter.h
@@ -0,0 +1,8 @@
+#ifndef __ASM_SPARC_PERF_COUNTER_H
+#define __ASM_SPARC_PERF_COUNTER_H
+
+static inline void set_perf_counter_pending(void) { }
+
+#define PERF_COUNTER_INDEX_OFFSET 0
+
+#endif
diff --git a/arch/sparc/include/asm/unistd.h b/arch/sparc/include/asm/unistd.h
index b2c406d..706df66 100644
--- a/arch/sparc/include/asm/unistd.h
+++ b/arch/sparc/include/asm/unistd.h
@@ -395,8 +395,9 @@
#define __NR_preadv 324
#define __NR_pwritev 325
#define __NR_rt_tgsigqueueinfo 326
+#define __NR_perf_counter_open 327

-#define NR_SYSCALLS 327
+#define NR_SYSCALLS 328

#ifdef __32bit_syscall_numbers__
/* Sparc 32-bit only has the "setresuid32", "getresuid32" variants,
diff --git a/arch/sparc/kernel/systbls_32.S b/arch/sparc/kernel/systbls_32.S
index 6909016..0418157 100644
--- a/arch/sparc/kernel/systbls_32.S
+++ b/arch/sparc/kernel/systbls_32.S
@@ -82,5 +82,5 @@ sys_call_table:
/*310*/ .long sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
/*315*/ .long sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
/*320*/ .long sys_dup3, sys_pipe2, sys_inotify_init1, sys_accept4, sys_preadv
-/*325*/ .long sys_pwritev, sys_rt_tgsigqueueinfo
+/*325*/ .long sys_pwritev, sys_rt_tgsigqueueinfo, sys_perf_counter_open

diff --git a/arch/sparc/kernel/systbls_64.S b/arch/sparc/kernel/systbls_64.S
index 2ee7250..91b06b7 100644
--- a/arch/sparc/kernel/systbls_64.S
+++ b/arch/sparc/kernel/systbls_64.S
@@ -83,7 +83,7 @@ sys_call_table32:
/*310*/ .word compat_sys_utimensat, compat_sys_signalfd, sys_timerfd_create, sys_eventfd, compat_sys_fallocate
.word compat_sys_timerfd_settime, compat_sys_timerfd_gettime, compat_sys_signalfd4, sys_eventfd2, sys_epoll_create1
/*320*/ .word sys_dup3, sys_pipe2, sys_inotify_init1, sys_accept4, compat_sys_preadv
- .word compat_sys_pwritev, compat_sys_rt_tgsigqueueinfo
+ .word compat_sys_pwritev, compat_sys_rt_tgsigqueueinfo, sys_perf_counter_open

#endif /* CONFIG_COMPAT */

@@ -158,4 +158,4 @@ sys_call_table:
/*310*/ .word sys_utimensat, sys_signalfd, sys_timerfd_create, sys_eventfd, sys_fallocate
.word sys_timerfd_settime, sys_timerfd_gettime, sys_signalfd4, sys_eventfd2, sys_epoll_create1
/*320*/ .word sys_dup3, sys_pipe2, sys_inotify_init1, sys_accept4, sys_preadv
- .word sys_pwritev, sys_rt_tgsigqueueinfo
+ .word sys_pwritev, sys_rt_tgsigqueueinfo, sys_perf_counter_open
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index e5148e2..2abeb20 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -41,6 +41,12 @@
#define cpu_relax() asm volatile("" ::: "memory");
#endif

+#ifdef __sparc__
+#include "../../arch/sparc/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>

--
Jens Axboe

2009-09-04 06:44:34

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc


* Jens Axboe <[email protected]> wrote:

> On Fri, Sep 04 2009, Jens Axboe wrote:
> > On Thu, Sep 03 2009, David Miller wrote:
> > > From: Ingo Molnar <[email protected]>
> > > Date: Fri, 4 Sep 2009 07:02:56 +0200
> > >
> > > >
> > > > * David Miller <[email protected]> wrote:
> > > >
> > > >> Does this build for you without adding an
> > > >> arch/sparc/include/asm/perf_counter.h that looks
> > > >> something like the following?
> > > >>
> > > >> #ifndef _ASM_SPARC_PERF_COUNTER_H
> > > >> #define _ASM_SPARC_PERF_COUNTER_H
> > > >>
> > > >> #define PERF_COUNTER_INDEX_OFFSET 0
> > > >>
> > > >> #endif
> > > >>
> > > >> Or is this somehow now required in the -tip trees?
> > > >
> > > > This used to be required but i recently fixed this (and that fix is
> > > > upstream as well) via:
> > > >
> > > > f738eb1: perf_counter: Fix the PARISC build
> > > >
> > > > there's now a default define of 0 so you dont have to define it and
> > > > can leave out this chunk.
> > > >
> > > > ( That index is only interesting if the architecture has a way to
> > > > allow unprivileged user-space to access counter registers
> > > > directly. In that case the index reflects the offset from the
> > > > (constantly changing) dynamix index which we put into the mmap
> > > > header. With Sparc not having a hw-PMU implementation this index
> > > > is entirely uninteresting at this stage. )
> > >
> > > But you still do need at least an empty perf_counter.h file
> > > right? Jens must have left the file out of his submission
> > > by accident, and that's what I'm trying to get to the bottom
> > > of here :-)
> > >
> > > I assume there was a similar change to deal with references to
> > > set_perf_counter_pending() too or is at least a NOP definition
> > > still needed?
> >
> > It wasn't required when I built and used it (and sent the
> > patch), I used the posted patch as-is. It's been a few weeks
> > since I last updated and ran that box, let me double check after
> > morning coffee and send you a fresh patch (if needed) :-)
>
> It seems I had a left-over arch/sparc/include/asm/perf_counter.h
> from earlier experiments that was never checked in, so that is why
> it worked for me. include/linux/sched.h does:
>
> #ifdef CONFIG_PERF_COUNTERS
> # include <asm/perf_counter.h>
> #endif
>
> so that's no way around an empty stub file. Updated patch below,
> this is what I have been using.

That stub is not wasted: it will be filled in with real arch hw-PMU
details, once that's implemented too.

hw-PMU support has its advantages: it can provide NMI sampling that
allows 'perf report' to pierce irqs-off critical sections, and it
also gives access to non-time based metrics such as instructions,
cache-misses, etc. - depending on what the CPU can do.

That way you can tell at a glance what a workload does:

aldebaran:~> perf stat ./hackbench 10
Time: 0.109

Performance counter stats for './hackbench 10':

1191.574039 task-clock-msecs # 7.768 CPUs
50363 context-switches # 0.042 M/sec
4249 CPU-migrations # 0.004 M/sec
17710 page-faults # 0.015 M/sec
3600730931 cycles # 3021.827 M/sec
1573681316 instructions # 0.437 IPC
15394883 cache-references # 12.920 M/sec
5005241 cache-misses # 4.201 M/sec

0.153389368 seconds time elapsed

without hw-PMU support it looks like this:

venus:~> perf stat ./hackbench 1
Time: 7.600

Performance counter stats for './hackbench 1':

28218.316944 task-clock-ticks # 3.590 CPU utilization factor
62700 context-switches # 0.002 M/sec
11112 CPU-migrations # 0.000 M/sec
1919 page-faults # 0.000 M/sec
<not counted> cycles
<not counted> instructions
<not counted> cache-references
<not counted> cache-misses

Wall-clock time elapsed: 7860.000004 msecs

The soft stats and time measurements work just fine - the hardware
metrics are not counted.

Ingo

2009-09-04 09:56:58

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] basic perf support for sparc

From: Jens Axboe <[email protected]>
Date: Fri, 4 Sep 2009 08:34:19 +0200

> It seems I had a left-over arch/sparc/include/asm/perf_counter.h from
> earlier experiments that was never checked in, so that is why it worked
> for me. include/linux/sched.h does:

That explains everything :-)

> sparc: add basic support for 'perf'
>
> This wires up the perf_counter_open() syscall so that basic
> software support for perf is working.
>
> Signed-off-by: Jens Axboe <[email protected]>

Applied to sparc-next-2.6, thanks!

As time allows I'll work on hw counter support.