[PATCH 1/2] perf symbols: Fix inverted logic for showing kallsyms as the source of symbols

[PATCH 1/2] perf symbols: Fix inverted logic for showing kallsyms as the source of symbols

[Arnaldo Carvalho de Melo]

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

Only if we parsed /proc/kallsyms (or a copy found in the buildid cache)
we should set the dso long name to "[kernel.kallsyms]".

Reported-by: Mike Galbraith <efault@gmx.de>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 6f30fe1..1270cf8 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1671,7 +1671,7 @@ do_kallsyms:
 out_try_fixup:
 	if (err > 0) {
 out_fixup:
-		if (kallsyms_filename == NULL)
+		if (kallsyms_filename != NULL)
 			dso__set_long_name(self, strdup("[kernel.kallsyms]"));
 		map__fixup_start(map);
 		map__fixup_end(map);
-- 
1.6.2.5

[PATCH 2/2] perf symbols: Use the right variable to check for kallsyms in the cache

[Arnaldo Carvalho de Melo]

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

Probably this wasn't noticed when testing this on my parisc machine
because I must have copied manually to its cache the vmlinux file used
in the x86_64 machine, now that I tried looking on a x86-32 machine with
a fresh cache, kernel symbols weren't being resolved even with the right
kallsyms copy on its cache, duh.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 1270cf8..f1f609d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1650,12 +1650,12 @@ static int dso__load_kernel_sym(struct dso *self, struct map *map,
 			     getenv("HOME"), sbuild_id) == -1)
 			return -1;
 
+		kallsyms_filename = kallsyms_allocated_filename;
+
 		if (access(kallsyms_filename, F_OK)) {
 			free(kallsyms_allocated_filename);
 			return -1;
 		}
-
-		kallsyms_filename = kallsyms_allocated_filename;
 	} else {
 		/*
 		 * Last resort, if we don't have a build-id and couldn't find
-- 
1.6.2.5

Re: [PATCH 1/2] perf symbols: Fix inverted logic for showing kallsyms as the source of symbols

[Mike Galbraith]

On Fri, 2010-01-22 at 14:35 -0200, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Only if we parsed /proc/kallsyms (or a copy found in the buildid cache)
> we should set the dso long name to "[kernel.kallsyms]".

Yup, works just as well as my busted one :)

	-Mike

perf regression testing. was Re: [PATCH 1/2] perf symbols: Fix inverted logic for showing kallsyms as the source of symbols

[Arnaldo Carvalho de Melo]

Em Fri, Jan 22, 2010 at 05:43:28PM +0100, Mike Galbraith escreveu:
> On Fri, 2010-01-22 at 14:35 -0200, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > Only if we parsed /proc/kallsyms (or a copy found in the buildid cache)
> > we should set the dso long name to "[kernel.kallsyms]".
> 
> Yup, works just as well as my busted one :)

yeah :-)

I'll soon start working on 'perf regtest', to start accumulating
regression tests a la 'perf bench', as these silly bugs are worrying
me...

Some will require more than one machine, and of different archs, so that
we can properly test 'perf archive' and the buildid machinery, etc. Some
need to compare what we get from kallsyms with what we get from vmlinux
of same buildid, etc.

We'll use this when developing new tools or changing the core code, and
a subset of it can also be used by users to sanity test their
environment.

If not I'll have to continue relying on you as my regression test ;-)

- Arnaldo

Re: perf regression testing. was Re: [PATCH 1/2] perf symbols: Fix inverted logic for showing kallsyms as the source of symbols

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Em Fri, Jan 22, 2010 at 05:43:28PM +0100, Mike Galbraith escreveu:
> > On Fri, 2010-01-22 at 14:35 -0200, Arnaldo Carvalho de Melo wrote:
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > 
> > > Only if we parsed /proc/kallsyms (or a copy found in the buildid cache)
> > > we should set the dso long name to "[kernel.kallsyms]".
> > 
> > Yup, works just as well as my busted one :)
> 
> yeah :-)
> 
> I'll soon start working on 'perf regtest', to start accumulating regression 
> tests a la 'perf bench', as these silly bugs are worrying me...

Excellent, it's much needed!

The best example i know about is KVM: they have built an impressive regression 
test suite and add testcases for new regressions all the time.

The key is to have something simple to run, which developers (and 
maintainers) can run after every new perf commit or so.

	Ingo

Re: perf regression testing. was Re: [PATCH 1/2] perf symbols: Fix inverted logic for showing kallsyms as the source of symbols

[Arnaldo Carvalho de Melo]

Em Sat, Jan 23, 2010 at 08:04:51AM +0100, Ingo Molnar escreveu:
> 
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> 
> > Em Fri, Jan 22, 2010 at 05:43:28PM +0100, Mike Galbraith escreveu:
> > > On Fri, 2010-01-22 at 14:35 -0200, Arnaldo Carvalho de Melo wrote:
> > > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > > 
> > > > Only if we parsed /proc/kallsyms (or a copy found in the buildid cache)
> > > > we should set the dso long name to "[kernel.kallsyms]".
> > > 
> > > Yup, works just as well as my busted one :)
> > 
> > yeah :-)
> > 
> > I'll soon start working on 'perf regtest', to start accumulating regression 
> > tests a la 'perf bench', as these silly bugs are worrying me...
> 
> Excellent, it's much needed!
> 
> The best example i know about is KVM: they have built an impressive regression 
> test suite and add testcases for new regressions all the time.
> 
> The key is to have something simple to run, which developers (and 
> maintainers) can run after every new perf commit or so.

Yes, that is the idea, I'm finishing the first test (/proc/kallsyms
versus what we get from a auto-located vmlinux symtab) and will submit
it soon:

[acme@doppio linux-2.6-tip]$ perf regtest
 1: vmlinux symtab matches kallsyms... 
Segmentation fault
[acme@doppio linux-2.6-tip]$

Not quite working yet, tho :o)

- Arnaldo