[PATCH 1/3] perf symbols: fixup kernel_maps__fixup_end end map

[PATCH 1/3] perf symbols: fixup kernel_maps__fixup_end end map

[Arnaldo Carvalho de Melo]

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

We better call this routine after both the kernel and modules are
loaded, because as it was if there weren't modules it would be called,
resulting in kernel_map->end remaining at zero, so no map would be found
and consequently the kernel symtab wouldn't get loaded, i.e. no kernel
symbols would be resolved.

Also this fixes another case, that is when we _have_ modules, but the
last map would have its ->end address not set before we loaded its
symbols, which would never happen because ->end was not set.

Reported-by: Ingo Molnar <mingo@elte.hu>
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 |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 48f87f0..e161a51 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -71,6 +71,12 @@ static void kernel_maps__fixup_end(void)
 		curr = rb_entry(nd, struct map, rb_node);
 		prev->end = curr->start - 1;
 	}
+
+	/*
+	 * We still haven't the actual symbols, so guess the
+	 * last map final address.
+	 */
+	curr->end = ~0UL;
 }
 
 static struct symbol *symbol__new(u64 start, u64 len, const char *name)
@@ -1319,12 +1325,6 @@ static int kernel_maps__create_module_maps(void)
 	free(line);
 	fclose(file);
 
-	/*
-	 * Now that we have all sorted out, just set the ->end of all
-	 * maps:
-	 */
-	kernel_maps__fixup_end();
-
 	return dsos__set_modules_path();
 
 out_delete_line:
@@ -1493,7 +1493,10 @@ int kernel_maps__init(bool use_modules)
 	if (use_modules && kernel_maps__create_module_maps() < 0)
 		pr_warning("Failed to load list of modules in use, "
 			   "continuing...\n");
-
+	/*
+	 * Now that we have all the maps created, just set the ->end of them:
+	 */
+	kernel_maps__fixup_end();
 	return 0;
 }
 
-- 
1.6.2.5

[PATCH 2/3] perf symbols: Old versions of elf.h don't have NT_GNU_BUILD_ID

[Arnaldo Carvalho de Melo]

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

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, 4 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index e161a51..86ec6c7 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -12,6 +12,10 @@
 #include <limits.h>
 #include <sys/utsname.h>
 
+#ifndef NT_GNU_BUILD_ID
+#define NT_GNU_BUILD_ID 3
+#endif
+
 enum dso_origin {
 	DSO__ORIG_KERNEL = 0,
 	DSO__ORIG_JAVA_JIT,
-- 
1.6.2.5

[PATCH 3/3] perf trace: read_tracing_data should die() another day

[Arnaldo Carvalho de Melo]

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

It better propagate errors, also if we do a simple:

[root@doppio linux-2.6-tip]# perf record -R -a -f sleep 3s ; perf trace
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.182 MB perf.data (~7972 samples) ]
  Fatal: not an trace data file
[root@doppio linux-2.6-tip]#

That is what is expected, right? I.e. as we didn't specify any
tracepoint event via -e, it should gracefully bail out and not SEGFAULT.

Cc: Steven Rostedt <rostedt@goodmis.org>
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/trace-event-info.c |   22 +++++++++++++++-------
 tools/perf/util/trace-event.h      |    2 +-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/trace-event-info.c b/tools/perf/util/trace-event-info.c
index 831052d..cace355 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -33,11 +33,11 @@
 #include <ctype.h>
 #include <errno.h>
 #include <stdbool.h>
+#include <linux/kernel.h>
 
 #include "../perf.h"
 #include "trace-event.h"
 
-
 #define VERSION "0.5"
 
 #define _STR(x) #x
@@ -483,23 +483,31 @@ static struct tracepoint_path *
 get_tracepoints_path(struct perf_event_attr *pattrs, int nb_events)
 {
 	struct tracepoint_path path, *ppath = &path;
-	int i;
+	int i, nr_tracepoints = 0;
 
 	for (i = 0; i < nb_events; i++) {
 		if (pattrs[i].type != PERF_TYPE_TRACEPOINT)
 			continue;
+		++nr_tracepoints;
 		ppath->next = tracepoint_id_to_path(pattrs[i].config);
 		if (!ppath->next)
 			die("%s\n", "No memory to alloc tracepoints list");
 		ppath = ppath->next;
 	}
 
-	return path.next;
+	return nr_tracepoints > 0 ? path.next : NULL;
 }
-void read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events)
+
+int read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events)
 {
 	char buf[BUFSIZ];
-	struct tracepoint_path *tps;
+	struct tracepoint_path *tps = get_tracepoints_path(pattrs, nb_events);
+
+	/*
+	 * What? No tracepoints? No sense writing anything here, bail out.
+	 */
+	if (tps == NULL)
+		return -1;
 
 	output_fd = fd;
 
@@ -528,11 +536,11 @@ void read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events)
 	page_size = getpagesize();
 	write_or_die(&page_size, 4);
 
-	tps = get_tracepoints_path(pattrs, nb_events);
-
 	read_header_files();
 	read_ftrace_files(tps);
 	read_event_files(tps);
 	read_proc_kallsyms();
 	read_ftrace_printk();
+
+	return 0;
 }
diff --git a/tools/perf/util/trace-event.h b/tools/perf/util/trace-event.h
index f6637c2..dd51c68 100644
--- a/tools/perf/util/trace-event.h
+++ b/tools/perf/util/trace-event.h
@@ -248,7 +248,7 @@ unsigned long long
 raw_field_value(struct event *event, const char *name, void *data);
 void *raw_field_ptr(struct event *event, const char *name, void *data);
 
-void read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events);
+int read_tracing_data(int fd, struct perf_event_attr *pattrs, int nb_events);
 
 /* taken from kernel/trace/trace.h */
 enum trace_flag_type {
-- 
1.6.2.5

Re: [PATCH 1/3] perf symbols: fixup kernel_maps__fixup_end end map

[Arnaldo Carvalho de Melo]

Em Sat, Nov 21, 2009 at 02:31:24PM -0200, Arnaldo Carvalho de Melo escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> We better call this routine after both the kernel and modules are
> loaded, because as it was if there weren't modules it would be called,

s/would/would not/g

Ingo, if you see this before merging, please ammend the above comment
line :)

> resulting in kernel_map->end remaining at zero, so no map would be found
> and consequently the kernel symtab wouldn't get loaded, i.e. no kernel
> symbols would be resolved.
> 
> Also this fixes another case, that is when we _have_ modules, but the
> last map would have its ->end address not set before we loaded its
> symbols, which would never happen because ->end was not set.