From 9aca7f1792c5d2d5d367bbe5cfe204fe40517929 Mon Sep 17 00:00:00 2001 From: David Ahern Date: Wed, 4 Dec 2013 19:41:39 -0700 Subject: [PATCH 01/34] perf trace: Add support for syscalls vs raw_syscalls Older kernels (e.g., RHEL6) do system call tracing via syscalls:sys_{enter,exit} rather than raw_syscalls. Update perf-trace to detect lack of raw_syscalls support and try syscalls. Signed-off-by: David Ahern Link: http://lkml.kernel.org/r/1386211302-31303-2-git-send-email-dsahern@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 56afe339661a..a7aa771a98e6 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -12,6 +12,7 @@ #include "util/thread_map.h" #include "util/stat.h" #include "trace-event.h" +#include "util/parse-events.h" #include #include @@ -173,6 +174,10 @@ static struct perf_evsel *perf_evsel__syscall_newtp(const char *direction, void { struct perf_evsel *evsel = perf_evsel__newtp("raw_syscalls", direction); + /* older kernel (e.g., RHEL6) use syscalls:{enter,exit} */ + if (evsel == NULL) + evsel = perf_evsel__newtp("syscalls", direction); + if (evsel) { if (perf_evsel__init_syscall_tp(evsel, handler)) goto out_delete; @@ -1801,10 +1806,11 @@ static int trace__record(int argc, const char **argv) "-R", "-m", "1024", "-c", "1", - "-e", "raw_syscalls:sys_enter,raw_syscalls:sys_exit", + "-e", }; - rec_argc = ARRAY_SIZE(record_args) + argc; + /* +1 is for the event string below */ + rec_argc = ARRAY_SIZE(record_args) + 1 + argc; rec_argv = calloc(rec_argc + 1, sizeof(char *)); if (rec_argv == NULL) @@ -1813,6 +1819,17 @@ static int trace__record(int argc, const char **argv) for (i = 0; i < ARRAY_SIZE(record_args); i++) rec_argv[i] = record_args[i]; + /* event string may be different for older kernels - e.g., RHEL6 */ + if (is_valid_tracepoint("raw_syscalls:sys_enter")) + rec_argv[i] = "raw_syscalls:sys_enter,raw_syscalls:sys_exit"; + else if (is_valid_tracepoint("syscalls:sys_enter")) + rec_argv[i] = "syscalls:sys_enter,syscalls:sys_exit"; + else { + pr_err("Neither raw_syscalls nor syscalls events exist.\n"); + return -1; + } + i++; + for (j = 0; j < (unsigned int)argc; j++, i++) rec_argv[i] = argv[j]; @@ -2048,6 +2065,10 @@ static int trace__replay(struct trace *trace) evsel = perf_evlist__find_tracepoint_by_name(session->evlist, "raw_syscalls:sys_enter"); + /* older kernels have syscalls tp versus raw_syscalls */ + if (evsel == NULL) + evsel = perf_evlist__find_tracepoint_by_name(session->evlist, + "syscalls:sys_enter"); if (evsel == NULL) { pr_err("Data file does not have raw_syscalls:sys_enter event\n"); goto out; @@ -2061,6 +2082,9 @@ static int trace__replay(struct trace *trace) evsel = perf_evlist__find_tracepoint_by_name(session->evlist, "raw_syscalls:sys_exit"); + if (evsel == NULL) + evsel = perf_evlist__find_tracepoint_by_name(session->evlist, + "syscalls:sys_exit"); if (evsel == NULL) { pr_err("Data file does not have raw_syscalls:sys_exit event\n"); goto out; From 3160565f0e005d2ec736ae25cf0a79988c0cbe71 Mon Sep 17 00:00:00 2001 From: David Ahern Date: Wed, 4 Dec 2013 19:41:41 -0700 Subject: [PATCH 02/34] perf trace: Fix summary percentage when processing files Getting a divide by 0 when events are processed from a file: perf trace -i perf.data -s ... dnsmasq (1684), 10 events, inf%, 0.000 msec The problem is that the event count is not incremented as events are processed. With this patch: perf trace -i perf.data -s ... dnsmasq (1684), 10 events, 8.9%, 0.000 msec Signed-off-by: David Ahern Link: http://lkml.kernel.org/r/1386211302-31303-4-git-send-email-dsahern@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index a7aa771a98e6..56bbca5bc2dc 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -1770,8 +1770,10 @@ static int trace__process_sample(struct perf_tool *tool, if (!trace->full_time && trace->base_time == 0) trace->base_time = sample->time; - if (handler) + if (handler) { + ++trace->nr_events; handler(trace, evsel, sample); + } return err; } From 3a3ffa2e82205921d1189f1055c22dae4c72819a Mon Sep 17 00:00:00 2001 From: Steven Rostedt Date: Mon, 18 Nov 2013 21:38:20 -0500 Subject: [PATCH 03/34] tools lib traceevent: Report better error message on bad function args When Jiri Olsa was writing a function callback for scsi_trace_parse_cdb(), he thought that the traceevent library had a bug in it because he was getting this error: Error: expected ')' but read ',' Error: expected ')' but read ',' Error: expected ')' but read ',' Error: expected ')' but read ',' But in truth, he didn't have the write number of arguments for the function callback, and the error was the library detecting the discrepancy. A better error message would have prevented the confusion: Error: function 'scsi_trace_parse_cdb()' only expects 2 arguments but event scsi_dispatch_cmd_timeout has more Error: function 'scsi_trace_parse_cdb()' only expects 2 arguments but event scsi_dispatch_cmd_start has more Error: function 'scsi_trace_parse_cdb()' only expects 2 arguments but event scsi_dispatch_cmd_error has more Error: function 'scsi_trace_parse_cdb()' only expects 2 arguments but event scsi_dispatch_cmd_done has more Or Error: function 'scsi_trace_parse_cdb()' expects 4 arguments but event scsi_dispatch_cmd_timeout only uses 3 Error: function 'scsi_trace_parse_cdb()' expects 4 arguments but event scsi_dispatch_cmd_start only uses 3 Error: function 'scsi_trace_parse_cdb()' expects 4 arguments but event scsi_dispatch_cmd_error only uses 3 Error: function 'scsi_trace_parse_cdb()' expects 4 arguments but event scsi_dispatch_cmd_done only uses 3 Signed-off-by: Steven Rostedt Link: http://lkml.kernel.org/n/tip-a4c34w62vl0diitvxb7bt3er@git.kernel.org Signed-off-by: Jiri Olsa Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c index 9849873265d4..22566c271275 100644 --- a/tools/lib/traceevent/event-parse.c +++ b/tools/lib/traceevent/event-parse.c @@ -2710,7 +2710,6 @@ process_func_handler(struct event_format *event, struct pevent_function_handler struct print_arg *farg; enum event_type type; char *token; - const char *test; int i; arg->type = PRINT_FUNC; @@ -2727,15 +2726,19 @@ process_func_handler(struct event_format *event, struct pevent_function_handler } type = process_arg(event, farg, &token); - if (i < (func->nr_args - 1)) - test = ","; - else - test = ")"; - - if (test_type_token(type, token, EVENT_DELIM, test)) { - free_arg(farg); - free_token(token); - return EVENT_ERROR; + if (i < (func->nr_args - 1)) { + if (type != EVENT_DELIM || strcmp(token, ",") != 0) { + warning("Error: function '%s()' expects %d arguments but event %s only uses %d", + func->name, func->nr_args, + event->name, i + 1); + goto err; + } + } else { + if (type != EVENT_DELIM || strcmp(token, ")") != 0) { + warning("Error: function '%s()' only expects %d arguments but event %s has more", + func->name, func->nr_args, event->name); + goto err; + } } *next_arg = farg; @@ -2747,6 +2750,11 @@ process_func_handler(struct event_format *event, struct pevent_function_handler *tok = token; return type; + +err: + free_arg(farg); + free_token(token); + return EVENT_ERROR; } static enum event_type From a4eb24a49566db77ee999b46603f602a0302f481 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 6 Dec 2013 09:42:56 +0200 Subject: [PATCH 04/34] perf script: Fix symoff printing in callchains The address being used to calculate the offset was the memory address but the address needed is the address mapped to the dso. i.e. the 'addr' member of 'struct addr_location' Signed-off-by: Adrian Hunter Acked-by: David Ahern Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386315778-11633-2-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/session.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index 8a7da6f4a569..c236b38ed02b 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1515,6 +1515,8 @@ void perf_evsel__print_ip(struct perf_evsel *evsel, struct perf_sample *sample, node_al = *al; while (stack_depth) { + u64 addr = 0; + node = callchain_cursor_current(&callchain_cursor); if (!node) break; @@ -1525,10 +1527,13 @@ void perf_evsel__print_ip(struct perf_evsel *evsel, struct perf_sample *sample, if (print_ip) printf("%c%16" PRIx64, s, node->ip); + if (node->map) + addr = node->map->map_ip(node->map, node->ip); + if (print_sym) { printf(" "); if (print_symoffset) { - node_al.addr = node->ip; + node_al.addr = addr; node_al.map = node->map; symbol__fprintf_symname_offs(node->sym, &node_al, stdout); } else From cc8fae1d81648e85587f5d18b4f93e0b771fb02d Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 6 Dec 2013 09:42:57 +0200 Subject: [PATCH 05/34] perf script: Add an option to print the source line number Add field 'srcline' that displays the source file name and line number associated with the sample ip. The information displayed is the same as from addr2line. $ perf script -f comm,tid,pid,time,ip,sym,dso,symoff,srcline grep 10701/10701 2497321.421013: ffffffff81043ffa native_write_msr_safe+0xa ([kernel.kallsyms]) /usr/src/debug/kernel-3.9.fc17/linux-3.9.10-100.fc17.x86_64/arch/x86/include/asm/msr.h:95 grep 10701/10701 2497321.421984: ffffffff8165b6b3 _raw_spin_lock+0x13 ([kernel.kallsyms]) /usr/src/debug/kernel-3.9.fc17/linux-3.9.10-100.fc17.x86_64/arch/x86/include/asm/spinlock.h:54 grep 10701/10701 2497321.421990: ffffffff810b64b3 tick_sched_timer+0x53 ([kernel.kallsyms]) /usr/src/debug/kernel-3.9.fc17/linux-3.9.10-100.fc17.x86_64/kernel/time/tick-sched.c:840 grep 10701/10701 2497321.421992: ffffffff8106f63f run_timer_softirq+0x2f ([kernel.kallsyms]) /usr/src/debug/kernel-3.9.fc17/linux-3.9.10-100.fc17.x86_64/kernel/timer.c:1372 Signed-off-by: Adrian Hunter Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386315778-11633-3-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-script.txt | 2 +- tools/perf/builtin-script.c | 10 ++++++++++ tools/perf/util/map.c | 17 +++++++++++++++++ tools/perf/util/map.h | 2 ++ tools/perf/util/session.c | 8 ++++++++ tools/perf/util/session.h | 1 + 6 files changed, 39 insertions(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt index cfdbb1e045b5..c2a5071cf8f8 100644 --- a/tools/perf/Documentation/perf-script.txt +++ b/tools/perf/Documentation/perf-script.txt @@ -115,7 +115,7 @@ OPTIONS -f:: --fields:: Comma separated list of fields to print. Options are: - comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff. + comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline. Field list can be prepended with the type, trace, sw or hw, to indicate to which event type the field list applies. e.g., -f sw:comm,tid,time,ip,sym and -f trace:time,cpu,trace diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 4484886dcf08..7a571fb7eb8a 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -43,6 +43,7 @@ enum perf_output_field { PERF_OUTPUT_DSO = 1U << 9, PERF_OUTPUT_ADDR = 1U << 10, PERF_OUTPUT_SYMOFFSET = 1U << 11, + PERF_OUTPUT_SRCLINE = 1U << 12, }; struct output_option { @@ -61,6 +62,7 @@ struct output_option { {.str = "dso", .field = PERF_OUTPUT_DSO}, {.str = "addr", .field = PERF_OUTPUT_ADDR}, {.str = "symoff", .field = PERF_OUTPUT_SYMOFFSET}, + {.str = "srcline", .field = PERF_OUTPUT_SRCLINE}, }; /* default set to maintain compatibility with current format */ @@ -210,6 +212,11 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel, "to DSO.\n"); return -EINVAL; } + if (PRINT_FIELD(SRCLINE) && !PRINT_FIELD(IP)) { + pr_err("Display of source line number requested but sample IP is not\n" + "selected. Hence, no address to lookup the source line number.\n"); + return -EINVAL; + } if ((PRINT_FIELD(PID) || PRINT_FIELD(TID)) && perf_evsel__check_stype(evsel, PERF_SAMPLE_TID, "TID", @@ -245,6 +252,9 @@ static void set_print_ip_opts(struct perf_event_attr *attr) if (PRINT_FIELD(SYMOFFSET)) output[type].print_ip_opts |= PRINT_IP_OPT_SYMOFFSET; + + if (PRINT_FIELD(SRCLINE)) + output[type].print_ip_opts |= PRINT_IP_OPT_SRCLINE; } /* diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index ef5bc913ca7a..9b9bd719aa19 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -11,6 +11,7 @@ #include "strlist.h" #include "vdso.h" #include "build-id.h" +#include "util.h" #include const char *map_type__name[MAP__NR_TYPES] = { @@ -252,6 +253,22 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp) return fprintf(fp, "%s", dsoname); } +int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix, + FILE *fp) +{ + char *srcline; + int ret = 0; + + if (map && map->dso) { + srcline = get_srcline(map->dso, + map__rip_2objdump(map, addr)); + if (srcline != SRCLINE_UNKNOWN) + ret = fprintf(fp, "%s%s", prefix, srcline); + free_srcline(srcline); + } + return ret; +} + /** * map__rip_2objdump - convert symbol start address to objdump address. * @map: memory map diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index e4e259c3ba16..18068c6b71c1 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -103,6 +103,8 @@ struct map *map__clone(struct map *map); int map__overlap(struct map *l, struct map *r); size_t map__fprintf(struct map *map, FILE *fp); size_t map__fprintf_dsoname(struct map *map, FILE *fp); +int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix, + FILE *fp); int map__load(struct map *map, symbol_filter_t filter); struct symbol *map__find_symbol(struct map *map, diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index c236b38ed02b..e748f29c53cf 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1497,6 +1497,7 @@ void perf_evsel__print_ip(struct perf_evsel *evsel, struct perf_sample *sample, int print_dso = print_opts & PRINT_IP_OPT_DSO; int print_symoffset = print_opts & PRINT_IP_OPT_SYMOFFSET; int print_oneline = print_opts & PRINT_IP_OPT_ONELINE; + int print_srcline = print_opts & PRINT_IP_OPT_SRCLINE; char s = print_oneline ? ' ' : '\t'; if (symbol_conf.use_callchain && sample->callchain) { @@ -1546,6 +1547,10 @@ void perf_evsel__print_ip(struct perf_evsel *evsel, struct perf_sample *sample, printf(")"); } + if (print_srcline) + map__fprintf_srcline(node->map, addr, "\n ", + stdout); + if (!print_oneline) printf("\n"); @@ -1575,6 +1580,9 @@ void perf_evsel__print_ip(struct perf_evsel *evsel, struct perf_sample *sample, map__fprintf_dsoname(al->map, stdout); printf(")"); } + + if (print_srcline) + map__fprintf_srcline(al->map, al->addr, "\n ", stdout); } } diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h index 004d3e8116aa..2a3955ea4fd8 100644 --- a/tools/perf/util/session.h +++ b/tools/perf/util/session.h @@ -45,6 +45,7 @@ struct perf_session { #define PRINT_IP_OPT_DSO (1<<2) #define PRINT_IP_OPT_SYMOFFSET (1<<3) #define PRINT_IP_OPT_ONELINE (1<<4) +#define PRINT_IP_OPT_SRCLINE (1<<5) struct perf_tool; From 53653d70a0784a997748cc9e315ddf19d310e812 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Mon, 9 Dec 2013 15:18:40 +0200 Subject: [PATCH 06/34] perf record: Fix display of incorrect mmap pages 'mmap_pages' is 'unsigned int' not 'int' e.g. perf record -m2147483648 uname Permission error mapping pages. Consider increasing /proc/sys/kernel/perf_event_mlock_kb, or try again with a smaller value of -m/--mmap_pages. (current value: -2147483648) Fixed: perf record -m2147483648 uname Permission error mapping pages. Consider increasing /proc/sys/kernel/perf_event_mlock_kb, or try again with a smaller value of -m/--mmap_pages. (current value: 2147483648) Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386595120-22978-5-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-record.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index d93e2eef0979..c1c1200d2f0a 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -224,7 +224,7 @@ static int perf_record__open(struct perf_record *rec) "Consider increasing " "/proc/sys/kernel/perf_event_mlock_kb,\n" "or try again with a smaller value of -m/--mmap_pages.\n" - "(current value: %d)\n", opts->mmap_pages); + "(current value: %u)\n", opts->mmap_pages); rc = -errno; } else { pr_err("failed to mmap with %d (%s)\n", errno, strerror(errno)); From 2bcab6c146fde0d0286b132068111f98b6217460 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Mon, 9 Dec 2013 15:18:37 +0200 Subject: [PATCH 07/34] perf evlist: Remove unnecessary parentheses Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386595120-22978-2-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 7bb6ee1ca19f..4d0945c96ab2 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -732,7 +732,7 @@ static long parse_pages_arg(const char *str, unsigned long min, return -EINVAL; } - if ((pages == 0) && (min == 0)) { + if (pages == 0 && min == 0) { /* leave number of pages at 0 */ } else if (pages < (1UL << 31) && !is_power_of_2(pages)) { /* round pages up to next power of 2 */ From f5ae9c424e4c80c44e9ab7e5ff4a6b79490c23be Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Mon, 9 Dec 2013 15:18:38 +0200 Subject: [PATCH 08/34] perf evlist: Fix max mmap_pages 'SIZE_MAX / page_size' is an upper limit for the maximum number of mmap pages, not a lower limit. Change the condition accordingly. Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386595120-22978-3-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 4d0945c96ab2..98ec96b3a744 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -754,7 +754,7 @@ int perf_evlist__parse_mmap_pages(const struct option *opt, const char *str, unsigned long max = UINT_MAX; long pages; - if (max < SIZE_MAX / page_size) + if (max > SIZE_MAX / page_size) max = SIZE_MAX / page_size; pages = parse_pages_arg(str, 1, max); From 1dbfa9387b397f2c4b8c65411b3e3fdf9284d2b1 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Mon, 9 Dec 2013 15:18:39 +0200 Subject: [PATCH 09/34] perf evlist: Fix mmap pages rounding to power of 2 'next_pow2()' only works for 'unsigned int' but the argument is 'unsigned long'. Checking for values less than (1 << 31) ensures that 'next_pow2()' is not passed a value out of range but lets anything else go through unvalidated. As a result mmap_pages of zero is used e.g. perf record -v -m2147483649 uname mmap size 0B failed to mmap with 22 (Invalid argument) Fixed: perf record -m2147483649 uname rounding mmap pages size to 17592186044416 bytes (4294967296 pages) Invalid argument for --mmap_pages/-m Signed-off-by: Adrian Hunter Acked-by: Jiri Olsa Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386595120-22978-4-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 6 ++++-- tools/perf/util/util.h | 12 ++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 98ec96b3a744..af250556b33f 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -734,9 +734,11 @@ static long parse_pages_arg(const char *str, unsigned long min, if (pages == 0 && min == 0) { /* leave number of pages at 0 */ - } else if (pages < (1UL << 31) && !is_power_of_2(pages)) { + } else if (!is_power_of_2(pages)) { /* round pages up to next power of 2 */ - pages = next_pow2(pages); + pages = next_pow2_l(pages); + if (!pages) + return -EINVAL; pr_info("rounding mmap pages size to %lu bytes (%lu pages)\n", pages * page_size, pages); } diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h index adb39f251f90..659abf30e01b 100644 --- a/tools/perf/util/util.h +++ b/tools/perf/util/util.h @@ -73,6 +73,7 @@ #include #include #include +#include extern const char *graph_line; extern const char *graph_dotted_line; @@ -281,6 +282,17 @@ static inline unsigned next_pow2(unsigned x) return 1ULL << (32 - __builtin_clz(x - 1)); } +static inline unsigned long next_pow2_l(unsigned long x) +{ +#if BITS_PER_LONG == 64 + if (x <= (1UL << 31)) + return next_pow2(x); + return (unsigned long)next_pow2(x >> 32) << 32; +#else + return next_pow2(x); +#endif +} + size_t hex_width(u64 v); int hex2u64(const char *ptr, u64 *val); From 100b907350c87aa1f3b5dbd95bac3ad5aad3e108 Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Mon, 9 Dec 2013 12:15:11 -0500 Subject: [PATCH 10/34] perf kvm: Introduce option -v for perf kvm command. As there is no -v option for perf kvm, the all debug message for perf kvm will nerver be printed out to user. Example: # perf kvm --guestmount /tmp/guestmount/ record -a Not enough memory for reading perf file header It is confusing message for newbies such as me. With this patch applied, we can use -v option to get the detail. Example: # perf kvm --guestmount /tmp/guestmount/ record -a -v Can't access file /tmp/guestmount//15069/proc/kallsyms Not enough memory for reading perf file header Signed-off-by: Dongsheng Yang Cc: David Ahern Link: http://lkml.kernel.org/r/1386609311-23889-1-git-send-email-yangds.fnst@cn.fujitsu.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-kvm.txt | 7 +++++-- tools/perf/builtin-kvm.c | 2 ++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/perf/Documentation/perf-kvm.txt b/tools/perf/Documentation/perf-kvm.txt index 96a9a1dea727..52276a6d2b75 100644 --- a/tools/perf/Documentation/perf-kvm.txt +++ b/tools/perf/Documentation/perf-kvm.txt @@ -10,9 +10,9 @@ SYNOPSIS [verse] 'perf kvm' [--host] [--guest] [--guestmount= [--guestkallsyms= --guestmodules= | --guestvmlinux=]] - {top|record|report|diff|buildid-list} + {top|record|report|diff|buildid-list} [] 'perf kvm' [--host] [--guest] [--guestkallsyms= --guestmodules= - | --guestvmlinux=] {top|record|report|diff|buildid-list|stat} + | --guestvmlinux=] {top|record|report|diff|buildid-list|stat} [] 'perf kvm stat [record|report|live] [] DESCRIPTION @@ -93,6 +93,9 @@ OPTIONS kernel module information. Users copy it out from guest os. --guestvmlinux=:: Guest os kernel vmlinux. +-v:: +--verbose:: + Be more verbose (show counter open errors, etc). STAT REPORT OPTIONS ------------------- diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c index f8bf5f244d77..d9cc0e3f284f 100644 --- a/tools/perf/builtin-kvm.c +++ b/tools/perf/builtin-kvm.c @@ -1690,6 +1690,8 @@ int cmd_kvm(int argc, const char **argv, const char *prefix __maybe_unused) "file", "file saving guest os /proc/kallsyms"), OPT_STRING(0, "guestmodules", &symbol_conf.default_guest_modules, "file", "file saving guest os /proc/modules"), + OPT_INCR('v', "verbose", &verbose, + "be more verbose (show counter open errors, etc)"), OPT_END() }; From 476b3a865f8a3734f74cf659cfa510856a105b1a Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Mon, 9 Dec 2013 18:47:03 -0500 Subject: [PATCH 11/34] perf kvm: Fix bug in 'stat report' When we use perf kvm record-report, there is a bug in report subcommand. Example: # perf kvm stat record -a sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.678 MB perf.data.guest (~29641 samples) ] # perf kvm stat report failed to open perf.data: No such file or directory (try 'perf record' first) Initializing perf session failed This bug was introduced by f5fc14124. + struct perf_data_file file = { + .path = input_name, + .mode = PERF_DATA_MODE_READ, + }; kvm->tool = eops; - kvm->session = perf_session__new(kvm->file_name, O_RDONLY, 0, false, - &kvm->tool); + kvm->session = perf_session__new(&file, false, &kvm->tool); It changed the path from kvm->file_name to input_name, this patch change the path back to 'kvm->file_name', then it works well. Verification: # perf kvm stat record -a sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.807 MB perf.data.guest (~35264 samples) ] # perf kvm stat report Analyze events for all VCPUs: VM-EXIT Samples Samples% Time% Min Time Max Time Avg time EPT_VIOLATION 200 32.79% 1.25% 0us 12064us 62.35us ( +- 96.74% ) EPT_MISCONFIG 134 21.97% 0.21% 0us 35us 15.25us ( +- 4.14% ) EXCEPTION_NMI 96 15.74% 0.02% 0us 11us 1.95us ( +- 9.81% ) APIC_ACCESS 79 12.95% 0.02% 0us 13us 2.94us ( +- 11.20% ) HLT 65 10.66% 98.47% 0us 16706us 15084.86us ( +- 1.89% ) IO_INSTRUCTION 27 4.43% 0.02% 0us 29us 6.42us ( +- 15.53% ) EXTERNAL_INTERRUPT 5 0.82% 0.01% 0us 77us 23.65us ( +- 57.90% ) TPR_BELOW_THRESHOLD 4 0.66% 0.00% 0us 1us 1.22us ( +- 4.36% ) Total Samples:610, Total events handled time:995745.54us. Signed-off-by: Dongsheng Yang Link: http://lkml.kernel.org/r/1386632823-17539-1-git-send-email-yangds.fnst@cn.fujitsu.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-kvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c index d9cc0e3f284f..c2e5d56a291d 100644 --- a/tools/perf/builtin-kvm.c +++ b/tools/perf/builtin-kvm.c @@ -1232,7 +1232,7 @@ static int read_events(struct perf_kvm_stat *kvm) .ordered_samples = true, }; struct perf_data_file file = { - .path = input_name, + .path = kvm->file_name, .mode = PERF_DATA_MODE_READ, }; From f113bee01956d4a2efde461443b09ab1020f4b17 Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Mon, 9 Dec 2013 17:47:47 -0500 Subject: [PATCH 12/34] perf archive: Remove duplicated 'runs' in man page Two 'runs' here breaks the sentence in Description of 'perf archive' command. Signed-off-by: Dongsheng Yang Link: http://lkml.kernel.org/r/78a15a9f4f500b6074a1e25917d6e8251f894628.1386629050.git.yangds.fnst@cn.fujitsu.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-archive.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/Documentation/perf-archive.txt b/tools/perf/Documentation/perf-archive.txt index 5032a142853e..ac6ecbb3e669 100644 --- a/tools/perf/Documentation/perf-archive.txt +++ b/tools/perf/Documentation/perf-archive.txt @@ -12,9 +12,9 @@ SYNOPSIS DESCRIPTION ----------- -This command runs runs perf-buildid-list --with-hits, and collects the files -with the buildids found so that analysis of perf.data contents can be possible -on another machine. +This command runs perf-buildid-list --with-hits, and collects the files with the +buildids found so that analysis of perf.data contents can be possible on another +machine. SEE ALSO From 6f1d0c866261ec1e42850b2bf760484c100b6dd6 Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Mon, 9 Dec 2013 17:47:48 -0500 Subject: [PATCH 13/34] perf annotate: Fix typo A typo in comment of builtin-annotate.c about 'that'. Signed-off-by: Dongsheng Yang Link: http://lkml.kernel.org/r/46cb069a4ce21141057a07c0b50baa9968e3228c.1386629050.git.yangds.fnst@cn.fujitsu.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-annotate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 4087ab19823c..6fd52c8fa682 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -373,7 +373,7 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused) if (argc) { /* - * Special case: if there's an argument left then assume tha + * Special case: if there's an argument left then assume that * it's a symbol filter: */ if (argc > 1) From e1a2b174dbbe08dce12bde9f05f64dbbae652bed Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Fri, 6 Dec 2013 17:25:51 -0500 Subject: [PATCH 14/34] perf kvm: Move code to generate filename for perf-kvm to function. The code in builtin-kvm.c to generate filename for perf-kvm is useful to other command such as builtin-diff. This patch move the related code form builtin-kvm.c to util/util.c and wrap them in a function named get_filename_for_perf_kvm. Signed-off-by: Dongsheng Yang Link: http://lkml.kernel.org/r/5e09a5c47e8a495e888cbdc65a6fafb2c950f529.1386368672.git.yangds.fnst@cn.fujitsu.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-kvm.c | 7 +------ tools/perf/util/util.c | 14 ++++++++++++++ tools/perf/util/util.h | 2 ++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c index c2e5d56a291d..c6fa3cbd45a9 100644 --- a/tools/perf/builtin-kvm.c +++ b/tools/perf/builtin-kvm.c @@ -1713,12 +1713,7 @@ int cmd_kvm(int argc, const char **argv, const char *prefix __maybe_unused) perf_guest = 1; if (!file_name) { - if (perf_host && !perf_guest) - file_name = strdup("perf.data.host"); - else if (!perf_host && perf_guest) - file_name = strdup("perf.data.guest"); - else - file_name = strdup("perf.data.kvm"); + file_name = get_filename_for_perf_kvm(); if (!file_name) { pr_err("Failed to allocate memory for filename\n"); diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index bae8756a4eb1..4a57609c0b43 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -482,3 +482,17 @@ int filename__read_str(const char *filename, char **buf, size_t *sizep) close(fd); return err; } + +const char *get_filename_for_perf_kvm(void) +{ + const char *filename; + + if (perf_host && !perf_guest) + filename = strdup("perf.data.host"); + else if (!perf_host && perf_guest) + filename = strdup("perf.data.guest"); + else + filename = strdup("perf.data.kvm"); + + return filename; +} diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h index 659abf30e01b..0171213d1d4d 100644 --- a/tools/perf/util/util.h +++ b/tools/perf/util/util.h @@ -321,4 +321,6 @@ void free_srcline(char *srcline); int filename__read_int(const char *filename, int *value); int filename__read_str(const char *filename, char **buf, size_t *sizep); + +const char *get_filename_for_perf_kvm(void); #endif /* GIT_COMPAT_UTIL_H */ From d8d9608fdd19f85a524db0a41bc2def5c88cbdd0 Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Fri, 6 Dec 2013 17:25:52 -0500 Subject: [PATCH 15/34] perf kvm: Make perf kvm diff support --guestmount. In manpage of perf-kvm, --guestmount is supported by diff command, but it does not work well. This patch change the extend the checking in buildid-diff from guestkallsyms or guestmodules to perf_guest. Then this checking can cover the all cases perf kvm is used for. Signed-off-by: Dongsheng Yang Link: http://lkml.kernel.org/r/72857ed89642e0633f5e88f7e7abbc9645359e8e.1386368672.git.yangds.fnst@cn.fujitsu.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-diff.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c index 3b67ea2444bd..2a85cc9a2d09 100644 --- a/tools/perf/builtin-diff.c +++ b/tools/perf/builtin-diff.c @@ -1000,8 +1000,7 @@ static int data_init(int argc, const char **argv) data__files_cnt = argc; use_default = false; } - } else if (symbol_conf.default_guest_vmlinux_name || - symbol_conf.default_guest_kallsyms) { + } else if (perf_guest) { defaults[0] = "perf.data.host"; defaults[1] = "perf.data.guest"; } From 4f24416331e9a507e953e90d4534e9a9802cbc12 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 9 Dec 2013 14:34:00 +0900 Subject: [PATCH 16/34] tools lib traceevent: Get rid of malloc_or_die() in pevent_filter_alloc() It returns NULL when allocation fails so the users should check the return value from now on. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386567251-22751-4-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 2500e75583fc..b3a61d4fe38e 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -182,7 +182,10 @@ struct event_filter *pevent_filter_alloc(struct pevent *pevent) { struct event_filter *filter; - filter = malloc_or_die(sizeof(*filter)); + filter = malloc(sizeof(*filter)); + if (filter == NULL) + return NULL; + memset(filter, 0, sizeof(*filter)); filter->pevent = pevent; pevent_ref(pevent); From 234520d3fbe43ef72268c4959f85ae326459378c Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 9 Dec 2013 14:34:04 +0900 Subject: [PATCH 17/34] tools lib traceevent: Get rid of malloc_or_die() in add_event() Make it return error value since its only caller find_event() now can handle allocation error properly. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386567251-22751-8-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index b3a61d4fe38e..2b73abfb0c9f 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -245,15 +245,19 @@ static void free_arg(struct filter_arg *arg) free(arg); } -static void add_event(struct event_list **events, +static int add_event(struct event_list **events, struct event_format *event) { struct event_list *list; - list = malloc_or_die(sizeof(*list)); + list = malloc(sizeof(*list)); + if (list == NULL) + return -1; + list->next = *events; *events = list; list->event = event; + return 0; } static int event_match(struct event_format *event, @@ -276,6 +280,7 @@ find_event(struct pevent *pevent, struct event_list **events, regex_t ereg; regex_t sreg; int match = 0; + int fail = 0; char *reg; int ret; int i; @@ -310,7 +315,10 @@ find_event(struct pevent *pevent, struct event_list **events, event = pevent->events[i]; if (event_match(event, sys_name ? &sreg : NULL, &ereg)) { match = 1; - add_event(events, event); + if (add_event(events, event) < 0) { + fail = 1; + break; + } } } @@ -320,6 +328,8 @@ find_event(struct pevent *pevent, struct event_list **events, if (!match) return -1; + if (fail) + return -2; return 0; } From 2036fcd1c7ce455424c11bdb1c8a2ac906430e2f Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 9 Dec 2013 14:34:05 +0900 Subject: [PATCH 18/34] tools lib traceevent: Get rid of die() in create_arg_item() Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386567251-22751-9-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 2b73abfb0c9f..53e48eb112c3 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -362,8 +362,11 @@ create_arg_item(struct event_format *event, const char *token, arg->value.type = type == EVENT_DQUOTE ? FILTER_STRING : FILTER_CHAR; arg->value.str = strdup(token); - if (!arg->value.str) - die("malloc string"); + if (!arg->value.str) { + free_arg(arg); + show_error(error_str, "failed to allocate string filter arg"); + return NULL; + } break; case EVENT_ITEM: /* if it is a number, then convert it */ From 28942c87e5e907f591d77547203e86ad1089b499 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 9 Dec 2013 14:34:08 +0900 Subject: [PATCH 19/34] tools lib traceevent: Get rid of malloc_or_die() in pevent_filter_add_filter_str() Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386567251-22751-12-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 53e48eb112c3..a4d5bb23a110 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -1226,7 +1226,13 @@ int pevent_filter_add_filter_str(struct event_filter *filter, else len = strlen(filter_str); - this_event = malloc_or_die(len + 1); + this_event = malloc(len + 1); + if (this_event == NULL) { + show_error(error_str, "Memory allocation failure"); + /* This can only happen when events is NULL, but still */ + free_events(events); + return -1; + } memcpy(this_event, filter_str, len); this_event[len] = 0; From 7ef2e813476273ac9c9138f002d8f4cb28e5adad Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Mon, 9 Dec 2013 14:34:09 +0900 Subject: [PATCH 20/34] tools lib traceevent: Get rid of die() in pevent_filter_clear_trivial() Change the function signature to return error code and not call die() anymore. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386567251-22751-13-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 2 +- tools/lib/traceevent/parse-filter.c | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 620c27a72960..6e23f197175f 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -860,7 +860,7 @@ int pevent_event_filtered(struct event_filter *filter, void pevent_filter_reset(struct event_filter *filter); -void pevent_filter_clear_trivial(struct event_filter *filter, +int pevent_filter_clear_trivial(struct event_filter *filter, enum filter_trivial_type type); void pevent_filter_free(struct event_filter *filter); diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index a4d5bb23a110..ab402fb2dcf7 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -1504,8 +1504,10 @@ int pevent_update_trivial(struct event_filter *dest, struct event_filter *source * @type: remove only true, false, or both * * Removes filters that only contain a TRUE or FALES boolean arg. + * + * Returns 0 on success and -1 if there was a problem. */ -void pevent_filter_clear_trivial(struct event_filter *filter, +int pevent_filter_clear_trivial(struct event_filter *filter, enum filter_trivial_type type) { struct filter_type *filter_type; @@ -1514,13 +1516,15 @@ void pevent_filter_clear_trivial(struct event_filter *filter, int i; if (!filter->filters) - return; + return 0; /* * Two steps, first get all ids with trivial filters. * then remove those ids. */ for (i = 0; i < filter->filters; i++) { + int *new_ids; + filter_type = &filter->event_filters[i]; if (filter_type->filter->type != FILTER_ARG_BOOLEAN) continue; @@ -1535,19 +1539,24 @@ void pevent_filter_clear_trivial(struct event_filter *filter, break; } - ids = realloc(ids, sizeof(*ids) * (count + 1)); - if (!ids) - die("Can't allocate ids"); + new_ids = realloc(ids, sizeof(*ids) * (count + 1)); + if (!new_ids) { + free(ids); + return -1; + } + + ids = new_ids; ids[count++] = filter_type->event_id; } if (!count) - return; + return 0; for (i = 0; i < count; i++) pevent_filter_remove_event(filter, ids[i]); free(ids); + return 0; } /** From 5cfe2c82f3eb6876cf4b55e99decea0bd015d6b8 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 9 Dec 2013 11:02:49 +0100 Subject: [PATCH 21/34] perf report: Add --header/--header-only options Currently the perf.data header is always displayed for stdio output, which is no always useful. Disabling header information by default and adding following options to control header output: --header - display header information (old default) --header-only - display header information only w/o further processing, forces stdio output Signed-off-by: Jiri Olsa Acked-by: David Ahern Cc: Corey Ashford Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386583370-1699-2-git-send-email-jolsa@redhat.com [ Added single line explaining talking about the new --header* options, to address David Ahern comment; better man page entry for the new options, from Namhyung Kim ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-report.txt | 9 +++++++++ tools/perf/builtin-report.c | 22 +++++++++++++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt index 10a279871251..8eab8a4bdeb8 100644 --- a/tools/perf/Documentation/perf-report.txt +++ b/tools/perf/Documentation/perf-report.txt @@ -237,6 +237,15 @@ OPTIONS Do not show entries which have an overhead under that percent. (Default: 0). +--header:: + Show header information in the perf.data file. This includes + various information like hostname, OS and perf version, cpu/mem + info, perf command line, event list and so on. Currently only + --stdio output supports this feature. + +--header-only:: + Show only perf.data header (forces --stdio). + SEE ALSO -------- linkperf:perf-stat[1], linkperf:perf-annotate[1] diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 8cf8e66ba594..3a14dbed387c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -49,6 +49,8 @@ struct perf_report { bool show_threads; bool inverted_callchain; bool mem_mode; + bool header; + bool header_only; int max_stack; struct perf_read_values show_threads_values; const char *pretty_printing_style; @@ -514,9 +516,6 @@ static int __cmd_report(struct perf_report *rep) return ret; } - if (use_browser <= 0) - perf_session__fprintf_info(session, stdout, rep->show_full_info); - if (rep->show_threads) perf_read_values_init(&rep->show_threads_values); @@ -820,6 +819,9 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) OPT_BOOLEAN(0, "gtk", &report.use_gtk, "Use the GTK2 interface"), OPT_BOOLEAN(0, "stdio", &report.use_stdio, "Use the stdio interface"), + OPT_BOOLEAN(0, "header", &report.header, "Show data header."), + OPT_BOOLEAN(0, "header-only", &report.header_only, + "Show only data header."), OPT_STRING('s', "sort", &sort_order, "key[,key2...]", "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline," " dso_to, dso_from, symbol_to, symbol_from, mispredict," @@ -963,6 +965,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) goto error; } + /* Force tty output for header output. */ + if (report.header || report.header_only) + use_browser = 0; + if (strcmp(input_name, "-") != 0) setup_browser(true); else { @@ -970,6 +976,16 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) perf_hpp__init(); } + if (report.header || report.header_only) { + perf_session__fprintf_info(session, stdout, + report.show_full_info); + if (report.header_only) + return 0; + } else if (use_browser == 0) { + fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n", + stdout); + } + /* * Only in the TUI browser we are doing integrated annotation, * so don't allocate extra space that won't be used in the stdio From e90debddf8f26094cd90162b9af2a8ed37ed57cb Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 9 Dec 2013 11:02:50 +0100 Subject: [PATCH 22/34] perf script: Add --header/--header-only options Currently the perf.data header is always displayed for stdio output, which is no always useful. Disabling header information by default and adding following options to control header output: --header - display header information --header-only - display header information only w/o further processing Signed-off-by: Jiri Olsa Link: http://lkml.kernel.org/n/tip-0ehaawv5xc83w6ag03c5hi10@git.kernel.org Cc: Corey Ashford Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386583370-1699-3-git-send-email-jolsa@redhat.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-script.txt | 6 ++++++ tools/perf/builtin-script.c | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt index c2a5071cf8f8..05f9a0a6784c 100644 --- a/tools/perf/Documentation/perf-script.txt +++ b/tools/perf/Documentation/perf-script.txt @@ -209,6 +209,12 @@ OPTIONS --show-mmap-events Display mmap related events (e.g. MMAP, MMAP2). +--header + Show perf.data header. + +--header-only + Show only perf.data header. + SEE ALSO -------- linkperf:perf-record[1], linkperf:perf-script-perl[1], diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c index 7a571fb7eb8a..f8ab125aac48 100644 --- a/tools/perf/builtin-script.c +++ b/tools/perf/builtin-script.c @@ -1494,6 +1494,8 @@ static int have_cmd(int argc, const char **argv) int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused) { bool show_full_info = false; + bool header = false; + bool header_only = false; char *rec_script_path = NULL; char *rep_script_path = NULL; struct perf_session *session; @@ -1532,6 +1534,8 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused) OPT_STRING('i', "input", &input_name, "file", "input file name"), OPT_BOOLEAN('d', "debug-mode", &debug_mode, "do various checks like samples ordering and lost events"), + OPT_BOOLEAN(0, "header", &header, "Show data header."), + OPT_BOOLEAN(0, "header-only", &header_only, "Show only data header."), OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name, "file", "vmlinux pathname"), OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, @@ -1748,6 +1752,12 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused) if (session == NULL) return -ENOMEM; + if (header || header_only) { + perf_session__fprintf_info(session, stdout, show_full_info); + if (header_only) + return 0; + } + script.session = session; if (cpu_list) { @@ -1755,9 +1765,6 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused) return -1; } - if (!script_name && !generate_script_lang) - perf_session__fprintf_info(session, stdout, show_full_info); - if (!no_callchain) symbol_conf.use_callchain = true; else From c7282f2efff9f115378b450b7aea51210fabb6ef Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 10:44:37 -0300 Subject: [PATCH 23/34] perf symbols: Rename [sl]name_alloc to match the members they refer to So we now have: dso->short_name dso->short_name_len dso->short_name_allocated Ditto for the 'long variants. To more quickly grasp what they refer to. Suggested-by: Ingo Molnar Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-nu228f8vlp9w0lr7c0q77dqi@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 4 ++-- tools/perf/util/dso.h | 4 ++-- tools/perf/util/machine.c | 4 ++-- tools/perf/util/symbol.c | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index a0c7c591f4b2..55c983586b05 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -465,9 +465,9 @@ void dso__delete(struct dso *dso) int i; for (i = 0; i < MAP__NR_TYPES; ++i) symbols__delete(&dso->symbols[i]); - if (dso->sname_alloc) + if (dso->short_name_allocated) free((char *)dso->short_name); - if (dso->lname_alloc) + if (dso->long_name_allocated) free(dso->long_name); dso_cache__free(&dso->cache); dso__free_a2l(dso); diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 384f2d97e38e..00a232d89607 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -89,8 +89,8 @@ struct dso { u8 has_srcline:1; u8 hit:1; u8 annotate_warned:1; - u8 sname_alloc:1; - u8 lname_alloc:1; + u8 short_name_allocated:1; + u8 long_name_allocated:1; u8 sorted_by_name; u8 loaded; u8 rel; diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index bac817ab2068..f66f309a091a 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -764,7 +764,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *mg, goto out; } dso__set_long_name(map->dso, long_name); - map->dso->lname_alloc = 1; + map->dso->long_name_allocated = 1; dso__kernel_module_get_build_id(map->dso, ""); } } @@ -936,7 +936,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine, goto out_problem; map->dso->short_name = name; - map->dso->sname_alloc = 1; + map->dso->short_name_allocated = 1; map->end = map->start + event->mmap.len; } else if (is_kernel_mmap) { const char *symbol_name = (event->mmap.filename + diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index de87dbac50a0..265a149bc43f 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1458,7 +1458,7 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map, if (filename != NULL) { err = dso__load_vmlinux(dso, map, filename, filter); if (err > 0) { - dso->lname_alloc = 1; + dso->long_name_allocated = 1; goto out; } free(filename); @@ -1468,7 +1468,7 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map, err = dso__load_vmlinux(dso, map, vmlinux_path[i], filter); if (err > 0) { dso__set_long_name(dso, strdup(vmlinux_path[i])); - dso->lname_alloc = 1; + dso->long_name_allocated = 1; break; } } @@ -1612,7 +1612,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map, if (err > 0) { dso__set_long_name(dso, strdup(symbol_conf.vmlinux_name)); - dso->lname_alloc = 1; + dso->long_name_allocated = 1; return err; } return err; From 7521ab592550d9e6542a496bcea11b40900690da Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 10:58:47 -0300 Subject: [PATCH 24/34] perf machine: Don't open code assign dso->short_name Use dso__set_short_name instead, as it will release any previously, possibly allocated, short name. Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-1v39elw7v6nxczpntpp7ljwr@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index f66f309a091a..f85da9a9a5d9 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -935,7 +935,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine, if (name == NULL) goto out_problem; - map->dso->short_name = name; + dso__set_short_name(map->dso, name); map->dso->short_name_allocated = 1; map->end = map->start + event->mmap.len; } else if (is_kernel_mmap) { From 58a98c9cc583435784a93f23754128363b4cca94 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Tue, 10 Dec 2013 11:11:46 -0300 Subject: [PATCH 25/34] perf symbols: Remove open coded management of short_name_allocated member Instead of expecting callers to set this member accodingly so that later at dso destruction it can, if needed, be correctly free()d, make it a requirement by passing it as a parameter to dso__set_short_name. Cc: Andi Kleen Cc: David Ahern Cc: Dongsheng Yang Cc: Frederic Weisbecker CC: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Steven Rostedt Signed-off-by: Adrian Hunter Link: http://lkml.kernel.org/r/52A707A2.5020802@intel.com [ Renamed the 'allocated' parameter to clearly indicate to which variable it refers to. ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 17 +++++++++++------ tools/perf/util/dso.h | 2 +- tools/perf/util/machine.c | 3 +-- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 55c983586b05..f8c849767c4d 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -379,7 +379,7 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name, * processing we had no idea this was the kernel dso. */ if (dso != NULL) { - dso__set_short_name(dso, short_name); + dso__set_short_name(dso, short_name, false); dso->kernel = dso_type; } @@ -394,17 +394,22 @@ void dso__set_long_name(struct dso *dso, char *name) dso->long_name_len = strlen(name); } -void dso__set_short_name(struct dso *dso, const char *name) +void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) { if (name == NULL) return; - dso->short_name = name; - dso->short_name_len = strlen(name); + + if (dso->short_name_allocated) + free((char *)dso->short_name); + + dso->short_name = name; + dso->short_name_len = strlen(name); + dso->short_name_allocated = name_allocated; } static void dso__set_basename(struct dso *dso) { - dso__set_short_name(dso, basename(dso->long_name)); + dso__set_short_name(dso, basename(dso->long_name), false); } int dso__name_len(const struct dso *dso) @@ -440,7 +445,7 @@ struct dso *dso__new(const char *name) int i; strcpy(dso->name, name); dso__set_long_name(dso, dso->name); - dso__set_short_name(dso, dso->name); + dso__set_short_name(dso, dso->name, false); for (i = 0; i < MAP__NR_TYPES; ++i) dso->symbols[i] = dso->symbol_names[i] = RB_ROOT; dso->cache = RB_ROOT; diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 00a232d89607..8eceab78088f 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -110,7 +110,7 @@ static inline void dso__set_loaded(struct dso *dso, enum map_type type) struct dso *dso__new(const char *name); void dso__delete(struct dso *dso); -void dso__set_short_name(struct dso *dso, const char *name); +void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated); void dso__set_long_name(struct dso *dso, char *name); int dso__name_len(const struct dso *dso); diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index f85da9a9a5d9..09d5c66d4087 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -935,8 +935,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine, if (name == NULL) goto out_problem; - dso__set_short_name(map->dso, name); - map->dso->short_name_allocated = 1; + dso__set_short_name(map->dso, name, true); map->end = map->start + event->mmap.len; } else if (is_kernel_mmap) { const char *symbol_name = (event->mmap.filename + From 5230fb7db42914e47786a6e920d2624739b0f896 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 11:58:52 -0300 Subject: [PATCH 26/34] perf symbols: Set alloc flag close to setting the long_name This is a preparatory patch to do with dso__set_long_name what was done with the short name variant. Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-mb7eqhkyejq1qcf3p22wz2x7@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/probe-event.c | 2 +- tools/perf/util/symbol.c | 31 +++++++++++-------------------- tools/perf/util/symbol.h | 3 ++- 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 9c6989ca2bea..d7cff57945c2 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -154,7 +154,7 @@ static struct dso *kernel_get_module_dso(const char *module) vmlinux_name = symbol_conf.vmlinux_name; if (vmlinux_name) { - if (dso__load_vmlinux(dso, map, vmlinux_name, NULL) <= 0) + if (dso__load_vmlinux(dso, map, vmlinux_name, false, NULL) <= 0) return NULL; } else { if (dso__load_vmlinux_path(dso, map, NULL) <= 0) { diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 265a149bc43f..9a5de8837d6d 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1408,7 +1408,8 @@ struct map *map_groups__find_by_name(struct map_groups *mg, } int dso__load_vmlinux(struct dso *dso, struct map *map, - const char *vmlinux, symbol_filter_t filter) + const char *vmlinux, bool vmlinux_allocated, + symbol_filter_t filter) { int err = -1; struct symsrc ss; @@ -1438,6 +1439,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map, else dso->data_type = DSO_BINARY_TYPE__VMLINUX; dso__set_long_name(dso, (char *)vmlinux); + dso->long_name_allocated = vmlinux_allocated; dso__set_loaded(dso, map->type); pr_debug("Using %s for symbols\n", symfs_vmlinux); } @@ -1456,21 +1458,16 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map, filename = dso__build_id_filename(dso, NULL, 0); if (filename != NULL) { - err = dso__load_vmlinux(dso, map, filename, filter); - if (err > 0) { - dso->long_name_allocated = 1; + err = dso__load_vmlinux(dso, map, filename, true, filter); + if (err > 0) goto out; - } free(filename); } for (i = 0; i < vmlinux_path__nr_entries; ++i) { - err = dso__load_vmlinux(dso, map, vmlinux_path[i], filter); - if (err > 0) { - dso__set_long_name(dso, strdup(vmlinux_path[i])); - dso->long_name_allocated = 1; + err = dso__load_vmlinux(dso, map, vmlinux_path[i], false, filter); + if (err > 0) break; - } } out: return err; @@ -1607,15 +1604,8 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map, } if (!symbol_conf.ignore_vmlinux && symbol_conf.vmlinux_name != NULL) { - err = dso__load_vmlinux(dso, map, - symbol_conf.vmlinux_name, filter); - if (err > 0) { - dso__set_long_name(dso, - strdup(symbol_conf.vmlinux_name)); - dso->long_name_allocated = 1; - return err; - } - return err; + return dso__load_vmlinux(dso, map, symbol_conf.vmlinux_name, + false, filter); } if (!symbol_conf.ignore_vmlinux && vmlinux_path != NULL) { @@ -1671,7 +1661,8 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map, */ if (symbol_conf.default_guest_vmlinux_name != NULL) { err = dso__load_vmlinux(dso, map, - symbol_conf.default_guest_vmlinux_name, filter); + symbol_conf.default_guest_vmlinux_name, + false, filter); return err; } diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h index f1031a1358a1..6de9c2b8a601 100644 --- a/tools/perf/util/symbol.h +++ b/tools/perf/util/symbol.h @@ -206,7 +206,8 @@ bool symsrc__possibly_runtime(struct symsrc *ss); int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter); int dso__load_vmlinux(struct dso *dso, struct map *map, - const char *vmlinux, symbol_filter_t filter); + const char *vmlinux, bool vmlinux_allocated, + symbol_filter_t filter); int dso__load_vmlinux_path(struct dso *dso, struct map *map, symbol_filter_t filter); int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map, From 7e155d4d5e2912f75443c18c02dd6f1dbd4eef84 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 15:08:44 -0300 Subject: [PATCH 27/34] perf symbols: Remove open coded management of long_name_allocated member Instead of expecting callers to set this member accodingly so that later at dso destruction it can, if needed, be correctly free()d, make it a requirement by passing it as a parameter to dso__set_long_name. Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-na7t1tqim22vuqkt4zq5n4ri@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 13 +++++++++---- tools/perf/util/dso.h | 2 +- tools/perf/util/machine.c | 3 +-- tools/perf/util/symbol.c | 9 ++++----- tools/perf/util/vdso.c | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index f8c849767c4d..ecb37d62f814 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -386,12 +386,17 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name, return dso; } -void dso__set_long_name(struct dso *dso, char *name) +void dso__set_long_name(struct dso *dso, char *name, bool name_allocated) { if (name == NULL) return; - dso->long_name = name; - dso->long_name_len = strlen(name); + + if (dso->long_name_allocated) + free(dso->long_name); + + dso->long_name = name; + dso->long_name_len = strlen(name); + dso->long_name_allocated = name_allocated; } void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) @@ -444,7 +449,7 @@ struct dso *dso__new(const char *name) if (dso != NULL) { int i; strcpy(dso->name, name); - dso__set_long_name(dso, dso->name); + dso__set_long_name(dso, dso->name, false); dso__set_short_name(dso, dso->name, false); for (i = 0; i < MAP__NR_TYPES; ++i) dso->symbols[i] = dso->symbol_names[i] = RB_ROOT; diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 8eceab78088f..7b434691525a 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -111,7 +111,7 @@ struct dso *dso__new(const char *name); void dso__delete(struct dso *dso); void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated); -void dso__set_long_name(struct dso *dso, char *name); +void dso__set_long_name(struct dso *dso, char *name, bool name_allocated); int dso__name_len(const struct dso *dso); diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 09d5c66d4087..751454bcde69 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -763,8 +763,7 @@ static int map_groups__set_modules_path_dir(struct map_groups *mg, ret = -1; goto out; } - dso__set_long_name(map->dso, long_name); - map->dso->long_name_allocated = 1; + dso__set_long_name(map->dso, long_name, true); dso__kernel_module_get_build_id(map->dso, ""); } } diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 9a5de8837d6d..5029ee1a9421 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1159,7 +1159,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map, dso->data_type = DSO_BINARY_TYPE__GUEST_KCORE; else dso->data_type = DSO_BINARY_TYPE__KCORE; - dso__set_long_name(dso, strdup(kcore_filename)); + dso__set_long_name(dso, strdup(kcore_filename), true); close(fd); @@ -1438,8 +1438,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map, dso->data_type = DSO_BINARY_TYPE__GUEST_VMLINUX; else dso->data_type = DSO_BINARY_TYPE__VMLINUX; - dso__set_long_name(dso, (char *)vmlinux); - dso->long_name_allocated = vmlinux_allocated; + dso__set_long_name(dso, (char *)vmlinux, vmlinux_allocated); dso__set_loaded(dso, map->type); pr_debug("Using %s for symbols\n", symfs_vmlinux); } @@ -1631,7 +1630,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map, free(kallsyms_allocated_filename); if (err > 0 && !dso__is_kcore(dso)) { - dso__set_long_name(dso, strdup("[kernel.kallsyms]")); + dso__set_long_name(dso, strdup("[kernel.kallsyms]"), true); map__fixup_start(map); map__fixup_end(map); } @@ -1679,7 +1678,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map, pr_debug("Using %s for symbols\n", kallsyms_filename); if (err > 0 && !dso__is_kcore(dso)) { machine__mmap_name(machine, path, sizeof(path)); - dso__set_long_name(dso, strdup(path)); + dso__set_long_name(dso, strdup(path), true); map__fixup_start(map); map__fixup_end(map); } diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c index 39159822d58f..0ddb3b8a89ec 100644 --- a/tools/perf/util/vdso.c +++ b/tools/perf/util/vdso.c @@ -103,7 +103,7 @@ struct dso *vdso__dso_findnew(struct list_head *head) dso = dso__new(VDSO__MAP_NAME); if (dso != NULL) { dsos__add(head, dso); - dso__set_long_name(dso, file); + dso__set_long_name(dso, file, false); } } From bf4414ae7b86cddca60a5b510954a37d30583a1f Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 15:19:23 -0300 Subject: [PATCH 28/34] perf symbols: Constify dso->long_name Same reason as for dso->short_name, it may point to a const string, and in most places it is treated as const, i.e. it is just accessed for using its contents as a key or to show it on reports. Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-nf7mxf33zt5qw207pbxxryot@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/annotate.c | 2 +- tools/perf/util/dso.c | 10 +++++----- tools/perf/util/dso.h | 4 ++-- tools/perf/util/header.c | 6 +++--- tools/perf/util/srcline.c | 2 +- tools/perf/util/symbol.c | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index cf6242c92ee2..0fcd81ea31ae 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -900,7 +900,7 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize) * cache, or is just a kallsyms file, well, lets hope that this * DSO is the same as when 'perf record' ran. */ - filename = dso->long_name; + filename = (char *)dso->long_name; snprintf(symfs_filename, sizeof(symfs_filename), "%s%s", symbol_conf.symfs, filename); free_filename = false; diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index ecb37d62f814..2c7e1899a735 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -67,7 +67,7 @@ int dso__binary_type_file(struct dso *dso, enum dso_binary_type type, case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO: { - char *last_slash; + const char *last_slash; size_t len; size_t dir_size; @@ -386,13 +386,13 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name, return dso; } -void dso__set_long_name(struct dso *dso, char *name, bool name_allocated) +void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated) { if (name == NULL) return; if (dso->long_name_allocated) - free(dso->long_name); + free((char *)dso->long_name); dso->long_name = name; dso->long_name_len = strlen(name); @@ -414,7 +414,7 @@ void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) static void dso__set_basename(struct dso *dso) { - dso__set_short_name(dso, basename(dso->long_name), false); + dso__set_short_name(dso, basename((char *)dso->long_name), false); } int dso__name_len(const struct dso *dso) @@ -478,7 +478,7 @@ void dso__delete(struct dso *dso) if (dso->short_name_allocated) free((char *)dso->short_name); if (dso->long_name_allocated) - free(dso->long_name); + free((char *)dso->long_name); dso_cache__free(&dso->cache); dso__free_a2l(dso); free(dso->symsrc_filename); diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 7b434691525a..a2d71292f746 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -96,7 +96,7 @@ struct dso { u8 rel; u8 build_id[BUILD_ID_SIZE]; const char *short_name; - char *long_name; + const char *long_name; u16 long_name_len; u16 short_name_len; char name[0]; @@ -111,7 +111,7 @@ struct dso *dso__new(const char *name); void dso__delete(struct dso *dso); void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated); -void dso__set_long_name(struct dso *dso, char *name, bool name_allocated); +void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated); int dso__name_len(const struct dso *dso); diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 125cdc9250ee..0bb830f6b49c 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -177,7 +177,7 @@ perf_header__set_cmdline(int argc, const char **argv) continue; \ else -static int write_buildid(char *name, size_t name_len, u8 *build_id, +static int write_buildid(const char *name, size_t name_len, u8 *build_id, pid_t pid, u16 misc, int fd) { int err; @@ -209,7 +209,7 @@ static int __dsos__write_buildid_table(struct list_head *head, dsos__for_each_with_build_id(pos, head) { int err; - char *name; + const char *name; size_t name_len; if (!pos->hit) @@ -387,7 +387,7 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine, { bool is_kallsyms = dso->kernel && dso->long_name[0] != '/'; bool is_vdso = is_vdso_map(dso->short_name); - char *name = dso->long_name; + const char *name = dso->long_name; char nm[PATH_MAX]; if (dso__is_kcore(dso)) { diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c index 0c075560ad46..58b2bd8f38c9 100644 --- a/tools/perf/util/srcline.c +++ b/tools/perf/util/srcline.c @@ -255,7 +255,7 @@ char *get_srcline(struct dso *dso, unsigned long addr) char *file = NULL; unsigned line = 0; char *srcline; - char *dso_name; + const char *dso_name; if (!dso->has_srcline) return SRCLINE_UNKNOWN; diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 5029ee1a9421..e377c2e96191 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -1438,7 +1438,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map, dso->data_type = DSO_BINARY_TYPE__GUEST_VMLINUX; else dso->data_type = DSO_BINARY_TYPE__VMLINUX; - dso__set_long_name(dso, (char *)vmlinux, vmlinux_allocated); + dso__set_long_name(dso, vmlinux, vmlinux_allocated); dso__set_loaded(dso, map->type); pr_debug("Using %s for symbols\n", symfs_vmlinux); } @@ -1630,7 +1630,7 @@ static int dso__load_kernel_sym(struct dso *dso, struct map *map, free(kallsyms_allocated_filename); if (err > 0 && !dso__is_kcore(dso)) { - dso__set_long_name(dso, strdup("[kernel.kallsyms]"), true); + dso__set_long_name(dso, "[kernel.kallsyms]", false); map__fixup_start(map); map__fixup_end(map); } From ee021d42238daadc7ba49274bb0ba7dff219c6ab Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 15:26:55 -0300 Subject: [PATCH 29/34] perf symbols: Set freed members to NULL in dso destructor To help in debugging use after free bugs. Reported-by: Ingo Molnar Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-3ckwsob2g1q23s77nuhexrq7@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 2c7e1899a735..19babb0d365d 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -475,13 +475,23 @@ void dso__delete(struct dso *dso) int i; for (i = 0; i < MAP__NR_TYPES; ++i) symbols__delete(&dso->symbols[i]); - if (dso->short_name_allocated) + + if (dso->short_name_allocated) { free((char *)dso->short_name); - if (dso->long_name_allocated) + dso->short_name = NULL; + dso->short_name_allocated = false; + } + + if (dso->long_name_allocated) { free((char *)dso->long_name); + dso->long_name = NULL; + dso->long_name_allocated = false; + } + dso_cache__free(&dso->cache); dso__free_a2l(dso); free(dso->symsrc_filename); + dso->symsrc_filename = NULL; free(dso); } From 3344996e4f2980be568ecf0cd59cb85e646da029 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 15:46:29 -0300 Subject: [PATCH 30/34] perf symbols: Constify some DSO methods parameters Those methods are not supposed to change the data structures they manipulate, so make that clearer by using the const qualifier in the function signature and in some variables. Suggested-by: Ingo Molnar Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-j7oyakex7zy3r82h33rdw25x@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/build-id.c | 2 +- tools/perf/util/build-id.h | 2 +- tools/perf/util/dso.c | 11 +++++------ tools/perf/util/dso.h | 4 ++-- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index a92770c98cc7..6baabe63182b 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -89,7 +89,7 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf) return raw - build_id; } -char *dso__build_id_filename(struct dso *dso, char *bf, size_t size) +char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size) { char build_id_hex[BUILD_ID_SIZE * 2 + 1]; diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h index 929f28a7c14d..845ef865eced 100644 --- a/tools/perf/util/build-id.h +++ b/tools/perf/util/build-id.h @@ -10,7 +10,7 @@ extern struct perf_tool build_id__mark_dso_hit_ops; struct dso; int build_id__sprintf(const u8 *build_id, int len, char *bf); -char *dso__build_id_filename(struct dso *dso, char *bf, size_t size); +char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size); int build_id__mark_dso_hit(struct perf_tool *tool, union perf_event *event, struct perf_sample *sample, struct perf_evsel *evsel, diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 19babb0d365d..fbc66fde6b30 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -28,7 +28,7 @@ char dso__symtab_origin(const struct dso *dso) return origin[dso->symtab_type]; } -int dso__binary_type_file(struct dso *dso, enum dso_binary_type type, +int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, char *root_dir, char *file, size_t size) { char build_id_hex[BUILD_ID_SIZE * 2 + 1]; @@ -200,11 +200,10 @@ dso_cache__free(struct rb_root *root) } } -static struct dso_cache* -dso_cache__find(struct rb_root *root, u64 offset) +static struct dso_cache *dso_cache__find(const struct rb_root *root, u64 offset) { - struct rb_node **p = &root->rb_node; - struct rb_node *parent = NULL; + struct rb_node * const *p = &root->rb_node; + const struct rb_node *parent = NULL; struct dso_cache *cache; while (*p != NULL) { @@ -566,7 +565,7 @@ void dsos__add(struct list_head *head, struct dso *dso) list_add_tail(&dso->node, head); } -struct dso *dsos__find(struct list_head *head, const char *name, bool cmp_short) +struct dso *dsos__find(const struct list_head *head, const char *name, bool cmp_short) { struct dso *pos; diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index a2d71292f746..99f3c647d683 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -128,7 +128,7 @@ void dso__read_running_kernel_build_id(struct dso *dso, int dso__kernel_module_get_build_id(struct dso *dso, const char *root_dir); char dso__symtab_origin(const struct dso *dso); -int dso__binary_type_file(struct dso *dso, enum dso_binary_type type, +int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, char *root_dir, char *file, size_t size); int dso__data_fd(struct dso *dso, struct machine *machine); @@ -143,7 +143,7 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name, const char *short_name, int dso_type); void dsos__add(struct list_head *head, struct dso *dso); -struct dso *dsos__find(struct list_head *head, const char *name, +struct dso *dsos__find(const struct list_head *head, const char *name, bool cmp_short); struct dso *__dsos__findnew(struct list_head *head, const char *name); bool __dsos__read_build_ids(struct list_head *head, bool with_hits); From 7d2a5122ca973cdf3c1469187811ae01dc07f67a Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 10 Dec 2013 16:02:50 -0300 Subject: [PATCH 31/34] perf symbols: Rename filename argument The 'file' is more commonly associated with a file descriptor of some sort, rename it to 'filename' as this is the more common idiom for a file name argument. Cc: Adrian Hunter Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-0ehaawv5xc83w6ag03c5hi10@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 32 ++++++++++++++++---------------- tools/perf/util/dso.h | 2 +- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index fbc66fde6b30..582b5d344aa3 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -29,7 +29,7 @@ char dso__symtab_origin(const struct dso *dso) } int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, - char *root_dir, char *file, size_t size) + char *root_dir, char *filename, size_t size) { char build_id_hex[BUILD_ID_SIZE * 2 + 1]; int ret = 0; @@ -38,30 +38,30 @@ int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, case DSO_BINARY_TYPE__DEBUGLINK: { char *debuglink; - strncpy(file, dso->long_name, size); - debuglink = file + dso->long_name_len; - while (debuglink != file && *debuglink != '/') + strncpy(filename, dso->long_name, size); + debuglink = filename + dso->long_name_len; + while (debuglink != filename && *debuglink != '/') debuglink--; if (*debuglink == '/') debuglink++; filename__read_debuglink(dso->long_name, debuglink, - size - (debuglink - file)); + size - (debuglink - filename)); } break; case DSO_BINARY_TYPE__BUILD_ID_CACHE: /* skip the locally configured cache if a symfs is given */ if (symbol_conf.symfs[0] || - (dso__build_id_filename(dso, file, size) == NULL)) + (dso__build_id_filename(dso, filename, size) == NULL)) ret = -1; break; case DSO_BINARY_TYPE__FEDORA_DEBUGINFO: - snprintf(file, size, "%s/usr/lib/debug%s.debug", + snprintf(filename, size, "%s/usr/lib/debug%s.debug", symbol_conf.symfs, dso->long_name); break; case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO: - snprintf(file, size, "%s/usr/lib/debug%s", + snprintf(filename, size, "%s/usr/lib/debug%s", symbol_conf.symfs, dso->long_name); break; @@ -75,14 +75,14 @@ int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, while (last_slash != dso->long_name && *last_slash != '/') last_slash--; - len = scnprintf(file, size, "%s", symbol_conf.symfs); + len = scnprintf(filename, size, "%s", symbol_conf.symfs); dir_size = last_slash - dso->long_name + 2; if (dir_size > (size - len)) { ret = -1; break; } - len += scnprintf(file + len, dir_size, "%s", dso->long_name); - len += scnprintf(file + len , size - len, ".debug%s", + len += scnprintf(filename + len, dir_size, "%s", dso->long_name); + len += scnprintf(filename + len , size - len, ".debug%s", last_slash); break; } @@ -96,7 +96,7 @@ int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, build_id__sprintf(dso->build_id, sizeof(dso->build_id), build_id_hex); - snprintf(file, size, + snprintf(filename, size, "%s/usr/lib/debug/.build-id/%.2s/%s.debug", symbol_conf.symfs, build_id_hex, build_id_hex + 2); break; @@ -104,23 +104,23 @@ int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, case DSO_BINARY_TYPE__VMLINUX: case DSO_BINARY_TYPE__GUEST_VMLINUX: case DSO_BINARY_TYPE__SYSTEM_PATH_DSO: - snprintf(file, size, "%s%s", + snprintf(filename, size, "%s%s", symbol_conf.symfs, dso->long_name); break; case DSO_BINARY_TYPE__GUEST_KMODULE: - snprintf(file, size, "%s%s%s", symbol_conf.symfs, + snprintf(filename, size, "%s%s%s", symbol_conf.symfs, root_dir, dso->long_name); break; case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE: - snprintf(file, size, "%s%s", symbol_conf.symfs, + snprintf(filename, size, "%s%s", symbol_conf.symfs, dso->long_name); break; case DSO_BINARY_TYPE__KCORE: case DSO_BINARY_TYPE__GUEST_KCORE: - snprintf(file, size, "%s", dso->long_name); + snprintf(filename, size, "%s", dso->long_name); break; default: diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index 99f3c647d683..e1cc50698137 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -129,7 +129,7 @@ int dso__kernel_module_get_build_id(struct dso *dso, const char *root_dir); char dso__symtab_origin(const struct dso *dso); int dso__binary_type_file(const struct dso *dso, enum dso_binary_type type, - char *root_dir, char *file, size_t size); + char *root_dir, char *filename, size_t size); int dso__data_fd(struct dso *dso, struct machine *machine); ssize_t dso__data_read_offset(struct dso *dso, struct machine *machine, From ac5e7f84c0e050fe19146d9bf51f69890beabcef Mon Sep 17 00:00:00 2001 From: Stephane Eranian Date: Thu, 5 Dec 2013 19:26:42 +0100 Subject: [PATCH 32/34] perf symbols: Fix bug in usage of the basename() function The basename() implementation varies a lot between systems. The Linux man page says: "basename may modify the content of the path, so it may be desirable to pass a copy when calling the function". On some other systems, the returned address may come from an internal buffer which can be reused in subsequent calls, thus the results should also be copied. The dso__set_basename() function was not doing this causing problems on some systems with wrong library names being shown by perf report, such as on Android systems. This patch fixes the problem. The patch is relative to tip.git. In v2, we clean up the comments based on Ingo's feedback. Reported-by: Ben Cheng Signed-off-by: Stephane Eranian Acked-by: Ingo Molnar Cc: Ben Cheng Cc: David Ahern Cc: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20131205182642.GA14614@quad [ v3: Fixed up wrt allocated flag now being set in dso__set_short_name ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 582b5d344aa3..436922f1f9d9 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -413,7 +413,28 @@ void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) static void dso__set_basename(struct dso *dso) { - dso__set_short_name(dso, basename((char *)dso->long_name), false); + /* + * basename() may modify path buffer, so we must pass + * a copy. + */ + char *base, *lname = strdup(dso->long_name); + + if (!lname) + return; + + /* + * basename() may return a pointer to internal + * storage which is reused in subsequent calls + * so copy the result. + */ + base = strdup(basename(lname)); + + free(lname); + + if (!base) + return; + + dso__set_short_name(dso, base, true); } int dso__name_len(const struct dso *dso) From ef517c6bee5d16404b06bee1d07f00ffb74fc21a Mon Sep 17 00:00:00 2001 From: Jean Pihet Date: Tue, 10 Dec 2013 13:24:03 +0100 Subject: [PATCH 33/34] perf tools: Add per-feature check flags Add CFLAGS and LDFLAGS for each feature to be checked. This allows to pass flags and parameters to the feature checks compilation. Also simplifies the feature check makefile, to come in a subsequent patch. Signed-off-by: Jean Pihet Acked-by: Ingo Molnar Acked-by: Jiri Olsa Cc: Ingo Molnar Cc: Jiri Olsa Cc: Will Deacon Cc: linaro-kernel@lists.linaro.org Cc: patches@linaro.org Link: http://lkml.kernel.org/r/1386678244-13535-2-git-send-email-jean.pihet@linaro.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index bae10720a136..2afb132db51a 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -102,7 +102,7 @@ endif feature_check = $(eval $(feature_check_code)) define feature_check_code - feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS="$(LDFLAGS)" LIBUNWIND_LIBS="$(LIBUNWIND_LIBS)" -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0) + feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" LIBUNWIND_LIBS="$(LIBUNWIND_LIBS)" -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0) endef feature_set = $(eval $(feature_set_code)) From 1448fef40af6079de38380c3a81bcf9994a1037d Mon Sep 17 00:00:00 2001 From: Jean Pihet Date: Tue, 10 Dec 2013 13:24:04 +0100 Subject: [PATCH 34/34] perf unwinding: Use the per-feature check flags Use the per-feature check flags for the unwinding feature in order to correctly compile the test-all, libunwind and libunwind-debug-frame feature checks. Tested on x86_64, ARMv7 and ARMv8 with and without LIBUNWIND_DIR set in 'make -C tools/perf' Signed-off-by: Jean Pihet Acked-by: Ingo Molnar Acked-by: Jiri Olsa Cc: Ingo Molnar Cc: Jiri Olsa Cc: Will Deacon Cc: linaro-kernel@lists.linaro.org Cc: patches@linaro.org Link: http://lkml.kernel.org/r/1386678244-13535-3-git-send-email-jean.pihet@linaro.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 52 +++++++++++++---------- tools/perf/config/feature-checks/Makefile | 8 ++-- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 2afb132db51a..5a1f4df3c3a8 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -36,6 +36,30 @@ ifeq ($(ARCH),arm) LIBUNWIND_LIBS = -lunwind -lunwind-arm endif +ifeq ($(LIBUNWIND_LIBS),) + NO_LIBUNWIND := 1 +else + # + # For linking with debug library, run like: + # + # make DEBUG=1 LIBUNWIND_DIR=/opt/libunwind/ + # + ifdef LIBUNWIND_DIR + LIBUNWIND_CFLAGS = -I$(LIBUNWIND_DIR)/include + LIBUNWIND_LDFLAGS = -L$(LIBUNWIND_DIR)/lib + endif + LIBUNWIND_LDFLAGS += $(LIBUNWIND_LIBS) + + # Set per-feature check compilation flags + FEATURE_CHECK_CFLAGS-libunwind = $(LIBUNWIND_CFLAGS) + FEATURE_CHECK_LDFLAGS-libunwind = $(LIBUNWIND_LDFLAGS) + FEATURE_CHECK_CFLAGS-libunwind-debug-frame = $(LIBUNWIND_CFLAGS) + FEATURE_CHECK_LDFLAGS-libunwind-debug-frame = $(LIBUNWIND_LDFLAGS) + # and the flags for the test-all case + FEATURE_CHECK_CFLAGS-all += $(LIBUNWIND_CFLAGS) + FEATURE_CHECK_LDFLAGS-all += $(LIBUNWIND_LDFLAGS) +endif + ifeq ($(NO_PERF_REGS),0) CFLAGS += -DHAVE_PERF_REGS_SUPPORT endif @@ -102,7 +126,7 @@ endif feature_check = $(eval $(feature_check_code)) define feature_check_code - feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" LIBUNWIND_LIBS="$(LIBUNWIND_LIBS)" -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0) + feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS) $(FEATURE_CHECK_CFLAGS-$(1))" LDFLAGS="$(LDFLAGS) $(FEATURE_CHECK_LDFLAGS-$(1))" -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0) endef feature_set = $(eval $(feature_set_code)) @@ -305,21 +329,7 @@ ifndef NO_LIBELF endif # NO_DWARF endif # NO_LIBELF -ifeq ($(LIBUNWIND_LIBS),) - NO_LIBUNWIND := 1 -endif - ifndef NO_LIBUNWIND - # - # For linking with debug library, run like: - # - # make DEBUG=1 LIBUNWIND_DIR=/opt/libunwind/ - # - ifdef LIBUNWIND_DIR - LIBUNWIND_CFLAGS := -I$(LIBUNWIND_DIR)/include - LIBUNWIND_LDFLAGS := -L$(LIBUNWIND_DIR)/lib - endif - ifneq ($(feature-libunwind), 1) msg := $(warning No libunwind found, disabling post unwind support. Please install libunwind-dev[el] >= 1.1); NO_LIBUNWIND := 1 @@ -334,14 +344,12 @@ ifndef NO_LIBUNWIND # non-ARM has no dwarf_find_debug_frame() function: CFLAGS += -DNO_LIBUNWIND_DEBUG_FRAME endif - endif -endif -ifndef NO_LIBUNWIND - CFLAGS += -DHAVE_LIBUNWIND_SUPPORT - EXTLIBS += $(LIBUNWIND_LIBS) - CFLAGS += $(LIBUNWIND_CFLAGS) - LDFLAGS += $(LIBUNWIND_LDFLAGS) + CFLAGS += -DHAVE_LIBUNWIND_SUPPORT + EXTLIBS += $(LIBUNWIND_LIBS) + CFLAGS += $(LIBUNWIND_CFLAGS) + LDFLAGS += $(LIBUNWIND_LDFLAGS) + endif # ifneq ($(feature-libunwind), 1) endif ifndef NO_LIBAUDIT diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile index b8bb749c3392..bc86462e80a2 100644 --- a/tools/perf/config/feature-checks/Makefile +++ b/tools/perf/config/feature-checks/Makefile @@ -32,12 +32,12 @@ CC := $(CC) -MD all: $(FILES) -BUILD = $(CC) $(CFLAGS) $(LDFLAGS) -o $(OUTPUT)$@ $@.c +BUILD = $(CC) $(CFLAGS) -o $(OUTPUT)$@ $@.c $(LDFLAGS) ############################### test-all: - $(BUILD) -Werror -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma $(LIBUNWIND_LIBS) -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl + $(BUILD) -Werror -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl test-hello: $(BUILD) @@ -70,10 +70,10 @@ test-libnuma: $(BUILD) -lnuma test-libunwind: - $(BUILD) $(LIBUNWIND_LIBS) -lelf + $(BUILD) -lelf test-libunwind-debug-frame: - $(BUILD) $(LIBUNWIND_LIBS) -lelf + $(BUILD) -lelf test-libaudit: $(BUILD) -laudit