* [PATCH 1/3] perf symbols: Improve kallsyms symbol end addr calculation
2010-12-22 22:36 [GIT PULL 0/3] perf/core fixes and improvements Arnaldo Carvalho de Melo
@ 2010-12-22 22:36 ` Arnaldo Carvalho de Melo
2010-12-22 22:36 ` [PATCH 2/3] perf test: Look forward for symbol aliases Arnaldo Carvalho de Melo
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-12-22 22:36 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Mike Galbraith, Peter Zijlstra, Paul Mackerras, Stephane Eranian
From: Arnaldo Carvalho de Melo <acme@redhat.com>
For kallsyms we don't have the symbol address end, so we do an extra pass and
set the symbol end addr as being the start of the next minus one.
But this was being done just after we filtered the symbols of a
particular type (functions, variables), so the symbol end was sometimes
after what it really is.
Fixing up symbol end also was falling apart when we have symbol aliases,
then the end address of all but the last alias was being set to be
before its start.
Fix it up by checking for symbol aliases and making the kallsyms__parse
routine use the next symbol, whatever its type, as the limit for the
previous symbol, passing that end address to the callback.
This was detected by the 'perf test' synthetic paranoid regression
tests, fix it up so that even that case doesn't mislead us.
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/event.c | 3 +-
tools/perf/util/symbol.c | 56 ++++++++++++++++++++++++++++++++++-----------
tools/perf/util/symbol.h | 2 +-
3 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 183aedd..2302ec0 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -332,7 +332,8 @@ struct process_symbol_args {
u64 start;
};
-static int find_symbol_cb(void *arg, const char *name, char type, u64 start)
+static int find_symbol_cb(void *arg, const char *name, char type,
+ u64 start, u64 end __used)
{
struct process_symbol_args *args = arg;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 561db63..2ea1a2e 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -22,6 +22,10 @@
#include <limits.h>
#include <sys/utsname.h>
+#ifndef KSYM_NAME_LEN
+#define KSYM_NAME_LEN 128
+#endif
+
#ifndef NT_GNU_BUILD_ID
#define NT_GNU_BUILD_ID 3
#endif
@@ -93,7 +97,7 @@ static void symbols__fixup_end(struct rb_root *self)
prev = curr;
curr = rb_entry(nd, struct symbol, rb_node);
- if (prev->end == prev->start)
+ if (prev->end == prev->start && prev->end != curr->start)
prev->end = curr->start - 1;
}
@@ -426,16 +430,25 @@ size_t dso__fprintf(struct dso *self, enum map_type type, FILE *fp)
int kallsyms__parse(const char *filename, void *arg,
int (*process_symbol)(void *arg, const char *name,
- char type, u64 start))
+ char type, u64 start, u64 end))
{
char *line = NULL;
size_t n;
- int err = 0;
+ int err = -1;
+ u64 prev_start = 0;
+ char prev_symbol_type = 0;
+ char *prev_symbol_name;
FILE *file = fopen(filename, "r");
if (file == NULL)
goto out_failure;
+ prev_symbol_name = malloc(KSYM_NAME_LEN);
+ if (prev_symbol_name == NULL)
+ goto out_close;
+
+ err = 0;
+
while (!feof(file)) {
u64 start;
int line_len, len;
@@ -455,14 +468,33 @@ int kallsyms__parse(const char *filename, void *arg,
continue;
symbol_type = toupper(line[len]);
- symbol_name = line + len + 2;
+ len += 2;
+ symbol_name = line + len;
+ len = line_len - len;
- err = process_symbol(arg, symbol_name, symbol_type, start);
- if (err)
+ if (len >= KSYM_NAME_LEN) {
+ err = -1;
break;
+ }
+
+ if (prev_symbol_type) {
+ u64 end = start;
+ if (end != prev_start)
+ --end;
+ err = process_symbol(arg, prev_symbol_name,
+ prev_symbol_type, prev_start, end);
+ if (err)
+ break;
+ }
+
+ memcpy(prev_symbol_name, symbol_name, len + 1);
+ prev_symbol_type = symbol_type;
+ prev_start = start;
}
+ free(prev_symbol_name);
free(line);
+out_close:
fclose(file);
return err;
@@ -484,7 +516,7 @@ static u8 kallsyms2elf_type(char type)
}
static int map__process_kallsym_symbol(void *arg, const char *name,
- char type, u64 start)
+ char type, u64 start, u64 end)
{
struct symbol *sym;
struct process_kallsyms_args *a = arg;
@@ -493,11 +525,8 @@ static int map__process_kallsym_symbol(void *arg, const char *name,
if (!symbol_type__is_a(type, a->map->type))
return 0;
- /*
- * Will fix up the end later, when we have all symbols sorted.
- */
- sym = symbol__new(start, 0, kallsyms2elf_type(type), name);
-
+ sym = symbol__new(start, end - start + 1,
+ kallsyms2elf_type(type), name);
if (sym == NULL)
return -ENOMEM;
/*
@@ -650,7 +679,6 @@ int dso__load_kallsyms(struct dso *self, const char *filename,
if (dso__load_all_kallsyms(self, filename, map) < 0)
return -1;
- symbols__fixup_end(&self->symbols[map->type]);
if (self->kernel == DSO_TYPE_GUEST_KERNEL)
self->origin = DSO__ORIG_GUEST_KERNEL;
else
@@ -2162,7 +2190,7 @@ struct process_args {
};
static int symbol__in_kernel(void *arg, const char *name,
- char type __used, u64 start)
+ char type __used, u64 start, u64 end __used)
{
struct process_args *args = arg;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index bcd2f98..7b8c27b 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -215,7 +215,7 @@ bool __dsos__read_build_ids(struct list_head *head, bool with_hits);
int build_id__sprintf(const u8 *self, int len, char *bf);
int kallsyms__parse(const char *filename, void *arg,
int (*process_symbol)(void *arg, const char *name,
- char type, u64 start));
+ char type, u64 start, u64 end));
void machine__destroy_kernel_maps(struct machine *self);
int __machine__create_kernel_maps(struct machine *self, struct dso *kernel);
--
1.6.2.5
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/3] perf test: Look forward for symbol aliases
2010-12-22 22:36 [GIT PULL 0/3] perf/core fixes and improvements Arnaldo Carvalho de Melo
2010-12-22 22:36 ` [PATCH 1/3] perf symbols: Improve kallsyms symbol end addr calculation Arnaldo Carvalho de Melo
@ 2010-12-22 22:36 ` Arnaldo Carvalho de Melo
2010-12-22 22:36 ` [PATCH 3/3] perf probe: Fix wrong warning in __show_one_line() if read(1) errors happen Arnaldo Carvalho de Melo
2010-12-23 13:20 ` [GIT PULL 0/3] perf/core fixes and improvements Ingo Molnar
3 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-12-22 22:36 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo, Frederic Weisbecker,
Han Pingtian, Mike Galbraith, Peter Zijlstra, Paul Mackerras,
Stephane Eranian
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Not just before, fixing these false positives:
[acme@mica linux]$ perf test -v 1
1: vmlinux symtab matches kallsyms:
--- start ---
Looking at the vmlinux_path (6 entries long)
Using //lib/modules/2.6.37-rc5-00180-ge06b6bf/build/vmlinux for symbols
0xffffffff81058dc0: diff name v: sys_vm86old k: sys_ni_syscall
0xffffffff81058dc0: diff name v: sys_vm86 k: sys_ni_syscall
0xffffffff81058dc0: diff name v: sys_subpage_prot k: sys_ni_syscall
0xffffffff810b5f7c: diff name v: probe_kernel_write k: __probe_kernel_write
0xffffffff810b5fe5: diff name v: probe_kernel_read k: __probe_kernel_read
0xffffffff811bc380: diff name v: __memset k: memset
0xffffffff81384a98: diff name v: __sched_text_start k: sleep_on_common
0xffffffff81386750: diff name v: __sched_text_end k: _raw_spin_trylock
0xffffffff8138cee8: diff name v: __irqentry_text_start k: do_IRQ
0xffffffff8138f079: diff name v: __start_notes k: _etext
0xffffffff8138f079: diff name v: __stop_notes k: _etext
---- end ----
vmlinux symtab matches kallsyms: FAILED!
[acme@mica linux]$
Some are weak functions, others are just markers, etc. They get in the rb tree
with the same addr, so we need to look around to find the symbol with the same
name.
We were looking just at the previous entries with the same addr, look forward
too.
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Han Pingtian <phan@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Stephane Eranian <eranian@google.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-test.c | 23 +++++++++++++++++++----
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 035b9fa..e0c3f47 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -119,10 +119,16 @@ static int test__vmlinux_matches_kallsyms(void)
* end addresses too.
*/
for (nd = rb_first(&vmlinux_map->dso->symbols[type]); nd; nd = rb_next(nd)) {
- struct symbol *pair;
+ struct symbol *pair, *first_pair;
+ bool backwards = true;
sym = rb_entry(nd, struct symbol, rb_node);
- pair = machine__find_kernel_symbol(&kallsyms, type, sym->start, NULL, NULL);
+
+ if (sym->start == sym->end)
+ continue;
+
+ first_pair = machine__find_kernel_symbol(&kallsyms, type, sym->start, NULL, NULL);
+ pair = first_pair;
if (pair && pair->start == sym->start) {
next_pair:
@@ -143,8 +149,10 @@ next_pair:
pr_debug("%#Lx: diff end addr for %s v: %#Lx k: %#Lx\n",
sym->start, sym->name, sym->end, pair->end);
} else {
- struct rb_node *nnd = rb_prev(&pair->rb_node);
-
+ struct rb_node *nnd;
+detour:
+ nnd = backwards ? rb_prev(&pair->rb_node) :
+ rb_next(&pair->rb_node);
if (nnd) {
struct symbol *next = rb_entry(nnd, struct symbol, rb_node);
@@ -153,6 +161,13 @@ next_pair:
goto next_pair;
}
}
+
+ if (backwards) {
+ backwards = false;
+ pair = first_pair;
+ goto detour;
+ }
+
pr_debug("%#Lx: diff name v: %s k: %s\n",
sym->start, sym->name, pair->name);
}
--
1.6.2.5
^ permalink raw reply [flat|nested] 5+ messages in thread