From a55e5663761366fb883f6f25375dd68bc958b9db Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 17 Feb 2016 10:57:19 -0300 Subject: [PATCH 01/22] perf evlist: Reference count the cpu and thread maps at set_maps() We were dropping the reference we possibly held but not obtaining one for the new maps, which we will drop at perf_evlist__delete(), fix it. This was caught by Steven Noonan in some of the machines which would produce this output when caught by glibc debug mechanisms: $ sudo perf test 21 21: Test object code reading :*** Error in `perf': corrupted double-linked list: 0x00000000023ffcd0 *** ======= Backtrace: ========= /usr/lib/libc.so.6(+0x72055)[0x7f25be0f3055] /usr/lib/libc.so.6(+0x779b6)[0x7f25be0f89b6] /usr/lib/libc.so.6(+0x7a0ed)[0x7f25be0fb0ed] /usr/lib/libc.so.6(__libc_calloc+0xba)[0x7f25be0fceda] perf(parse_events_lex_init_extra+0x38)[0x4cfff8] perf(parse_events+0x55)[0x4a0615] perf(perf_evlist__config+0xcf)[0x4eeb2f] perf[0x479f82] perf(test__code_reading+0x1e)[0x47ad4e] perf(cmd_test+0x5dd)[0x46452d] perf[0x47f4e3] perf(main+0x603)[0x42c723] /usr/lib/libc.so.6(__libc_start_main+0xf0)[0x7f25be0a1610] perf(_start+0x29)[0x42c859] Further investigation using valgrind led to the reference count imbalance fixed in this patch. Reported-and-Tested-by: Steven Noonan Report-Link: http://lkml.kernel.org/r/CAKbGBLjC2Dx5vshxyGmQkcD+VwiAQLbHoXA9i7kvRB2-2opHZQ@mail.gmail.com Cc: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Namhyung Kim Cc: Wang Nan Fixes: f30a79b012e5 ("perf tools: Add reference counting for cpu_map object") Link: http://lkml.kernel.org/n/tip-j0u1bdhr47sa511sgg76kb8h@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index d81f13de2476..a7eb0eae9938 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1181,12 +1181,12 @@ void perf_evlist__set_maps(struct perf_evlist *evlist, struct cpu_map *cpus, */ if (cpus != evlist->cpus) { cpu_map__put(evlist->cpus); - evlist->cpus = cpus; + evlist->cpus = cpu_map__get(cpus); } if (threads != evlist->threads) { thread_map__put(evlist->threads); - evlist->threads = threads; + evlist->threads = thread_map__get(threads); } perf_evlist__propagate_maps(evlist); From 85723885feb823b4fc352b727ece0b6d00306c4d Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 15 Feb 2016 09:34:31 +0100 Subject: [PATCH 02/22] perf record: Add --all-user/--all-kernel options Allow user to easily switch all events to user or kernel space with simple --all-user or --all-kernel options. This will be handy within perf mem/c2c wrappers to switch easily monitoring modes. Committer note: Testing it: # perf record --all-kernel --all-user -a sleep 2 Error: option `all-user' cannot be used with all-kernel Usage: perf record [] [] or: perf record [] -- [] --all-user Configure all used events to run in user space. --all-kernel Configure all used events to run in kernel space. # perf record --all-user --all-kernel -a sleep 2 Error: option `all-kernel' cannot be used with all-user Usage: perf record [] [] or: perf record [] -- [] --all-kernel Configure all used events to run in kernel space. --all-user Configure all used events to run in user space. # perf record --all-user -a sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 1.416 MB perf.data (162 samples) ] # perf report | grep '\[k\]' # perf record --all-kernel -a sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 1.423 MB perf.data (296 samples) ] # perf report | grep '\[\.\]' # Signed-off-by: Jiri Olsa Tested-by: Arnaldo Carvalho de Melo Cc: Andi Kleen Cc: David Ahern Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1455525293-8671-2-git-send-email-jolsa@kernel.org [ Made those options to be mutually exclusive ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Documentation/perf-record.txt | 6 ++++++ tools/perf/builtin-record.c | 6 ++++++ tools/perf/perf.h | 2 ++ tools/perf/util/evsel.c | 10 ++++++++++ 4 files changed, 24 insertions(+) diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt index fbceb631387c..19aa17532a16 100644 --- a/tools/perf/Documentation/perf-record.txt +++ b/tools/perf/Documentation/perf-record.txt @@ -341,6 +341,12 @@ Specify vmlinux path which has debuginfo. --buildid-all:: Record build-id of all DSOs regardless whether it's actually hit or not. +--all-kernel:: +Configure all used events to run in kernel space. + +--all-user:: +Configure all used events to run in user space. + SEE ALSO -------- linkperf:perf-stat[1], linkperf:perf-list[1] diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 0ee0d5cd31a7..cf3a28d83066 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -1140,6 +1140,12 @@ struct option __record_options[] = { "per thread proc mmap processing timeout in ms"), OPT_BOOLEAN(0, "switch-events", &record.opts.record_switch_events, "Record context switch events"), + OPT_BOOLEAN_FLAG(0, "all-kernel", &record.opts.all_kernel, + "Configure all used events to run in kernel space.", + PARSE_OPT_EXCLUSIVE), + OPT_BOOLEAN_FLAG(0, "all-user", &record.opts.all_user, + "Configure all used events to run in user space.", + PARSE_OPT_EXCLUSIVE), OPT_STRING(0, "clang-path", &llvm_param.clang_path, "clang path", "clang binary to use for compiling BPF scriptlets"), OPT_STRING(0, "clang-opt", &llvm_param.clang_opt, "clang options", diff --git a/tools/perf/perf.h b/tools/perf/perf.h index 90129accffbe..5381a01c0610 100644 --- a/tools/perf/perf.h +++ b/tools/perf/perf.h @@ -58,6 +58,8 @@ struct record_opts { bool full_auxtrace; bool auxtrace_snapshot_mode; bool record_switch_events; + bool all_kernel; + bool all_user; unsigned int freq; unsigned int mmap_pages; unsigned int auxtrace_mmap_pages; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 467808680ee4..6ae20d0056de 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -898,6 +898,16 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts) if (evsel->precise_max) perf_event_attr__set_max_precise_ip(attr); + if (opts->all_user) { + attr->exclude_kernel = 1; + attr->exclude_user = 0; + } + + if (opts->all_kernel) { + attr->exclude_kernel = 0; + attr->exclude_user = 1; + } + /* * Apply event specific term settings, * it overloads any global configuration. From d9aade7fd27a604bbffd363e6a68416ef51bab88 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 18 Feb 2016 13:34:09 -0300 Subject: [PATCH 03/22] perf evlist: Handle -EINVAL for sample_freq > max_sample_rate in strerror_open() When running the "code reading" test we get: # perf test -v "code reading" 2>&1 | tail -5 Parsing event 'cycles:u' perf_evlist__open failed test child finished with -1 ---- end ---- Test object code reading: FAILED! # And with -vv we get the errno value, -22, i.e. -EINVAL, but we can do better and handle the case at hand, with this patch it becomes: # perf test -v "code reading" 2>&1 | tail -7 perf_evlist__open() failed! Error: Invalid argument. Hint: Check /proc/sys/kernel/perf_event_max_sample_rate. Hint: The current value is 1000 and 4000 is being requested. test child finished with -1 ---- end ---- Test object code reading: FAILED! # Next patch will make this 'perf test' entry to use perf_evlist__strerror() Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Noonan Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-i31ai6kfefn75eapejjokfhc@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index a7eb0eae9938..0f577162c699 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1624,7 +1624,7 @@ size_t perf_evlist__fprintf(struct perf_evlist *evlist, FILE *fp) return printed + fprintf(fp, "\n"); } -int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused, +int perf_evlist__strerror_open(struct perf_evlist *evlist, int err, char *buf, size_t size) { int printed, value; @@ -1652,7 +1652,25 @@ int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused, "Hint:\tTry: 'sudo sh -c \"echo -1 > /proc/sys/kernel/perf_event_paranoid\"'\n" "Hint:\tThe current value is %d.", value); break; + case EINVAL: { + struct perf_evsel *first = perf_evlist__first(evlist); + int max_freq; + + if (sysctl__read_int("kernel/perf_event_max_sample_rate", &max_freq) < 0) + goto out_default; + + if (first->attr.sample_freq < (u64)max_freq) + goto out_default; + + printed = scnprintf(buf, size, + "Error:\t%s.\n" + "Hint:\tCheck /proc/sys/kernel/perf_event_max_sample_rate.\n" + "Hint:\tThe current value is %d and %" PRIu64 " is being requested.", + emsg, max_freq, first->attr.sample_freq); + break; + } default: +out_default: scnprintf(buf, size, "%s", emsg); break; } From 6880bbf96930ec6f8b40b5b93f21973f3297672a Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 18 Feb 2016 13:40:57 -0300 Subject: [PATCH 04/22] perf tests: Use perf_evlist__strerror_open() to provide hints about max_freq Before: # perf test -v "code reading" 2>&1 | tail -4 perf_evlist__open failed test child finished with -1 ---- end ---- Test object code reading: FAILED! # After: # perf test -v "code reading" 2>&1 | tail -7 perf_evlist__open() failed! Error: Invalid argument. Hint: Check /proc/sys/kernel/perf_event_max_sample_rate. Hint: The current value is 1000 and 4000 is being requested. test child finished with -1 ---- end ---- Test object code reading: FAILED! # Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Noonan Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-ifbx7vmrc38loe6317owz2jx@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/code-reading.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c index 313a48c6b2bc..f84339cb7f95 100644 --- a/tools/perf/tests/code-reading.c +++ b/tools/perf/tests/code-reading.c @@ -559,7 +559,13 @@ static int do_test_code_reading(bool try_kcore) evlist = NULL; continue; } - pr_debug("perf_evlist__open failed\n"); + + if (verbose) { + char errbuf[512]; + perf_evlist__strerror_open(evlist, errno, errbuf, sizeof(errbuf)); + pr_debug("perf_evlist__open() failed!\n%s\n", errbuf); + } + goto out_put; } break; From 5243ba76a585a6481c4d7b931e7e3d98900cbdbe Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 18 Feb 2016 13:45:25 -0300 Subject: [PATCH 05/22] perf test: Reduce the sample_freq for the 'object code reading' test Using 4 kHz is not necessary and sometimes is more than what was auto-tuned: # dmesg | grep max_sample_rate | tail -2 [ 2499.144373] perf interrupt took too long (2501 > 2500), lowering kernel.perf_event_max_sample_rate to 50000 [ 3592.413606] perf interrupt took too long (5069 > 5000), lowering kernel.perf_event_max_sample_rate to 25000 Simulating a auto-tune of 2000 we make the test fail, as reported by Steven Noonan for one of his machines, so reduce it to 500 HZ, it is enough to get a good number of samples for this test: # perf test -v 21 2>&1 | grep '^Reading object code for memory address' | tee /tmp/out | tail -5 Reading object code for memory address: 0x479f40 Reading object code for memory address: 0x7f29b7eea80d Reading object code for memory address: 0x7f29b7eea80d Reading object code for memory address: 0x7f29b7eea800 Reading object code for memory address: 0xffffffff813b2f23 [root@jouet ~]# wc -l /tmp/out 40 /tmp/out [root@jouet ~]# For systems that auto-tune below that, the previous patches will tell the user what is happening so that he may either ignore the result of this test or bump /proc/sys/kernel/perf_event_max_sample_rate. Cc: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Cc: Steven Noonan Cc: Wang Nan Link: http://lkml.kernel.org/n/tip-6kufyy1iprdfzrbtuqgxir70@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/code-reading.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c index f84339cb7f95..afc9ad0a0515 100644 --- a/tools/perf/tests/code-reading.c +++ b/tools/perf/tests/code-reading.c @@ -439,7 +439,7 @@ static int do_test_code_reading(bool try_kcore) .mmap_pages = UINT_MAX, .user_freq = UINT_MAX, .user_interval = ULLONG_MAX, - .freq = 4000, + .freq = 500, .target = { .uses_mmap = true, }, From b002f3bbd321993c1a6d56b86544065420156ab9 Mon Sep 17 00:00:00 2001 From: Andi Kleen Date: Wed, 17 Feb 2016 14:44:00 -0800 Subject: [PATCH 06/22] perf stat: Handled scaled == -1 case for counters Arnaldo pointed out that the earlier cb110f471025 ("perf stat: Move noise/running printing into printout") change changed behavior for not counted counters. This patch fixes it again. Signed-off-by: Andi Kleen Cc: Jiri Olsa Cc: Stephane Eranian Fixes: cb110f471025 ("perf stat: Move noise/running printing into printout") Link: http://lkml.kernel.org/r/1455749045-18098-2-git-send-email-andi@firstfloor.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-stat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 15e4fcf34e0c..86289dfcb452 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -860,7 +860,7 @@ static void printout(int id, int nr, struct perf_evsel *counter, double uval, nl = new_line_std; - if (run == 0 || ena == 0) { + if (run == 0 || ena == 0 || counter->counts->scaled == -1) { aggr_printout(counter, id, nr); fprintf(stat_config.output, "%*s%s", From 80cdce7666924158af63ba5071805a28800ebe4b Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:43:51 +0000 Subject: [PATCH 07/22] perf bpf: Rename bpf_prog_priv__clear() to clear_prog_priv() The name of bpf_prog_priv__clear() doesn't follow perf's naming convention. bpf_prog_priv__delete() seems to be a better name. However, bpf_prog_priv__delete() should be a method of 'struct bpf_prog_priv', but its first parameter is 'struct bpf_program'. It is callback from libbpf to clear priv structures when destroying a bpf program. It is actually a method of bpf_program (libbpf object), but bpf_program__ functions should be provided by libbpf. This patch removes the prefix of that function. Signed-off-by: Wang Nan Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1455882283-79592-4-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/bpf-loader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c index 540a7efa657e..0bdccf423b27 100644 --- a/tools/perf/util/bpf-loader.c +++ b/tools/perf/util/bpf-loader.c @@ -108,8 +108,8 @@ void bpf__clear(void) } static void -bpf_prog_priv__clear(struct bpf_program *prog __maybe_unused, - void *_priv) +clear_prog_priv(struct bpf_program *prog __maybe_unused, + void *_priv) { struct bpf_prog_priv *priv = _priv; @@ -337,7 +337,7 @@ config_bpf_program(struct bpf_program *prog) } pr_debug("bpf: config '%s' is ok\n", config_str); - err = bpf_program__set_private(prog, priv, bpf_prog_priv__clear); + err = bpf_program__set_private(prog, priv, clear_prog_priv); if (err) { pr_debug("Failed to set priv for program '%s'\n", config_str); goto errout; From 26dee028d365fbc0e3326606a8520260b4462381 Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:43:52 +0000 Subject: [PATCH 08/22] perf tools: Fix checking asprintf return value According to man pages, asprintf returns -1 when failure. This patch fixes two incorrect return value checker. Signed-off-by: Wang Nan Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Cc: stable@vger.kernel.org # v4.4+ Fixes: ffeb883e5662 ("perf tools: Show proper error message for wrong terms of hw/sw events") Link: http://lkml.kernel.org/r/1455882283-79592-5-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index e5583fd4e7bd..72524c755b11 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2110,11 +2110,11 @@ char *parse_events_formats_error_string(char *additional_terms) /* valid terms */ if (additional_terms) { - if (!asprintf(&str, "valid terms: %s,%s", - additional_terms, static_terms)) + if (asprintf(&str, "valid terms: %s,%s", + additional_terms, static_terms) < 0) goto fail; } else { - if (!asprintf(&str, "valid terms: %s", static_terms)) + if (asprintf(&str, "valid terms: %s", static_terms) < 0) goto fail; } return str; From 17cb5f84b89fd39a143f1c899836f40420a6b799 Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:43:57 +0000 Subject: [PATCH 09/22] perf tools: Create config_term_names array config_term_names[] is introduced for future commits which will be able to retrieve the config name through the config term. Utilize this array in parse_events_formats_error_string() so the missing '{,no-}inherit' terms are added. Signed-off-by: Wang Nan Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1455882283-79592-10-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 51 ++++++++++++++++++++++++++++++++-- tools/perf/util/parse-events.h | 3 +- tools/perf/util/parse-events.l | 3 +- 3 files changed, 51 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 72524c755b11..fd085d5f5c79 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -746,6 +746,25 @@ static int check_type_val(struct parse_events_term *term, return -EINVAL; } +/* + * Update according to parse-events.l + */ +static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = { + [PARSE_EVENTS__TERM_TYPE_USER] = "", + [PARSE_EVENTS__TERM_TYPE_CONFIG] = "config", + [PARSE_EVENTS__TERM_TYPE_CONFIG1] = "config1", + [PARSE_EVENTS__TERM_TYPE_CONFIG2] = "config2", + [PARSE_EVENTS__TERM_TYPE_NAME] = "name", + [PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD] = "period", + [PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ] = "freq", + [PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE] = "branch_type", + [PARSE_EVENTS__TERM_TYPE_TIME] = "time", + [PARSE_EVENTS__TERM_TYPE_CALLGRAPH] = "call-graph", + [PARSE_EVENTS__TERM_TYPE_STACKSIZE] = "stack-size", + [PARSE_EVENTS__TERM_TYPE_NOINHERIT] = "no-inherit", + [PARSE_EVENTS__TERM_TYPE_INHERIT] = "inherit", +}; + typedef int config_term_func_t(struct perf_event_attr *attr, struct parse_events_term *term, struct parse_events_error *err); @@ -2097,6 +2116,31 @@ void parse_events_evlist_error(struct parse_events_evlist *data, WARN_ONCE(!err->str, "WARNING: failed to allocate error string"); } +static void config_terms_list(char *buf, size_t buf_sz) +{ + int i; + bool first = true; + + buf[0] = '\0'; + for (i = 0; i < __PARSE_EVENTS__TERM_TYPE_NR; i++) { + const char *name = config_term_names[i]; + + if (!name) + continue; + if (name[0] == '<') + continue; + + if (strlen(buf) + strlen(name) + 2 >= buf_sz) + return; + + if (!first) + strcat(buf, ","); + else + first = false; + strcat(buf, name); + } +} + /* * Return string contains valid config terms of an event. * @additional_terms: For terms such as PMU sysfs terms. @@ -2104,10 +2148,11 @@ void parse_events_evlist_error(struct parse_events_evlist *data, char *parse_events_formats_error_string(char *additional_terms) { char *str; - static const char *static_terms = "config,config1,config2,name," - "period,freq,branch_type,time," - "call-graph,stack-size\n"; + /* "branch_type" is the longest name */ + char static_terms[__PARSE_EVENTS__TERM_TYPE_NR * + (sizeof("branch_type") - 1)]; + config_terms_list(static_terms, sizeof(static_terms)); /* valid terms */ if (additional_terms) { if (asprintf(&str, "valid terms: %s,%s", diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 53628bf3da67..b50d50b96f95 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -68,7 +68,8 @@ enum { PARSE_EVENTS__TERM_TYPE_CALLGRAPH, PARSE_EVENTS__TERM_TYPE_STACKSIZE, PARSE_EVENTS__TERM_TYPE_NOINHERIT, - PARSE_EVENTS__TERM_TYPE_INHERIT + PARSE_EVENTS__TERM_TYPE_INHERIT, + __PARSE_EVENTS__TERM_TYPE_NR, }; struct parse_events_term { diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l index 58c5831ffd5c..99486e6a8b97 100644 --- a/tools/perf/util/parse-events.l +++ b/tools/perf/util/parse-events.l @@ -178,8 +178,7 @@ modifier_bp [rwx]{1,3} { /* - * Please update parse_events_formats_error_string any time - * new static term is added. + * Please update config_term_names when new static term is added. */ config { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG); } config1 { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CONFIG1); } From 1669e509ea25e4e3e871d913d21b1cac4a96d1e8 Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:43:58 +0000 Subject: [PATCH 10/22] perf stat: Bail out on unsupported event config modifiers 'perf stat' accepts some config terms but doesn't apply them. For example: # perf stat -e 'instructions/no-inherit/' -e 'instructions/inherit/' bash # ls # exit Performance counter stats for 'bash': 266258061 instructions/no-inherit/ 266258061 instructions/inherit/ 1.402183915 seconds time elapsed The result is confusing, because user may expect the first 'instructions' event exclude the 'ls' command. This patch forbid most of these config terms for 'perf stat'. Result: # ./perf stat -e 'instructions/no-inherit/' -e 'instructions/inherit/' bash event syntax error: 'instructions/no-inherit/' \___ 'no-inherit' is not usable in 'perf stat' ... We can add blocked config terms back when 'perf stat' really supports them. This patch also removes unavailable config term from error message: # ./perf stat -e 'instructions/badterm/' ls event syntax error: 'instructions/badterm/' \___ unknown term valid terms: config,config1,config2,name # ./perf stat -e 'cpu/badterm/' ls event syntax error: 'cpu/badterm/' \___ unknown term valid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2,name Signed-off-by: Wang Nan Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1455882283-79592-11-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-stat.c | 1 + tools/perf/util/parse-events.c | 48 ++++++++++++++++++++++++++++++++++ tools/perf/util/parse-events.h | 1 + 3 files changed, 50 insertions(+) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 86289dfcb452..8c0bc0fe5179 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -1831,6 +1831,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused) if (evsel_list == NULL) return -ENOMEM; + parse_events__shrink_config_terms(); argc = parse_options_subcommand(argc, argv, stat_options, stat_subcommands, (const char **) stat_usage, PARSE_OPT_STOP_AT_NON_OPTION); diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index fd085d5f5c79..eb5df43ec68f 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -765,6 +765,41 @@ static const char *config_term_names[__PARSE_EVENTS__TERM_TYPE_NR] = { [PARSE_EVENTS__TERM_TYPE_INHERIT] = "inherit", }; +static bool config_term_shrinked; + +static bool +config_term_avail(int term_type, struct parse_events_error *err) +{ + if (term_type < 0 || term_type >= __PARSE_EVENTS__TERM_TYPE_NR) { + err->str = strdup("Invalid term_type"); + return false; + } + if (!config_term_shrinked) + return true; + + switch (term_type) { + case PARSE_EVENTS__TERM_TYPE_CONFIG: + case PARSE_EVENTS__TERM_TYPE_CONFIG1: + case PARSE_EVENTS__TERM_TYPE_CONFIG2: + case PARSE_EVENTS__TERM_TYPE_NAME: + return true; + default: + if (!err) + return false; + + /* term_type is validated so indexing is safe */ + if (asprintf(&err->str, "'%s' is not usable in 'perf stat'", + config_term_names[term_type]) < 0) + err->str = NULL; + return false; + } +} + +void parse_events__shrink_config_terms(void) +{ + config_term_shrinked = true; +} + typedef int config_term_func_t(struct perf_event_attr *attr, struct parse_events_term *term, struct parse_events_error *err); @@ -834,6 +869,17 @@ do { \ return -EINVAL; } + /* + * Check term availbility after basic checking so + * PARSE_EVENTS__TERM_TYPE_USER can be found and filtered. + * + * If check availbility at the entry of this function, + * user will see "'' is not usable in 'perf stat'" + * if an invalid config term is provided for legacy events + * (for example, instructions/badterm/...), which is confusing. + */ + if (!config_term_avail(term->type_term, err)) + return -EINVAL; return 0; #undef CHECK_TYPE_VAL } @@ -2125,6 +2171,8 @@ static void config_terms_list(char *buf, size_t buf_sz) for (i = 0; i < __PARSE_EVENTS__TERM_TYPE_NR; i++) { const char *name = config_term_names[i]; + if (!config_term_avail(i, NULL)) + continue; if (!name) continue; if (name[0] == '<') diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index b50d50b96f95..76151f9f00d2 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -105,6 +105,7 @@ struct parse_events_terms { struct list_head *terms; }; +void parse_events__shrink_config_terms(void); int parse_events__is_hardcoded_term(struct parse_events_term *term); int parse_events_term__num(struct parse_events_term **term, int type_term, char *config, u64 num, From e814fddde18fec43fa41a27ae94c09b54772697e Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:43:59 +0000 Subject: [PATCH 11/22] perf tools: Rename and move pmu_event_name to get_config_name Following commits will make more events obey /name=newname/ options. This patch makes pmu_event_name() a generic helper. Makes new get_config_name() accept NULL input to make life easier. Signed-off-by: Wang Nan Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1455882283-79592-12-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 35 +++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index eb5df43ec68f..3243e95eb1c7 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -279,7 +279,24 @@ const char *event_type(int type) return "unknown"; } +static int parse_events__is_name_term(struct parse_events_term *term) +{ + return term->type_term == PARSE_EVENTS__TERM_TYPE_NAME; +} +static char *get_config_name(struct list_head *head_terms) +{ + struct parse_events_term *term; + + if (!head_terms) + return NULL; + + list_for_each_entry(term, head_terms, list) + if (parse_events__is_name_term(term)) + return term->val.str; + + return NULL; +} static struct perf_evsel * __add_event(struct list_head *list, int *idx, @@ -1029,22 +1046,6 @@ int parse_events_add_numeric(struct parse_events_evlist *data, return add_event(list, &data->idx, &attr, NULL, &config_terms); } -static int parse_events__is_name_term(struct parse_events_term *term) -{ - return term->type_term == PARSE_EVENTS__TERM_TYPE_NAME; -} - -static char *pmu_event_name(struct list_head *head_terms) -{ - struct parse_events_term *term; - - list_for_each_entry(term, head_terms, list) - if (parse_events__is_name_term(term)) - return term->val.str; - - return NULL; -} - int parse_events_add_pmu(struct parse_events_evlist *data, struct list_head *list, char *name, struct list_head *head_config) @@ -1089,7 +1090,7 @@ int parse_events_add_pmu(struct parse_events_evlist *data, return -EINVAL; evsel = __add_event(list, &data->idx, &attr, - pmu_event_name(head_config), pmu->cpus, + get_config_name(head_config), pmu->cpus, &config_terms); if (evsel) { evsel->unit = info.unit; From 1d55e8ef340dad1ccd5aaf53071de41fc3d8dba4 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 19 Feb 2016 18:45:12 -0300 Subject: [PATCH 12/22] perf tools: Introduce opt_event_config nonterminal To remove duplicated code that differs only in using the matching '/a,b,c/' part or NULL if no event configuration is done ('//' or no pair of slashes at all). Will be used by some new targets allowing the configuration of hardware events, etc. Lifted part of the 'opt_event_config' nonterminal from a patch by Wang Nan. Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/n/tip-e3xzpx9cqsmwnaguaxyw6r42@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.y | 47 +++++++++++++--------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index c0eac88ef474..ce68746bdc89 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -64,6 +64,7 @@ static inc_group_count(struct list_head *list, %type PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT %type value_sym %type event_config +%type opt_event_config %type event_term %type event_pmu %type event_legacy_symbol @@ -222,16 +223,6 @@ PE_NAME '/' event_config '/' $$ = list; } | -PE_NAME '/' '/' -{ - struct parse_events_evlist *data = _data; - struct list_head *list; - - ALLOC_LIST(list); - ABORT_ON(parse_events_add_pmu(data, list, $1, NULL)); - $$ = list; -} -| PE_KERNEL_PMU_EVENT sep_dc { struct parse_events_evlist *data = _data; @@ -378,7 +369,7 @@ PE_PREFIX_MEM PE_VALUE sep_dc } event_legacy_tracepoint: -tracepoint_name +tracepoint_name opt_event_config { struct parse_events_evlist *data = _data; struct parse_events_error *error = data->error; @@ -389,24 +380,7 @@ tracepoint_name error->idx = @1.first_column; if (parse_events_add_tracepoint(list, &data->idx, $1.sys, $1.event, - error, NULL)) - return -1; - - $$ = list; -} -| -tracepoint_name '/' event_config '/' -{ - struct parse_events_evlist *data = _data; - struct parse_events_error *error = data->error; - struct list_head *list; - - ALLOC_LIST(list); - if (error) - error->idx = @1.first_column; - - if (parse_events_add_tracepoint(list, &data->idx, $1.sys, $1.event, - error, $3)) + error, $2)) return -1; $$ = list; @@ -476,6 +450,21 @@ PE_BPF_SOURCE $$ = list; } +opt_event_config: +'/' event_config '/' +{ + $$ = $2; +} +| +'/' '/' +{ + $$ = NULL; +} +| +{ + $$ = NULL; +} + start_terms: event_config { struct parse_events_terms *data = _data; From 10bf358a1b79fa1311eb05ee31f2cefdcad01741 Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:44:00 +0000 Subject: [PATCH 13/22] perf tools: Enable config raw and numeric events This patch allows setting config terms for raw and numeric events. For example: # perf stat -e cycles/name=cyc/ ls ... 1821108 cyc ... # perf stat -e r6530160/name=event/ ls ... 1103195 event ... # perf record -e cycles -e 4:0x6530160/name=evtx,call-graph=fp/ -a sleep 1 ... # perf report --stdio ... # Samples: 124 of event 'cycles' 46.61% 0.00% swapper [kernel.vmlinux] [k] cpu_startup_entry 41.26% 0.00% swapper [kernel.vmlinux] [k] start_secondary ... # Samples: 91 of event 'evtx' ... 93.76% 0.00% swapper [kernel.vmlinux] [k] cpu_startup_entry | ---cpu_startup_entry | |--66.63%--call_cpuidle | cpuidle_enter | | ... 3 test cases are introduced to test config terms for symbol, raw and numeric events. Committer note: Further testing shows that we can retrieve the event name using 'perf evlist -v' and looking at the 'config' perf_event_attr field, i.e.: # perf record -e cycles -e 4:0x6530160/name=evtx,call-graph=fp/ -a sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 1.724 MB perf.data (2076 samples) ] # perf evlist cycles evtx # perf evlist -v cycles: size: 112, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CPU|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, mmap: 1, comm: 1, freq: 1, task: 1, sample_id_all: 1, exclude_guest: 1, mmap2: 1, comm_exec: 1 evtx: type: 4, size: 112, config: 0x6530160, { sample_period, sample_freq }: 4000, sample_type: IP|TID|TIME|CALLCHAIN|CPU|PERIOD|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, freq: 1, sample_id_all: 1, exclude_guest: 1 # Signed-off-by: Wang Nan Acked-by: Jiri Olsa Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1455882283-79592-13-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 40 +++++++++++++++++++++++++++++++++ tools/perf/util/parse-events.c | 3 ++- tools/perf/util/parse-events.y | 10 +++++---- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index 6648274f4601..15e2d055321e 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1271,6 +1271,31 @@ static int test__checkevent_precise_max_modifier(struct perf_evlist *evlist) return 0; } +static int test__checkevent_config_symbol(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel = perf_evlist__first(evlist); + + TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "insn") == 0); + return 0; +} + +static int test__checkevent_config_raw(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel = perf_evlist__first(evlist); + + TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "rawpmu") == 0); + return 0; +} + +static int test__checkevent_config_num(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel = perf_evlist__first(evlist); + + TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "numpmu") == 0); + return 0; +} + + static int count_tracepoints(void) { struct dirent *events_ent; @@ -1579,6 +1604,21 @@ static struct evlist_test test__events[] = { .check = test__checkevent_precise_max_modifier, .id = 47, }, + { + .name = "instructions/name=insn/", + .check = test__checkevent_config_symbol, + .id = 48, + }, + { + .name = "r1234/name=rawpmu/", + .check = test__checkevent_config_raw, + .id = 49, + }, + { + .name = "4:0x6530160/name=numpmu/", + .check = test__checkevent_config_num, + .id = 50, + }, }; static struct evlist_test test__events_pmu[] = { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 3243e95eb1c7..75576e130e16 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1043,7 +1043,8 @@ int parse_events_add_numeric(struct parse_events_evlist *data, return -ENOMEM; } - return add_event(list, &data->idx, &attr, NULL, &config_terms); + return add_event(list, &data->idx, &attr, + get_config_name(head_config), &config_terms); } int parse_events_add_pmu(struct parse_events_evlist *data, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index ce68746bdc89..82029f92c4d2 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -407,24 +407,26 @@ PE_NAME ':' PE_NAME } event_legacy_numeric: -PE_VALUE ':' PE_VALUE +PE_VALUE ':' PE_VALUE opt_event_config { struct parse_events_evlist *data = _data; struct list_head *list; ALLOC_LIST(list); - ABORT_ON(parse_events_add_numeric(data, list, (u32)$1, $3, NULL)); + ABORT_ON(parse_events_add_numeric(data, list, (u32)$1, $3, $4)); + parse_events_terms__delete($4); $$ = list; } event_legacy_raw: -PE_RAW +PE_RAW opt_event_config { struct parse_events_evlist *data = _data; struct list_head *list; ALLOC_LIST(list); - ABORT_ON(parse_events_add_numeric(data, list, PERF_TYPE_RAW, $1, NULL)); + ABORT_ON(parse_events_add_numeric(data, list, PERF_TYPE_RAW, $1, $2)); + parse_events_terms__delete($2); $$ = list; } From 43d0b97817a41b274aaec0476e912dae3ae1f93d Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Fri, 19 Feb 2016 11:44:01 +0000 Subject: [PATCH 14/22] perf tools: Enable config and setting names for legacy cache events This patch allows setting config terms for legacy cache events. For example: # perf stat -e L1-icache-misses/name=valA/ -e branches/name=valB/ ls ... Performance counter stats for 'ls': 11299 valA 451605 valB 0.000779091 seconds time elapsed # perf record -e cache-misses/name=inh/ -e cache-misses/name=noinh,no-inherit/ bash # ls # exit [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.023 MB perf.data (131 samples) ] # perf report --stdio | grep -B 1 'Event count' # Samples: 105 of event 'inh' # Event count (approx.): 109118 -- # Samples: 26 of event 'noinh' # Event count (approx.): 48302 A test case is introduced to test this feature. Signed-off-by: Wang Nan Tested-by: Arnaldo Carvalho de Melo Cc: Adrian Hunter Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Cody P Schafer Cc: He Kuang Cc: Jeremie Galarneau Cc: Jiri Olsa Cc: Kirill Smelkov Cc: Li Zefan Cc: Masami Hiramatsu Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1455882283-79592-14-git-send-email-wangnan0@huawei.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-events.c | 12 ++++++++++++ tools/perf/util/parse-events.c | 30 +++++++++++++++++++++++++++--- tools/perf/util/parse-events.h | 4 +++- tools/perf/util/parse-events.y | 18 ++++++++++++------ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c index 15e2d055321e..7865f68dc0d8 100644 --- a/tools/perf/tests/parse-events.c +++ b/tools/perf/tests/parse-events.c @@ -1295,6 +1295,13 @@ static int test__checkevent_config_num(struct perf_evlist *evlist) return 0; } +static int test__checkevent_config_cache(struct perf_evlist *evlist) +{ + struct perf_evsel *evsel = perf_evlist__first(evlist); + + TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "cachepmu") == 0); + return 0; +} static int count_tracepoints(void) { @@ -1619,6 +1626,11 @@ static struct evlist_test test__events[] = { .check = test__checkevent_config_num, .id = 50, }, + { + .name = "L1-dcache-misses/name=cachepmu/", + .check = test__checkevent_config_cache, + .id = 51, + }, }; static struct evlist_test test__events_pmu[] = { diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 75576e130e16..2996aa4207bd 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -350,11 +350,25 @@ static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES] return -1; } +typedef int config_term_func_t(struct perf_event_attr *attr, + struct parse_events_term *term, + struct parse_events_error *err); +static int config_term_common(struct perf_event_attr *attr, + struct parse_events_term *term, + struct parse_events_error *err); +static int config_attr(struct perf_event_attr *attr, + struct list_head *head, + struct parse_events_error *err, + config_term_func_t config_term); + int parse_events_add_cache(struct list_head *list, int *idx, - char *type, char *op_result1, char *op_result2) + char *type, char *op_result1, char *op_result2, + struct parse_events_error *error, + struct list_head *head_config) { struct perf_event_attr attr; - char name[MAX_NAME_LEN]; + LIST_HEAD(config_terms); + char name[MAX_NAME_LEN], *config_name; int cache_type = -1, cache_op = -1, cache_result = -1; char *op_result[2] = { op_result1, op_result2 }; int i, n; @@ -368,6 +382,7 @@ int parse_events_add_cache(struct list_head *list, int *idx, if (cache_type == -1) return -EINVAL; + config_name = get_config_name(head_config); n = snprintf(name, MAX_NAME_LEN, "%s", type); for (i = 0; (i < 2) && (op_result[i]); i++) { @@ -408,7 +423,16 @@ int parse_events_add_cache(struct list_head *list, int *idx, memset(&attr, 0, sizeof(attr)); attr.config = cache_type | (cache_op << 8) | (cache_result << 16); attr.type = PERF_TYPE_HW_CACHE; - return add_event(list, idx, &attr, name, NULL); + + if (head_config) { + if (config_attr(&attr, head_config, error, + config_term_common)) + return -EINVAL; + + if (get_config_terms(head_config, &config_terms)) + return -ENOMEM; + } + return add_event(list, idx, &attr, config_name ? : name, &config_terms); } static void tracepoint_error(struct parse_events_error *e, int err, diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index 76151f9f00d2..d5eb2af78826 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -140,7 +140,9 @@ int parse_events_add_numeric(struct parse_events_evlist *data, u32 type, u64 config, struct list_head *head_config); int parse_events_add_cache(struct list_head *list, int *idx, - char *type, char *op_result1, char *op_result2); + char *type, char *op_result1, char *op_result2, + struct parse_events_error *error, + struct list_head *head_config); int parse_events_add_breakpoint(struct list_head *list, int *idx, void *ptr, char *type, u64 len); int parse_events_add_pmu(struct parse_events_evlist *data, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 82029f92c4d2..6a2d006ea77f 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -293,33 +293,39 @@ value_sym sep_slash_dc } event_legacy_cache: -PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT +PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT opt_event_config { struct parse_events_evlist *data = _data; + struct parse_events_error *error = data->error; struct list_head *list; ALLOC_LIST(list); - ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, $5)); + ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, $5, error, $6)); + parse_events_terms__delete($6); $$ = list; } | -PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT +PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT opt_event_config { struct parse_events_evlist *data = _data; + struct parse_events_error *error = data->error; struct list_head *list; ALLOC_LIST(list); - ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, NULL)); + ABORT_ON(parse_events_add_cache(list, &data->idx, $1, $3, NULL, error, $4)); + parse_events_terms__delete($4); $$ = list; } | -PE_NAME_CACHE_TYPE +PE_NAME_CACHE_TYPE opt_event_config { struct parse_events_evlist *data = _data; + struct parse_events_error *error = data->error; struct list_head *list; ALLOC_LIST(list); - ABORT_ON(parse_events_add_cache(list, &data->idx, $1, NULL, NULL)); + ABORT_ON(parse_events_add_cache(list, &data->idx, $1, NULL, NULL, error, $2)); + parse_events_terms__delete($2); $$ = list; } From 467ef10c68b90b940412390dcd14bbfe8cc40e73 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:19 +0900 Subject: [PATCH 15/22] perf hists browser: Fix percentage update on key press Currently 'perf top --tui' decrements percentage of all entries on any key press. This is because it adds total period as new samples are added to hists. As perf-top does it currently but added samples are not passed to the display thread, the percentages are decresing continuously. So separate total period stat into a different variable so that it cannot affect the output total period. This new total period stats are used only for calcualating callchain percent limit. Signed-off-by: Namhyung Kim Tested-by: Arnaldo Carvalho de Melo Cc: Andi Kleen Cc: David Ahern Cc: Jiri Olsa Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Fixes: 0f58474ec835 ("perf hists: Update hists' total period when adding entries") Link: http://lkml.kernel.org/r/1455631723-17345-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/hist.c | 26 +++++++++++++++++++------- tools/perf/util/hist.h | 2 ++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 561e9473a915..a856617be744 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -405,6 +405,16 @@ static u8 symbol__parent_filter(const struct symbol *parent) return 0; } +static void hist_entry__add_callchain_period(struct hist_entry *he, u64 period) +{ + if (!symbol_conf.use_callchain) + return; + + he->hists->callchain_period += period; + if (!he->filtered) + he->hists->callchain_non_filtered_period += period; +} + static struct hist_entry *hists__findnew_entry(struct hists *hists, struct hist_entry *entry, struct addr_location *al, @@ -434,9 +444,7 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists, if (!cmp) { if (sample_self) { he_stat__add_period(&he->stat, period, weight); - hists->stats.total_period += period; - if (!he->filtered) - hists->stats.total_non_filtered_period += period; + hist_entry__add_callchain_period(he, period); } if (symbol_conf.cumulate_callchain) he_stat__add_period(he->stat_acc, period, weight); @@ -471,9 +479,8 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists, return NULL; if (sample_self) - hists__inc_stats(hists, he); - else - hists->nr_entries++; + hist_entry__add_callchain_period(he, period); + hists->nr_entries++; rb_link_node(&he->rb_node_in, parent, p); rb_insert_color(&he->rb_node_in, hists->entries_in); @@ -1227,9 +1234,14 @@ static void output_resort(struct hists *hists, struct ui_progress *prog, struct rb_root *root; struct rb_node *next; struct hist_entry *n; + u64 callchain_total; u64 min_callchain_hits; - min_callchain_hits = hists__total_period(hists) * (callchain_param.min_percent / 100); + callchain_total = hists->callchain_period; + if (symbol_conf.filter_relative) + callchain_total = hists->callchain_non_filtered_period; + + min_callchain_hits = callchain_total * (callchain_param.min_percent / 100); if (sort__need_collapse) root = &hists->entries_collapsed; diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 840b6d6aa44f..045a9e785a34 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -66,6 +66,8 @@ struct hists { struct rb_root entries_collapsed; u64 nr_entries; u64 nr_non_filtered_entries; + u64 callchain_period; + u64 callchain_non_filtered_period; struct thread *thread_filter; const struct dso *dso_filter; const char *uid_filter_str; From 7565bd39c1a63c82350d26a66ea1a1f1bb49ad2e Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:20 +0900 Subject: [PATCH 16/22] perf callchain: Check return value of add_child() The create_child() in add_child() can return NULL in case of memory allocation failure. So check the return value and bail out. The proper error handling will be added later. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-3-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/callchain.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index 53c43eb9489e..134d88b33fc1 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -453,6 +453,9 @@ add_child(struct callchain_node *parent, struct callchain_node *new; new = create_child(parent, false); + if (new == NULL) + return NULL; + fill_node(new, cursor); new->children_hit = 0; @@ -524,6 +527,8 @@ split_add_child(struct callchain_node *parent, node = callchain_cursor_current(cursor); new = add_child(parent, cursor, period); + if (new == NULL) + return; /* * This is second child since we moved parent's children @@ -585,6 +590,9 @@ append_chain_children(struct callchain_node *root, } /* nothing in children, add to the current node */ rnode = add_child(root, cursor, period); + if (rnode == NULL) + return; + rb_link_node(&rnode->rb_node_in, parent, p); rb_insert_color(&rnode->rb_node_in, &root->rb_root_in); From 8451cbb9b174a9b6e016d7f1bff81ff12dbd1990 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:21 +0900 Subject: [PATCH 17/22] perf callchain: Check return value of fill_node() Memory allocation in the fill_node() can fail so change its return type to int and check it in add_child() too. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-4-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/callchain.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index 134d88b33fc1..a82ea6f6fc0f 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -416,7 +416,7 @@ create_child(struct callchain_node *parent, bool inherit_children) /* * Fill the node with callchain values */ -static void +static int fill_node(struct callchain_node *node, struct callchain_cursor *cursor) { struct callchain_cursor_node *cursor_node; @@ -433,7 +433,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor) call = zalloc(sizeof(*call)); if (!call) { perror("not enough memory for the code path tree"); - return; + return -1; } call->ip = cursor_node->ip; call->ms.sym = cursor_node->sym; @@ -443,6 +443,7 @@ fill_node(struct callchain_node *node, struct callchain_cursor *cursor) callchain_cursor_advance(cursor); cursor_node = callchain_cursor_current(cursor); } + return 0; } static struct callchain_node * @@ -456,7 +457,16 @@ add_child(struct callchain_node *parent, if (new == NULL) return NULL; - fill_node(new, cursor); + if (fill_node(new, cursor) < 0) { + struct callchain_list *call, *tmp; + + list_for_each_entry_safe(call, tmp, &new->val, list) { + list_del(&call->list); + free(call); + } + free(new); + return NULL; + } new->children_hit = 0; new->hit = period; From 2d713b809d89a3d10c6a85162bf7cce0468e45d9 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:22 +0900 Subject: [PATCH 18/22] perf callchain: Add enum match_result for match_chain() The append_chain() might return either result of match_chain() or other (error) code. But match_chain() can return any value in s64 type so it's hard to check the error case. Add new enum match_result and make match_chain() return non-negative values only so that we can check the error cases. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-5-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/callchain.c | 52 ++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index a82ea6f6fc0f..dab2c1f1e86b 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -475,16 +475,32 @@ add_child(struct callchain_node *parent, return new; } -static s64 match_chain(struct callchain_cursor_node *node, - struct callchain_list *cnode) +enum match_result { + MATCH_ERROR = -1, + MATCH_EQ, + MATCH_LT, + MATCH_GT, +}; + +static enum match_result match_chain(struct callchain_cursor_node *node, + struct callchain_list *cnode) { struct symbol *sym = node->sym; + u64 left, right; if (cnode->ms.sym && sym && - callchain_param.key == CCKEY_FUNCTION) - return cnode->ms.sym->start - sym->start; - else - return cnode->ip - node->ip; + callchain_param.key == CCKEY_FUNCTION) { + left = cnode->ms.sym->start; + right = sym->start; + } else { + left = cnode->ip; + right = node->ip; + } + + if (left == right) + return MATCH_EQ; + + return left > right ? MATCH_GT : MATCH_LT; } /* @@ -549,7 +565,7 @@ split_add_child(struct callchain_node *parent, cnode = list_first_entry(&first->val, struct callchain_list, list); - if (match_chain(node, cnode) < 0) + if (match_chain(node, cnode) == MATCH_LT) pp = &p->rb_left; else pp = &p->rb_right; @@ -562,7 +578,7 @@ split_add_child(struct callchain_node *parent, } } -static int +static enum match_result append_chain(struct callchain_node *root, struct callchain_cursor *cursor, u64 period); @@ -583,17 +599,17 @@ append_chain_children(struct callchain_node *root, /* lookup in childrens */ while (*p) { - s64 ret; + enum match_result ret; parent = *p; rnode = rb_entry(parent, struct callchain_node, rb_node_in); /* If at least first entry matches, rely to children */ ret = append_chain(rnode, cursor, period); - if (ret == 0) + if (ret == MATCH_EQ) goto inc_children_hit; - if (ret < 0) + if (ret == MATCH_LT) p = &parent->rb_left; else p = &parent->rb_right; @@ -611,7 +627,7 @@ append_chain_children(struct callchain_node *root, root->children_count++; } -static int +static enum match_result append_chain(struct callchain_node *root, struct callchain_cursor *cursor, u64 period) @@ -620,7 +636,7 @@ append_chain(struct callchain_node *root, u64 start = cursor->pos; bool found = false; u64 matches; - int cmp = 0; + enum match_result cmp = MATCH_ERROR; /* * Lookup in the current node @@ -636,7 +652,7 @@ append_chain(struct callchain_node *root, break; cmp = match_chain(node, cnode); - if (cmp) + if (cmp != MATCH_EQ) break; found = true; @@ -646,7 +662,7 @@ append_chain(struct callchain_node *root, /* matches not, relay no the parent */ if (!found) { - WARN_ONCE(!cmp, "Chain comparison error\n"); + WARN_ONCE(cmp == MATCH_ERROR, "Chain comparison error\n"); return cmp; } @@ -655,20 +671,20 @@ append_chain(struct callchain_node *root, /* we match only a part of the node. Split it and add the new chain */ if (matches < root->val_nr) { split_add_child(root, cursor, cnode, start, matches, period); - return 0; + return MATCH_EQ; } /* we match 100% of the path, increment the hit */ if (matches == root->val_nr && cursor->pos == cursor->nr) { root->hit += period; root->count++; - return 0; + return MATCH_EQ; } /* We match the node and still have a part remaining */ append_chain_children(root, cursor, period); - return 0; + return MATCH_EQ; } int callchain_append(struct callchain_root *root, From f2bb4c5af4fe16d8b1e4ae371e1ceaa817380a88 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:23 +0900 Subject: [PATCH 19/22] perf callchain: Check return value of split_add_child() Now create_child() and add_child() return errors so check and pass it to the caller. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-6-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/callchain.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index dab2c1f1e86b..5259379892e1 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -508,7 +508,7 @@ static enum match_result match_chain(struct callchain_cursor_node *node, * give a part of its callchain to the created child. * Then create another child to host the given callchain of new branch */ -static void +static int split_add_child(struct callchain_node *parent, struct callchain_cursor *cursor, struct callchain_list *to_split, @@ -520,6 +520,8 @@ split_add_child(struct callchain_node *parent, /* split */ new = create_child(parent, true); + if (new == NULL) + return -1; /* split the callchain and move a part to the new child */ old_tail = parent->val.prev; @@ -554,7 +556,7 @@ split_add_child(struct callchain_node *parent, node = callchain_cursor_current(cursor); new = add_child(parent, cursor, period); if (new == NULL) - return; + return -1; /* * This is second child since we moved parent's children @@ -576,6 +578,7 @@ split_add_child(struct callchain_node *parent, parent->hit = period; parent->count = 1; } + return 0; } static enum match_result @@ -670,7 +673,10 @@ append_chain(struct callchain_node *root, /* we match only a part of the node. Split it and add the new chain */ if (matches < root->val_nr) { - split_add_child(root, cursor, cnode, start, matches, period); + if (split_add_child(root, cursor, cnode, start, matches, + period) < 0) + return MATCH_ERROR; + return MATCH_EQ; } From dca0d122e498c054b117bd4aa5568ce90ee142d5 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:24 +0900 Subject: [PATCH 20/22] perf callchain: Check return value of append_chain_children() Now it can check the error case, so check and pass it to the caller. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-7-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/callchain.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c index 5259379892e1..24b4bd0d7754 100644 --- a/tools/perf/util/callchain.c +++ b/tools/perf/util/callchain.c @@ -586,7 +586,7 @@ append_chain(struct callchain_node *root, struct callchain_cursor *cursor, u64 period); -static void +static int append_chain_children(struct callchain_node *root, struct callchain_cursor *cursor, u64 period) @@ -598,7 +598,7 @@ append_chain_children(struct callchain_node *root, node = callchain_cursor_current(cursor); if (!node) - return; + return -1; /* lookup in childrens */ while (*p) { @@ -611,6 +611,8 @@ append_chain_children(struct callchain_node *root, ret = append_chain(rnode, cursor, period); if (ret == MATCH_EQ) goto inc_children_hit; + if (ret == MATCH_ERROR) + return -1; if (ret == MATCH_LT) p = &parent->rb_left; @@ -620,7 +622,7 @@ append_chain_children(struct callchain_node *root, /* nothing in children, add to the current node */ rnode = add_child(root, cursor, period); if (rnode == NULL) - return; + return -1; rb_link_node(&rnode->rb_node_in, parent, p); rb_insert_color(&rnode->rb_node_in, &root->rb_root_in); @@ -628,6 +630,7 @@ append_chain_children(struct callchain_node *root, inc_children_hit: root->children_hit += period; root->children_count++; + return 0; } static enum match_result @@ -688,7 +691,8 @@ append_chain(struct callchain_node *root, } /* We match the node and still have a part remaining */ - append_chain_children(root, cursor, period); + if (append_chain_children(root, cursor, period) < 0) + return MATCH_ERROR; return MATCH_EQ; } @@ -702,7 +706,8 @@ int callchain_append(struct callchain_root *root, callchain_cursor_commit(cursor); - append_chain_children(&root->node, cursor, period); + if (append_chain_children(&root->node, cursor, period) < 0) + return -1; if (cursor->nr > root->max_depth) root->max_depth = cursor->nr; @@ -730,7 +735,8 @@ merge_chain_branch(struct callchain_cursor *cursor, if (src->hit) { callchain_cursor_commit(cursor); - append_chain_children(dst, cursor, src->hit); + if (append_chain_children(dst, cursor, src->hit) < 0) + return -1; } n = rb_first(&src->rb_root_in); From bba58cdfaace2eb96d2b3cabc610d2ba033371c8 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:25 +0900 Subject: [PATCH 21/22] perf hists: Return error from hists__collapse_resort() Currently hists__collapse_resort() and hists__collapse_insert_entry() don't return an error code. Now that callchain_merge() can check for errors, abort and pass the error to the user. A later patch can add more work which also can fail. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-8-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/hist.c | 29 +++++++++++++++++++---------- tools/perf/util/hist.h | 4 ++-- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index a856617be744..827c6cbcd05d 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -1046,8 +1046,8 @@ int hist_entry__snprintf_alignment(struct hist_entry *he, struct perf_hpp *hpp, * collapse the histogram */ -bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, - struct rb_root *root, struct hist_entry *he) +int hists__collapse_insert_entry(struct hists *hists, struct rb_root *root, + struct hist_entry *he) { struct rb_node **p = &root->rb_node; struct rb_node *parent = NULL; @@ -1061,18 +1061,21 @@ bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, cmp = hist_entry__collapse(iter, he); if (!cmp) { + int ret = 0; + he_stat__add_stat(&iter->stat, &he->stat); if (symbol_conf.cumulate_callchain) he_stat__add_stat(iter->stat_acc, he->stat_acc); if (symbol_conf.use_callchain) { callchain_cursor_reset(&callchain_cursor); - callchain_merge(&callchain_cursor, - iter->callchain, - he->callchain); + if (callchain_merge(&callchain_cursor, + iter->callchain, + he->callchain) < 0) + ret = -1; } hist_entry__delete(he); - return false; + return ret; } if (cmp < 0) @@ -1084,7 +1087,7 @@ bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, rb_link_node(&he->rb_node_in, parent, p); rb_insert_color(&he->rb_node_in, root); - return true; + return 1; } struct rb_root *hists__get_rotate_entries_in(struct hists *hists) @@ -1110,14 +1113,15 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he) hists__filter_entry_by_socket(hists, he); } -void hists__collapse_resort(struct hists *hists, struct ui_progress *prog) +int hists__collapse_resort(struct hists *hists, struct ui_progress *prog) { struct rb_root *root; struct rb_node *next; struct hist_entry *n; + int ret; if (!sort__need_collapse) - return; + return 0; hists->nr_entries = 0; @@ -1132,7 +1136,11 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog) next = rb_next(&n->rb_node_in); rb_erase(&n->rb_node_in, root); - if (hists__collapse_insert_entry(hists, &hists->entries_collapsed, n)) { + ret = hists__collapse_insert_entry(hists, &hists->entries_collapsed, n); + if (ret < 0) + return -1; + + if (ret) { /* * If it wasn't combined with one of the entries already * collapsed, we need to apply the filters that may have @@ -1143,6 +1151,7 @@ void hists__collapse_resort(struct hists *hists, struct ui_progress *prog) if (prog) ui_progress__update(prog, 1); } + return 0; } static int hist_entry__sort(struct hist_entry *a, struct hist_entry *b) diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 045a9e785a34..97baa1d6ae5f 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -138,7 +138,7 @@ void hist_entry__delete(struct hist_entry *he); void perf_evsel__output_resort(struct perf_evsel *evsel, struct ui_progress *prog); void hists__output_resort(struct hists *hists, struct ui_progress *prog); -void hists__collapse_resort(struct hists *hists, struct ui_progress *prog); +int hists__collapse_resort(struct hists *hists, struct ui_progress *prog); void hists__decay_entries(struct hists *hists, bool zap_user, bool zap_kernel); void hists__delete_entries(struct hists *hists); @@ -197,7 +197,7 @@ int hists__init(void); int __hists__init(struct hists *hists, struct perf_hpp_list *hpp_list); struct rb_root *hists__get_rotate_entries_in(struct hists *hists); -bool hists__collapse_insert_entry(struct hists *hists __maybe_unused, +int hists__collapse_insert_entry(struct hists *hists, struct rb_root *root, struct hist_entry *he); struct perf_hpp { From 5b2ea6f2f6ac81a230e6cc68e1473e796a583f00 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 16 Feb 2016 23:08:26 +0900 Subject: [PATCH 22/22] perf report: Check error during report__collapse_hists() If it returns an error, warn user and bail out instead of silently ignoring it. Signed-off-by: Namhyung Kim Acked-by: Jiri Olsa Cc: Andi Kleen Cc: David Ahern Cc: Peter Zijlstra Cc: Stephane Eranian Cc: Wang Nan Link: http://lkml.kernel.org/r/1455631723-17345-9-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 1eab50ac1ef6..760e886ca9d9 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -469,10 +469,11 @@ static int report__browse_hists(struct report *rep) return ret; } -static void report__collapse_hists(struct report *rep) +static int report__collapse_hists(struct report *rep) { struct ui_progress prog; struct perf_evsel *pos; + int ret = 0; ui_progress__init(&prog, rep->nr_entries, "Merging related events..."); @@ -484,7 +485,9 @@ static void report__collapse_hists(struct report *rep) hists->socket_filter = rep->socket_filter; - hists__collapse_resort(hists, &prog); + ret = hists__collapse_resort(hists, &prog); + if (ret < 0) + break; /* Non-group events are considered as leader */ if (symbol_conf.event_group && @@ -497,6 +500,7 @@ static void report__collapse_hists(struct report *rep) } ui_progress__finish(); + return ret; } static void report__output_resort(struct report *rep) @@ -564,7 +568,11 @@ static int __cmd_report(struct report *rep) } } - report__collapse_hists(rep); + ret = report__collapse_hists(rep); + if (ret) { + ui__error("failed to process hist entry\n"); + return ret; + } if (session_done()) return 0;