[GIT PULL 0/2] perf annotate fix and report improvoment

[GIT PULL 0/2] perf annotate fix and report improvoment

[Arnaldo Carvalho de Melo]

Hi Ingo,

        Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf

Regards,

- Arnaldo

Arnaldo Carvalho de Melo (2):
  perf report: Support multiple events on the TUI
  perf annotate: Fix up usage of the build id cache

 tools/perf/builtin-report.c |   60 +++++++++++++++++++++--------------
 tools/perf/util/hist.c      |   13 ++++++-
 tools/perf/util/hist.h      |   14 +++++++-
 tools/perf/util/newt.c      |   74 ++++++++++++++++++++++++++++++++++--------
 4 files changed, 119 insertions(+), 42 deletions(-)

[PATCH 1/2] perf report: Support multiple events on the TUI

[Arnaldo Carvalho de Melo]

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

The hists__tty_browse_tree function was created with the loop to print
all events, and its equivalent, hists__tui_browse_tree, was created in a
similar fashion, where it is possible to switch among the multiple
events, if present, using TAB to go the next event, and shift+TAB
(UNTAB) to go to the previous.

The report TUI now shows as the window title the name of the event and a
leak was fixed wrt pstacks.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c |   60 +++++++++++++++++++++--------------
 tools/perf/util/hist.h      |   14 +++++++-
 tools/perf/util/newt.c      |   74 ++++++++++++++++++++++++++++++++++--------
 3 files changed, 108 insertions(+), 40 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index a7b8760..3ff4e66 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -288,6 +288,38 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
 	return ret + fprintf(fp, "\n#\n");
 }
 
+static int hists__tty_browse_tree(struct rb_root *tree, const char *help)
+{
+	struct rb_node *next = rb_first(tree);
+
+	while (next) {
+		struct hists *hists = rb_entry(next, struct hists, rb_node);
+		const char *evname = NULL;
+
+		if (rb_first(&hists->entries) != rb_last(&hists->entries))
+			evname = __event_name(hists->type, hists->config);
+
+		hists__fprintf_nr_sample_events(hists, evname, stdout);
+		hists__fprintf(hists, NULL, false, stdout);
+		fprintf(stdout, "\n\n");
+		next = rb_next(&hists->rb_node);
+	}
+
+	if (sort_order == default_sort_order &&
+	    parent_pattern == default_parent_pattern) {
+		fprintf(stdout, "#\n# (%s)\n#\n", help);
+
+		if (show_threads) {
+			bool style = !strcmp(pretty_printing_style, "raw");
+			perf_read_values_display(stdout, &show_threads_values,
+						 style);
+			perf_read_values_destroy(&show_threads_values);
+		}
+	}
+
+	return 0;
+}
+
 static int __cmd_report(void)
 {
 	int ret = -EINVAL;
@@ -330,34 +362,14 @@ static int __cmd_report(void)
 		hists = rb_entry(next, struct hists, rb_node);
 		hists__collapse_resort(hists);
 		hists__output_resort(hists);
-		if (use_browser > 0)
-			hists__browse(hists, help, input_name);
-		else {
-			const char *evname = NULL;
-			if (rb_first(&session->hists.entries) !=
-			    rb_last(&session->hists.entries))
-				evname = __event_name(hists->type, hists->config);
-
-			hists__fprintf_nr_sample_events(hists, evname, stdout);
-
-			hists__fprintf(hists, NULL, false, stdout);
-			fprintf(stdout, "\n\n");
-		}
-
 		next = rb_next(&hists->rb_node);
 	}
 
-	if (use_browser <= 0 && sort_order == default_sort_order &&
-	    parent_pattern == default_parent_pattern) {
-		fprintf(stdout, "#\n# (%s)\n#\n", help);
+	if (use_browser)
+		hists__tui_browse_tree(&session->hists_tree, help);
+	else
+		hists__tty_browse_tree(&session->hists_tree, help);
 
-		if (show_threads) {
-			bool style = !strcmp(pretty_printing_style, "raw");
-			perf_read_values_display(stdout, &show_threads_values,
-						 style);
-			perf_read_values_destroy(&show_threads_values);
-		}
-	}
 out_delete:
 	perf_session__delete(session);
 	return ret;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2d5203f..83fa33a 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -98,10 +98,17 @@ void hists__filter_by_thread(struct hists *self, const struct thread *thread);
 #ifdef NO_NEWT_SUPPORT
 static inline int hists__browse(struct hists *self __used,
 				const char *helpline __used,
-				const char *input_name __used)
+				const char *ev_name __used)
 {
 	return 0;
 }
+
+static inline int hists__tui_browse_tree(struct rb_root *self __used,
+					 const char *help __used)
+{
+	return 0;
+}
+
 static inline int hist_entry__tui_annotate(struct hist_entry *self __used)
 {
 	return 0;
@@ -111,9 +118,12 @@ static inline int hist_entry__tui_annotate(struct hist_entry *self __used)
 #else
 #include <newt.h>
 int hists__browse(struct hists *self, const char *helpline,
-		  const char *input_name);
+		  const char *ev_name);
 int hist_entry__tui_annotate(struct hist_entry *self);
+
 #define KEY_LEFT NEWT_KEY_LEFT
 #define KEY_RIGHT NEWT_KEY_RIGHT
+
+int hists__tui_browse_tree(struct rb_root *self, const char *help);
 #endif
 #endif	/* __PERF_HIST_H */
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index ffd0472..d54c540 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -842,6 +842,8 @@ static int hist_browser__populate(struct hist_browser *self, struct hists *hists
 	newtFormAddHotKey(self->form, 'h');
 	newtFormAddHotKey(self->form, NEWT_KEY_F1);
 	newtFormAddHotKey(self->form, NEWT_KEY_RIGHT);
+	newtFormAddHotKey(self->form, NEWT_KEY_TAB);
+	newtFormAddHotKey(self->form, NEWT_KEY_UNTAB);
 	newtFormAddComponents(self->form, self->tree, NULL);
 	self->selection = newt__symbol_tree_get_current(self->tree);
 
@@ -873,7 +875,7 @@ static struct thread *hist_browser__selected_thread(struct hist_browser *self)
 	return he ? he->thread : NULL;
 }
 
-static int hist_browser__title(char *bf, size_t size, const char *input_name,
+static int hist_browser__title(char *bf, size_t size, const char *ev_name,
 			       const struct dso *dso, const struct thread *thread)
 {
 	int printed = 0;
@@ -887,18 +889,18 @@ static int hist_browser__title(char *bf, size_t size, const char *input_name,
 		printed += snprintf(bf + printed, size - printed,
 				    "%sDSO: %s", thread ? " " : "",
 				    dso->short_name);
-	return printed ?: snprintf(bf, size, "Report: %s", input_name);
+	return printed ?: snprintf(bf, size, "Event: %s", ev_name);
 }
 
-int hists__browse(struct hists *self, const char *helpline, const char *input_name)
+int hists__browse(struct hists *self, const char *helpline, const char *ev_name)
 {
 	struct hist_browser *browser = hist_browser__new();
-	struct pstack *fstack = pstack__new(2);
+	struct pstack *fstack;
 	const struct thread *thread_filter = NULL;
 	const struct dso *dso_filter = NULL;
 	struct newtExitStruct es;
 	char msg[160];
-	int err = -1;
+	int key = -1;
 
 	if (browser == NULL)
 		return -1;
@@ -909,7 +911,7 @@ int hists__browse(struct hists *self, const char *helpline, const char *input_na
 
 	ui_helpline__push(helpline);
 
-	hist_browser__title(msg, sizeof(msg), input_name,
+	hist_browser__title(msg, sizeof(msg), ev_name,
 			    dso_filter, thread_filter);
 	if (hist_browser__populate(browser, self, msg) < 0)
 		goto out_free_stack;
@@ -927,10 +929,23 @@ int hists__browse(struct hists *self, const char *helpline, const char *input_na
 		dso = browser->selection->map ? browser->selection->map->dso : NULL;
 
 		if (es.reason == NEWT_EXIT_HOTKEY) {
-			if (es.u.key == NEWT_KEY_F1)
+			key = es.u.key;
+
+			switch (key) {
+			case NEWT_KEY_F1:
 				goto do_help;
+			case NEWT_KEY_TAB:
+			case NEWT_KEY_UNTAB:
+				/*
+				 * Exit the browser, let hists__browser_tree
+				 * go to the next or previous
+				 */
+				goto out_free_stack;
+			default:;
+			}
 
-			switch (toupper(es.u.key)) {
+			key = toupper(key);
+			switch (key) {
 			case 'A':
 				if (browser->selection->map == NULL &&
 				    browser->selection->map->dso->annotate_warned)
@@ -953,8 +968,8 @@ do_help:
 				continue;
 			default:;
 			}
-			if (is_exit_key(es.u.key)) {
-				if (es.u.key == NEWT_KEY_ESCAPE) {
+			if (is_exit_key(key)) {
+				if (key == NEWT_KEY_ESCAPE) {
 					if (dialog_yesno("Do you really want to exit?"))
 						break;
 					else
@@ -1041,7 +1056,7 @@ zoom_out_dso:
 				pstack__push(fstack, &dso_filter);
 			}
 			hists__filter_by_dso(self, dso_filter);
-			hist_browser__title(msg, sizeof(msg), input_name,
+			hist_browser__title(msg, sizeof(msg), ev_name,
 					    dso_filter, thread_filter);
 			if (hist_browser__populate(browser, self, msg) < 0)
 				goto out;
@@ -1060,18 +1075,49 @@ zoom_out_thread:
 				pstack__push(fstack, &thread_filter);
 			}
 			hists__filter_by_thread(self, thread_filter);
-			hist_browser__title(msg, sizeof(msg), input_name,
+			hist_browser__title(msg, sizeof(msg), ev_name,
 					    dso_filter, thread_filter);
 			if (hist_browser__populate(browser, self, msg) < 0)
 				goto out;
 		}
 	}
-	err = 0;
 out_free_stack:
 	pstack__delete(fstack);
 out:
 	hist_browser__delete(browser);
-	return err;
+	return key;
+}
+
+int hists__tui_browse_tree(struct rb_root *self, const char *help)
+{
+	struct rb_node *first = rb_first(self), *nd = first, *next;
+	int key = 0;
+
+	while (nd) {
+		struct hists *hists = rb_entry(nd, struct hists, rb_node);
+		const char *ev_name = __event_name(hists->type, hists->config);
+
+		key = hists__browse(hists, help, ev_name);
+
+		if (is_exit_key(key))
+			break;
+
+		switch (key) {
+		case NEWT_KEY_TAB:
+			next = rb_next(nd);
+			if (next)
+				nd = next;
+			break;
+		case NEWT_KEY_UNTAB:
+			if (nd == first)
+				continue;
+			nd = rb_prev(nd);
+		default:
+			break;
+		}
+	}
+
+	return key;
 }
 
 static struct newtPercentTreeColors {
-- 
1.6.2.5

[PATCH 2/2] perf annotate: Fix up usage of the build id cache

[Arnaldo Carvalho de Melo]

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

It was assuming that the cache was always available and also wasn't
checking if the file found in the build id cache was just a kallsyms
file, that is not supported by objdump for disassembly.

Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 682a6d8..cbf7eae 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -990,6 +990,7 @@ int hist_entry__annotate(struct hist_entry *self, struct list_head *head)
 	struct map *map = self->ms.map;
 	struct dso *dso = map->dso;
 	char *filename = dso__build_id_filename(dso, NULL, 0);
+	bool free_filename = true;
 	char command[PATH_MAX * 2];
 	FILE *file;
 	int err = 0;
@@ -1001,11 +1002,19 @@ int hist_entry__annotate(struct hist_entry *self, struct list_head *head)
 			       sym->name);
 			return -ENOMEM;
 		}
+		goto fallback;
+	} else if (readlink(filename, command, sizeof(command)) < 0 ||
+		   strstr(command, "[kernel.kallsyms]") ||
+		   access(filename, R_OK)) {
+		free(filename);
+fallback:
 		/*
-		 * If we don't have build-ids, well, lets hope that this
+		 * If we don't have build-ids or the build-id file isn't in the
+		 * cache, or is just a kallsyms file, well, lets hope that this
 		 * DSO is the same as when 'perf record' ran.
 		 */
 		filename = dso->long_name;
+		free_filename = false;
 	}
 
 	if (dso->origin == DSO__ORIG_KERNEL) {
@@ -1045,7 +1054,7 @@ int hist_entry__annotate(struct hist_entry *self, struct list_head *head)
 
 	pclose(file);
 out_free_filename:
-	if (dso->has_build_id)
+	if (free_filename)
 		free(filename);
 	return err;
 }
-- 
1.6.2.5

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Ingo Molnar]

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

> Hi Ingo,
> 
>         Please pull from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf
> 
> Regards,
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (2):
>   perf report: Support multiple events on the TUI
>   perf annotate: Fix up usage of the build id cache
> 
>  tools/perf/builtin-report.c |   60 +++++++++++++++++++++--------------
>  tools/perf/util/hist.c      |   13 ++++++-
>  tools/perf/util/hist.h      |   14 +++++++-
>  tools/perf/util/newt.c      |   74 ++++++++++++++++++++++++++++++++++--------
>  4 files changed, 119 insertions(+), 42 deletions(-)

Pulled, thanks Arnaldo!

	Ingo

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Stephane Eranian]

On Mon, May 24, 2010 at 3:31 AM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Hi Ingo,
>
>        Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf
>
> Regards,
>
> - Arnaldo
>
> Arnaldo Carvalho de Melo (2):
>  perf report: Support multiple events on the TUI
>  perf annotate: Fix up usage of the build id cache
>
With these patches, can I analyze a binary on a remote
machine? If I copy perf.data + .debug subdir, then can
I run perf annotate on a different machine, where I don't
have the sampled binaries installed?

I have tried that today using -tip, and perf annotate still
looked for the binary using its original pathname in perf.data.
perf report worked fine.

Thanks.

>  tools/perf/builtin-report.c |   60 +++++++++++++++++++++--------------
>  tools/perf/util/hist.c      |   13 ++++++-
>  tools/perf/util/hist.h      |   14 +++++++-
>  tools/perf/util/newt.c      |   74 ++++++++++++++++++++++++++++++++++--------
>  4 files changed, 119 insertions(+), 42 deletions(-)
>
>



-- 
Stephane Eranian  | EMEA Software Engineering
Google France | 38 avenue de l'Opéra | 75002 Paris
Tel : +33 (0) 1 42 68 53 00
This email may be confidential or privileged. If you received this
communication by mistake, please
don't forward it to anyone else, please erase all copies and
attachments, and please let me know that
it went to the wrong person. Thanks

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Arnaldo Carvalho de Melo]

Em Tue, May 25, 2010 at 11:08:29PM +0200, Stephane Eranian escreveu:
> On Mon, May 24, 2010 at 3:31 AM, Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > Arnaldo Carvalho de Melo (2):
> >  perf report: Support multiple events on the TUI
> >  perf annotate: Fix up usage of the build id cache

> With these patches, can I analyze a binary on a remote
> machine? If I copy perf.data + .debug subdir, then can
> I run perf annotate on a different machine, where I don't
> have the sampled binaries installed?

It should, either directly thru 'perf annotate foo_symbol' or via the
TUI 'perf report' browser.
 
> I have tried that today using -tip, and perf annotate still
> looked for the binary using its original pathname in perf.data.
> perf report worked fine.

Can you please try it with -vvvv and make the output available
somewhere?

- Arnaldo

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Stephane Eranian]

Hi,

+lkml et al.

On Wed, May 26, 2010 at 8:23 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Wed, May 26, 2010 at 06:45:18PM +0200, Stephane Eranian escreveu:
>> Attached is the trace for
>> perf annotate -i ~/perf.data noploop
>>
>> I get no output at all (TUI is off). Copied over .debug tarball + perf.data.
>
> Looks like the bug I'm investigating now:
>
> build id event received for [kernel.kallsyms]: 54b1e7cc3cf52e0db255aab44ce7538eb62655b8
> build id event received for /tmp/noploop: 875ae61623e89f408b425ca0486a9ec99e3ac73e
> Looking at the vmlinux_path (5 entries long)
> No build_id in vmlinux, ignoring it
> No build_id in /boot/vmlinux, ignoring it
> No build_id in /boot/vmlinux-2.6.34-tip-default+, ignoring it
> build_id in /lib/modules/2.6.34-tip-default+/build/vmlinux is
> 3635a0e8dfc9a1afbd5627c2ba789e41d77d3cd1 while expected is
> 54b1e7cc3cf52e0db255aab44ce7538eb62655b8, ignoring it
> No build_id in /usr/lib/debug/lib/modules/2.6.34-tip-default+/vmlinux, ignoring it
>
> In my case it is not finding the vmlinux because I'm using RHEL6 Beta
> kernel without the respective kernel-debuginfo{-common} packages so it
> doesn't find the vmlinux, uses just /proc/kallsyms and that is not
> enough for annotation.
>
> In your case the problem is that we only cache the kallsyms file in the
> build-id cache ($HOME/.debug) and that is not enough for annotation, so
> I have to fix two things:
>
I can understand the issue with the kernel symbols.
But in this example, I only really care about the symbols in the
noploop program (/tmp/noploop).

Missing symbol support for the kernel should not cause perf to avoid
trying to resolve the symbols in other modules such as my user program
here.

> 1. tell about these constraints when a symbol cannot be annotated, not
> just silently fail
>
Agreed, print something like <uknown symbol> or <cannot resolve symbol>


> 2. cache the vmlinux file in the build-id cache instead of the vmlinux,
> if the vmlinux file was found, if not, fallback to caching the kallsyms
> file.
>
Don't you need to match buildid here too, the vmlinux linux on the host
may not match the one on the target, i.e., monitored system.

> Both can't be cached at the same time as they will have the same
> build-id and thus at least the symlink would be to one of them.
>
> I'll make this even configurable in ~/.perfconfig, on a new [symbol]
> section, something like:
>
> [symbol]
>
>        cache_vmlinux = on/off
>
> So that people that have issues with the size of these beasts can trade
> off disk space used by the cache versus being able to do annotation.
>
Yes, disk space is an issue.

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Arnaldo Carvalho de Melo]

Em Wed, May 26, 2010 at 09:07:05PM +0200, Stephane Eranian escreveu:
> Hi,
> 
> +lkml et al.
> 
> On Wed, May 26, 2010 at 8:23 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > Em Wed, May 26, 2010 at 06:45:18PM +0200, Stephane Eranian escreveu:
> >> Attached is the trace for
> >> perf annotate -i ~/perf.data noploop
> >>
> >> I get no output at all (TUI is off). Copied over .debug tarball + perf.data.
> >
> > Looks like the bug I'm investigating now:
> >
> > build id event received for [kernel.kallsyms]: 54b1e7cc3cf52e0db255aab44ce7538eb62655b8
> > build id event received for /tmp/noploop: 875ae61623e89f408b425ca0486a9ec99e3ac73e
> > Looking at the vmlinux_path (5 entries long)
> > No build_id in vmlinux, ignoring it
> > No build_id in /boot/vmlinux, ignoring it
> > No build_id in /boot/vmlinux-2.6.34-tip-default+, ignoring it
> > build_id in /lib/modules/2.6.34-tip-default+/build/vmlinux is
> > 3635a0e8dfc9a1afbd5627c2ba789e41d77d3cd1 while expected is
> > 54b1e7cc3cf52e0db255aab44ce7538eb62655b8, ignoring it
> > No build_id in /usr/lib/debug/lib/modules/2.6.34-tip-default+/vmlinux, ignoring it
> >
> > In my case it is not finding the vmlinux because I'm using RHEL6 Beta
> > kernel without the respective kernel-debuginfo{-common} packages so it
> > doesn't find the vmlinux, uses just /proc/kallsyms and that is not
> > enough for annotation.
> >
> > In your case the problem is that we only cache the kallsyms file in the
> > build-id cache ($HOME/.debug) and that is not enough for annotation, so
> > I have to fix two things:
> >
> I can understand the issue with the kernel symbols.

That has to be fixed and I've got some patches already for that, testing
them now.

> But in this example, I only really care about the symbols in the
> noploop program (/tmp/noploop).
> 
> Missing symbol support for the kernel should not cause perf to avoid
> trying to resolve the symbols in other modules such as my user program
> here.

Right, my bad, I thought that the problem was about the kernel symbols.

Then can you try replacing:

perf annotate -i ~/perf.data noploop

with:

perf annotate -i ~/perf.data -d noploop

And see if that helps?

Because, in the debug output you sent me we have:

Thread 11487 noploop
Functions:
Map: 400000-401000 0 /tmp/noploop
dso: noploop (/tmp/noploop, Functions, loaded, 875ae61623e89f408b425ca0486a9ec99e3ac73e)
 508-58f _init
 590-5bb _start
 5bc-5df call_gmon_start
 5e0-64f __do_global_dtors_aux
 650-67f frame_dummy
 680-681 noploop
 6e0-6ea handler
 6f0-6f1 __libc_csu_fini
 700-788 __libc_csu_init
 790-7c7 __do_global_ctors_aux
 7c8-1000 _fini

So yes, there is a symbol called noploop and it is in the binary
noploop, that _has_ a build-id, and that is in the cache, perf managed
to load its symtab and knows where it is mapped, when it created this
symbol it did:

symbol__new: noploop 0x680-0x681
dso__load_sym: adjusting symbol: st_value: 0x400690 sh_addr: 0x400590 sh_offset: 0x590

Now with perf annotate -D you should get a raw dump that will tell you
if it managed to find the hist_entry for this symbol.

And I checked and there are no other "noploop" symbol in any of the
other DSOs involved.

> > 1. tell about these constraints when a symbol cannot be annotated, not
> > just silently fail
> >
> Agreed, print something like <uknown symbol> or <cannot resolve symbol>
> 
> 
> > 2. cache the vmlinux file in the build-id cache instead of the vmlinux,
> > if the vmlinux file was found, if not, fallback to caching the kallsyms
> > file.
> >
> Don't you need to match buildid here too, the vmlinux linux on the host
> may not match the one on the target, i.e., monitored system.

Right, it is checked if the build-id is in the perf.data file.
 
> > Both can't be cached at the same time as they will have the same
> > build-id and thus at least the symlink would be to one of them.
> >
> > I'll make this even configurable in ~/.perfconfig, on a new [symbol]
> > section, something like:
> >
> > [symbol]
> >
> >        cache_vmlinux = on/off
> >
> > So that people that have issues with the size of these beasts can trade
> > off disk space used by the cache versus being able to do annotation.
> >
> Yes, disk space is an issue.

- Arnaldo

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Stephane Eranian]

On Wed, May 26, 2010 at 9:32 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Wed, May 26, 2010 at 09:07:05PM +0200, Stephane Eranian escreveu:
>> Hi,
>>
>> +lkml et al.
>>
>> On Wed, May 26, 2010 at 8:23 PM, Arnaldo Carvalho de Melo
>> <acme@infradead.org> wrote:
>> > Em Wed, May 26, 2010 at 06:45:18PM +0200, Stephane Eranian escreveu:
>> >> Attached is the trace for
>> >> perf annotate -i ~/perf.data noploop
>> >>
>> >> I get no output at all (TUI is off). Copied over .debug tarball + perf.data.
>> >
>> > Looks like the bug I'm investigating now:
>> >
>> > build id event received for [kernel.kallsyms]: 54b1e7cc3cf52e0db255aab44ce7538eb62655b8
>> > build id event received for /tmp/noploop: 875ae61623e89f408b425ca0486a9ec99e3ac73e
>> > Looking at the vmlinux_path (5 entries long)
>> > No build_id in vmlinux, ignoring it
>> > No build_id in /boot/vmlinux, ignoring it
>> > No build_id in /boot/vmlinux-2.6.34-tip-default+, ignoring it
>> > build_id in /lib/modules/2.6.34-tip-default+/build/vmlinux is
>> > 3635a0e8dfc9a1afbd5627c2ba789e41d77d3cd1 while expected is
>> > 54b1e7cc3cf52e0db255aab44ce7538eb62655b8, ignoring it
>> > No build_id in /usr/lib/debug/lib/modules/2.6.34-tip-default+/vmlinux, ignoring it
>> >
>> > In my case it is not finding the vmlinux because I'm using RHEL6 Beta
>> > kernel without the respective kernel-debuginfo{-common} packages so it
>> > doesn't find the vmlinux, uses just /proc/kallsyms and that is not
>> > enough for annotation.
>> >
>> > In your case the problem is that we only cache the kallsyms file in the
>> > build-id cache ($HOME/.debug) and that is not enough for annotation, so
>> > I have to fix two things:
>> >
>> I can understand the issue with the kernel symbols.
>
> That has to be fixed and I've got some patches already for that, testing
> them now.
>
>> But in this example, I only really care about the symbols in the
>> noploop program (/tmp/noploop).
>>
>> Missing symbol support for the kernel should not cause perf to avoid
>> trying to resolve the symbols in other modules such as my user program
>> here.
>
> Right, my bad, I thought that the problem was about the kernel symbols.
>
> Then can you try replacing:
>
> perf annotate -i ~/perf.data noploop
>
> with:
>
> perf annotate -i ~/perf.data -d noploop
>
> And see if that helps?
>
Ok that works. But if I turned on TUI, then I cannot obtain
the same result. I am guessing it does not use the -d option.

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Arnaldo Carvalho de Melo]

Em Wed, May 26, 2010 at 10:11:14PM +0200, Stephane Eranian escreveu:
> On Wed, May 26, 2010 at 9:32 PM, Arnaldo Carvalho de Melo
> <acme@infradead.org> wrote:
> > Em Wed, May 26, 2010 at 09:07:05PM +0200, Stephane Eranian escreveu:
> >> But in this example, I only really care about the symbols in the
> >> noploop program (/tmp/noploop).
> >>
> >> Missing symbol support for the kernel should not cause perf to avoid
> >> trying to resolve the symbols in other modules such as my user program
> >> here.
> >
> > Right, my bad, I thought that the problem was about the kernel symbols.
> >
> > Then can you try replacing:
> >
> > perf annotate -i ~/perf.data noploop
> >
> > with:
> >
> > perf annotate -i ~/perf.data -d noploop
> >
> > And see if that helps?
> >
> Ok that works. But if I turned on TUI, then I cannot obtain
> the same result. I am guessing it does not use the -d option.

Humm, it should be working, i.e. -d processing is about adding a filter,
lemme see...

Yeah, in builtin-annotate.c

        OPT_STRING('d', "dsos", &symbol_conf.dso_list_str, "dso[,dso...]",
                   "only consider symbols in these dsos"),

It sets the dso_list_str and that is used when processing events in:

	perf_session__process_events
		process_sample_event (in builtin-annotate.c)
			event__preprocess_sample

In event__preprocess_sample:

     if (symbol_conf.dso_list &&
         (!al->map || !al->map->dso ||
          !(strlist__has_entry(symbol_conf.dso_list,
                               al->map->dso->short_name) ||
            (al->map->dso->short_name != al->map->dso->long_name &&
             strlist__has_entry(symbol_conf.dso_list,
                                al->map->dso->long_name)))))
             goto out_filtered;

Yeah, this is all done no matter what frontend is used :-\

I just tried it here with:

[root@emilia linux-2.6-tip]# perf report -d libc-2.12.so
[root@emilia linux-2.6-tip]# perf annotate -d libc-2.12.so

with

[root@emilia linux-2.6-tip]# cat ~/.perfconfig 
[tui]

	report = on
	annotate = on
[root@emilia linux-2.6-tip]#

And it works as expected.

- Arnaldo

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Stephane Eranian]

On Wed, May 26, 2010 at 10:33 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Wed, May 26, 2010 at 10:11:14PM +0200, Stephane Eranian escreveu:
>> On Wed, May 26, 2010 at 9:32 PM, Arnaldo Carvalho de Melo
>> <acme@infradead.org> wrote:
>> > Em Wed, May 26, 2010 at 09:07:05PM +0200, Stephane Eranian escreveu:
>> >> But in this example, I only really care about the symbols in the
>> >> noploop program (/tmp/noploop).
>> >>
>> >> Missing symbol support for the kernel should not cause perf to avoid
>> >> trying to resolve the symbols in other modules such as my user program
>> >> here.
>> >
>> > Right, my bad, I thought that the problem was about the kernel symbols.
>> >
>> > Then can you try replacing:
>> >
>> > perf annotate -i ~/perf.data noploop
>> >
>> > with:
>> >
>> > perf annotate -i ~/perf.data -d noploop
>> >
>> > And see if that helps?
>> >
>> Ok that works. But if I turned on TUI, then I cannot obtain
>> the same result. I am guessing it does not use the -d option.
>
> Humm, it should be working, i.e. -d processing is about adding a filter,
> lemme see...
>
> Yeah, in builtin-annotate.c
>
>        OPT_STRING('d', "dsos", &symbol_conf.dso_list_str, "dso[,dso...]",
>                   "only consider symbols in these dsos"),
>
> It sets the dso_list_str and that is used when processing events in:
>
>        perf_session__process_events
>                process_sample_event (in builtin-annotate.c)
>                        event__preprocess_sample
>
> In event__preprocess_sample:
>
>     if (symbol_conf.dso_list &&
>         (!al->map || !al->map->dso ||
>          !(strlist__has_entry(symbol_conf.dso_list,
>                               al->map->dso->short_name) ||
>            (al->map->dso->short_name != al->map->dso->long_name &&
>             strlist__has_entry(symbol_conf.dso_list,
>                                al->map->dso->long_name)))))
>             goto out_filtered;
>
> Yeah, this is all done no matter what frontend is used :-\
>
> I just tried it here with:
>
> [root@emilia linux-2.6-tip]# perf report -d libc-2.12.so
> [root@emilia linux-2.6-tip]# perf annotate -d libc-2.12.so
>
> with
>
> [root@emilia linux-2.6-tip]# cat ~/.perfconfig
> [tui]
>
>        report = on
>        annotate = on
> [root@emilia linux-2.6-tip]#
>
> And it works as expected.
>
I did not explain my testcase enough ;-<

With TUI, I started from perf report -i perf.data, then chose to zoom into
the noploop symbol.

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Arnaldo Carvalho de Melo]

Em Wed, May 26, 2010 at 10:40:24PM +0200, Stephane Eranian escreveu:
> On Wed, May 26, 2010 at 10:33 PM, Arnaldo Carvalho de Melo
> > I just tried it here with:
> >
> > [root@emilia linux-2.6-tip]# perf report -d libc-2.12.so
> > [root@emilia linux-2.6-tip]# perf annotate -d libc-2.12.so
> >
> > with
> >
> > [root@emilia linux-2.6-tip]# cat ~/.perfconfig
> > [tui]
> >
> >        report = on
> >        annotate = on
> > [root@emilia linux-2.6-tip]#
> >
> > And it works as expected.

> I did not explain my testcase enough ;-<
> 
> With TUI, I started from perf report -i perf.data, then chose to zoom into
> the noploop symbol.

You can zoom into DSOs and threads, additionally you can Annotate
symbols, so you mean that annotating the 'noploop' in the 'noploop' DSO
didn't work?

Lets try this offline thing ourselves, can you please put the
perf.data.tar.bz2 and the perf.data files somewhere I can grab? So that
I can try to do these operations here and figure out the problem :-)

I guess there is nothing sensitive in those files, lemme know if making
them available is not an option and I'll try other ways to get this
fixed.

- Arnaldo

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Stephane Eranian]

On Wed, May 26, 2010 at 10:50 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
> Em Wed, May 26, 2010 at 10:40:24PM +0200, Stephane Eranian escreveu:
>> On Wed, May 26, 2010 at 10:33 PM, Arnaldo Carvalho de Melo
>> > I just tried it here with:
>> >
>> > [root@emilia linux-2.6-tip]# perf report -d libc-2.12.so
>> > [root@emilia linux-2.6-tip]# perf annotate -d libc-2.12.so
>> >
>> > with
>> >
>> > [root@emilia linux-2.6-tip]# cat ~/.perfconfig
>> > [tui]
>> >
>> >        report = on
>> >        annotate = on
>> > [root@emilia linux-2.6-tip]#
>> >
>> > And it works as expected.
>
>> I did not explain my testcase enough ;-<
>>
>> With TUI, I started from perf report -i perf.data, then chose to zoom into
>> the noploop symbol.
>
> You can zoom into DSOs and threads, additionally you can Annotate
> symbols, so you mean that annotating the 'noploop' in the 'noploop' DSO
> didn't work?
>
Yes, it does not show me the assembly.

> Lets try this offline thing ourselves, can you please put the
> perf.data.tar.bz2 and the perf.data files somewhere I can grab? So that
> I can try to do these operations here and figure out the problem :-)
>
> I guess there is nothing sensitive in those files, lemme know if making
> them available is not an option and I'll try other ways to get this
> fixed.
>
I will send you the files in a private email.

Re: [GIT PULL 0/2] perf annotate fix and report improvoment

[Arnaldo Carvalho de Melo]

Em Thu, May 27, 2010 at 01:13:24AM +0200, Stephane Eranian escreveu:
> On Wed, May 26, 2010 at 10:50 PM, Arnaldo Carvalho de Melo
> > You can zoom into DSOs and threads, additionally you can Annotate
> > symbols, so you mean that annotating the 'noploop' in the 'noploop' DSO
> > didn't work?

> Yes, it does not show me the assembly.

We'll get this fixed.

> > Lets try this offline thing ourselves, can you please put the
> > perf.data.tar.bz2 and the perf.data files somewhere I can grab? So that
> > I can try to do these operations here and figure out the problem :-)

> > I guess there is nothing sensitive in those files, lemme know if making
> > them available is not an option and I'll try other ways to get this
> > fixed.

> I will send you the files in a private email.

Thanks a lot!

- Arnaldo