From 3b01a413c196c91040d41c86e5b56f76bb369f74 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 22 Dec 2010 01:08:36 -0200 Subject: perf symbols: Improve kallsyms symbol end addr calculation 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 Cc: Mike Galbraith Cc: Peter Zijlstra Cc: Paul Mackerras Cc: Stephane Eranian LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo --- 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(-) (limited to 'tools') diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index 183aedd4db83..2302ec051bb4 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 561db6361f57..2ea1a2e4c0c2 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -22,6 +22,10 @@ #include #include +#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 bcd2f986927e..7b8c27bb1e6b 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); -- cgit v1.2.3-59-g8ed1b From d3678758048308049cdad31ec3eae063be17c0db Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 21 Dec 2010 23:38:37 -0200 Subject: perf test: Look forward for symbol aliases 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 Cc: Han Pingtian Cc: Mike Galbraith Cc: Peter Zijlstra Cc: Paul Mackerras Cc: Stephane Eranian LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-test.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'tools') diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c index 035b9fa063a9..e0c3f471f22d 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); } -- cgit v1.2.3-59-g8ed1b From 32b2b6ec57a3adb3ab7215fbf36ec61c15de06ee Mon Sep 17 00:00:00 2001 From: Franck Bui-Huu Date: Wed, 22 Dec 2010 17:37:13 +0100 Subject: perf probe: Fix wrong warning in __show_one_line() if read(1) errors happen This was introduced by commit fde52dbd7f71934aba4e150f3d1d51e826a08850. Cc: H. Peter Anvin Cc: Masami Hiramatsu Cc: Ingo Molnar Cc: Thomas Gleixner LKML-Reference: Signed-off-by: Franck Bui-Huu Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/probe-event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 099336ed34b4..4bde988a0b04 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -309,7 +309,7 @@ static int __show_one_line(FILE *fp, int l, bool skip, bool show_num) return 1; error: if (ferror(fp)) { - pr_warning("Source file is shorter than expected.\n"); + pr_warning("File read error: %s\n", strerror(errno)); return -1; } return 0; -- cgit v1.2.3-59-g8ed1b