From c1a3a4b90a5a47adcca0e587f5d7e9ea61329b26 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 22 Nov 2010 14:01:55 -0200 Subject: perf record: Handle restrictive permissions in /proc/{kallsyms,modules} The 59365d1 commit, even being reverted by 33e0d57, showed a non robust behavior in 'perf record': it really should just warn the user that some functionality will not be available. The new behavior then becomes: [acme@felicio linux]$ ls -la /proc/{kallsyms,modules} -r-------- 1 root root 0 Nov 22 12:19 /proc/kallsyms -r-------- 1 root root 0 Nov 22 12:19 /proc/modules [acme@felicio linux]$ perf record ls -R > /dev/null Couldn't record kernel reference relocation symbol Symbol resolution may be skewed if relocation was used (e.g. kexec). Check /proc/kallsyms permission or run as root. [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.004 MB perf.data (~161 samples) ] [acme@felicio linux]$ perf report --stdio [kernel.kallsyms] with build id 77b05e00e64e4de1c9347d83879779b540d69f00 not found, continuing without symbols # Events: 98 cycles # # Overhead Command Shared Object Symbol # ........ ....... ............... .................... # 48.26% ls [kernel] [k] ffffffff8102b92b 22.49% ls libc-2.12.90.so [.] __strlen_sse2 8.35% ls libc-2.12.90.so [.] __GI___strcoll_l 8.17% ls ls [.] 11580 3.35% ls libc-2.12.90.so [.] _IO_new_file_xsputn 3.33% ls libc-2.12.90.so [.] _int_malloc 1.88% ls libc-2.12.90.so [.] _int_free 0.84% ls libc-2.12.90.so [.] malloc_consolidate 0.84% ls libc-2.12.90.so [.] __readdir64 0.83% ls ls [.] strlen@plt 0.83% ls libc-2.12.90.so [.] __GI_fwrite_unlocked 0.83% ls libc-2.12.90.so [.] __memcpy_sse2 # # (For a higher level overview, try: perf report --sort comm,dso) # [acme@felicio linux]$ It still has the build-ids for DSOs in the maps with hits: [acme@felicio linux]$ perf buildid-list 77b05e00e64e4de1c9347d83879779b540d69f00 [kernel.kallsyms] 09c4a431a4a8b648fcfc2c2bdda70f56050ddff1 /bin/ls af75ea9ad951d25e0f038901a11b3846dccb29a4 /lib64/libc-2.12.90.so [acme@felicio linux]$ That can be used in another machine to resolve kernel symbols. Cc: Eugene Teo Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jesper Juhl Cc: Marcus Meissner Cc: Mike Galbraith Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Sarah Sharp Cc: Stephane Eranian Cc: Tejun Heo Cc: Tom Zanussi LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-record.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'tools') diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 93bd2ff001fb..e2c2de201eec 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -697,17 +697,18 @@ static int __cmd_record(int argc, const char **argv) if (err < 0) err = event__synthesize_kernel_mmap(process_synthesized_event, session, machine, "_stext"); - if (err < 0) { - pr_err("Couldn't record kernel reference relocation symbol.\n"); - return err; - } + if (err < 0) + pr_err("Couldn't record kernel reference relocation symbol\n" + "Symbol resolution may be skewed if relocation was used (e.g. kexec).\n" + "Check /proc/kallsyms permission or run as root.\n"); err = event__synthesize_modules(process_synthesized_event, session, machine); - if (err < 0) { - pr_err("Couldn't record kernel reference relocation symbol.\n"); - return err; - } + if (err < 0) + pr_err("Couldn't record kernel module information.\n" + "Symbol resolution may be skewed if relocation was used (e.g. kexec).\n" + "Check /proc/modules permission or run as root.\n"); + if (perf_guest) perf_session__process_machines(session, event__synthesize_guest_os); -- cgit v1.2.2 From 02a9d03772aa1ff33a26180a2da0bfb191240eda Mon Sep 17 00:00:00 2001 From: Rabin Vincent Date: Tue, 23 Nov 2010 22:08:18 +0530 Subject: perf symbols: Remove incorrect open-coded container_of() At least on ARM, padding is inserted between rb_node and sym in struct symbol_name_rb_node, causing "((void *)sym) - sizeof(struct rb_node)" to point inside rb_node rather than to the symbol_name_rb_node. Fix this by converting the code to use container_of(). Cc: Ian Munsie Cc: Ingo Molnar Cc: Ming Lei Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Tom Zanussi LKML-Reference: <20101123163106.GA25677@debian> Signed-off-by: Rabin Vincent Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/symbol.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index b39f499e575a..0500895a45af 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -295,7 +295,9 @@ static void symbols__insert_by_name(struct rb_root *self, struct symbol *sym) { struct rb_node **p = &self->rb_node; struct rb_node *parent = NULL; - struct symbol_name_rb_node *symn = ((void *)sym) - sizeof(*parent), *s; + struct symbol_name_rb_node *symn, *s; + + symn = container_of(sym, struct symbol_name_rb_node, sym); while (*p != NULL) { parent = *p; -- cgit v1.2.2 From 8a9533123f43f2cdb3eb601c17ff2ad336882eff Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 29 Nov 2010 12:44:15 -0200 Subject: perf symbols: Fix kallsyms kernel/module map splitting On ARM, module addresss space is ahead of kernel space, so the module symbols are handled before kernel symbol in dso__split_kallsyms, then was causing one map to be created for each kernel symbol. Reported-by: Ming Lei Tested-by: Ming Lei Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Mike Galbraith Cc: Ming Lei Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Tom Zanussi LKML-Reference: <20101124144540.GB15875@ghostprotocols.net> Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/symbol.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 0500895a45af..2af4d7d22373 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -532,7 +532,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map, struct machine *machine = kmaps->machine; struct map *curr_map = map; struct symbol *pos; - int count = 0; + int count = 0, moved = 0; struct rb_root *root = &self->symbols[map->type]; struct rb_node *next = rb_first(root); int kernel_range = 0; @@ -590,6 +590,11 @@ static int dso__split_kallsyms(struct dso *self, struct map *map, char dso_name[PATH_MAX]; struct dso *dso; + if (count == 0) { + curr_map = map; + goto filter_symbol; + } + if (self->kernel == DSO_TYPE_GUEST_KERNEL) snprintf(dso_name, sizeof(dso_name), "[guest.kernel].%d", @@ -615,7 +620,7 @@ static int dso__split_kallsyms(struct dso *self, struct map *map, map_groups__insert(kmaps, curr_map); ++kernel_range; } - +filter_symbol: if (filter && filter(curr_map, pos)) { discard_symbol: rb_erase(&pos->rb_node, root); symbol__delete(pos); @@ -623,8 +628,9 @@ discard_symbol: rb_erase(&pos->rb_node, root); if (curr_map != map) { rb_erase(&pos->rb_node, root); symbols__insert(&curr_map->dso->symbols[curr_map->type], pos); - } - count++; + ++moved; + } else + ++count; } } @@ -634,7 +640,7 @@ discard_symbol: rb_erase(&pos->rb_node, root); dso__set_loaded(curr_map->dso, curr_map->type); } - return count; + return count + moved; } int dso__load_kallsyms(struct dso *self, const char *filename, -- cgit v1.2.2 From d214afbd81405d4da2c5745fe867e6b313fd4178 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 25 Nov 2010 19:27:25 +0800 Subject: perf symbols: Figure out start address of kernel map from kallsyms On ARM, module symbol start address is ahead of kernel symbol start address, so we can't suppose that the start address of kernel map always is zero, otherwise may cause incorrect .start and .end of kernel map (caused by fixup) when there are modules loaded, then map_groups__find may return incorrect map for symbol query. This patch always figures out the start address of kernel map from /proc/kallsyms if the file is available, so fix the issues on ARM for module loaded case. This patch fixes the following issues on ARM when modules are loaded: - vmlinux symbol can't be found by kallsyms maps doing 'perf test' - module symbols are parsed mistakenlly when doing 'perf top'/'perf report' Cc: Ian Munsie Cc: Ingo Molnar Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: Tom Zanussi LKML-Reference: <20101125192725.62d31b42@tom-lei> Signed-off-by: Ming Lei Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/symbol.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 2af4d7d22373..d628c8d1cf5e 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -2131,14 +2131,55 @@ static struct dso *machine__create_kernel(struct machine *self) return kernel; } +struct process_args { + u64 start; +}; + +static int symbol__in_kernel(void *arg, const char *name, + char type __used, u64 start) +{ + struct process_args *args = arg; + + if (strchr(name, '[')) + return 0; + + args->start = start; + return 1; +} + +/* Figure out the start address of kernel map from /proc/kallsyms */ +static u64 machine__get_kernel_start_addr(struct machine *machine) +{ + const char *filename; + char path[PATH_MAX]; + struct process_args args; + + if (machine__is_host(machine)) { + filename = "/proc/kallsyms"; + } else { + if (machine__is_default_guest(machine)) + filename = (char *)symbol_conf.default_guest_kallsyms; + else { + sprintf(path, "%s/proc/kallsyms", machine->root_dir); + filename = path; + } + } + + if (kallsyms__parse(filename, &args, symbol__in_kernel) <= 0) + return 0; + + return args.start; +} + int __machine__create_kernel_maps(struct machine *self, struct dso *kernel) { enum map_type type; + u64 start = machine__get_kernel_start_addr(self); for (type = 0; type < MAP__NR_TYPES; ++type) { struct kmap *kmap; - self->vmlinux_maps[type] = map__new2(0, kernel, type); + self->vmlinux_maps[type] = map__new2(start, kernel, type); if (self->vmlinux_maps[type] == NULL) return -1; -- cgit v1.2.2 From 60e677373be9c0bf7c9a22937601d5a40e51c042 Mon Sep 17 00:00:00 2001 From: Franck Bui-Huu Date: Tue, 30 Nov 2010 12:50:14 +0100 Subject: perf header: Don't assume there's no attr info if no sample ids is provided This primarily fixes perf-report, which didn't report the correct type of event if perf-record was called to record one event different from 'cycles': $ perf record -e instructions true [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.007 MB perf.data (~295 samples) ] $ perf report | head -n1 # Events: 7 cycles LPU-Reference: Signed-off-by: Franck Bui-Huu --- tools/perf/util/header.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index d7e67b167ea3..64a85bafde63 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -946,11 +946,16 @@ perf_header__find_attr(u64 id, struct perf_header *header) /* * We set id to -1 if the data file doesn't contain sample - * ids. Check for this and avoid walking through the entire - * list of ids which may be large. + * ids. This can happen when the data file contains one type + * of event and in that case, the header can still store the + * event attribute information. Check for this and avoid + * walking through the entire list of ids which may be large. */ - if (id == -1ULL) + if (id == -1ULL) { + if (header->attrs > 0) + return &header->attr[0]->attr; return NULL; + } for (i = 0; i < header->attrs; i++) { struct perf_header_attr *attr = header->attr[i]; -- cgit v1.2.2 From 18483b81ee7e70ee68d4b18be618be5cfcc0b290 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 6 Dec 2010 15:13:38 -0200 Subject: perf record: Fix eternal wait for stillborn child When execvp fails to find the specified command on the path we won't get SIGCHLD, so send a SIGUSR1 and exit right away. Current situation would require a SIGINT performed by the user and would produce meaningless summary. Now: [acme@emilia linux]$ ./foo -bash: ./foo: No such file or directory [acme@emilia linux]$ perf record ./foo ./foo: No such file or directory [acme@emilia linux]$ Acked-by: Thomas Gleixner Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Mike Galbraith Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Tom Zanussi Cc: Thomas Gleixner LKML-Reference: Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-record.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index e2c2de201eec..564491fa18b2 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -197,7 +197,7 @@ static void sig_atexit(void) if (child_pid > 0) kill(child_pid, SIGTERM); - if (signr == -1) + if (signr == -1 || signr == SIGUSR1) return; signal(signr, SIG_DFL); @@ -515,6 +515,7 @@ static int __cmd_record(int argc, const char **argv) atexit(sig_atexit); signal(SIGCHLD, sig_handler); signal(SIGINT, sig_handler); + signal(SIGUSR1, sig_handler); if (forks && (pipe(child_ready_pipe) < 0 || pipe(go_pipe) < 0)) { perror("failed to create pipes"); @@ -606,6 +607,7 @@ static int __cmd_record(int argc, const char **argv) execvp(argv[0], (char **)argv); perror(argv[0]); + kill(getppid(), SIGUSR1); exit(-1); } @@ -762,7 +764,7 @@ static int __cmd_record(int argc, const char **argv) } } - if (quiet) + if (quiet || signr == SIGUSR1) return 0; fprintf(stderr, "[ perf record: Woken up %ld times to write data ]\n", waking); -- cgit v1.2.2