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

From: Arnaldo Carvalho de Melo <acme.RemoveThis@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.RemoveThis@elte.hu>
Cc: Frédéric Weisbecker <fweisbec.RemoveThis@gmail.com>
Cc: Mike Galbraith <efault.RemoveThis@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra.RemoveThis@chello.nl>
Cc: Paul Mackerras <paulus.RemoveThis@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme.RemoveThis@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.RemoveThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

From: Arnaldo Carvalho de Melo <acme.DeleteThis@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.DeleteThis@goodmis.org>
Cc: Frédéric Weisbecker <fweisbec.DeleteThis@gmail.com>
Cc: Mike Galbraith <efault.DeleteThis@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra.DeleteThis@chello.nl>
Cc: Paul Mackerras <paulus.DeleteThis@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme.DeleteThis@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo.DeleteThis@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/