perf/core improvements and fixes
Arnaldo Carvalho de Melo
maintainerHi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit a9a3cd900fbbcbf837d65653105e7bfc583ced09:
Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-06-20 01:11:11 +0200)
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 83b2ea257eb1d43e52f76d756722aeb899a2852c:
perf tools: Allow auxtrace data alignment (2015-06-23 18:28:37 -0300)
----------------------------------------------------------------
perf/core improvements and fixes:
User visible:
- Move toggling event logic from 'perf top' and into hists browser, allowing freeze/unfreeze with event lists with more than one entry (Namhyung Kim)
- Add missing newlines when dumping PERF_RECORD_FINISHED_ROUND and showing the Aggregated stats in 'perf report -D' (Adrian Hunter)
Infrastructure:
- Allow auxtrace data alignment (Adrian Hunter)
- Allow events with dot (Andi Kleen)
- Fix failure to 'perf probe' events on arm (He Kuang)
- Add testing for Makefile.perf (Jiri Olsa)
- Add test for make install with prefix (Jiri Olsa)
- Fix single target build dependency check (Jiri Olsa)
- Access thread_map entries via accessors, prep patch to hold more info per entry, for ongoing 'perf stat --per-thread' work (Jiri Olsa)
- Use __weak definition from compiler.h (Sukadev Bhattiprolu)
- Split perf_pmu__new_alias() (Sukadev Bhattiprolu)
----------------------------------------------------------------
Adrian Hunter (3):
perf session: Print a newline when dumping PERF_RECORD_FINISHED_ROUND perf tools: Print a newline before dumping Aggregated stats perf tools: Allow auxtrace data alignment
Andi Kleen (1):
perf tools: Allow events with dot
He Kuang (1):
perf probe: Fix failure to probe events on arm
Jiri Olsa (5):
perf tests: Add testing for Makefile.perf
perf tests: Add test for make install with prefix perf build: Fix single target build dependency check perf thread_map: Don't access the array entries directly perf thread_map: Change map entries into a struct
Namhyung Kim (1):
perf top: Move toggling event logic into hists browser
Sukadev Bhattiprolu (2):
perf pmu: Use __weak definition from <linux/compiler.h> perf pmu: Split perf_pmu__new_alias()
tools/perf/Makefile | 4 +-- tools/perf/builtin-top.c | 24 ++------------- tools/perf/builtin-trace.c | 4 +-- tools/perf/tests/make | 31 ++++++++++++++++++-- tools/perf/tests/openat-syscall-tp-fields.c | 2 +- tools/perf/ui/browsers/hists.c | 19 ++++++++++-- tools/perf/util/auxtrace.c | 11 +++++-- tools/perf/util/auxtrace.h | 1 + tools/perf/util/event.c | 6 ++-- tools/perf/util/evlist.c | 4 +-- tools/perf/util/evsel.c | 2 +- tools/perf/util/parse-events.l | 5 ++-- tools/perf/util/pmu.c | 45 +++++++++++++++++++---------- tools/perf/util/probe-event.c | 6 +++- tools/perf/util/session.c | 4 ++- tools/perf/util/thread_map.c | 24 ++++++++------- tools/perf/util/thread_map.h | 16 +++++++++- 17 files changed, 136 insertions(+), 72 deletions(-)
Arnaldo Carvalho de Melo
maintainerPATCH 1/13 perf top: Move toggling event logic into hists browser
From: Namhyung Kim <[email protected]>
Current 'f' key action to enable/disable events won't work if there're more than one event since perf_evsel_menu__run() doesn't return the key. So move it to the hists browser loop so that it can be processed as like other key action, and it's more natural to handle it there IMHO.
--- tools/perf/builtin-top.c | 24 +++--------------------- tools/perf/ui/browsers/hists.c | 19 +++++++++++++++++-- 2 files changed, 20 insertions(+), 23 deletions(-)
tools/perf/builtin-top.c +3 -21
@@ -586,27 +586,9 @@ static void *display_thread_tui(void *arg) hists->uid_filter_str = top->record_opts.target.uid_str; } -while (true) { -int key =perf_evlist__tui_browse_hists (top->evlist, help, &hbt, - top->min_percent, - &top->session->header.env); - -if (key !='f' ) -break ; - -perf_evlist__toggle_enable (top->evlist); -/* -* No need to refresh, resort/decay histogram entries -* if we are not collecting samples: -*/ -if (top->evlist->enabled) { - hbt.refresh = top->delay_secs; - help ="Press 'f' to disable the events or 'h' to see other hotkeys" ; - }else { - help ="Press 'f' again to re-enable the events" ; - hbt.refresh =0 ; - } - } +perf_evlist__tui_browse_hists (top->evlist, help, &hbt, + top->min_percent, + &top->session->header.env); done =1 ;return NULL ;
tools/perf/ui/browsers/hists.c +17 -2
@@ -1902,8 +1902,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,case CTRL ('c' ): goto out_free_stack;case 'f' : -if (!is_report_browser (hbt)) - goto out_free_stack; +if (!is_report_browser (hbt)) { +struct perf_top *top = hbt->arg; + +perf_evlist__toggle_enable (top->evlist); +/* +* No need to refresh, resort/decay histogram +* entries if we are not collecting samples: +*/ +if (top->evlist->enabled) { + helpline ="Press 'f' to disable the events or 'h' to see other hotkeys" ; + hbt->refresh = delay_secs; + }else { + helpline ="Press 'f' again to re-enable the events" ; + hbt->refresh =0 ; + } +continue ; + }/* Fall thru */ default : helpline ="Press '?' for help on key bindings" ;
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 2/13 perf tests: Add testing for Makefile.perf
In reply to Arnaldo Carvalho de MeloFrom: Jiri Olsa <[email protected]>
Currently we test only builds through top level Makefile, but seems like there's a bunch of users using Makefile.perf directly.
Changing the make suite to be run for Makefile.perf as well. It takes now considerable amount of time, but hopefully we catch more issues.
Also fixing the output indentation for make_kernelsrc and make_kernelsrc_tools tests.
--- tools/perf/tests/make | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-)
tools/perf/tests/make +20 -3
@@ -1,5 +1,16 @@ +ifndef MK +ifeq ($(MAKECMDGOALS),) +# no target specified, trigger the whole suite +all: + @echo "Testing Makefile"; $(MAKE) -sf tests/make MK=Makefile + @echo "Testing Makefile.perf"; $(MAKE) -sf tests/make MK=Makefile.perf +else +# run only specific test over 'Makefile' +%: + @echo "Testing Makefile"; $(MAKE) -sf tests/make MK=Makefile $@ +endif +else PERF := . -MK := Makefile include config/Makefile.arch
@@ -57,7 +68,12 @@ make_minimal += NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 # $(run) contains all available tests run := make_pure +# Targets 'clean all' can be run together only through top level +# Makefile because we detect clean target in Makefile.perf and +# disable features detection +ifeq ($(MK),Makefile) run += make_clean_all +endif run += make_python_perf_so run += make_debug run += make_no_libperl
@@ -226,13 +242,13 @@ tarpkg: ( eval $$cmd ) >> $@ 2>&1 make_kernelsrc: - @echo " - make -C <kernelsrc> tools/perf" + @echo "- make -C <kernelsrc> tools/perf" $(call clean); \ (make -C ../.. tools/perf) > $@ 2>&1 && \ test -x perf && rm -f $@ || (cat $@ ; false) make_kernelsrc_tools: - @echo " - make -C <kernelsrc>/tools perf" + @echo "- make -C <kernelsrc>/tools perf" $(call clean); \ (make -C ../../tools perf) > $@ 2>&1 && \ test -x perf && rm -f $@ || (cat $@ ; false)
@@ -244,3 +260,4 @@ out: $(run_O) @echo OK .PHONY: all $(run) $(run_O) tarpkg clean +endif # ifndef MK
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 3/13 perf tests: Add test for make install with prefix
In reply to Arnaldo Carvalho de MeloFrom: Jiri Olsa <[email protected]>
Lukas Wunner reported issue (and fix[1]) with 'make install prefix=...'. Adding automated test for this, so it wouldn't happen again.
[1]: 75e84ab906ef ("perf tools: Fix build breakage if prefix= is specified")
--- tools/perf/tests/make | 8 ++++++++ 1 file changed, 8 insertions(+)
tools/perf/tests/make +8 -0
@@ -58,6 +58,7 @@ make_install_man := install-man make_install_html := install-html make_install_info := install-info make_install_pdf := install-pdf +make_install_prefix := install prefix=/tmp/krava make_static := LDFLAGS=-static # all the NO_* variable combined
@@ -99,6 +100,7 @@ run += make_util_map_o run += make_util_pmu_bison_o run += make_install run += make_install_bin +run += make_install_prefix # FIXME 'install-*' commented out till they're fixed # run += make_install_doc # run += make_install_man
@@ -173,6 +175,12 @@ test_make_install_O := $(call test_dest_files,$(installed_files_all)) test_make_install_bin := $(call test_dest_files,$(installed_files_bin)) test_make_install_bin_O := $(call test_dest_files,$(installed_files_bin)) +# We prefix all installed files for make_install_prefix +# with '/tmp/krava' to match installed/prefix-ed files. +installed_files_all_prefix := $(addprefix /tmp/krava/,$(installed_files_all)) +test_make_install_prefix := $(call test_dest_files,$(installed_files_all_prefix)) +test_make_install_prefix_O := $(call test_dest_files,$(installed_files_all_prefix)) + # FIXME nothing gets installed test_make_install_man := test -f $$TMP_DEST/share/man/man1/perf.1 test_make_install_man_O := $(test_make_install_man)
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 4/13 perf build: Fix single target build dependency check
In reply to Arnaldo Carvalho de MeloFrom: Jiri Olsa <[email protected]>
Currently if we build a single target like:
$ touch util/map.c && make util/map.o
It will not rebuild util/map.o if it already exists and util/map.c is modified.
The reason is that the top-level 'Makefile' processes util/map.o as an implicit rule and if util/map.o exists make considers the 'util/map.o' target as done and will not nest into Makefile.perf.
Adding FORCE for '%', because that's what we want to nest into Makefile.perf for any target.
Adding Makefile into phony targets, because make tries to rebuild it and it's also resolved as '%' target.
--- tools/perf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
tools/perf/Makefile +2 -2
@@ -83,8 +83,8 @@ build-test: # # All other targets get passed through: # -%: +%: FORCE $(print_msg) $(make) -.PHONY: tags TAGS +.PHONY: tags TAGS FORCE Makefile
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 5/13 perf pmu: Use __weak definition from <linux/compiler.h>
In reply to Arnaldo Carvalho de MeloFrom: Sukadev Bhattiprolu <[email protected]>
Jiri Olsa pointed out, that the <linux/compiler.h> defines the attribute '__weak'. We might as well use that.
--- tools/perf/util/pmu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
tools/perf/util/pmu.c +2 -1
@@ -1,4 +1,5 @@#include <linux/list.h> +#include <linux/compiler.h> #include <sys/types.h> #include <unistd.h> #include <stdio.h>
@@ -436,7 +437,7 @@ static struct cpu_map *pmu_cpumask(const char *name)return cpus; } -struct perf_event_attr *__attribute__((weak)) +struct perf_event_attr *__weak perf_pmu__get_default_config (struct perf_pmu *pmu__maybe_unused ) {return NULL ;
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 6/13 perf pmu: Split perf_pmu__new_alias()
In reply to Arnaldo Carvalho de MeloFrom: Sukadev Bhattiprolu <[email protected]>
Separate the event parsing code in perf_pmu__new_alias() out into a separate function __perf_pmu__new_alias() so that code can be called indepdently.
This is based on an earlier patch from Andi Kleen.
--- tools/perf/util/pmu.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)
tools/perf/util/pmu.c +27 -15
@@ -206,17 +206,12 @@ static int perf_pmu__parse_snapshot(struct perf_pmu_alias *alias,return 0 ; } -static int perf_pmu__new_alias (struct list_head *list,char *dir,char *name,FILE *file) +static int __perf_pmu__new_alias (struct list_head *list,char *dir,char *name, +char *desc__maybe_unused ,char *val) {struct perf_pmu_alias *alias; -char buf[256 ];int ret; - ret =fread (buf,1 ,sizeof (buf), file); -if (ret ==0 ) -return -EINVAL ; - buf[ret] =0 ; - alias =malloc (sizeof (*alias));if (!alias)return -ENOMEM ;
@@ -226,26 +221,43 @@ static int perf_pmu__new_alias(struct list_head *list, char *dir, char *name, FI alias->unit[0 ] ='\0' ; alias->per_pkg = false; - ret =parse_events_terms (&alias->terms, buf); + ret =parse_events_terms (&alias->terms, val);if (ret) { +pr_err ("Cannot parse alias %s: %d\n" , val, ret);free (alias);return ret; } alias->name =strdup (name); -/* -* load unit name and scale if available -*/ -perf_pmu__parse_unit (alias, dir, name); -perf_pmu__parse_scale (alias, dir, name); -perf_pmu__parse_per_pkg (alias, dir, name); -perf_pmu__parse_snapshot (alias, dir, name); +if (dir) { +/* +* load unit name and scale if available +*/ +perf_pmu__parse_unit (alias, dir, name); +perf_pmu__parse_scale (alias, dir, name); +perf_pmu__parse_per_pkg (alias, dir, name); +perf_pmu__parse_snapshot (alias, dir, name); + }list_add_tail (&alias->list, list);return 0 ; } +static int perf_pmu__new_alias (struct list_head *list,char *dir,char *name,FILE *file) +{ +char buf[256 ]; +int ret; + + ret =fread (buf,1 ,sizeof (buf), file); +if (ret ==0 ) +return -EINVAL ; + + buf[ret] =0 ; + +return __perf_pmu__new_alias (list, dir, name,NULL , buf); +} +static inline bool pmu_alias_info_file (char *name) {size_t len;
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 7/13 perf tools: Allow events with dot
In reply to Arnaldo Carvalho de MeloFrom: Andi Kleen <[email protected]>
The Intel events use a dot to separate event name and unit mask. Allow dot in names in the scanner, and remove special handling of dot as EOF. Also remove the hack in jevents to replace dot with underscore. This way dotted events can be specified directly by the user.
I'm not fully sure this change to the scanner is correct (what was the dot special case good for?), but I haven't found anything that breaks with it so far at least.
--- tools/perf/util/parse-events.l | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
tools/perf/util/parse-events.l +2 -3
@@ -119,8 +119,8 @@ event [^,{}/]+ num_dec [0-9]+ num_hex 0x[a-fA-F0-9]+ num_raw_hex [a-fA-F0-9]+ -name [a-zA-Z_*?][a-zA-Z0-9_*?]* -name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?]* +name [a-zA-Z_*?][a-zA-Z0-9_*?.]* +name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?.]* /* If you add a modifier you need to update check_modifier() */ modifier_event [ukhpGHSDI]+ modifier_bp [rwx]{1,3}
@@ -165,7 +165,6 @@ modifier_bp [rwx]{1,3} return PE_EVENT_NAME; } -. | <<EOF>> { BEGIN(INITIAL); REWIND(0);
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 8/13 perf session: Print a newline when dumping PERF_RECORD_FINISHED_ROUND
In reply to Arnaldo Carvalho de MeloFrom: Adrian Hunter <[email protected]>
With 'perf report -D' the PERF_RECORD_FINISHED_ROUND event was printed without a newline, resulting in:
0x91a18 [0x8]: PERF_RECORD_FINISHED_ROUNDAggregated stats
Other events print their details, but PERF_RECORD_FINISHED_ROUND doesn't have any so just add a print for a newline.
--- tools/perf/util/session.c | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/util/session.c +2 -0
@@ -686,6 +686,8 @@ static int process_finished_round(struct perf_tool *tool __maybe_unused, union perf_event *event __maybe_unused, struct ordered_events *oe) { + if (dump_trace) + fprintf(stdout, "\n"); return ordered_events__flush(oe, OE_FLUSH__ROUND); }
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 9/13 perf tools: Print a newline before dumping Aggregated stats
In reply to Arnaldo Carvalho de MeloFrom: Adrian Hunter <[email protected]>
When dumping events with 'perf report -D' the event print always starts with a newline (see dump_event()).
Do the same with the "Aggregated stats" print so that it is not jammed up against the last event print.
--- tools/perf/util/session.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/session.c +1 -1
@@ -1728,7 +1728,7 @@ size_t perf_session__fprintf_nr_events(struct perf_session *session, FILE *fp)if (perf_header__has_feat (&session->header,HEADER_AUXTRACE )) msg =" (excludes AUX area (e.g. instruction trace) decoded / synthesized events)" ; - ret =fprintf (fp,"Aggregated stats:%s\n" , msg); + ret =fprintf (fp,"\nAggregated stats:%s\n" , msg); ret +=events_stats__fprintf (&session->evlist->stats, fp);return ret;
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 10/13 perf probe: Fix failure to probe events on arm
In reply to Arnaldo Carvalho de MeloFrom: He Kuang <[email protected]>
Fix failure to probe events on arm, the problem was introduced by commit 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text").
For some architectures, the '_etext' label is not in the .text section (in the .notes section for arm/arm64). Labels out of the .text section are not loaded as symbols and we get a zero value when looking up its addresses, which causes all events to be wrongly skipped.
This patch skips checking the text address range when failing to get the address of '_etext' and thus fixes the problem.
The problem can be reproduced on arm as follows:
# perf probe --add='generic_perform_write'
generic_perform_write+0 is out of .text, skip it.
Probe point 'generic_perform_write' not found.
Error: Failed to add events.After this patch:
# perf probe --add='generic_perform_write'
Added new event:
probe:generic_perform_write (on generic_perform_write)You can now use it in all perf tools, such as:
perf record -e probe:generic_perform_write -aR sleep 1
--- tools/perf/util/probe-event.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
tools/perf/util/probe-event.c +5 -1
@@ -249,8 +249,12 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)static bool kprobe_blacklist__listed (unsigned long address);static bool kprobe_warn_out_range (const char *symbol,unsigned long address) { +u64 etext_addr; +/* Get the address of _etext for checking non-probable text symbol */ -if (kernel_get_symbol_address_by_name ("_etext" , false) < address) + etext_addr =kernel_get_symbol_address_by_name ("_etext" , false); + +if (etext_addr !=0 && etext_addr < address)pr_warning ("%s is out of .text, skip it.\n" , symbol);else if (kprobe_blacklist__listed (address))pr_warning ("%s is blacklisted function, skip it.\n" , symbol);
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 11/13 perf thread_map: Don't access the array entries directly
In reply to Arnaldo Carvalho de MeloFrom: Jiri Olsa <[email protected]>
Instead provide a method to set the array entries, and another to access the contents.
--- tools/perf/builtin-trace.c | 4 ++-- tools/perf/tests/openat-syscall-tp-fields.c | 2 +- tools/perf/util/auxtrace.c | 4 ++-- tools/perf/util/event.c | 6 +++--- tools/perf/util/evlist.c | 4 ++-- tools/perf/util/evsel.c | 2 +- tools/perf/util/thread_map.c | 24 +++++++++++++----------- tools/perf/util/thread_map.h | 10 ++++++++++ 8 files changed, 34 insertions(+), 22 deletions(-)
tools/perf/builtin-trace.c +2 -2
@@ -2325,7 +2325,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)*/ if (trace->filter_pids.nr >0 ) err =perf_evlist__set_filter_pids (evlist, trace->filter_pids.nr, trace->filter_pids.entries); -else if (evlist->threads->map[0 ] ==-1 ) +else if (thread_map__pid (evlist->threads,0 ) ==-1 ) err =perf_evlist__set_filter_pid (evlist,getpid ());if (err <0 ) {
@@ -2343,7 +2343,7 @@ static int trace__run(struct trace *trace, int argc, const char **argv)if (forks)perf_evlist__start_workload (evlist); - trace->multiple_threads = evlist->threads->map[0 ] ==-1 || + trace->multiple_threads =thread_map__pid (evlist->threads,0 ) ==-1 || evlist->threads->nr >1 ||perf_evlist__first (evlist)->attr.inherit; again:
tools/perf/tests/openat-syscall-tp-fields.c +1 -1
@@ -45,7 +45,7 @@ int test__syscall_openat_tp_fields(void)perf_evsel__config (evsel, &opts); - evlist->threads->map[0 ] =getpid (); +thread_map__set_pid (evlist->threads,0 ,getpid ()); err =perf_evlist__open (evlist);if (err <0 ) {
tools/perf/util/auxtrace.c +2 -2
@@ -119,12 +119,12 @@ void auxtrace_mmap_params__set_idx(struct auxtrace_mmap_params *mp,if (per_cpu) { mp->cpu = evlist->cpus->map[idx];if (evlist->threads) - mp->tid = evlist->threads->map[0 ]; + mp->tid =thread_map__pid (evlist->threads,0 );else mp->tid =-1 ; }else { mp->cpu =-1 ; - mp->tid = evlist->threads->map[idx]; + mp->tid =thread_map__pid (evlist->threads, idx); } }
tools/perf/util/event.c +3 -3
@@ -504,7 +504,7 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,for (thread =0 ; thread < threads->nr; ++thread) {if (__event__synthesize_thread (comm_event, mmap_event, fork_event, - threads->map[thread],0 , +thread_map__pid (threads, thread),0 , process, tool, machine, mmap_data, proc_map_timeout)) { err =-1 ;
@@ -515,12 +515,12 @@ int perf_event__synthesize_thread_map(struct perf_tool *tool,* comm.pid is set to thread group id by * perf_event__synthesize_comm */ -if ((int ) comm_event->comm.pid != threads->map[thread]) { +if ((int ) comm_event->comm.pid !=thread_map__pid (threads, thread)) {bool need_leader = true;/* is thread group leader in thread_map? */ for (j =0 ; j < threads->nr; ++j) { -if ((int ) comm_event->comm.pid == threads->map[j]) { +if ((int ) comm_event->comm.pid ==thread_map__pid (threads, j)) { need_leader = false;break ; }
tools/perf/util/evlist.c +2 -2
@@ -548,7 +548,7 @@ static void perf_evlist__set_sid_idx(struct perf_evlist *evlist,else sid->cpu =-1 ;if (!evsel->system_wide && evlist->threads && thread >=0 ) - sid->tid = evlist->threads->map[thread]; + sid->tid =thread_map__pid (evlist->threads, thread);else sid->tid =-1 ; }
@@ -1475,7 +1475,7 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist, struct target *tar __func__, __LINE__); goto out_close_pipes; } - evlist->threads->map[0 ] = evlist->workload.pid; +thread_map__set_pid (evlist->threads,0 , evlist->workload.pid); }close (child_ready_pipe[1 ]);
tools/perf/util/evsel.c +1 -1
@@ -1167,7 +1167,7 @@ retry_sample_id:int group_fd;if (!evsel->cgrp && !evsel->system_wide) - pid = threads->map[thread]; + pid =thread_map__pid (threads, thread); group_fd =get_group_fd (evsel, cpu, thread); retry_open:
tools/perf/util/thread_map.c +13 -11
@@ -45,7 +45,7 @@ struct thread_map *thread_map__new_by_pid(pid_t pid) threads =thread_map__alloc (items);if (threads !=NULL ) {for (i =0 ; i < items; i++) - threads->map[i] =atoi (namelist[i]->d_name); +thread_map__set_pid (threads, i,atoi (namelist[i]->d_name)); threads->nr = items; }
@@ -61,8 +61,8 @@ struct thread_map *thread_map__new_by_tid(pid_t tid)struct thread_map *threads =thread_map__alloc (1 );if (threads !=NULL ) { - threads->map[0 ] = tid; - threads->nr =1 ; +thread_map__set_pid (threads,0 , tid); + threads->nr =1 ; }return threads;
@@ -123,8 +123,10 @@ struct thread_map *thread_map__new_by_uid(uid_t uid) threads = tmp; } -for (i =0 ; i < items; i++) - threads->map[threads->nr + i] =atoi (namelist[i]->d_name); +for (i =0 ; i < items; i++) { +thread_map__set_pid (threads, threads->nr + i, +atoi (namelist[i]->d_name)); + }for (i =0 ; i < items; i++)zfree (&namelist[i]);
@@ -201,7 +203,7 @@ static struct thread_map *thread_map__new_by_pid_str(const char *pid_str) threads = nt;for (i =0 ; i < items; i++) { - threads->map[j++] =atoi (namelist[i]->d_name); +thread_map__set_pid (threads, j++,atoi (namelist[i]->d_name));zfree (&namelist[i]); } threads->nr = total_tasks;
@@ -227,8 +229,8 @@ struct thread_map *thread_map__new_dummy(void)struct thread_map *threads =thread_map__alloc (1 );if (threads !=NULL ) { - threads->map[0 ] =-1 ; - threads->nr =1 ; +thread_map__set_pid (threads,0 ,-1 ); + threads->nr =1 ; }return threads; }
@@ -267,8 +269,8 @@ static struct thread_map *thread_map__new_by_tid_str(const char *tid_str) goto out_free_threads; threads = nt; - threads->map[ntasks -1 ] = tid; - threads->nr = ntasks; +thread_map__set_pid (threads, ntasks -1 , tid); + threads->nr = ntasks; } out:return threads;
@@ -301,7 +303,7 @@ size_t thread_map__fprintf(struct thread_map *threads, FILE *fp)size_t printed =fprintf (fp,"%d thread%s: " , threads->nr, threads->nr >1 ?"s" :"" );for (i =0 ; i < threads->nr; ++i) - printed +=fprintf (fp,"%s%d" , i ?", " :"" , threads->map[i]); + printed +=fprintf (fp,"%s%d" , i ?", " :"" ,thread_map__pid (threads, i));return printed +fprintf (fp,"\n" ); }
tools/perf/util/thread_map.h +10 -0
@@ -27,4 +27,14 @@ static inline int thread_map__nr(struct thread_map *threads) return threads ? threads->nr : 1; } +static inline pid_t thread_map__pid (struct thread_map *map,int thread) +{ +return map->map[thread]; +} + +static inline void +thread_map__set_pid (struct thread_map *map,int thread,pid_t pid) +{ + map->map[thread] = pid; +}#endif /* __PERF_THREAD_MAP_H */
Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerPATCH 12/13 perf thread_map: Change map entries into a struct
In reply to Arnaldo Carvalho de MeloFrom: Jiri Olsa <[email protected]>
We need to store command names with the pid. Changing map entries to be a struct holding pid. Process name is coming in shortly.
--- tools/perf/util/thread_map.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
tools/perf/util/thread_map.h +7 -3
@@ -4,9 +4,13 @@#include <sys/types.h> #include <stdio.h> +struct thread_map_data { +pid_t pid; +}; +struct thread_map {int nr; -pid_t map[]; +struct thread_map_data map[]; };struct thread_map *thread_map__new_dummy (void );
@@ -29,12 +33,12 @@ static inline int thread_map__nr(struct thread_map *threads)static inline pid_t thread_map__pid (struct thread_map *map,int thread) { -return map->map[thread]; +return map->map[thread].pid; }static inline void thread_map__set_pid (struct thread_map *map,int thread,pid_t pid) { - map->map[thread] = pid; + map->map[thread].pid = pid; }#endif /* __PERF_THREAD_MAP_H */
Signature · 1 line
2.1.0
Re: [PATCH 12/13] perf thread_map: Change map entries into a struct
Quoted 5 lines · Arnaldo Carvalho de Melo
From: Jiri Olsa <[email protected]>
We need to store command names with the pid. Changing map entries to be a struct holding pid. Process name is coming in shortly.
Quoted 9 lines
Signed-off-by Jiri Olsa <[email protected]>Cc Adrian Hunter <[email protected]>Cc Andi Kleen <[email protected]>Cc David Ahern <[email protected]>Cc Namhyung Kim <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Stephane Eranian <[email protected]>[ Split providing the set/get accessors from transforming the entries structs ]
the comment in [] is wrong..
and you missed following hunk:
---
static struct thread_map *thread_map__realloc(struct thread_map *map, int nr) {
- size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+ size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr;
---
shouldn't be fatal, it probably has the same or biggger size ATM I'll include the fix in the next version
jirka
Quoted 42 lines
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>--- tools/perf/util/thread_map.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)tools/perf/util/thread_map.h +7
-3@@ -4,9 +4,13 @@#include <sys/types.h> #include <stdio.h> +struct thread_map_data { +pid_t pid; +}; +struct thread_map {int nr; -pid_t map[]; +struct thread_map_data map[]; };struct thread_map *thread_map__new_dummy (void );@@ -29,12 +33,12 @@ static inline int thread_map__nr(struct thread_map *threads)static inline pid_t thread_map__pid (struct thread_map *map,int thread) { -return map->map[thread]; +return map->map[thread].pid; }static inline void thread_map__set_pid (struct thread_map *map,int thread,pid_t pid) { - map->map[thread] = pid; + map->map[thread].pid = pid; }#endif /* __PERF_THREAD_MAP_H */ Signature · 1 line
2.1.0
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 12/13] perf thread_map: Change map entries into a struct
Em Thu, Jun 25, 2015 at 10:38:41AM +0200, Jiri Olsa escreveu:
Quoted 27 lines · Arnaldo Carvalho de Melo
Quoted 5 lines
From: Jiri Olsa <[email protected]>
We need to store command names with the pid. Changing map entries to be a struct holding pid. Process name is coming in shortly.
Quoted 9 lines
Signed-off-by Jiri Olsa <[email protected]>Cc Adrian Hunter <[email protected]>Cc Andi Kleen <[email protected]>Cc David Ahern <[email protected]>Cc Namhyung Kim <[email protected]>Cc Peter Zijlstra <[email protected]>Cc Stephane Eranian <[email protected]>[ Split providing the set/get accessors from transforming the entries structs ]the comment in [] is wrong..
and you missed following hunk:
---
static structthread_map*thread_map__realloc(structthread_map*map, int nr) {(quoted diff) +1
-2- size_t size = sizeof(*map) + sizeof(pid_t) * nr; + size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr; ---
Quoted 2 lines
shouldn't be fatal, it probably has the same or biggger size ATM I'll include the fix in the next version
Yeah, sorry about that, should have noticed that, but as you said, code wise it is harmless.
- Arnaldo
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 12/13] perf thread_map: Change map entries into a struct
Em Thu, Jun 25, 2015 at 10:45:05AM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 10 lines
Em Thu, Jun 25, 2015 at 10:38:41AM +0200, Jiri Olsa escreveu:
Quoted 8 lines
and you missed following hunk:
---
static structthread_map*thread_map__realloc(structthread_map*map, int nr) {(quoted diff) +1
-2- size_t size = sizeof(*map) + sizeof(pid_t) * nr; + size_t size = sizeof(*map) + sizeof(struct thread_map_data) * nr; ---
Quoted 5 lines
Quoted 2 lines
shouldn't be fatal, it probably has the same or biggger size ATM I'll include the fix in the next version
Yeah, sorry about that, should have noticed that, but as you said, code wise it is harmless.
Adding this fix for it, should have been there since when I implemented thread_map...
static struct thread_map *thread_map__realloc(struct thread_map *map,
int nr)
{
- size_t size = sizeof(*map) + sizeof(pid_t) * nr;
+ size_t size = sizeof(*map) + sizeof(map->map[0]) * nr;
-----------
[acme@zoo ~]$ cat thread_map.c
#include <sys/types.h>
#include <stdio.h>
struct thread_map_data {
pid_t pid;
};struct thread_map {
int nr;
struct thread_map_data map[];
};int main(void)
{
struct thread_map *map;
printf("sizeof(map->map[0]:\t\t%zd\n", sizeof(map->map[0]));
printf("sizeof(struct thread_map_data):\t%zd\n", sizeof(struct thread_map_data));
printf("sizeof(pid_t):\t\t\t%zd\n", sizeof(pid_t));
return 0;
}[acme@zoo ~]$ make thread_map
cc thread_map.c -o thread_map
[acme@zoo ~]$ ./thread_map
sizeof(map->map[0]: 4sizeof(struct thread_map_data): 4sizeof(pid_t): 4
[acme@zoo ~]$ file thread_mapthread_map: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, BuildID[sha1]=a6a9894fb6127e23621ef02e1110ff6635082bcd, not stripped [acme@zoo ~]$
- Arnaldo
tip-bot for Arnaldo Carvalho de Melo
merge bot[tip:perf/urgent] perf tools: Future-proof thread_map allocation size calculation
perf tools: Future-proof thread_map allocation size calculation
Commit-ID: 060664f3b9dff37860e48b5158e8429b2467e526 Gitweb: http://git.kernel.org/tip/060664f3b9dff37860e48b5158e8429b2467e526 Author: Arnaldo Carvalho de Melo <[email protected]> AuthorDate: Thu, 25 Jun 2015 14:48:49 -0300 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 25 Jun 2015 15:15:49 -0300
perf tools: Future-proof thread_map allocation size calculation
--- tools/perf/util/thread_map.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/util/thread_map.c +1 -1
@@ -22,7 +22,7 @@ static int filter(const struct dirent *dir)static struct thread_map *thread_map__realloc (struct thread_map *map,int nr) { -size_t size =sizeof (*map) +sizeof (pid_t) * nr; +size_t size =sizeof (*map) +sizeof (map->map[0 ]) * nr;return realloc (map, size); }
Arnaldo Carvalho de Melo
maintainerPATCH 13/13 perf tools: Allow auxtrace data alignment
In reply to Arnaldo Carvalho de MeloFrom: Adrian Hunter <[email protected]>
Allow auxtrace data to be a multiple of something other than page size. That is needed for BTS where the buffer contains 24-byte records.
--- tools/perf/util/auxtrace.c | 7 +++++++ tools/perf/util/auxtrace.h | 1 + 2 files changed, 8 insertions(+)
tools/perf/util/auxtrace.c +7 -0
@@ -1182,6 +1182,13 @@ static int __auxtrace_mmap__read(struct auxtrace_mmap *mm, data2 =NULL ; } +if (itr->alignment) { +unsigned int unwanted = len1 % itr->alignment; + + len1 -= unwanted; + size -= unwanted; + } +/* padding must be written by fn() e.g. record__process_auxtrace() */ padding = size &7 ;if (padding)
tools/perf/util/auxtrace.h +1 -0
@@ -303,6 +303,7 @@ struct auxtrace_record { const char *str);u64 (*reference)(struct auxtrace_record *itr);int (*read_finish)(struct auxtrace_record *itr,int idx); +unsigned int alignment; };#ifdef HAVE_AUXTRACE_SUPPORT
Signature · 1 line
2.1.0
Quoted 88 lines · Arnaldo Carvalho de Melo
Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit
a9a3cd900fbbcbf837d65653105e7bfc583ced09:Merge tag 'perf-core-for-mingo' of
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linuxinto perf/core (2015-06-20 01:11:11 +0200)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
83b2ea257eb1d43e52f76d756722aeb899a2852c:perf tools: Allow auxtrace data alignment (2015-06-23 18:28:37 -0300)
----------------------------------------------------------------
perf/core improvements and fixes:User visible:
- Move toggling event logic from 'perf top' and into hists browser, allowing freeze/unfreeze with event lists with more than one entry (Namhyung Kim)
- Add missing newlines when dumping
PERF_RECORD_FINISHED_ROUNDand showing the Aggregated stats in 'perf report -D' (Adrian Hunter)Infrastructure:
- Allow auxtrace data alignment (Adrian Hunter)
- Allow events with dot (Andi Kleen)
- Fix failure to 'perf probe' events on arm (He Kuang)
- Add testing for Makefile.perf (Jiri Olsa)
- Add test for make install with prefix (Jiri Olsa)
- Fix single target build dependency check (Jiri Olsa)
- Access
thread_mapentries via accessors, prep patch to hold more info per entry, for ongoing 'perf stat --per-thread' work (Jiri Olsa)- Use
__weakdefinition fromcompiler.h(Sukadev Bhattiprolu)- Split
perf_pmu__new_alias()(Sukadev Bhattiprolu)Signed-off-by Arnaldo Carvalho de Melo <[email protected]>----------------------------------------------------------------
Adrian Hunter (3):
perf session: Print a newline when dumpingPERF_RECORD_FINISHED_ROUNDperf tools: Print a newline before dumping Aggregated stats perf tools: Allow auxtrace data alignmentAndi Kleen (1):
perf tools: Allow events with dotHe Kuang (1):
perf probe: Fix failure to probe events on armJiri Olsa (5):
perf tests: Add testing for Makefile.perf
perf tests: Add test for make install with prefix perf build: Fix single target build dependency check perfthread_map: Don't access the array entries directly perfthread_map: Change map entries into a structNamhyung Kim (1):
perf top: Move toggling event logic into hists browserSukadev Bhattiprolu (2):
perf pmu: Use__weakdefinition from <linux/compiler.h> perf pmu: Splitperf_pmu__new_alias()tools/perf/Makefile | 4 +-- tools/perf/builtin-top.c | 24 ++------------- tools/perf/builtin-trace.c | 4 +-- tools/perf/tests/make | 31 ++++++++++++++++++-- tools/perf/tests/openat-syscall-tp-fields.c | 2 +- tools/perf/ui/browsers/hists.c | 19 ++++++++++-- tools/perf/util/auxtrace.c | 11 +++++-- tools/perf/util/auxtrace.h | 1 + tools/perf/util/event.c | 6 ++-- tools/perf/util/evlist.c | 4 +--
Quoted 8 lines
tools/perf/util/evsel.c | 2 +- tools/perf/util/parse-events.l | 5 ++-- tools/perf/util/pmu.c | 45 +++++++++++++++++++---------- tools/perf/util/probe-event.c | 6 +++- tools/perf/util/session.c | 4 ++- tools/perf/util/thread_map.c | 24 ++++++++------- tools/perf/util/thread_map.h | 16 +++++++++- 17 files changed, 136 insertions(+), 72 deletions(-)
Pulled, thanks a lot Arnaldo!
Btw., one small thing I noticed about the status line in perf top: if I ever use 'f' to freeze/unfreeze events, the following message:
Press 'f' to disable the events or 'h' to see other hotkeys
sticks around forever, even after I look into annotation and exit it, etc.
So I don't mind some default, helpful message there (such as 'Press 'h' to see hotkeys'), but it appears this particular message is context and usage sensitive, which wasn't really the goal, right?
Thanks,
Ingo
Arnaldo Carvalho de Melo
maintainerEm Thu, Jun 25, 2015 at 09:31:41AM +0200, Ingo Molnar escreveu:
Quoted 5 lines
Pulled, thanks a lot Arnaldo!
Btw., one small thing I noticed about the status line in perf top: if I ever use 'f' to freeze/unfreeze events, the following message:
Quoted 7 lines
Press 'f' to disable the events or 'h' to see other hotkeys
sticks around forever, even after I look into annotation and exit it, etc.
So I don't mind some default, helpful message there (such as 'Press 'h' to see hotkeys'), but it appears this particular message is context and usage sensitive, which wasn't really the goal, right?
Agreed, some more work is needed to change that message in more places, will do it eventually.
- Arnaldo