perf/core improvements and fixes
Subsystems (2)
Arnaldo Carvalho de Melo
maintainerHi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit 3b364d7b587db0f0eeafde0f271e0698187de776:
perf/core: Remove unused arguments from a bunch of functions (2016-02-17 10:37:48 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
for you to fetch changes up to 5b2ea6f2f6ac81a230e6cc68e1473e796a583f00:
perf report: Check error during report__collapse_hists() (2016-02-19 19:17:50 -0300)
----------------------------------------------------------------
perf/core improvements and fixes:
User visible:
- Add 'perf record' --all-user/--all-kernel options, so that one can tell that all the events in the command line should be restricted to the user or kernel levels (Jiri Olsa), i.e.:
perf record -e cycles:u,instructions:u
is equivalent to:
perf record --all-user -e cycles,instructions
- Fix percentage update on key press, due to the buffering code (that creates hist_entries that will later be consumed) touching per hists state that is used by the display thread (Namhyung Kim)
- Bail out when event modifiers not supported by 'perf stat' are specified, i.e.: (Wang Nan)
# perf stat -e cycles/no-inherit/ usleep 1
event syntax error: 'cycles/no-inherit/'
\___ 'no-inherit' is not usable in 'perf stat'
# perf stat -e cycles/foo/ usleep 1
event syntax error: 'cycles/foo/'
\___ unknown term valid terms: config,config1,config2,name
#
- Enable setting names for legacy cache, raw and numeric events, e.g: (Wang Nan)
# 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.659 MB perf.data (844 samples) ] # perf evlist cycles evtx #
Miscelaneous/Infrastructure:
- Handled scaled == -1 case for counters in 'perf stat', fixing recent, only in perf/core, regression (Andi Kleen)
- Reference count the cpu and thread maps at set_maps(), fixing the 'object code reading' 'perf test' entry when it was requesting a perf_event_attr.sample_freq > /proc/sys/kernel/perf_event_max_sample_rate (Arnaldo Carvalho de Melo)
- Improve perf_evlist__strerror_open() to provide hints for -EINVAL due to perf_event_attr.sample_freq > /proc/sys/kernel/perf_event_max_sample_rate (Arnaldo Carvalho de Melo)
- Add checks to various callchain and histogram routines (Namhyung Kim)
- Fix checking asprintf return value when parsing additional event config terms (Wang Nan)
----------------------------------------------------------------
Andi Kleen (1):
perf stat: Handled scaled == -1 case for counters
Arnaldo Carvalho de Melo (5):
perf evlist: Reference count the cpu and thread maps at set_maps() perf evlist: Handle -EINVAL for sample_freq > max_sample_rate in strerror_open() perf tests: Use perf_evlist__strerror_open() to provide hints about max_freq perf test: Reduce the sample_freq for the 'object code reading' test perf tools: Introduce opt_event_config nonterminal
Jiri Olsa (1):
perf record: Add --all-user/--all-kernel options
Namhyung Kim (8):
perf hists browser: Fix percentage update on key press perf callchain: Check return value of add_child() perf callchain: Check return value of fill_node() perf callchain: Add enum match_result for match_chain() perf callchain: Check return value of split_add_child() perf callchain: Check return value of append_chain_children() perf hists: Return error from hists__collapse_resort() perf report: Check error during report__collapse_hists()
Wang Nan (7):
perf bpf: Rename bpf_prog_priv__clear() to clear_prog_priv() perf tools: Fix checking asprintf return value
perf tools: Create config_term_names array
perf stat: Bail out on unsupported event config modifiers perf tools: Rename and move pmu_event_name to get_config_name perf tools: Enable config raw and numeric events
perf tools: Enable config and setting names for legacy cache events
tools/perf/Documentation/perf-record.txt | 6 ++ tools/perf/builtin-record.c | 6 ++ tools/perf/builtin-report.c | 14 ++- tools/perf/builtin-stat.c | 3 +- tools/perf/perf.h | 2 + tools/perf/tests/code-reading.c | 10 +- tools/perf/tests/parse-events.c | 52 ++++++++++ tools/perf/util/bpf-loader.c | 6 +- tools/perf/util/callchain.c | 102 +++++++++++++----- tools/perf/util/evlist.c | 24 ++++- tools/perf/util/evsel.c | 10 ++ tools/perf/util/hist.c | 55 +++++++--- tools/perf/util/hist.h | 6 +- tools/perf/util/parse-events.c | 173 ++++++++++++++++++++++++++----- tools/perf/util/parse-events.h | 8 +- tools/perf/util/parse-events.l | 3 +- tools/perf/util/parse-events.y | 75 +++++++------- 17 files changed, 426 insertions(+), 129 deletions(-)
Arnaldo Carvalho de Melo
maintainerPATCH 1/22 perf evlist: Reference count the cpu and thread maps at set_maps()
From: Arnaldo Carvalho de Melo <[email protected]>
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.
Report-Link: http://lkml.kernel.org/r/CAKbGBLjC2Dx5vshxyGmQkcD+VwiAQLbHoXA9i7kvRB2-2opHZQ@mail.gmail.com
--- tools/perf/util/evlist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/util/evlist.c +2 -2
@@ -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);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 2/22 perf record: Add --all-user/--all-kernel options
In reply to Arnaldo Carvalho de MeloFrom: Jiri Olsa <[email protected]>
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 [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>] --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 [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>] --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 '\[\.\]' #
--- 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(+)
tools/perf/Documentation/perf-record.txt +6 -0
@@ -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]
tools/perf/builtin-record.c +6 -0
@@ -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",
tools/perf/perf.h +2 -0
@@ -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;
tools/perf/util/evsel.c +10 -0
@@ -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.
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 3/22 perf evlist: Handle -EINVAL for sample_freq > max_sample_rate in strerror_open()
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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()
--- tools/perf/util/evlist.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
tools/perf/util/evlist.c +19 -1
@@ -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 ; }
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 4/22 perf tests: Use perf_evlist__strerror_open() to provide hints about max_freq
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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! #
--- tools/perf/tests/code-reading.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
tools/perf/tests/code-reading.c +7 -1
@@ -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 ;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 5/22 perf test: Reduce the sample_freq for the 'object code reading' test
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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.
--- tools/perf/tests/code-reading.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/tests/code-reading.c +1 -1
@@ -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, },
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 6/22 perf stat: Handled scaled == -1 case for counters
In reply to Arnaldo Carvalho de MeloFrom: Andi Kleen <[email protected]>
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.
--- tools/perf/builtin-stat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-stat.c +1 -1
@@ -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",
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 7/22 perf bpf: Rename bpf_prog_priv__clear() to clear_prog_priv()
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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.
--- tools/perf/util/bpf-loader.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tools/perf/util/bpf-loader.c +3 -3
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 8/22 perf tools: Fix checking asprintf return value
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
According to man pages, asprintf returns -1 when failure. This patch fixes two incorrect return value checker.
--- tools/perf/util/parse-events.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
tools/perf/util/parse-events.c +3 -3
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 9/22 perf tools: Create config_term_names array
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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.
--- 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(-)
tools/perf/util/parse-events.c +48 -3
@@ -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 ] ="<sysfs term>" , + [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",
tools/perf/util/parse-events.h +2 -1
@@ -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 {
tools/perf/util/parse-events.l +1 -2
@@ -178,8 +178,7 @@ modifier_bp [rwx]{1,3} <config>{ /* - * 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); }
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 10/22 perf stat: Bail out on unsupported event config modifiers
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
'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 elapsedThe 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 termvalid terms: config,config1,config2,name
# ./perf stat -e 'cpu/badterm/' ls
event syntax error: 'cpu/badterm/'
\___ unknown termvalid terms: pc,any,inv,edge,cmask,event,in_tx,ldlat,umask,in_tx_cp,offcore_rsp,config,config1,config2,name
--- 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(+)
tools/perf/builtin-stat.c +1 -0
@@ -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 );
tools/perf/util/parse-events.c +48 -0
66 lines not on this page · raw message
tools/perf/util/parse-events.h +1 -0
@@ -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,
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 11/22 perf tools: Rename and move pmu_event_name to get_config_name
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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.
--- tools/perf/util/parse-events.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-)
tools/perf/util/parse-events.c +18 -17
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 12/22 perf tools: Introduce opt_event_config nonterminal
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
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.
--- tools/perf/util/parse-events.y | 47 ++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 29 deletions(-)
tools/perf/util/parse-events.y +18 -29
@@ -64,6 +64,7 @@ static inc_group_count(struct list_head *list, %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT %type <num> value_sym %type <head> event_config +%type <head> opt_event_config %type <term> event_term %type <head> event_pmu %type <head> 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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 13/22 perf tools: Enable config raw and numeric events
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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
#--- 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(-)
tools/perf/tests/parse-events.c +40 -0
@@ -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[] = {
tools/perf/util/parse-events.c +2 -1
@@ -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,
tools/perf/util/parse-events.y +6 -4
@@ -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; }
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 14/22 perf tools: Enable config and setting names for legacy cache events
In reply to Arnaldo Carvalho de MeloFrom: Wang Nan <[email protected]>
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.
--- 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(-)
tools/perf/tests/parse-events.c +12 -0
@@ -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[] = {
tools/perf/util/parse-events.c +27 -3
@@ -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,
tools/perf/util/parse-events.h +3 -1
@@ -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,
tools/perf/util/parse-events.y +12 -6
@@ -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; }
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 15/22 perf hists browser: Fix percentage update on key press
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
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.
--- tools/perf/util/hist.c | 26 +++++++++++++++++++------- tools/perf/util/hist.h | 2 ++ 2 files changed, 21 insertions(+), 7 deletions(-)
tools/perf/util/hist.c +19 -7
@@ -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;
tools/perf/util/hist.h +2 -0
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 16/22 perf callchain: Check return value of add_child()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
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.
--- tools/perf/util/callchain.c | 8 ++++++++ 1 file changed, 8 insertions(+)
tools/perf/util/callchain.c +8 -0
@@ -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);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 17/22 perf callchain: Check return value of fill_node()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
Memory allocation in the fill_node() can fail so change its return type to int and check it in add_child() too.
--- tools/perf/util/callchain.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)
tools/perf/util/callchain.c +13 -3
@@ -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;
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 18/22 perf callchain: Add enum match_result for match_chain()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
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.
--- tools/perf/util/callchain.c | 52 +++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 18 deletions(-)
tools/perf/util/callchain.c +34 -18
@@ -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 @@ inc_children_hit: 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,
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 19/22 perf callchain: Check return value of split_add_child()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
Now create_child() and add_child() return errors so check and pass it to the caller.
--- tools/perf/util/callchain.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
tools/perf/util/callchain.c +9 -3
@@ -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 ; }
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 20/22 perf callchain: Check return value of append_chain_children()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
Now it can check the error case, so check and pass it to the caller.
--- tools/perf/util/callchain.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
tools/perf/util/callchain.c +12 -6
@@ -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);
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 21/22 perf hists: Return error from hists__collapse_resort()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
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.
--- tools/perf/util/hist.c | 29 +++++++++++++++++++---------- tools/perf/util/hist.h | 4 ++-- 2 files changed, 21 insertions(+), 12 deletions(-)
tools/perf/util/hist.c +19 -10
@@ -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)
tools/perf/util/hist.h +2 -2
@@ -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 {
Signature · 1 line
2.5.0
Arnaldo Carvalho de Melo
maintainerPATCH 22/22 perf report: Check error during report__collapse_hists()
In reply to Arnaldo Carvalho de MeloFrom: Namhyung Kim <[email protected]>
If it returns an error, warn user and bail out instead of silently ignoring it.
--- tools/perf/builtin-report.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
tools/perf/builtin-report.c +11 -3
@@ -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 ;
Signature · 1 line
2.5.0
Quoted 123 lines · Arnaldo Carvalho de Melo
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit
3b364d7b587db0f0eeafde0f271e0698187de776:perf/core: Remove unused arguments from a bunch of functions (2016-02-17 10:37:48 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.gittags/perf-core-for-mingofor you to fetch changes up to
5b2ea6f2f6ac81a230e6cc68e1473e796a583f00:perf report: Check error during
report__collapse_hists()(2016-02-19 19:17:50 -0300)----------------------------------------------------------------
perf/core improvements and fixes:User visible:
- Add 'perf record' --all-user/--all-kernel options, so that one can tell that all the events in the command line should be restricted to the user or kernel levels (Jiri Olsa), i.e.:
perf record -e cycles:u,instructions:u
is equivalent to:
perf record --all-user -e cycles,instructions- Fix percentage update on key press, due to the buffering code (that creates
hist_entriesthat will later be consumed) touching per hists state that is used by the display thread (Namhyung Kim)- Bail out when event modifiers not supported by 'perf stat' are specified, i.e.: (Wang Nan)
# perf stat -e cycles/no-inherit/ usleep 1 event syntax error: 'cycles/no-inherit/' \___ 'no-inherit' is not usable in 'perf stat' # perf stat -e cycles/foo/ usleep 1 event syntax error: 'cycles/foo/' \___ unknown termvalid terms: config,config1,config2,name
#- Enable setting names for legacy cache, raw and numeric events, e.g: (Wang Nan)
# 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.659 MB perf.data (844 samples) ] # perf evlist cycles evtx #Miscelaneous/Infrastructure:
- Handled scaled == -1 case for counters in 'perf stat', fixing recent, only in perf/core, regression (Andi Kleen)
- Reference count the cpu and thread maps at
set_maps(), fixing the 'object code reading' 'perf test' entry when it was requesting a perf_event_attr.sample_freq >/proc/sys/kernel/perf_event_max_sample_rate(Arnaldo Carvalho de Melo)- Improve
perf_evlist__strerror_open()to provide hints for-EINVALdue to perf_event_attr.sample_freq >/proc/sys/kernel/perf_event_max_sample_rate(Arnaldo Carvalho de Melo)- Add checks to various callchain and histogram routines (Namhyung Kim)
- Fix checking asprintf return value when parsing additional event config terms (Wang Nan)
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>----------------------------------------------------------------
Andi Kleen (1):perf stat: Handled scaled == -1 case for countersArnaldo Carvalho de Melo (5):
perf evlist: Reference count the cpu and thread maps atset_maps()perf evlist: Handle-EINVALforsample_freq>max_sample_rateinstrerror_open()perf tests: Useperf_evlist__strerror_open()to provide hints aboutmax_freqperf test: Reduce thesample_freqfor the 'object code reading' test perf tools: Introduceopt_event_confignonterminalJiri Olsa (1):
perf record: Add --all-user/--all-kernel optionsNamhyung Kim (8):
perf hists browser: Fix percentage update on key press perf callchain: Check return value ofadd_child()perf callchain: Check return value offill_node()perf callchain: Add enummatch_resultformatch_chain()perf callchain: Check return value ofsplit_add_child()perf callchain: Check return value ofappend_chain_children()perf hists: Return error fromhists__collapse_resort()perf report: Check error duringreport__collapse_hists()Wang Nan (7):
perf bpf: Renamebpf_prog_priv__clear()toclear_prog_priv()perf tools: Fix checking asprintf return value
perf tools: Createconfig_term_namesarray
perf stat: Bail out on unsupported event config modifiers perf tools: Rename and movepmu_event_nametoget_config_nameperf tools: Enable config raw and numeric events
perf tools: Enable config and setting names for legacy cache eventstools/perf/Documentation/perf-record.txt | 6 ++ tools/perf/builtin-record.c | 6 ++ tools/perf/builtin-report.c | 14 ++- tools/perf/builtin-stat.c | 3 +- tools/perf/perf.h | 2 + tools/perf/tests/code-reading.c | 10 +- tools/perf/tests/parse-events.c | 52 ++++++++++ tools/perf/util/bpf-loader.c | 6 +- tools/perf/util/callchain.c | 102 +++++++++++++----- tools/perf/util/evlist.c | 24 ++++-
Quoted 8 lines
tools/perf/util/evsel.c | 10 ++ tools/perf/util/hist.c | 55 +++++++--- tools/perf/util/hist.h | 6 +- tools/perf/util/parse-events.c | 173 ++++++++++++++++++++++++++----- tools/perf/util/parse-events.h | 8 +- tools/perf/util/parse-events.l | 3 +- tools/perf/util/parse-events.y | 75 +++++++------- 17 files changed, 426 insertions(+), 129 deletions(-)
Pulled, thanks a lot Arnaldo!
Ingo