public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] basic perf support for sparc
@ 2009-07-29 11:25 Jens Axboe
  2009-07-29 19:28 ` Jens Axboe
  2009-08-02 20:17 ` [PATCH] basic perf support for sparc David Miller
  0 siblings, 2 replies; 41+ messages in thread
From: Jens Axboe @ 2009-07-29 11:25 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, a.p.zijlstra, fweisbec, Ingo Molnar

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 <jens.axboe@oracle.com>

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


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

* Re: [PATCH] basic perf support for sparc
  2009-07-29 11:25 [PATCH] basic perf support for sparc Jens Axboe
@ 2009-07-29 19:28 ` Jens Axboe
  2009-08-01  1:14   ` Anton Blanchard
  2009-08-02 20:17 ` [PATCH] basic perf support for sparc David Miller
  1 sibling, 1 reply; 41+ messages in thread
From: Jens Axboe @ 2009-07-29 19:28 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, a.p.zijlstra, fweisbec, Ingo Molnar, benh, anton

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


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

* Re: [PATCH] basic perf support for sparc
  2009-07-29 19:28 ` Jens Axboe
@ 2009-08-01  1:14   ` Anton Blanchard
  2009-08-01  8:20     ` Jens Axboe
  0 siblings, 1 reply; 41+ messages in thread
From: Anton Blanchard @ 2009-08-01  1:14 UTC (permalink / raw)
  To: Jens Axboe
  Cc: davem, linux-kernel, a.p.zijlstra, fweisbec, Ingo Molnar, benh,
	penberg, vegard.nossum, paulus, williams, acme


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

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

* Re: [PATCH] basic perf support for sparc
  2009-08-01  1:14   ` Anton Blanchard
@ 2009-08-01  8:20     ` Jens Axboe
  2009-08-01 18:22       ` Arnaldo Carvalho de Melo
  2009-08-05 12:16       ` [tip:perfcounters/urgent] perf: Auto-detect libbfd tip-bot for Peter Zijlstra
  0 siblings, 2 replies; 41+ messages in thread
From: Jens Axboe @ 2009-08-01  8:20 UTC (permalink / raw)
  To: Anton Blanchard
  Cc: davem, linux-kernel, a.p.zijlstra, fweisbec, Ingo Molnar, benh,
	penberg, vegard.nossum, paulus, williams, acme

On Sat, Aug 01 2009, Anton Blanchard wrote:
> 
> 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.

Right, binutils is the ugly one. I got a libbfd.so built for both ppc
and sparc, but it wasn't just a make && make install job. Personally I
could not care less about losing c++ demangled name support, so that
approach sounds fine to me :-)

-- 
Jens Axboe


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

* Re: [PATCH] basic perf support for sparc
  2009-08-01  8:20     ` Jens Axboe
@ 2009-08-01 18:22       ` Arnaldo Carvalho de Melo
  2009-08-02 18:41         ` Ingo Molnar
  2009-08-05 12:16       ` [tip:perfcounters/urgent] perf: Auto-detect libbfd tip-bot for Peter Zijlstra
  1 sibling, 1 reply; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-08-01 18:22 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Anton Blanchard, davem, linux-kernel, a.p.zijlstra, fweisbec,
	Ingo Molnar, benh, penberg, vegard.nossum, paulus, williams, acme

Em Sat, Aug 01, 2009 at 10:20:48AM +0200, Jens Axboe escreveu:
> On Sat, Aug 01 2009, Anton Blanchard wrote:
> > 
> > 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.
> 
> Right, binutils is the ugly one. I got a libbfd.so built for both ppc
> and sparc, but it wasn't just a make && make install job. Personally I
> could not care less about losing c++ demangled name support, so that
> approach sounds fine to me :-)

Exactly, for a huge number of developers not being able to see demangled
C++ is okay, so I agree on adding smarts to not demangle when
binutils-devel is not available.

I thought about extracting the demangling bits out of binutils, ran away
screaming. I also hoped elfutils would have that by now, but it doesn't.

- Arnaldo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-01 18:22       ` Arnaldo Carvalho de Melo
@ 2009-08-02 18:41         ` Ingo Molnar
  2009-08-02 19:44           ` Kyle McMartin
  0 siblings, 1 reply; 41+ messages in thread
From: Ingo Molnar @ 2009-08-02 18:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jens Axboe, Anton Blanchard, davem, linux-kernel, a.p.zijlstra,
	fweisbec, benh, penberg, vegard.nossum, paulus, williams


* Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Em Sat, Aug 01, 2009 at 10:20:48AM +0200, Jens Axboe escreveu:
> > On Sat, Aug 01 2009, Anton Blanchard wrote:
> > > 
> > > 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.
> > 
> > Right, binutils is the ugly one. I got a libbfd.so built for 
> > both ppc and sparc, but it wasn't just a make && make install 
> > job. Personally I could not care less about losing c++ demangled 
> > name support, so that approach sounds fine to me :-)
> 
> Exactly, for a huge number of developers not being able to see 
> demangled C++ is okay, so I agree on adding smarts to not demangle 
> when binutils-devel is not available.
> 
> I thought about extracting the demangling bits out of binutils, 
> ran away screaming. I also hoped elfutils would have that by now, 
> but it doesn't.

Could we somehow define a weak symbol for those library functions 
ourselves and thus just fall back to that (which does nothing) 
instead of failing the link?

	Ingo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 18:41         ` Ingo Molnar
@ 2009-08-02 19:44           ` Kyle McMartin
  2009-08-02 19:50             ` Ingo Molnar
  2009-08-05 12:10             ` Peter Zijlstra
  0 siblings, 2 replies; 41+ messages in thread
From: Kyle McMartin @ 2009-08-02 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, Jens Axboe, Anton Blanchard, davem,
	linux-kernel, a.p.zijlstra, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

On Sun, Aug 02, 2009 at 08:41:48PM +0200, Ingo Molnar wrote:
> Could we somehow define a weak symbol for those library functions 
> ourselves and thus just fall back to that (which does nothing) 
> instead of failing the link?
> 

Well, you could just dlopen the object, but that's kind of gross...

Why not abuse the perf Makefile, which already has this kind of
portability gunk in it?

$ make NO_BFD_DEMANGLE=1
succeeds with no libbfd.

$ make
succeeds when libbfd is around.

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index a5e9b87..67ab097 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -345,7 +345,6 @@ BUILTIN_OBJS += builtin-stat.o
 BUILTIN_OBJS += builtin-top.o
 
 PERFLIBS = $(LIB_FILE)
-EXTLIBS = -lbfd
 
 #
 # Platform specific tweaks
@@ -541,6 +540,11 @@ endif
 ifdef NO_EXTERNAL_GREP
 	BASIC_CFLAGS += -DNO_EXTERNAL_GREP
 endif
+ifdef NO_BFD_DEMANGLE
+	BASIC_CFLAGS += -DNO_BFD_DEMANGLE
+else
+	EXTLIBS += -lbfd
+endif
 
 ifeq ($(PERL_PATH),)
 NO_PERL=NoThanks
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2810605..736b8db 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -6,10 +6,20 @@
 #include <libelf.h>
 #include <gelf.h>
 #include <elf.h>
+
+#ifndef NO_BFD_DEMANGLE
 #include <bfd.h>
+#endif /* NO_BFD_DEMANGLE */
 
 const char *sym_hist_filter;
 
+#ifdef NO_BFD_DEMANGLE
+static inline char *bfd_demangle(void __used *v, const char __used *c,
+				int __used i) {
+	return NULL;
+}
+#endif
+
 #ifndef DMGL_PARAMS
 #define DMGL_PARAMS      (1 << 0)       /* Include function args */
 #define DMGL_ANSI        (1 << 1)       /* Include const, volatile, etc */

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 19:44           ` Kyle McMartin
@ 2009-08-02 19:50             ` Ingo Molnar
  2009-08-02 20:11               ` Kyle McMartin
  2009-08-05 12:10             ` Peter Zijlstra
  1 sibling, 1 reply; 41+ messages in thread
From: Ingo Molnar @ 2009-08-02 19:50 UTC (permalink / raw)
  To: Kyle McMartin
  Cc: Arnaldo Carvalho de Melo, Jens Axboe, Anton Blanchard, davem,
	linux-kernel, a.p.zijlstra, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams


* Kyle McMartin <kyle@mcmartin.ca> wrote:

> On Sun, Aug 02, 2009 at 08:41:48PM +0200, Ingo Molnar wrote:
> > Could we somehow define a weak symbol for those library functions 
> > ourselves and thus just fall back to that (which does nothing) 
> > instead of failing the link?
> > 
> 
> Well, you could just dlopen the object, but that's kind of 
> gross...

hm, why is it gross? I think it's a nicely plug-and-play tooling 
solution - instead of some obscure make flag.

Since we define the prototype anyway:

> +static inline char *bfd_demangle(void __used *v, const char __used *c,
> +				int __used i) {
> +	return NULL;
> +}

We are sticking to a given version of the API. We could turn that 
into a function pointer and fill it in during startup via dlopen(). 
If it's NULL then we dont call it and assume a value of NULL.

Mind submitting such a version of your fix? It would nicely decrease 
the build requirements cross section surface of perf.

	Ingo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 19:50             ` Ingo Molnar
@ 2009-08-02 20:11               ` Kyle McMartin
  2009-08-02 20:33                 ` Ingo Molnar
  2009-08-02 20:47                 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 41+ messages in thread
From: Kyle McMartin @ 2009-08-02 20:11 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Kyle McMartin, Arnaldo Carvalho de Melo, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, a.p.zijlstra, fweisbec,
	benh, penberg, vegard.nossum, paulus, williams

On Sun, Aug 02, 2009 at 09:50:43PM +0200, Ingo Molnar wrote:
> We are sticking to a given version of the API. We could turn that 
> into a function pointer and fill it in during startup via dlopen(). 
> If it's NULL then we dont call it and assume a value of NULL.
> 
> Mind submitting such a version of your fix? It would nicely decrease 
> the build requirements cross section surface of perf.
> 

Assuming I remember how to use dlopen/dlsym, this might work.
Unfortunately I can't easily test anything this weekend, so
I don't actually know if it works... but it links ok.

cheers, Kyle

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index a5e9b87..8fb2e12 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -345,7 +345,7 @@ BUILTIN_OBJS += builtin-stat.o
 BUILTIN_OBJS += builtin-top.o
 
 PERFLIBS = $(LIB_FILE)
-EXTLIBS = -lbfd
+EXTLIBS = -ldl
 
 #
 # Platform specific tweaks
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 2810605..e41cb71 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,9 +4,9 @@
 #include "symbol.h"
 
 #include <libelf.h>
+#include <dlfcn.h>
 #include <gelf.h>
 #include <elf.h>
-#include <bfd.h>
 
 const char *sym_hist_filter;
 
@@ -512,6 +512,11 @@ out:
 	return 0;
 }
 
+static char *perf_bfd_demangle(void __used *v,
+	const char __used *name, int __used opt) {
+	return NULL;
+}
+
 static int dso__load_sym(struct dso *self, int fd, const char *name,
 			 symbol_filter_t filter, int verbose, struct module *mod)
 {
@@ -526,6 +531,8 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 	Elf_Scn *sec, *sec_strndx;
 	Elf *elf;
 	int nr = 0, kernel = !strcmp("[kernel]", self->name);
+	char *(*demangle)(void *v, const char *name, int opt) = NULL;
+	void *handle;
 
 	elf = elf_begin(fd, ELF_C_READ_MMAP, NULL);
 	if (elf == NULL) {
@@ -578,6 +585,13 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 						     NULL) != NULL);
 	} else self->adjust_symbols = 0;
 
+	handle = dlopen("libbfd.so", RTLD_NOW);
+	if (!handle)
+		demangle = perf_bfd_demangle;
+	demangle = dlsym(handle, "bfd_demangle");
+	if (!demangle)
+		demangle = perf_bfd_demangle;
+
 	elf_symtab__for_each_symbol(syms, nr_syms, index, sym) {
 		struct symbol *f;
 		const char *name;
@@ -592,7 +606,7 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 
 		sec = elf_getscn(elf, sym.st_shndx);
 		if (!sec)
-			goto out_elf_end;
+			goto out_dlclose;
 
 		gelf_getshdr(sec, &shdr);
 
@@ -617,7 +631,7 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 			else {
 				fprintf(stderr, "dso__load_sym() module %s lookup of %s failed\n",
 					mod->name, section_name);
-				goto out_elf_end;
+				goto out_dlclose;
 			}
 		}
 		/*
@@ -626,7 +640,7 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 		 * to it...
 		 */
 		name = elf_sym__name(&sym, symstrs);
-		demangled = bfd_demangle(NULL, name, DMGL_PARAMS | DMGL_ANSI);
+		demangled = demangle(NULL, name, DMGL_PARAMS | DMGL_ANSI);
 		if (demangled != NULL)
 			name = demangled;
 
@@ -634,7 +648,7 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 				self->sym_priv_size, obj_start, verbose);
 		free(demangled);
 		if (!f)
-			goto out_elf_end;
+			goto out_dlclose;
 
 		if (filter && filter(self, f))
 			symbol__delete(f, self->sym_priv_size);
@@ -646,6 +660,9 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 	}
 
 	err = nr;
+out_dlclose:
+	if (handle)
+		dlclose(handle);
 out_elf_end:
 	elf_end(elf);
 out_close:

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

* Re: [PATCH] basic perf support for sparc
  2009-07-29 11:25 [PATCH] basic perf support for sparc Jens Axboe
  2009-07-29 19:28 ` Jens Axboe
@ 2009-08-02 20:17 ` David Miller
  2009-08-02 20:25   ` Ingo Molnar
  2009-08-06  7:02   ` Jens Axboe
  1 sibling, 2 replies; 41+ messages in thread
From: David Miller @ 2009-08-02 20:17 UTC (permalink / raw)
  To: jens.axboe; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

From: Jens Axboe <jens.axboe@oracle.com>
Date: Wed, 29 Jul 2009 13:25:10 +0200

> -#define NR_SYSCALLS		327
> +#define NR_SYSCALLS		328

When you increase this value, you have to add entries to all of the
syscall tables.  The syscall dispatch checks against this as a limit,
so if you don't explicitly add an entry to all the tables, it's
possible to deref garbage past the end of the table and try to jump to
it as a syscall.

And if you somehow arrange for adding a compat syscall entry here for
this, and build the perf tools 32-bit, you can forego all of these
rediculious issues with trying to get a 64-bit BFD library.  If the
perf tools are written portably and use types like u64 etc. for
holding addresses and similar things, this should not be an issue.

The 32-bit sparc BFD library has full support for all the 64-bit
binary formats and whatnot.

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 20:17 ` [PATCH] basic perf support for sparc David Miller
@ 2009-08-02 20:25   ` Ingo Molnar
  2009-08-06  7:02   ` Jens Axboe
  1 sibling, 0 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-08-02 20:25 UTC (permalink / raw)
  To: David Miller; +Cc: jens.axboe, linux-kernel, a.p.zijlstra, fweisbec


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

> From: Jens Axboe <jens.axboe@oracle.com>
> Date: Wed, 29 Jul 2009 13:25:10 +0200
> 
> > -#define NR_SYSCALLS		327
> > +#define NR_SYSCALLS		328
> 
> When you increase this value, you have to add entries to all of 
> the syscall tables.  The syscall dispatch checks against this as a 
> limit, so if you don't explicitly add an entry to all the tables, 
> it's possible to deref garbage past the end of the table and try 
> to jump to it as a syscall.
> 
> And if you somehow arrange for adding a compat syscall entry here 
> for this, and build the perf tools 32-bit, you can forego all of 
> these rediculious issues with trying to get a 64-bit BFD library.  
> If the perf tools are written portably and use types like u64 etc. 
> for holding addresses and similar things, this should not be an 
> issue.
> 
> The 32-bit sparc BFD library has full support for all the 64-bit 
> binary formats and whatnot.

That would work too. On x86 perf works all across the compatibility 
spectrum, and we do use strict u32/u64 typing and ABIs.

Note that we'll also solve (remove) the binutils-devel dependency, 
it creates a way too large set of external build constraints for 
perf. But in any case both 32-bit and 64-bit perf should work just 
fine.

	Ingo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 20:11               ` Kyle McMartin
@ 2009-08-02 20:33                 ` Ingo Molnar
  2009-08-02 20:47                 ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-08-02 20:33 UTC (permalink / raw)
  To: Kyle McMartin
  Cc: Arnaldo Carvalho de Melo, Jens Axboe, Anton Blanchard, davem,
	linux-kernel, a.p.zijlstra, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams


* Kyle McMartin <kyle@mcmartin.ca> wrote:

> On Sun, Aug 02, 2009 at 09:50:43PM +0200, Ingo Molnar wrote:
> > We are sticking to a given version of the API. We could turn that 
> > into a function pointer and fill it in during startup via dlopen(). 
> > If it's NULL then we dont call it and assume a value of NULL.
> > 
> > Mind submitting such a version of your fix? It would nicely decrease 
> > the build requirements cross section surface of perf.
> > 
> 
> Assuming I remember how to use dlopen/dlsym, this might work. 
> Unfortunately I can't easily test anything this weekend, so I 
> don't actually know if it works... but it links ok.

looks good to me. Arnaldo, mind picking this up?

	Ingo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 20:11               ` Kyle McMartin
  2009-08-02 20:33                 ` Ingo Molnar
@ 2009-08-02 20:47                 ` Arnaldo Carvalho de Melo
  2009-08-03  1:54                   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-08-02 20:47 UTC (permalink / raw)
  To: Kyle McMartin
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, a.p.zijlstra, fweisbec,
	benh, penberg, vegard.nossum, paulus, williams

Em Sun, Aug 02, 2009 at 04:11:32PM -0400, Kyle McMartin escreveu:
> On Sun, Aug 02, 2009 at 09:50:43PM +0200, Ingo Molnar wrote:
> > We are sticking to a given version of the API. We could turn that 
> > into a function pointer and fill it in during startup via dlopen(). 
> > If it's NULL then we dont call it and assume a value of NULL.
> > 
> > Mind submitting such a version of your fix? It would nicely decrease 
> > the build requirements cross section surface of perf.
> > 
> 
> Assuming I remember how to use dlopen/dlsym, this might work.
> Unfortunately I can't easily test anything this weekend, so
> I don't actually know if it works... but it links ok.
> 
> cheers, Kyle

Doesn't seem to work, tried with firefox, with matching
xulrunner-debuginfo package installed and symbols didn't got properly
demangled, gotta be afk now, but will try to figure this out later.

[]s.

- Arnaldo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 20:47                 ` Arnaldo Carvalho de Melo
@ 2009-08-03  1:54                   ` Arnaldo Carvalho de Melo
  2009-08-04  3:33                     ` Kyle McMartin
  2009-08-04  9:25                     ` Ingo Molnar
  0 siblings, 2 replies; 41+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-08-03  1:54 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Kyle McMartin, Ingo Molnar, Jens Axboe, Anton Blanchard, davem,
	linux-kernel, a.p.zijlstra, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

Em Sun, Aug 02, 2009 at 05:47:01PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sun, Aug 02, 2009 at 04:11:32PM -0400, Kyle McMartin escreveu:
> > On Sun, Aug 02, 2009 at 09:50:43PM +0200, Ingo Molnar wrote:
> > > We are sticking to a given version of the API. We could turn that 
> > > into a function pointer and fill it in during startup via dlopen(). 
> > > If it's NULL then we dont call it and assume a value of NULL.
> > > 
> > > Mind submitting such a version of your fix? It would nicely decrease 
> > > the build requirements cross section surface of perf.
> > > 
> > 
> > Assuming I remember how to use dlopen/dlsym, this might work.
> > Unfortunately I can't easily test anything this weekend, so
> > I don't actually know if it works... but it links ok.
> > 
> > cheers, Kyle
> 
> Doesn't seem to work, tried with firefox, with matching
> xulrunner-debuginfo package installed and symbols didn't got properly
> demangled, gotta be afk now, but will try to figure this out later.

The problem is that at least on f11, libbfd.so is a... linker script,
not an ELF shared library as the name seemed to imply.

But if I cheat and do a dlopen on the full name, that includes the
version, it works. Argh, I don't know why this has to be so contorted,
does anybody here understands this?

Anyway, the reworked patch below works and additionally doesn't dlopens
at each DSO, but just once and only if it finds a symbol starting with
an underscore, an heuristic to avoid loading bfd if we don't need it.

Looking at each DW_AT_language attribute in the DW_TAG_compile_unit
tags, trying to figure out if there is a C++ object in the mix seems too
much, we would have to traverse all of the compile units, touching most
of the debuginfo file :-\

- Arnaldo

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 4b20fa4..8fb2e12 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -345,7 +345,7 @@ BUILTIN_OBJS += builtin-stat.o
 BUILTIN_OBJS += builtin-top.o
 
 PERFLIBS = $(LIB_FILE)
-EXTLIBS = -lbfd -liberty
+EXTLIBS = -ldl
 
 #
 # Platform specific tweaks
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b4fe057..625f975 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -4,9 +4,9 @@
 #include "symbol.h"
 
 #include <libelf.h>
+#include <dlfcn.h>
 #include <gelf.h>
 #include <elf.h>
-#include <bfd.h>
 
 const char *sym_hist_filter;
 
@@ -512,6 +512,30 @@ out:
 	return 0;
 }
 
+static char *def_perf_bfd_demangle(void __used *v, const char __used *name,
+				   int __used opt)
+{
+	return NULL;
+}
+
+static char *(*perf_bfd_demangle)(void *v, const char *name, int opt);
+
+static void load_perf_bfd_demangle(void)
+{
+	void *handle = dlopen("libbfd-2.19.51.0.2-17.fc11.so", RTLD_NOW);
+
+	if (handle) {
+		perf_bfd_demangle = dlsym(handle, "bfd_demangle");
+		if (perf_bfd_demangle)
+			return;
+		dlclose(handle);
+	} else
+		fprintf(stderr, "%s: problems finding bfd_demangle: %s.\n",
+			__func__, dlerror());
+
+	perf_bfd_demangle = def_perf_bfd_demangle;
+}
+
 static int dso__load_sym(struct dso *self, int fd, const char *name,
 			 symbol_filter_t filter, int verbose, struct module *mod)
 {
@@ -581,7 +605,7 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 	elf_symtab__for_each_symbol(syms, nr_syms, index, sym) {
 		struct symbol *f;
 		const char *name;
-		char *demangled;
+		char *demangled = NULL;
 		u64 obj_start;
 		struct section *section = NULL;
 		int is_label = elf_sym__is_label(&sym);
@@ -626,9 +650,16 @@ static int dso__load_sym(struct dso *self, int fd, const char *name,
 		 * to it...
 		 */
 		name = elf_sym__name(&sym, symstrs);
-		demangled = bfd_demangle(NULL, name, DMGL_PARAMS | DMGL_ANSI);
-		if (demangled != NULL)
-			name = demangled;
+		if (name && name[0] == '_') {
+			if (perf_bfd_demangle == NULL)
+				load_perf_bfd_demangle();
+
+			if (perf_bfd_demangle != NULL) {
+				demangled = perf_bfd_demangle(NULL, name, DMGL_PARAMS | DMGL_ANSI);
+				if (demangled != NULL)
+					name = demangled;
+			}
+		}
 
 		f = symbol__new(sym.st_value, sym.st_size, name,
 				self->sym_priv_size, obj_start, verbose);

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

* Re: [PATCH] basic perf support for sparc
  2009-08-03  1:54                   ` Arnaldo Carvalho de Melo
@ 2009-08-04  3:33                     ` Kyle McMartin
  2009-08-04  9:25                     ` Ingo Molnar
  1 sibling, 0 replies; 41+ messages in thread
From: Kyle McMartin @ 2009-08-04  3:33 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Kyle McMartin, Ingo Molnar, Jens Axboe, Anton Blanchard, davem,
	linux-kernel, a.p.zijlstra, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

On Sun, Aug 02, 2009 at 10:54:30PM -0300, Arnaldo Carvalho de Melo wrote:
> The problem is that at least on f11, libbfd.so is a... linker script,
> not an ELF shared library as the name seemed to imply.
> 

Ugh, that's weird. My fedora boxes are the same, but it's correctly a
symlink on my Debian box.
lrwxrwxrwx 1 root root     26 Aug  3 23:30 /usr/lib/libbfd.so ->
libbfd-2.19.51.20090723.so

Kind of strange it's a reference to bfd.a too. :/

> But if I cheat and do a dlopen on the full name, that includes the
> version, it works. Argh, I don't know why this has to be so contorted,
> does anybody here understands this?
> 
> Anyway, the reworked patch below works and additionally doesn't dlopens
> at each DSO, but just once and only if it finds a symbol starting with
> an underscore, an heuristic to avoid loading bfd if we don't need it.
> 

Looks better than mine. :)

cheers, Kyle

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

* Re: [PATCH] basic perf support for sparc
  2009-08-03  1:54                   ` Arnaldo Carvalho de Melo
  2009-08-04  3:33                     ` Kyle McMartin
@ 2009-08-04  9:25                     ` Ingo Molnar
  2009-08-04  9:29                       ` Peter Zijlstra
                                         ` (2 more replies)
  1 sibling, 3 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-08-04  9:25 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Kyle McMartin, Jens Axboe, Anton Blanchard, davem, linux-kernel,
	a.p.zijlstra, fweisbec, benh, penberg, vegard.nossum, paulus,
	williams


* Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> +static void load_perf_bfd_demangle(void)
> +{
> +	void *handle = dlopen("libbfd-2.19.51.0.2-17.fc11.so", RTLD_NOW);

Hm, this does not look like a very generic solution. Is there some 
way to do a library search to figure out the name? I guess a glob 
match on /usr/lib/libbfd*.so?

	Ingo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-04  9:25                     ` Ingo Molnar
@ 2009-08-04  9:29                       ` Peter Zijlstra
  2009-08-04 13:02                         ` David Miller
  2009-08-04 10:32                       ` Frederic Riss
  2009-08-04 11:28                       ` Ingo Molnar
  2 siblings, 1 reply; 41+ messages in thread
From: Peter Zijlstra @ 2009-08-04  9:29 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, Kyle McMartin, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

On Tue, 2009-08-04 at 11:25 +0200, Ingo Molnar wrote:
> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> 
> > +static void load_perf_bfd_demangle(void)
> > +{
> > +	void *handle = dlopen("libbfd-2.19.51.0.2-17.fc11.so", RTLD_NOW);
> 
> Hm, this does not look like a very generic solution. Is there some 
> way to do a library search to figure out the name? I guess a glob 
> match on /usr/lib/libbfd*.so?

I'd simply go for libbfd.so and raise a bug report against fedora. A
link archive named .so is just stupid.


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

* Re: [PATCH] basic perf support for sparc
  2009-08-04  9:25                     ` Ingo Molnar
  2009-08-04  9:29                       ` Peter Zijlstra
@ 2009-08-04 10:32                       ` Frederic Riss
  2009-08-04 10:38                         ` Peter Zijlstra
  2009-08-04 11:28                       ` Ingo Molnar
  2 siblings, 1 reply; 41+ messages in thread
From: Frederic Riss @ 2009-08-04 10:32 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Arnaldo Carvalho de Melo, Kyle McMartin, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, a.p.zijlstra, fweisbec,
	benh, penberg, vegard.nossum, paulus, williams

2009/8/4 Ingo Molnar <mingo@elte.hu>:
>
> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
>
>> +static void load_perf_bfd_demangle(void)
>> +{
>> +     void *handle = dlopen("libbfd-2.19.51.0.2-17.fc11.so", RTLD_NOW);
>
> Hm, this does not look like a very generic solution. Is there some
> way to do a library search to figure out the name? I guess a glob
> match on /usr/lib/libbfd*.so?

If all you want is C++ symbols demangling, why not just look for the
c++filt binary on the system? It accepts mangled names on its stdin
and outputs the demangled name on stdout.  You can keep it running in
the background, and communicate through pipes. Of course it's not as
effective as a library call, but I guess that it'll be much more
generic and also much more likely to be found on a developer's system
than a libbfd shared library.

Fred.

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

* Re: [PATCH] basic perf support for sparc
  2009-08-04 10:32                       ` Frederic Riss
@ 2009-08-04 10:38                         ` Peter Zijlstra
  2009-08-04 11:23                           ` Frederic Riss
  0 siblings, 1 reply; 41+ messages in thread
From: Peter Zijlstra @ 2009-08-04 10:38 UTC (permalink / raw)
  To: Frederic Riss
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Kyle McMartin, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

On Tue, 2009-08-04 at 12:32 +0200, Frederic Riss wrote:
> 2009/8/4 Ingo Molnar <mingo@elte.hu>:
> >
> > * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> >
> >> +static void load_perf_bfd_demangle(void)
> >> +{
> >> +     void *handle = dlopen("libbfd-2.19.51.0.2-17.fc11.so", RTLD_NOW);
> >
> > Hm, this does not look like a very generic solution. Is there some
> > way to do a library search to figure out the name? I guess a glob
> > match on /usr/lib/libbfd*.so?
> 
> If all you want is C++ symbols demangling, why not just look for the
> c++filt binary on the system? It accepts mangled names on its stdin
> and outputs the demangled name on stdout.  You can keep it running in
> the background, and communicate through pipes. Of course it's not as
> effective as a library call, but I guess that it'll be much more
> generic and also much more likely to be found on a developer's system
> than a libbfd shared library.

Oddly enough c++filt uses libbfd :-)

$ ldd `which c++filt`
        linux-gate.so.1 =>  (0xffffe000)
        libbfd-2.19.1.so => /usr/lib/libbfd-2.19.1.so (0xf7de9000)
        libz.so.1 => /lib/libz.so.1 (0xf7dd3000)
        libc.so.6 => /lib/tls/i686/cmov/libc.so.6 (0xf7c6f000)
        /lib/ld-linux.so.2 (0xf7ef9000)



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

* Re: [PATCH] basic perf support for sparc
  2009-08-04 10:38                         ` Peter Zijlstra
@ 2009-08-04 11:23                           ` Frederic Riss
  0 siblings, 0 replies; 41+ messages in thread
From: Frederic Riss @ 2009-08-04 11:23 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Kyle McMartin, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

2009/8/4 Peter Zijlstra <a.p.zijlstra@chello.nl>:
> On Tue, 2009-08-04 at 12:32 +0200, Frederic Riss wrote:
>> If all you want is C++ symbols demangling, why not just look for the
>> c++filt binary on the system? It accepts mangled names on its stdin
>> and outputs the demangled name on stdout.  You can keep it running in
>> the background, and communicate through pipes. Of course it's not as
>> effective as a library call, but I guess that it'll be much more
>> generic and also much more likely to be found on a developer's system
>> than a libbfd shared library.
>
> Oddly enough c++filt uses libbfd :-)
>
> $ ldd `which c++filt`
>        linux-gate.so.1 =>  (0xffffe000)
>        libbfd-2.19.1.so => /usr/lib/libbfd-2.19.1.so (0xf7de9000)
>        libz.so.1 => /lib/libz.so.1 (0xf7dd3000)
>        libc.so.6 => /lib/tls/i686/cmov/libc.so.6 (0xf7c6f000)
>        /lib/ld-linux.so.2 (0xf7ef9000)

Hehe... It depends how distro build it. IIRC, the default binutils
configure links statically, but I might be wrong. Anyway, this
approach still has the advantage that the binary is called 'c++filt'
and not c++filt-2.19.1-fc11 :-)

Fred

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

* Re: [PATCH] basic perf support for sparc
  2009-08-04  9:25                     ` Ingo Molnar
  2009-08-04  9:29                       ` Peter Zijlstra
  2009-08-04 10:32                       ` Frederic Riss
@ 2009-08-04 11:28                       ` Ingo Molnar
  2 siblings, 0 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-08-04 11:28 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Kyle McMartin, Jens Axboe, Anton Blanchard, davem, linux-kernel,
	a.p.zijlstra, fweisbec, benh, penberg, vegard.nossum, paulus,
	williams


Btw., how about the solution below? Will the libbfd system library 
version override this weak alias properly?

	Ingo

-------------->
>From 238762507bb41689df191160ada1986b012b57df Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 4 Aug 2009 13:27:41 +0200
Subject: [PATCH] perf_counter tools: Provide default bfd_demangle() function in case it's not around

Provide weak alias (which does nothing) in case the system does not
offer one.

Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
LKML-Reference: <new-submission>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/util/symbol.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b4fe057..7bd5128 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -8,6 +8,17 @@
 #include <elf.h>
 #include <bfd.h>
 
+/*
+ * Weak wrapper in case a library version is not available:
+ */
+extern char * __attribute__((weak)) bfd_demangle (bfd *, const char *, int);
+
+char * __attribute__((weak))
+ bfd_demangle (bfd *bfd __used, const char *name __used, int flags __used)
+{
+	return NULL;
+}
+
 const char *sym_hist_filter;
 
 #ifndef DMGL_PARAMS

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

* Re: [PATCH] basic perf support for sparc
  2009-08-04  9:29                       ` Peter Zijlstra
@ 2009-08-04 13:02                         ` David Miller
  0 siblings, 0 replies; 41+ messages in thread
From: David Miller @ 2009-08-04 13:02 UTC (permalink / raw)
  To: a.p.zijlstra
  Cc: mingo, acme, kyle, jens.axboe, anton, linux-kernel, fweisbec,
	benh, penberg, vegard.nossum, paulus, williams

From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Tue, 04 Aug 2009 11:29:00 +0200

> On Tue, 2009-08-04 at 11:25 +0200, Ingo Molnar wrote:
>> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
>> 
>> > +static void load_perf_bfd_demangle(void)
>> > +{
>> > +	void *handle = dlopen("libbfd-2.19.51.0.2-17.fc11.so", RTLD_NOW);
>> 
>> Hm, this does not look like a very generic solution. Is there some 
>> way to do a library search to figure out the name? I guess a glob 
>> match on /usr/lib/libbfd*.so?
> 
> I'd simply go for libbfd.so and raise a bug report against fedora. A
> link archive named .so is just stupid.

It really isn't, I find it surprising why people are so up in
arms about this.  It's a quite common technique.

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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 19:44           ` Kyle McMartin
  2009-08-02 19:50             ` Ingo Molnar
@ 2009-08-05 12:10             ` Peter Zijlstra
  2009-08-05 12:21               ` Jens Axboe
  1 sibling, 1 reply; 41+ messages in thread
From: Peter Zijlstra @ 2009-08-05 12:10 UTC (permalink / raw)
  To: Kyle McMartin
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Jens Axboe,
	Anton Blanchard, davem, linux-kernel, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

On Sun, 2009-08-02 at 15:44 -0400, Kyle McMartin wrote:

> Why not abuse the perf Makefile, which already has this kind of
> portability gunk in it?

Does this work for people?

---
Subject: perf: autodetect libbfd
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Wed Aug 05 14:05:16 CEST 2009

Since the C++ demangling isn't needed for everybody and bfd/iberty
aren't widely/easily available on all machines, make it optional.

It allows you to forcefully disable demangling by using NO_DEMANGLE=1
and otherwise tries to detect libbfd/libiberty combinations that
result in a compiling demangler.

Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 tools/perf/Makefile      |   17 ++++++++++++++++-
 tools/perf/util/symbol.c |    9 +++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

Index: linux-2.6/tools/perf/Makefile
===================================================================
--- linux-2.6.orig/tools/perf/Makefile
+++ linux-2.6/tools/perf/Makefile
@@ -345,7 +345,6 @@ BUILTIN_OBJS += builtin-stat.o
 BUILTIN_OBJS += builtin-top.o
 
 PERFLIBS = $(LIB_FILE)
-EXTLIBS = -lbfd -liberty
 
 #
 # Platform specific tweaks
@@ -374,6 +373,22 @@ ifeq ($(uname_S),Darwin)
 	PTHREAD_LIBS =
 endif
 
+ifdef NO_DEMANGLE
+	BASIC_CFLAGS += -DNO_DEMANGLE
+else
+	has_bfd := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd > /dev/null 2>&1 && echo y")
+
+	has_bfd_iberty := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd -liberty > /dev/null 2>&1 && echo y")
+
+	ifeq ($(has_bfd),y)
+		EXTLIBS += -lbfd
+	else ifeq ($(has_bfd_iberty),y)
+		EXTLIBS += -lbfd -liberty
+	else
+		BASIC_CFLAGS += -DNO_DEMANGLE
+	endif
+endif
+
 ifndef CC_LD_DYNPATH
 	ifdef NO_R_TO_GCC_LINKER
 		# Some gcc does not accept and pass -R to the linker to specify
Index: linux-2.6/tools/perf/util/symbol.c
===================================================================
--- linux-2.6.orig/tools/perf/util/symbol.c
+++ linux-2.6/tools/perf/util/symbol.c
@@ -6,7 +6,16 @@
 #include <libelf.h>
 #include <gelf.h>
 #include <elf.h>
+
+#ifndef NO_DEMANGLE
 #include <bfd.h>
+#else
+static inline
+char *bfd_demangle(void __used *v, const char __used *c, int __used i)
+{
+	return NULL;
+}
+#endif
 
 const char *sym_hist_filter;
 



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

* [tip:perfcounters/urgent] perf: Auto-detect libbfd
  2009-08-01  8:20     ` Jens Axboe
  2009-08-01 18:22       ` Arnaldo Carvalho de Melo
@ 2009-08-05 12:16       ` tip-bot for Peter Zijlstra
  2009-08-05 14:29         ` Peter Zijlstra
  1 sibling, 1 reply; 41+ messages in thread
From: tip-bot for Peter Zijlstra @ 2009-08-05 12:16 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, acme, hpa, mingo, a.p.zijlstra, kyle, jens.axboe,
	tglx, mingo

Commit-ID:  2cdbc46d7b2cb0acb68c3ecad93b000552121fa6
Gitweb:     http://git.kernel.org/tip/2cdbc46d7b2cb0acb68c3ecad93b000552121fa6
Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
AuthorDate: Wed, 5 Aug 2009 14:05:16 +0200
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Wed, 5 Aug 2009 14:12:08 +0200

perf: Auto-detect libbfd

Since the C++ demangling isn't needed for everybody and
bfd/iberty aren't widely/easily available on all machines, make
it optional.

It also allows you to forcefully disable demangling by using
NO_DEMANGLE=1 and otherwise tries to detect libbfd/libiberty
combinations that result in a compiling demangler.

Reported-by: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Kyle McMartin <kyle@mcmartin.ca>
LKML-Reference: <20090801082048.GX12579@kernel.dk>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 tools/perf/Makefile      |   18 +++++++++++++++++-
 tools/perf/util/symbol.c |    9 +++++++++
 2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 4b20fa4..ff905ac 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -345,7 +345,6 @@ BUILTIN_OBJS += builtin-stat.o
 BUILTIN_OBJS += builtin-top.o
 
 PERFLIBS = $(LIB_FILE)
-EXTLIBS = -lbfd -liberty
 
 #
 # Platform specific tweaks
@@ -374,6 +373,23 @@ ifeq ($(uname_S),Darwin)
 	PTHREAD_LIBS =
 endif
 
+ifdef NO_DEMANGLE
+	BASIC_CFLAGS += -DNO_DEMANGLE
+else
+
+	has_bfd := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd > /dev/null 2>&1 && echo y")
+
+	has_bfd_iberty := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd -liberty > /dev/null 2>&1 && echo y")
+
+	ifeq ($(has_bfd),y)
+		EXTLIBS += -lbfd
+	else ifeq ($(has_bfd_iberty),y)
+		EXTLIBS += -lbfd -liberty
+	else
+		BASIC_CFLAGS += -DNO_DEMANGLE
+	endif
+endif
+
 ifndef CC_LD_DYNPATH
 	ifdef NO_R_TO_GCC_LINKER
 		# Some gcc does not accept and pass -R to the linker to specify
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index b4fe057..0580b94 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -6,7 +6,16 @@
 #include <libelf.h>
 #include <gelf.h>
 #include <elf.h>
+
+#ifndef NO_DEMANGLE
 #include <bfd.h>
+#else
+static inline
+char *bfd_demangle(void __used *v, const char __used *c, int __used i)
+{
+	return NULL;
+}
+#endif
 
 const char *sym_hist_filter;
 

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

* Re: [PATCH] basic perf support for sparc
  2009-08-05 12:10             ` Peter Zijlstra
@ 2009-08-05 12:21               ` Jens Axboe
  2009-08-05 12:33                 ` Ingo Molnar
  0 siblings, 1 reply; 41+ messages in thread
From: Jens Axboe @ 2009-08-05 12:21 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Kyle McMartin, Ingo Molnar, Arnaldo Carvalho de Melo,
	Anton Blanchard, davem, linux-kernel, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams

On Wed, Aug 05 2009, Peter Zijlstra wrote:
> On Sun, 2009-08-02 at 15:44 -0400, Kyle McMartin wrote:
> 
> > Why not abuse the perf Makefile, which already has this kind of
> > portability gunk in it?
> 
> Does this work for people?

Works for me, just tested it.

-- 
Jens Axboe


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

* Re: [PATCH] basic perf support for sparc
  2009-08-05 12:21               ` Jens Axboe
@ 2009-08-05 12:33                 ` Ingo Molnar
  0 siblings, 0 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-08-05 12:33 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Peter Zijlstra, Kyle McMartin, Arnaldo Carvalho de Melo,
	Anton Blanchard, davem, linux-kernel, fweisbec, benh, penberg,
	vegard.nossum, paulus, williams


* Jens Axboe <jens.axboe@oracle.com> wrote:

> On Wed, Aug 05 2009, Peter Zijlstra wrote:
> > On Sun, 2009-08-02 at 15:44 -0400, Kyle McMartin wrote:
> > 
> > > Why not abuse the perf Makefile, which already has this kind of
> > > portability gunk in it?
> > 
> > Does this work for people?
> 
> Works for me, just tested it.

Thanks Jens - i've committed it and will push it Linus-wards.

	Ingo

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

* Re: [tip:perfcounters/urgent] perf: Auto-detect libbfd
  2009-08-05 12:16       ` [tip:perfcounters/urgent] perf: Auto-detect libbfd tip-bot for Peter Zijlstra
@ 2009-08-05 14:29         ` Peter Zijlstra
  2009-08-05 18:58           ` Peter Zijlstra
  0 siblings, 1 reply; 41+ messages in thread
From: Peter Zijlstra @ 2009-08-05 14:29 UTC (permalink / raw)
  To: mingo, hpa, acme, linux-kernel, jens.axboe, kyle, tglx, mingo
  Cc: linux-tip-commits

On Wed, 2009-08-05 at 12:16 +0000, tip-bot for Peter Zijlstra wrote:
> Commit-ID:  2cdbc46d7b2cb0acb68c3ecad93b000552121fa6
> Gitweb:     http://git.kernel.org/tip/2cdbc46d7b2cb0acb68c3ecad93b000552121fa6
> Author:     Peter Zijlstra <a.p.zijlstra@chello.nl>
> AuthorDate: Wed, 5 Aug 2009 14:05:16 +0200
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Wed, 5 Aug 2009 14:12:08 +0200
> 
> perf: Auto-detect libbfd
> 
> Since the C++ demangling isn't needed for everybody and
> bfd/iberty aren't widely/easily available on all machines, make
> it optional.
> 
> It also allows you to forcefully disable demangling by using
> NO_DEMANGLE=1 and otherwise tries to detect libbfd/libiberty
> combinations that result in a compiling demangler.


The below simpifies the C proglet and extends it to cover auto-detection
of libelf as well, it also generates an error in case of libelf being
MIA and an warning for libbfd.

Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
 tools/perf/Makefile |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index ff905ac..e9db128 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -158,8 +158,10 @@ uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')
 uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
 
 # If we're on a 64-bit kernel, use -m64
-ifneq ($(patsubst %64,%,$(uname_M)),$(uname_M))
-  M64 := -m64
+ifndef NO_64BIT
+	ifneq ($(patsubst %64,%,$(uname_M)),$(uname_M))
+	  M64 := -m64
+	endif
 endif
 
 # CFLAGS and LDFLAGS are for the users to override from the command line.
@@ -373,19 +375,24 @@ ifeq ($(uname_S),Darwin)
 	PTHREAD_LIBS =
 endif
 
+ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf * elf = elf_begin(0, ELF_C_READ_MMAP, 0); return (int)elf; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
+	msg := $(error No libelf.h/libelf found, please install libelf-dev[el]);
+endif
+
 ifdef NO_DEMANGLE
 	BASIC_CFLAGS += -DNO_DEMANGLE
 else
 
-	has_bfd := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd > /dev/null 2>&1 && echo y")
+	has_bfd := $(shell sh -c "(echo '\#include <bfd.h>'; echo 'int main(void) { bfd_demangle(0, 0, 0); return 0; }') | $(CC) -x c - $(ALL_CFLAGS) -o /dev/null $(ALL_LDFLAGS) -lbfd > /dev/null 2>&1 && echo y")
 
-	has_bfd_iberty := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd -liberty > /dev/null 2>&1 && echo y")
+	has_bfd_iberty := $(shell sh -c "(echo '\#include <bfd.h>'; echo 'int main(void) { bfd_demangle(0, 0, 0); return 0; }') | $(CC) -x c - $(ALL_CFLAGS) -o /dev/null $(ALL_LDFLAGS) -lbfd -liberty > /dev/null 2>&1 && echo y")
 
 	ifeq ($(has_bfd),y)
 		EXTLIBS += -lbfd
 	else ifeq ($(has_bfd_iberty),y)
 		EXTLIBS += -lbfd -liberty
 	else
+		msg := $(warning No bfd.h/libbfd found, install binutils-dev[el] to gain symbol demangling)
 		BASIC_CFLAGS += -DNO_DEMANGLE
 	endif
 endif



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

* Re: [tip:perfcounters/urgent] perf: Auto-detect libbfd
  2009-08-05 14:29         ` Peter Zijlstra
@ 2009-08-05 18:58           ` Peter Zijlstra
  0 siblings, 0 replies; 41+ messages in thread
From: Peter Zijlstra @ 2009-08-05 18:58 UTC (permalink / raw)
  To: mingo
  Cc: hpa, acme, linux-kernel, jens.axboe, kyle, tglx, mingo,
	linux-tip-commits


> +ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int
> main(void) { Elf * elf = elf_begin(0, ELF_C_READ_MMAP, 0); return
> (int)elf; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE
> -D_FILE_OFFSET_BITS=64 -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 &&
> echo y"), y)

s/(int)/(long)/

---
Index: linux-2.6/tools/perf/Makefile
===================================================================
--- linux-2.6.orig/tools/perf/Makefile
+++ linux-2.6/tools/perf/Makefile
@@ -158,8 +158,10 @@ uname_P := $(shell sh -c 'uname -p 2>/de
 uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
 
 # If we're on a 64-bit kernel, use -m64
-ifneq ($(patsubst %64,%,$(uname_M)),$(uname_M))
-  M64 := -m64
+ifndef NO_64BIT
+	ifneq ($(patsubst %64,%,$(uname_M)),$(uname_M))
+	  M64 := -m64
+	endif
 endif
 
 # CFLAGS and LDFLAGS are for the users to override from the command line.
@@ -373,18 +375,24 @@ ifeq ($(uname_S),Darwin)
 	PTHREAD_LIBS =
 endif
 
+ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf * elf = elf_begin(0, ELF_C_READ_MMAP, 0); return (long)elf; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
+	msg := $(error No libelf.h/libelf found, please install libelf-dev[el]);
+endif
+
 ifdef NO_DEMANGLE
 	BASIC_CFLAGS += -DNO_DEMANGLE
 else
-	has_bfd := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd > /dev/null 2>&1 && echo y")
 
-	has_bfd_iberty := $(shell sh -c "(echo '\#include <stdio.h>'; echo '\#include <bfd.h>'; echo '\#ifndef DMGL_PARAMS'; echo '\#define DMGL_PARAMS (1 << 0)'; echo '\#define DMGL_ANSI (1 << 1)'; echo '\#endif'; echo 'int main(int argc, char **argv) { bfd_demangle(NULL, argv[0], DMGL_PARAMS | DMGL_ANSI); return 0; }') | gcc -x c - -lbfd -liberty > /dev/null 2>&1 && echo y")
+	has_bfd := $(shell sh -c "(echo '\#include <bfd.h>'; echo 'int main(void) { bfd_demangle(0, 0, 0); return 0; }') | $(CC) -x c - $(ALL_CFLAGS) -o /dev/null $(ALL_LDFLAGS) -lbfd > /dev/null 2>&1 && echo y")
+
+	has_bfd_iberty := $(shell sh -c "(echo '\#include <bfd.h>'; echo 'int main(void) { bfd_demangle(0, 0, 0); return 0; }') | $(CC) -x c - $(ALL_CFLAGS) -o /dev/null $(ALL_LDFLAGS) -lbfd -liberty > /dev/null 2>&1 && echo y")
 
 	ifeq ($(has_bfd),y)
 		EXTLIBS += -lbfd
 	else ifeq ($(has_bfd_iberty),y)
 		EXTLIBS += -lbfd -liberty
 	else
+		msg := $(warning No bfd.h/libbfd found, install binutils-dev[el] to gain symbol demangling)
 		BASIC_CFLAGS += -DNO_DEMANGLE
 	endif
 endif



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

* Re: [PATCH] basic perf support for sparc
  2009-08-02 20:17 ` [PATCH] basic perf support for sparc David Miller
  2009-08-02 20:25   ` Ingo Molnar
@ 2009-08-06  7:02   ` Jens Axboe
  2009-08-12 18:06     ` David Miller
  2009-08-17  1:31     ` David Miller
  1 sibling, 2 replies; 41+ messages in thread
From: Jens Axboe @ 2009-08-06  7:02 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

On Sun, Aug 02 2009, David Miller wrote:
> From: Jens Axboe <jens.axboe@oracle.com>
> Date: Wed, 29 Jul 2009 13:25:10 +0200
> 
> > -#define NR_SYSCALLS		327
> > +#define NR_SYSCALLS		328
> 
> When you increase this value, you have to add entries to all of the
> syscall tables.  The syscall dispatch checks against this as a limit,
> so if you don't explicitly add an entry to all the tables, it's
> possible to deref garbage past the end of the table and try to jump to
> it as a syscall.

Oops, missed the 32-bit table. Does the below look better?

> And if you somehow arrange for adding a compat syscall entry here for
> this, and build the perf tools 32-bit, you can forego all of these
> rediculious issues with trying to get a 64-bit BFD library.  If the
> perf tools are written portably and use types like u64 etc. for
> holding addresses and similar things, this should not be an issue.
> 
> The 32-bit sparc BFD library has full support for all the 64-bit
> binary formats and whatnot.

I'm assuming that this will happen automatically at some point, instead
of perf defaulting to building with the same 32/64-bit of the kernel.

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_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 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


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

* Re: [PATCH] basic perf support for sparc
  2009-08-06  7:02   ` Jens Axboe
@ 2009-08-12 18:06     ` David Miller
  2009-08-12 18:13       ` Jens Axboe
  2009-08-17  1:31     ` David Miller
  1 sibling, 1 reply; 41+ messages in thread
From: David Miller @ 2009-08-12 18:06 UTC (permalink / raw)
  To: jens.axboe; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

From: Jens Axboe <jens.axboe@oracle.com>
Date: Thu, 6 Aug 2009 09:02:42 +0200

> On Sun, Aug 02 2009, David Miller wrote:
>> From: Jens Axboe <jens.axboe@oracle.com>
>> Date: Wed, 29 Jul 2009 13:25:10 +0200
>> 
>> > -#define NR_SYSCALLS		327
>> > +#define NR_SYSCALLS		328
>> 
>> When you increase this value, you have to add entries to all of the
>> syscall tables.  The syscall dispatch checks against this as a limit,
>> so if you don't explicitly add an entry to all the tables, it's
>> possible to deref garbage past the end of the table and try to jump to
>> it as a syscall.
> 
> Oops, missed the 32-bit table. Does the below look better?

I'll take a look at this as soon as I can Jens, sorry for taking
so long.

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

* Re: [PATCH] basic perf support for sparc
  2009-08-12 18:06     ` David Miller
@ 2009-08-12 18:13       ` Jens Axboe
  0 siblings, 0 replies; 41+ messages in thread
From: Jens Axboe @ 2009-08-12 18:13 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

On Wed, Aug 12 2009, David Miller wrote:
> From: Jens Axboe <jens.axboe@oracle.com>
> Date: Thu, 6 Aug 2009 09:02:42 +0200
> 
> > On Sun, Aug 02 2009, David Miller wrote:
> >> From: Jens Axboe <jens.axboe@oracle.com>
> >> Date: Wed, 29 Jul 2009 13:25:10 +0200
> >> 
> >> > -#define NR_SYSCALLS		327
> >> > +#define NR_SYSCALLS		328
> >> 
> >> When you increase this value, you have to add entries to all of the
> >> syscall tables.  The syscall dispatch checks against this as a limit,
> >> so if you don't explicitly add an entry to all the tables, it's
> >> possible to deref garbage past the end of the table and try to jump to
> >> it as a syscall.
> > 
> > Oops, missed the 32-bit table. Does the below look better?
> 
> I'll take a look at this as soon as I can Jens, sorry for taking
> so long.

No problem, I figured you'd get to it eventually and in the mean time
I'll just keep patching it in.

-- 
Jens Axboe


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

* Re: [PATCH] basic perf support for sparc
  2009-08-06  7:02   ` Jens Axboe
  2009-08-12 18:06     ` David Miller
@ 2009-08-17  1:31     ` David Miller
  2009-08-17  6:48       ` Jens Axboe
  1 sibling, 1 reply; 41+ messages in thread
From: David Miller @ 2009-08-17  1:31 UTC (permalink / raw)
  To: jens.axboe; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

From: Jens Axboe <jens.axboe@oracle.com>
Date: Thu, 6 Aug 2009 09:02:42 +0200

> On Sun, Aug 02 2009, David Miller wrote:
>> From: Jens Axboe <jens.axboe@oracle.com>
>> Date: Wed, 29 Jul 2009 13:25:10 +0200
>> 
>> > -#define NR_SYSCALLS		327
>> > +#define NR_SYSCALLS		328
>> 
>> When you increase this value, you have to add entries to all of the
>> syscall tables.  The syscall dispatch checks against this as a limit,
>> so if you don't explicitly add an entry to all the tables, it's
>> possible to deref garbage past the end of the table and try to jump to
>> it as a syscall.
> 
> Oops, missed the 32-bit table. Does the below look better?

You got the native 32-bit table, and the native 64-bit table,
but you missed sparc64's 32-bit compat syscall table :-)

There are two syscall tables in arch/sparc/systbls.S, you only
got the native 64-bit one.

Please fix this up, and the patch is fine with that cured,
so make it a formal submission with commit message and signoffs
thanks!

> I'm assuming that this will happen automatically at some point, instead
> of perf defaulting to building with the same 32/64-bit of the kernel.

Right, it looks like Ingo and co. are going to take care of that.

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

* Re: [PATCH] basic perf support for sparc
  2009-08-17  1:31     ` David Miller
@ 2009-08-17  6:48       ` Jens Axboe
  2009-08-17  7:57         ` Ingo Molnar
  2009-09-04  4:37         ` David Miller
  0 siblings, 2 replies; 41+ messages in thread
From: Jens Axboe @ 2009-08-17  6:48 UTC (permalink / raw)
  To: David Miller; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

On Sun, Aug 16 2009, David Miller wrote:
> From: Jens Axboe <jens.axboe@oracle.com>
> Date: Thu, 6 Aug 2009 09:02:42 +0200
> 
> > On Sun, Aug 02 2009, David Miller wrote:
> >> From: Jens Axboe <jens.axboe@oracle.com>
> >> Date: Wed, 29 Jul 2009 13:25:10 +0200
> >> 
> >> > -#define NR_SYSCALLS		327
> >> > +#define NR_SYSCALLS		328
> >> 
> >> When you increase this value, you have to add entries to all of the
> >> syscall tables.  The syscall dispatch checks against this as a limit,
> >> so if you don't explicitly add an entry to all the tables, it's
> >> possible to deref garbage past the end of the table and try to jump to
> >> it as a syscall.
> > 
> > Oops, missed the 32-bit table. Does the below look better?
> 
> You got the native 32-bit table, and the native 64-bit table,
> but you missed sparc64's 32-bit compat syscall table :-)
> 
> There are two syscall tables in arch/sparc/systbls.S, you only
> got the native 64-bit one.
> 
> Please fix this up, and the patch is fine with that cured,
> so make it a formal submission with commit message and signoffs
> thanks!

Gotcha, I missed the compat table. So below is a full patch, signed off
and everything. It also adds the select HAVE_PERF_COUNTERS for sparc32,
but it's only tested on sparc64.

> > I'm assuming that this will happen automatically at some point, instead
> > of perf defaulting to building with the same 32/64-bit of the kernel.
> 
> Right, it looks like Ingo and co. are going to take care of that.

It's already much better, I can just type make and it'll work. Still
builds 64-bit, but at least it only requires zlib and elf which I
already got fixed.

commit 4c66ea6871ad059b35e35e915fd7910724dc0bd6
Author: Jens Axboe <jens.axboe@oracle.com>
Date:   Mon Aug 17 08:40:50 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 <jens.axboe@oracle.com>

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/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 6b3ee88..01e4849 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


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

* Re: [PATCH] basic perf support for sparc
  2009-08-17  6:48       ` Jens Axboe
@ 2009-08-17  7:57         ` Ingo Molnar
  2009-09-04  4:37         ` David Miller
  1 sibling, 0 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-08-17  7:57 UTC (permalink / raw)
  To: Jens Axboe; +Cc: David Miller, linux-kernel, a.p.zijlstra, fweisbec


* Jens Axboe <jens.axboe@oracle.com> wrote:

> On Sun, Aug 16 2009, David Miller wrote:
>
> > > I'm assuming that this will happen automatically at some point, instead
> > > of perf defaulting to building with the same 32/64-bit of the kernel.
> > 
> > Right, it looks like Ingo and co. are going to take care of that.
> 
> It's already much better, I can just type make and it'll work. 
> Still builds 64-bit, but at least it only requires zlib and elf 
> which I already got fixed.

Yes, those fixes/enhancements are already in -rc6 so if you base it 
on that or later kernels it should be fine out of box.

	Ingo

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

* Re: [PATCH] basic perf support for sparc
  2009-08-17  6:48       ` Jens Axboe
  2009-08-17  7:57         ` Ingo Molnar
@ 2009-09-04  4:37         ` David Miller
  2009-09-04  5:02           ` Ingo Molnar
  1 sibling, 1 reply; 41+ messages in thread
From: David Miller @ 2009-09-04  4:37 UTC (permalink / raw)
  To: jens.axboe; +Cc: linux-kernel, a.p.zijlstra, fweisbec, mingo

From: Jens Axboe <jens.axboe@oracle.com>
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 <jens.axboe@oracle.com>

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?

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

* Re: [PATCH] basic perf support for sparc
  2009-09-04  4:37         ` David Miller
@ 2009-09-04  5:02           ` Ingo Molnar
  2009-09-04  5:09             ` David Miller
  0 siblings, 1 reply; 41+ messages in thread
From: Ingo Molnar @ 2009-09-04  5:02 UTC (permalink / raw)
  To: David Miller; +Cc: jens.axboe, linux-kernel, a.p.zijlstra, fweisbec


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

> From: Jens Axboe <jens.axboe@oracle.com>
> 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 <jens.axboe@oracle.com>
> 
> 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

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

* Re: [PATCH] basic perf support for sparc
  2009-09-04  5:02           ` Ingo Molnar
@ 2009-09-04  5:09             ` David Miller
  2009-09-04  5:20               ` Jens Axboe
  0 siblings, 1 reply; 41+ messages in thread
From: David Miller @ 2009-09-04  5:09 UTC (permalink / raw)
  To: mingo; +Cc: jens.axboe, linux-kernel, a.p.zijlstra, fweisbec

From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 4 Sep 2009 07:02:56 +0200

> 
> * David Miller <davem@davemloft.net> 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?

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

* Re: [PATCH] basic perf support for sparc
  2009-09-04  5:09             ` David Miller
@ 2009-09-04  5:20               ` Jens Axboe
  2009-09-04  6:34                 ` Jens Axboe
  0 siblings, 1 reply; 41+ messages in thread
From: Jens Axboe @ 2009-09-04  5:20 UTC (permalink / raw)
  To: David Miller; +Cc: mingo, linux-kernel, a.p.zijlstra, fweisbec

On Thu, Sep 03 2009, David Miller wrote:
> From: Ingo Molnar <mingo@elte.hu>
> Date: Fri, 4 Sep 2009 07:02:56 +0200
> 
> > 
> > * David Miller <davem@davemloft.net> 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


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

* Re: [PATCH] basic perf support for sparc
  2009-09-04  5:20               ` Jens Axboe
@ 2009-09-04  6:34                 ` Jens Axboe
  2009-09-04  6:44                   ` Ingo Molnar
  2009-09-04  9:57                   ` David Miller
  0 siblings, 2 replies; 41+ messages in thread
From: Jens Axboe @ 2009-09-04  6:34 UTC (permalink / raw)
  To: David Miller; +Cc: mingo, linux-kernel, a.p.zijlstra, fweisbec

On Fri, Sep 04 2009, Jens Axboe wrote:
> On Thu, Sep 03 2009, David Miller wrote:
> > From: Ingo Molnar <mingo@elte.hu>
> > Date: Fri, 4 Sep 2009 07:02:56 +0200
> > 
> > > 
> > > * David Miller <davem@davemloft.net> 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 <jens.axboe@oracle.com>
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 <jens.axboe@oracle.com>

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


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

* Re: [PATCH] basic perf support for sparc
  2009-09-04  6:34                 ` Jens Axboe
@ 2009-09-04  6:44                   ` Ingo Molnar
  2009-09-04  9:57                   ` David Miller
  1 sibling, 0 replies; 41+ messages in thread
From: Ingo Molnar @ 2009-09-04  6:44 UTC (permalink / raw)
  To: Jens Axboe; +Cc: David Miller, linux-kernel, a.p.zijlstra, fweisbec


* Jens Axboe <jens.axboe@oracle.com> wrote:

> On Fri, Sep 04 2009, Jens Axboe wrote:
> > On Thu, Sep 03 2009, David Miller wrote:
> > > From: Ingo Molnar <mingo@elte.hu>
> > > Date: Fri, 4 Sep 2009 07:02:56 +0200
> > > 
> > > > 
> > > > * David Miller <davem@davemloft.net> 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

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

* Re: [PATCH] basic perf support for sparc
  2009-09-04  6:34                 ` Jens Axboe
  2009-09-04  6:44                   ` Ingo Molnar
@ 2009-09-04  9:57                   ` David Miller
  1 sibling, 0 replies; 41+ messages in thread
From: David Miller @ 2009-09-04  9:57 UTC (permalink / raw)
  To: jens.axboe; +Cc: mingo, linux-kernel, a.p.zijlstra, fweisbec

From: Jens Axboe <jens.axboe@oracle.com>
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 <jens.axboe@oracle.com>

Applied to sparc-next-2.6, thanks!

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

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

end of thread, other threads:[~2009-09-04  9:56 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-29 11:25 [PATCH] basic perf support for sparc Jens Axboe
2009-07-29 19:28 ` Jens Axboe
2009-08-01  1:14   ` Anton Blanchard
2009-08-01  8:20     ` Jens Axboe
2009-08-01 18:22       ` Arnaldo Carvalho de Melo
2009-08-02 18:41         ` Ingo Molnar
2009-08-02 19:44           ` Kyle McMartin
2009-08-02 19:50             ` Ingo Molnar
2009-08-02 20:11               ` Kyle McMartin
2009-08-02 20:33                 ` Ingo Molnar
2009-08-02 20:47                 ` Arnaldo Carvalho de Melo
2009-08-03  1:54                   ` Arnaldo Carvalho de Melo
2009-08-04  3:33                     ` Kyle McMartin
2009-08-04  9:25                     ` Ingo Molnar
2009-08-04  9:29                       ` Peter Zijlstra
2009-08-04 13:02                         ` David Miller
2009-08-04 10:32                       ` Frederic Riss
2009-08-04 10:38                         ` Peter Zijlstra
2009-08-04 11:23                           ` Frederic Riss
2009-08-04 11:28                       ` Ingo Molnar
2009-08-05 12:10             ` Peter Zijlstra
2009-08-05 12:21               ` Jens Axboe
2009-08-05 12:33                 ` Ingo Molnar
2009-08-05 12:16       ` [tip:perfcounters/urgent] perf: Auto-detect libbfd tip-bot for Peter Zijlstra
2009-08-05 14:29         ` Peter Zijlstra
2009-08-05 18:58           ` Peter Zijlstra
2009-08-02 20:17 ` [PATCH] basic perf support for sparc David Miller
2009-08-02 20:25   ` Ingo Molnar
2009-08-06  7:02   ` Jens Axboe
2009-08-12 18:06     ` David Miller
2009-08-12 18:13       ` Jens Axboe
2009-08-17  1:31     ` David Miller
2009-08-17  6:48       ` Jens Axboe
2009-08-17  7:57         ` Ingo Molnar
2009-09-04  4:37         ` David Miller
2009-09-04  5:02           ` Ingo Molnar
2009-09-04  5:09             ` David Miller
2009-09-04  5:20               ` Jens Axboe
2009-09-04  6:34                 ` Jens Axboe
2009-09-04  6:44                   ` Ingo Molnar
2009-09-04  9:57                   ` David Miller

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