perf tools: Assorted fixes
Jiri Olsa
reviewercover perf tools: Assorted fixes
hi,
sending assorted general fixes that queued
up in my other branches.
Also available in here:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes
thanks,
jirka
---
Jiri Olsa (17):
perf report: Ask ordered events for --tasks option
perf record: Put new line after target override warning
perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND tools lib api fs: Add filename__read_xll function
tools lib api fs: Add sysfs__read_xll function
tools lib symbol: Skip non-address kallsyms line
perf tools: Free root_dir in machine__init error path
perf tools: Move kernel mmap name into struct machine
perf tools: Don't search for active kernel start in __machine__create_kernel_maps perf tools: Generalize machine__set_kernel_mmap function
perf tools: Use machine__set_kernel_mmap instead of map_groups__fixup_end perf tools: Rename __map_groups__fixup_end to map_groups__fixup_end perf tools: Remove machine__load_kallsyms function
perf tools: Do not create kernel maps in sample__resolve
perf tools: Check if we read regular file in dso__load
perf tests: Fix dwarf unwind for stripped binaries
perf tools: Fix comment for sort__* compare functions
tools/lib/api/fs/fs.c | 44 ++++++++++++++++++++++++++++-------- tools/lib/api/fs/fs.h | 2 ++ tools/lib/symbol/kallsyms.c | 4 ++++ tools/perf/Documentation/perf-script.txt | 3 +++ tools/perf/builtin-record.c | 2 +- tools/perf/builtin-report.c | 1 + tools/perf/builtin-script.c | 17 ++++++++++++++ tools/perf/tests/dwarf-unwind.c | 46 +++++++++++++++++++++++++------------- tools/perf/tests/vmlinux-kallsyms.c | 2 +- tools/perf/util/build-id.c | 10 +++------ tools/perf/util/event.c | 16 +------------ tools/perf/util/machine.c | 147 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------- tools/perf/util/machine.h | 6 +---- tools/perf/util/sort.c | 7 ++++-- tools/perf/util/symbol-elf.c | 2 +- tools/perf/util/symbol.c | 20 ++++++----------- tools/perf/util/symbol.h | 2 +- 17 files changed, 180 insertions(+), 151 deletions(-)
Jiri Olsa
reviewerPATCH 1/17 perf report: Ask ordered events for --tasks option
If we have the time in, keep the events in time order.
--- tools/perf/builtin-report.c | 1 + 1 file changed, 1 insertion(+)
tools/perf/builtin-report.c +1 -0
@@ -614,6 +614,7 @@ static int stats_print(struct report *rep)static void tasks_setup (struct report *rep) {memset (&rep->tool,0 ,sizeof (rep->tool)); + rep->tool.ordered_events = true;if (rep->mmaps_mode) { rep->tool.mmap = perf_event__process_mmap; rep->tool.mmap2 = perf_event__process_mmap2;
Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 01/17] perf report: Ask ordered events for --tasks option
Em Tue, Feb 06, 2018 at 07:17:57PM +0100, Jiri Olsa escreveu:
Quoted 1 line
If we have the time in, keep the events in time order.
Try to be more verbose, what actual effect this will have in this particular case?
So, I had to try it to see the effects and explain them:
tmp/after 2018-02-06 15:40:51.963403599 -0300 renamed from tmp/before 2018-02-06 15:40:29.536411625 -0300 +2 -2
@@ -5,34 +5,34 @@ 2540 2540 1818 | gnome-terminal- 3489 3489 2540 | bash 32433 32433 3489 | perf - 32434 32434 32433 | perf + 32434 32434 32433 | make 32441 32441 32434 | make 32514 32514 32441 | make 511 511 32514 | sh - 512 512 511 | sh + 512 512 511 | install
We don't have perf calling perf calling make, etc, the second perf actually is 'make', i.e. there was reordering of PERF_RECORD_COMM/PERF_RECORD_FORK:
Look for FORK and COMM meta events, for those tids:
[root@jouet acme]# perf report -D | egrep 'PERF_RECORD_(FORK|COMM)' | egrep '3243[34]' 0 14774650990679 0x1a3cd8 [0x38]: PERF_RECORD_FORK(32433:32433):(3489:3489) 1 14774652080381 0x1d6568 [0x30]: PERF_RECORD_COMM exec: perf:32433/32433 1 14774742473340 0x1dbb48 [0x38]: PERF_RECORD_FORK(32434:32434):(32433:32433) 0 14774752005779 0x1a4af8 [0x30]: PERF_RECORD_COMM exec: make:32434/32434 0 14774753997960 0x1a5578 [0x38]: PERF_RECORD_FORK(32435:32435):(32434:32434) 0 14774756070782 0x1a5618 [0x38]: PERF_RECORD_FORK(32438:32438):(32434:32434) 0 14774757772939 0x1a5680 [0x38]: PERF_RECORD_FORK(32440:32440):(32434:32434) 0 14774758230600 0x1a56e8 [0x38]: PERF_RECORD_FORK(32441:32441):(32434:32434) [root@jouet acme]#
So they are on different CPUs, thus ring buffers, and when we don't use ordered_events, we end up mixing that up, right?
- Arnaldo
Quoted 20 lines
Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/builtin-report.c | 1 + 1 file changed, 1 insertion(+)tools/perf/builtin-report.c +1
-0@@ -614,6 +614,7 @@ static int stats_print(struct report *rep)static void tasks_setup (struct report *rep) {memset (&rep->tool,0 ,sizeof (rep->tool)); + rep->tool.ordered_events = true;if (rep->mmaps_mode) { rep->tool.mmap = perf_event__process_mmap; rep->tool.mmap2 = perf_event__process_mmap2;Signature · 1 line
2.13.6
Re: [PATCH 01/17] perf report: Ask ordered events for --tasks option
Quoted 38 lines · Arnaldo Carvalho de Melo
Em Tue, Feb 06, 2018 at 07:17:57PM +0100, Jiri Olsa escreveu:
Quoted 1 line
If we have the time in, keep the events in time order.
Try to be more verbose, what actual effect this will have in this particular case?
So, I had to try it to see the effects and explain them:
tmp/after 2018-02-06 15:40:51.963403599 -0300 renamed from tmp/before 2018-02-06 15:40:29.536411625 -0300 +2
-2@@ -5,34 +5,34 @@ 2540 2540 1818 | gnome-terminal- 3489 3489 2540 | bash 32433 32433 3489 | perf - 32434 32434 32433 | perf + 32434 32434 32433 | make 32441 32441 32434 | make 32514 32514 32441 | make 511 511 32514 | sh - 512 512 511 | sh + 512 512 511 | installWe don't have perf calling perf calling make, etc, the second perf actually is 'make', i.e. there was reordering of PERF_RECORD_COMM/PERF_RECORD_FORK:
Look for FORK and COMM meta events, for those tids:
[root@jouet acme]# perf report -D | egrep '
PERF_RECORD_(FORK|COMM)' | egrep '3243[34]' 0 147746509906790x1a3cd8[0x38]: PERF_RECORD_FORK(32433:32433):(3489:3489) 1 147746520803810x1d6568[0x30]:PERF_RECORD_COMMexec: perf:32433/32433 1 147747424733400x1dbb48[0x38]: PERF_RECORD_FORK(32434:32434):(32433:32433) 0 147747520057790x1a4af8[0x30]:PERF_RECORD_COMMexec: make:32434/32434 0 147747539979600x1a5578[0x38]: PERF_RECORD_FORK(32435:32435):(32434:32434) 0 147747560707820x1a5618[0x38]: PERF_RECORD_FORK(32438:32438):(32434:32434) 0 147747577729390x1a5680[0x38]: PERF_RECORD_FORK(32440:32440):(32434:32434) 0 147747582306000x1a56e8[0x38]: PERF_RECORD_FORK(32441:32441):(32434:32434) [root@jouet acme]#
Quoted 2 lines
So they are on different CPUs, thus ring buffers, and when we don't use
ordered_events, we end up mixing that up, right?
right ;-) time sorted is always better..
thanks,
jirka
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 01/17] perf report: Ask ordered events for --tasks option
Em Tue, Feb 06, 2018 at 07:59:51PM +0100, Jiri Olsa escreveu:
Quoted 39 lines · Arnaldo Carvalho de Melo
Quoted 38 lines
Em Tue, Feb 06, 2018 at 07:17:57PM +0100, Jiri Olsa escreveu:
Quoted 1 line
If we have the time in, keep the events in time order.
Try to be more verbose, what actual effect this will have in this particular case?
So, I had to try it to see the effects and explain them:
tmp/after 2018-02-06 15:40:51.963403599 -0300 renamed from tmp/before 2018-02-06 15:40:29.536411625 -0300 +2
-2@@ -5,34 +5,34 @@ 2540 2540 1818 | gnome-terminal- 3489 3489 2540 | bash 32433 32433 3489 | perf - 32434 32434 32433 | perf + 32434 32434 32433 | make 32441 32441 32434 | make 32514 32514 32441 | make 511 511 32514 | sh - 512 512 511 | sh + 512 512 511 | installWe don't have perf calling perf calling make, etc, the second perf actually is 'make', i.e. there was reordering of PERF_RECORD_COMM/PERF_RECORD_FORK:
Look for FORK and COMM meta events, for those tids:
[root@jouet acme]# perf report -D | egrep '
PERF_RECORD_(FORK|COMM)' | egrep '3243[34]' 0 147746509906790x1a3cd8[0x38]: PERF_RECORD_FORK(32433:32433):(3489:3489) 1 147746520803810x1d6568[0x30]:PERF_RECORD_COMMexec: perf:32433/32433 1 147747424733400x1dbb48[0x38]: PERF_RECORD_FORK(32434:32434):(32433:32433) 0 147747520057790x1a4af8[0x30]:PERF_RECORD_COMMexec: make:32434/32434 0 147747539979600x1a5578[0x38]: PERF_RECORD_FORK(32435:32435):(32434:32434) 0 147747560707820x1a5618[0x38]: PERF_RECORD_FORK(32438:32438):(32434:32434) 0 147747577729390x1a5680[0x38]: PERF_RECORD_FORK(32440:32440):(32434:32434) 0 147747582306000x1a56e8[0x38]: PERF_RECORD_FORK(32441:32441):(32434:32434) [root@jouet acme]#
Quoted 4 lines
Quoted 2 lines
So they are on different CPUs, thus ring buffers, and when we don't use
ordered_events, we end up mixing that up, right?right ;-) time sorted is always better..
Sure ;-) Adding the comments and applying...
- Arnaldo
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf report: Ask for ordered events for --tasks option
In reply to Jiri Olsaperf report: Ask for ordered events for --tasks option
Commit-ID: 8614ada0be7d7be84b85c006d526a9c8f76484fa Gitweb: https://git.kernel.org/tip/8614ada0be7d7be84b85c006d526a9c8f76484fa Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:17:57 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
perf report: Ask for ordered events for --tasks option
If we have the time in, keep the events in time order.
Committer notes:
Trying to be more verbose, what actual effect this will have in this particular case?
Before and after this patch shows the artifacts:
--- /tmp/before 2018-02-06 15:40:29.536411625 -0300 +++ /tmp/after 2018-02-06 15:40:51.963403599 -0300 @@ -5,34 +5,34 @@
2540 2540 1818 | gnome-terminal-
3489 3489 2540 | bash
32433 32433 3489 | perf
- 32434 32434 32433 | perf
+ 32434 32434 32433 | make
32441 32441 32434 | make
32514 32514 32441 | make
511 511 32514 | sh
- 512 512 511 | sh
+ 512 512 511 | install<SNIP>
We don't have 'perf' calling 'perf' calling 'make', etc, the second 'perf' actually is 'make', i.e. there was reordering of the relevant PERF_RECORD_COMM and PERF_RECORD_FORK records.
Ditto for sh/install later on.
Look for FORK and COMM meta events, for those tids:
# perf report -D | egrep 'PERF_RECORD_(FORK|COMM)' | egrep '3243[34]' 0 14774650990679 0x1a3cd8 [0x38]: PERF_RECORD_FORK(32433:32433):(3489:3489) 1 14774652080381 0x1d6568 [0x30]: PERF_RECORD_COMM exec: perf:32433/32433 1 14774742473340 0x1dbb48 [0x38]: PERF_RECORD_FORK(32434:32434):(32433:32433) 0 14774752005779 0x1a4af8 [0x30]: PERF_RECORD_COMM exec: make:32434/32434 0 14774753997960 0x1a5578 [0x38]: PERF_RECORD_FORK(32435:32435):(32434:32434) 0 14774756070782 0x1a5618 [0x38]: PERF_RECORD_FORK(32438:32438):(32434:32434) 0 14774757772939 0x1a5680 [0x38]: PERF_RECORD_FORK(32440:32440):(32434:32434) 0 14774758230600 0x1a56e8 [0x38]: PERF_RECORD_FORK(32441:32441):(32434:32434) #
First column is the cpu, second is the timestamp.
So they are on different CPUs, thus ring buffers, and when we don't use the ordered_events class, we end up mixing that up, use it to take advantage of the PERF_RECORD_FINISHED_ROUND meta events to go on ordering the events using the PERF_SAMPLE_TIME present in the PERF_RECORD_{FORK,COMM,EXIT,SAMPLE,etc} records in the ring buffer.
--- tools/perf/builtin-report.c | 1 + 1 file changed, 1 insertion(+)
tools/perf/builtin-report.c +1 -0
@@ -614,6 +614,7 @@ static int stats_print(struct report *rep)static void tasks_setup (struct report *rep) {memset (&rep->tool,0 ,sizeof (rep->tool)); + rep->tool.ordered_events = true;if (rep->mmaps_mode) { rep->tool.mmap = perf_event__process_mmap; rep->tool.mmap2 = perf_event__process_mmap2;
Jiri Olsa
reviewerPATCH 2/17 perf record: Put new line after target override warning
In reply to Jiri OlsaThere's no new-line after target-override warning, now:
$ perf record -a --per-thread Warning: SYSTEM/CPU switch overriding PER-THREAD^C[ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.705 MB perf.data (2939 samples) ]
with patch:
$ perf record -a --per-thread Warning: SYSTEM/CPU switch overriding PER-THREAD ^C[ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.705 MB perf.data (2939 samples) ]
--- tools/perf/builtin-record.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-record.c +1 -1
@@ -1803,7 +1803,7 @@ int cmd_record(int argc, const char **argv) err =target__validate (&rec->opts.target);if (err) {target__strerror (&rec->opts.target, err, errbuf,BUFSIZ ); -ui__warning ("%s" , errbuf); +ui__warning ("%s\n" , errbuf); } err =target__parse_uid (&rec->opts.target);
Signature · 1 line
2.13.6
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf record: Put new line after target override warning
perf record: Put new line after target override warning
Commit-ID: c3dec27b7f70a9ad5f777d943d51ecdfcd9824d0 Gitweb: https://git.kernel.org/tip/c3dec27b7f70a9ad5f777d943d51ecdfcd9824d0 Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:17:58 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
perf record: Put new line after target override warning
There's no new-line after target-override warning, now:
$ perf record -a --per-thread Warning: SYSTEM/CPU switch overriding PER-THREAD^C[ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.705 MB perf.data (2939 samples) ]
with patch:
$ perf record -a --per-thread Warning: SYSTEM/CPU switch overriding PER-THREAD ^C[ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.705 MB perf.data (2939 samples) ]
--- tools/perf/builtin-record.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-record.c +1 -1
@@ -1803,7 +1803,7 @@ int cmd_record(int argc, const char **argv) err =target__validate (&rec->opts.target);if (err) {target__strerror (&rec->opts.target, err, errbuf,BUFSIZ ); -ui__warning ("%s" , errbuf); +ui__warning ("%s\n" , errbuf); } err =target__parse_uid (&rec->opts.target);
Jiri Olsa
reviewerPATCH 3/17 perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND
In reply to Jiri OlsaAdding --show-round-event to display PERF_RECORD_FINISHED_ROUND events like:
# perf script --show-round-events 2>/dev/null
yes 8591 [002] 124177.397597: 18 cpu/mem-stores/P: ff...
yes 8591 [002] 124177.397615: 1 cpu/mem-loads,ldlat=30/P: ff...
PERF_RECORD_FINISHED_ROUND
perf 10380 [001] 124177.397622: 6 cpu/mem-loads,ldlat=30/P: ff...
PERF_RECORD_FINISHED_ROUND
swapper 0 [000] 124177.400518: 88 cpu/mem-stores/P: ff...
swapper 0 [000] 124177.400521: 88 cpu/mem-stores/P: ff...--- tools/perf/Documentation/perf-script.txt | 3 +++ tools/perf/builtin-script.c | 17 +++++++++++++++++ 2 files changed, 20 insertions(+)
tools/perf/Documentation/perf-script.txt +3 -0
@@ -303,6 +303,9 @@ OPTIONS --show-lost-events Display lost events i.e. events of type PERF_RECORD_LOST. +--show-round-events + Display finished round events i.e. events of type PERF_RECORD_FINISHED_ROUND. + --demangle:: Demangle symbol names to human readable form. It's enabled by default, disable with --no-demangle.
tools/perf/builtin-script.c +17 -0
@@ -1489,6 +1489,7 @@ struct perf_script {bool show_switch_events;bool show_namespace_events;bool show_lost_events; +bool show_round_events;bool allocated;bool per_event_dump;struct cpu_map *cpus;
@@ -2104,6 +2105,16 @@ process_lost_event(struct perf_tool *tool,return 0 ; } +static int +process_finished_round_event (struct perf_tool *tool__maybe_unused , +union perf_event *event, +struct ordered_events *oe__maybe_unused ) + +{ +perf_event__fprintf (event, stdout); +return 0 ; +} +static void sig_handler (int sig__maybe_unused ) { session_done =1 ;
@@ -2200,6 +2211,10 @@ static int __cmd_script(struct perf_script *script) script->tool.namespaces = process_namespaces_event;if (script->show_lost_events) script->tool.lost = process_lost_event; +if (script->show_round_events) { + script->tool.ordered_events = false; + script->tool.finished_round = process_finished_round_event; + }if (perf_script__setup_per_event_dump (script)) {pr_err ("Couldn't create the per event dump files\n" );
@@ -3139,6 +3154,8 @@ int cmd_script(int argc, const char **argv) "Show namespace events (if recorded)"),OPT_BOOLEAN ('\0' ,"show-lost-events" , &script.show_lost_events,"Show lost events (if recorded)" ), +OPT_BOOLEAN ('\0' ,"show-round-events" , &script.show_round_events, +"Show round events (if recorded)" ),OPT_BOOLEAN ('\0' ,"per-event-dump" , &script.per_event_dump,"Dump trace output to files named by the monitored events" ), OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
Signature · 1 line
2.13.6
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND
perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND
Commit-ID: 3233b37a71c794e25a0a794185df8d6abd9f277e Gitweb: https://git.kernel.org/tip/3233b37a71c794e25a0a794185df8d6abd9f277e Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:17:59 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND
Adding --show-round-event to display PERF_RECORD_FINISHED_ROUND events like:
# perf script --show-round-events 2>/dev/null
yes 8591 [002] 124177.397597: 18 cpu/mem-stores/P: ff...
yes 8591 [002] 124177.397615: 1 cpu/mem-loads,ldlat=30/P: ff...
PERF_RECORD_FINISHED_ROUND
perf 10380 [001] 124177.397622: 6 cpu/mem-loads,ldlat=30/P: ff...
PERF_RECORD_FINISHED_ROUND
swapper 0 [000] 124177.400518: 88 cpu/mem-stores/P: ff...
swapper 0 [000] 124177.400521: 88 cpu/mem-stores/P: ff...--- tools/perf/Documentation/perf-script.txt | 3 +++ tools/perf/builtin-script.c | 17 +++++++++++++++++ 2 files changed, 20 insertions(+)
tools/perf/Documentation/perf-script.txt +3 -0
@@ -303,6 +303,9 @@ OPTIONS --show-lost-events Display lost events i.e. events of type PERF_RECORD_LOST. +--show-round-events + Display finished round events i.e. events of type PERF_RECORD_FINISHED_ROUND. + --demangle:: Demangle symbol names to human readable form. It's enabled by default, disable with --no-demangle.
tools/perf/builtin-script.c +17 -0
@@ -1489,6 +1489,7 @@ struct perf_script {bool show_switch_events;bool show_namespace_events;bool show_lost_events; +bool show_round_events;bool allocated;bool per_event_dump;struct cpu_map *cpus;
@@ -2104,6 +2105,16 @@ process_lost_event(struct perf_tool *tool,return 0 ; } +static int +process_finished_round_event (struct perf_tool *tool__maybe_unused , +union perf_event *event, +struct ordered_events *oe__maybe_unused ) + +{ +perf_event__fprintf (event, stdout); +return 0 ; +} +static void sig_handler (int sig__maybe_unused ) { session_done =1 ;
@@ -2200,6 +2211,10 @@ static int __cmd_script(struct perf_script *script) script->tool.namespaces = process_namespaces_event;if (script->show_lost_events) script->tool.lost = process_lost_event; +if (script->show_round_events) { + script->tool.ordered_events = false; + script->tool.finished_round = process_finished_round_event; + }if (perf_script__setup_per_event_dump (script)) {pr_err ("Couldn't create the per event dump files\n" );
@@ -3139,6 +3154,8 @@ int cmd_script(int argc, const char **argv) "Show namespace events (if recorded)"),OPT_BOOLEAN ('\0' ,"show-lost-events" , &script.show_lost_events,"Show lost events (if recorded)" ), +OPT_BOOLEAN ('\0' ,"show-round-events" , &script.show_round_events, +"Show round events (if recorded)" ),OPT_BOOLEAN ('\0' ,"per-event-dump" , &script.per_event_dump,"Dump trace output to files named by the monitored events" ), OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
Jiri Olsa
reviewerPATCH 4/17 tools lib api fs: Add filename__read_xll function
In reply to Jiri OlsaAdding filename__read_xll function to be able to read files with hex numbers in, which do not have 0x prefix.
--- tools/lib/api/fs/fs.c | 29 ++++++++++++++++++++++------- tools/lib/api/fs/fs.h | 1 + 2 files changed, 23 insertions(+), 7 deletions(-)
tools/lib/api/fs/fs.c +22 -7
@@ -315,12 +315,8 @@ int filename__read_int(const char *filename, int *value)return err; } -/* -* Parses @value out of @filename with strtoull. -* By using 0 for base, the strtoull detects the -* base automatically (see man strtoull). -*/ -int filename__read_ull (const char *filename,unsigned long long *value) +static int filename__read_ull_base (const char *filename, +unsigned long long *value,int base) {char line[64 ];int fd =open (filename,O_RDONLY ), err =-1 ;
@@ -329,7 +325,7 @@ int filename__read_ull(const char *filename, unsigned long long *value)return -1 ;if (read (fd, line,sizeof (line)) >0 ) { - *value =strtoull (line,NULL ,0 ); + *value =strtoull (line,NULL , base);if (*value !=ULLONG_MAX ) err =0 ; }
@@ -338,6 +334,25 @@ int filename__read_ull(const char *filename, unsigned long long *value)return err; } +/* +* Parses @value out of @filename with strtoull. +* By using 16 for base to treat the number as hex. +*/ +int filename__read_xll (const char *filename,unsigned long long *value) +{ +return filename__read_ull_base (filename, value,16 ); +} + +/* +* Parses @value out of @filename with strtoull. +* By using 0 for base, the strtoull detects the +* base automatically (see man strtoull). +*/ +int filename__read_ull (const char *filename,unsigned long long *value) +{ +return filename__read_ull_base (filename, value,0 ); +} +#define STRERR_BUFSIZE 128/* For the buffer size of strerror_r */ int filename__read_str (const char *filename,char **buf,size_t *sizep)
tools/lib/api/fs/fs.h +1 -0
@@ -30,6 +30,7 @@ FS(bpf_fs)int filename__read_int (const char *filename,int *value);int filename__read_ull (const char *filename,unsigned long long *value); +int filename__read_xll (const char *filename,unsigned long long *value);int filename__read_str (const char *filename,char **buf,size_t *sizep);int filename__write_int (const char *filename,int value);
Signature · 1 line
2.13.6
tip-bot for Jiri Olsa
merge bot[tip:perf/core] tools lib api fs: Add filename__read_xll function
tools lib api fs: Add filename__read_xll function
Commit-ID: 6baddfc6900eca7f6b360c91ff737890ab4f1d55 Gitweb: https://git.kernel.org/tip/6baddfc6900eca7f6b360c91ff737890ab4f1d55 Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:18:00 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
tools lib api fs: Add filename__read_xll function
Adding filename__read_xll function to be able to read files with hex numbers in, which do not have 0x prefix.
--- tools/lib/api/fs/fs.c | 29 ++++++++++++++++++++++------- tools/lib/api/fs/fs.h | 1 + 2 files changed, 23 insertions(+), 7 deletions(-)
tools/lib/api/fs/fs.c +22 -7
@@ -315,12 +315,8 @@ int filename__read_int(const char *filename, int *value)return err; } -/* -* Parses @value out of @filename with strtoull. -* By using 0 for base, the strtoull detects the -* base automatically (see man strtoull). -*/ -int filename__read_ull (const char *filename,unsigned long long *value) +static int filename__read_ull_base (const char *filename, +unsigned long long *value,int base) {char line[64 ];int fd =open (filename,O_RDONLY ), err =-1 ;
@@ -329,7 +325,7 @@ int filename__read_ull(const char *filename, unsigned long long *value)return -1 ;if (read (fd, line,sizeof (line)) >0 ) { - *value =strtoull (line,NULL ,0 ); + *value =strtoull (line,NULL , base);if (*value !=ULLONG_MAX ) err =0 ; }
@@ -338,6 +334,25 @@ int filename__read_ull(const char *filename, unsigned long long *value)return err; } +/* +* Parses @value out of @filename with strtoull. +* By using 16 for base to treat the number as hex. +*/ +int filename__read_xll (const char *filename,unsigned long long *value) +{ +return filename__read_ull_base (filename, value,16 ); +} + +/* +* Parses @value out of @filename with strtoull. +* By using 0 for base, the strtoull detects the +* base automatically (see man strtoull). +*/ +int filename__read_ull (const char *filename,unsigned long long *value) +{ +return filename__read_ull_base (filename, value,0 ); +} +#define STRERR_BUFSIZE 128/* For the buffer size of strerror_r */ int filename__read_str (const char *filename,char **buf,size_t *sizep)
tools/lib/api/fs/fs.h +1 -0
@@ -30,6 +30,7 @@ FS(bpf_fs)int filename__read_int (const char *filename,int *value);int filename__read_ull (const char *filename,unsigned long long *value); +int filename__read_xll (const char *filename,unsigned long long *value);int filename__read_str (const char *filename,char **buf,size_t *sizep);int filename__write_int (const char *filename,int value);
Jiri Olsa
reviewerPATCH 5/17 tools lib api fs: Add sysfs__read_xll function
In reply to Jiri OlsaAdding sysfs__read_xll function to be able to read sysfs files with hex numbers in, which do not have 0x prefix.
--- tools/lib/api/fs/fs.c | 15 +++++++++++++-- tools/lib/api/fs/fs.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)
tools/lib/api/fs/fs.c +13 -2
@@ -432,7 +432,8 @@ int procfs__read_str(const char *entry, char **buf, size_t *sizep)return filename__read_str (path, buf, sizep); } -int sysfs__read_ull (const char *entry,unsigned long long *value) +static int sysfs__read_ull_base (const char *entry, +unsigned long long *value,int base) {char path[PATH_MAX ];const char *sysfs =sysfs__mountpoint ();
@@ -442,7 +443,17 @@ int sysfs__read_ull(const char *entry, unsigned long long *value)snprintf (path,sizeof (path),"%s/%s" , sysfs, entry); -return filename__read_ull (path, value); +return filename__read_ull_base (path, value, base); +} + +int sysfs__read_xll (const char *entry,unsigned long long *value) +{ +return sysfs__read_ull_base (entry, value,16 ); +} + +int sysfs__read_ull (const char *entry,unsigned long long *value) +{ +return sysfs__read_ull_base (entry, value,0 ); }int sysfs__read_int (const char *entry,int *value)
tools/lib/api/fs/fs.h +1 -0
@@ -40,6 +40,7 @@ int procfs__read_str(const char *entry, char **buf, size_t *sizep);int sysctl__read_int (const char *sysctl,int *value);int sysfs__read_int (const char *entry,int *value);int sysfs__read_ull (const char *entry,unsigned long long *value); +int sysfs__read_xll (const char *entry,unsigned long long *value);int sysfs__read_str (const char *entry,char **buf,size_t *sizep);int sysfs__read_bool (const char *entry,bool *value);
Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 05/17] tools lib api fs: Add sysfs__read_xll function
Em Tue, Feb 06, 2018 at 07:18:01PM +0100, Jiri Olsa escreveu:
Quoted 2 lines
Adding
sysfs__read_xllfunction to be able to read sysfs files with hex numbers in, which do not have 0x prefix.
Applied 2-5 in this series, continuing...
- Arnaldo
Quoted 54 lines
Signed-off-by Jiri Olsa <[email protected]>--- tools/lib/api/fs/fs.c | 15 +++++++++++++-- tools/lib/api/fs/fs.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)tools/lib/api/fs/fs.c +13
-2@@ -432,7 +432,8 @@ int procfs__read_str(const char *entry, char **buf, size_t *sizep)return filename__read_str (path, buf, sizep); } -int sysfs__read_ull (const char *entry,unsigned long long *value) +static int sysfs__read_ull_base (const char *entry, +unsigned long long *value,int base) {char path[PATH_MAX ];const char *sysfs =sysfs__mountpoint ();@@ -442,7 +443,17 @@ int sysfs__read_ull(const char *entry, unsigned long long *value)snprintf (path,sizeof (path),"%s/%s" , sysfs, entry); -return filename__read_ull (path, value); +return filename__read_ull_base (path, value, base); +} + +int sysfs__read_xll (const char *entry,unsigned long long *value) +{ +return sysfs__read_ull_base (entry, value,16 ); +} + +int sysfs__read_ull (const char *entry,unsigned long long *value) +{ +return sysfs__read_ull_base (entry, value,0 ); }int sysfs__read_int (const char *entry,int *value)tools/lib/api/fs/fs.h +1
-0@@ -40,6 +40,7 @@ int procfs__read_str(const char *entry, char **buf, size_t *sizep);int sysctl__read_int (const char *sysctl,int *value);int sysfs__read_int (const char *entry,int *value);int sysfs__read_ull (const char *entry,unsigned long long *value); +int sysfs__read_xll (const char *entry,unsigned long long *value);int sysfs__read_str (const char *entry,char **buf,size_t *sizep);int sysfs__read_bool (const char *entry,bool *value);Signature · 1 line
2.13.6
tip-bot for Jiri Olsa
merge bot[tip:perf/core] tools lib api fs: Add sysfs__read_xll function
In reply to Jiri Olsatools lib api fs: Add sysfs__read_xll function
Commit-ID: d9c5f32240f503481291a6d4e7246ee0a128d76d Gitweb: https://git.kernel.org/tip/d9c5f32240f503481291a6d4e7246ee0a128d76d Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:18:01 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
tools lib api fs: Add sysfs__read_xll function
Adding sysfs__read_xll function to be able to read sysfs files with hex numbers in, which do not have 0x prefix.
--- tools/lib/api/fs/fs.c | 15 +++++++++++++-- tools/lib/api/fs/fs.h | 1 + 2 files changed, 14 insertions(+), 2 deletions(-)
tools/lib/api/fs/fs.c +13 -2
@@ -432,7 +432,8 @@ int procfs__read_str(const char *entry, char **buf, size_t *sizep)return filename__read_str (path, buf, sizep); } -int sysfs__read_ull (const char *entry,unsigned long long *value) +static int sysfs__read_ull_base (const char *entry, +unsigned long long *value,int base) {char path[PATH_MAX ];const char *sysfs =sysfs__mountpoint ();
@@ -442,7 +443,17 @@ int sysfs__read_ull(const char *entry, unsigned long long *value)snprintf (path,sizeof (path),"%s/%s" , sysfs, entry); -return filename__read_ull (path, value); +return filename__read_ull_base (path, value, base); +} + +int sysfs__read_xll (const char *entry,unsigned long long *value) +{ +return sysfs__read_ull_base (entry, value,16 ); +} + +int sysfs__read_ull (const char *entry,unsigned long long *value) +{ +return sysfs__read_ull_base (entry, value,0 ); }int sysfs__read_int (const char *entry,int *value)
tools/lib/api/fs/fs.h +1 -0
@@ -40,6 +40,7 @@ int procfs__read_str(const char *entry, char **buf, size_t *sizep);int sysctl__read_int (const char *sysctl,int *value);int sysfs__read_int (const char *entry,int *value);int sysfs__read_ull (const char *entry,unsigned long long *value); +int sysfs__read_xll (const char *entry,unsigned long long *value);int sysfs__read_str (const char *entry,char **buf,size_t *sizep);int sysfs__read_bool (const char *entry,bool *value);
Jiri Olsa
reviewerPATCH 6/17 tools lib symbol: Skip non-address kallsyms line
In reply to Jiri OlsaAdding check on failed attempt to parse the address and skip the line parsing early in that case.
--- tools/lib/symbol/kallsyms.c | 4 ++++ 1 file changed, 4 insertions(+)
tools/lib/symbol/kallsyms.c +4 -0
@@ -38,6 +38,10 @@ int kallsyms__parse(const char *filename, void *arg, len =hex2u64 (line, &start); +/* Skip the line if we failed to parse the address. */ +if (!len) +continue ; + len++;if (len +2 >= line_len)continue ;
Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 06/17] tools lib symbol: Skip non-address kallsyms line
Em Tue, Feb 06, 2018 at 07:18:02PM +0100, Jiri Olsa escreveu:
Quoted 2 lines
Adding check on failed attempt to parse the address and skip the line parsing early in that case.
How did you stumble on that? Can you provide an example of a line or situation where this would happen?
- Arnaldo
Quoted 23 lines
Signed-off-by Jiri Olsa <[email protected]>--- tools/lib/symbol/kallsyms.c | 4 ++++ 1 file changed, 4 insertions(+)tools/lib/symbol/kallsyms.c +4
-0@@ -38,6 +38,10 @@ int kallsyms__parse(const char *filename, void *arg, len =hex2u64 (line, &start); +/* Skip the line if we failed to parse the address. */ +if (!len) +continue ; + len++;if (len +2 >= line_len)continue ;Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 06/17] tools lib symbol: Skip non-address kallsyms line
Em Tue, Feb 06, 2018 at 04:06:49PM -0300, Arnaldo Carvalho de Melo escreveu:
Quoted 6 lines
Em Tue, Feb 06, 2018 at 07:18:02PM +0100, Jiri Olsa escreveu:
Quoted 2 lines
Adding check on failed attempt to parse the address and skip the line parsing early in that case.
How did you stumble on that? Can you provide an example of a line or situation where this would happen?
Something like this:
[acme@jouet perf]$ cat /proc/kallsyms | head
(null) A irq_stack_union
(null) A __per_cpu_start
(null) A cpu_debug_store
(null) A cpu_tss_rw
(null) A gdt_page
(null) A exception_stacks
(null) A entry_stack_storage
(null) A espfix_waddr
(null) A espfix_stack
(null) A cpu_llc_id
[acme@jouet perf]$ id
uid=1000(acme) gid=1000(acme) groups=1000(acme),10(wheel),18(dialout),54(lock) context=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 [acme@jouet perf]$
?
- Arnaldo
Jiri Olsa
reviewerPATCH 7/17 perf tools: Free root_dir in machine__init error path
In reply to Jiri OlsaFreeing root_dir in machine__init error path.
--- tools/perf/util/machine.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
tools/perf/util/machine.c +7 -1
@@ -50,6 +50,8 @@ static void machine__threads_init(struct machine *machine)int machine__init (struct machine *machine,const char *root_dir,pid_t pid) { +int err = -ENOMEM ; +memset (machine,0 ,sizeof (*machine));map_groups__init (&machine->kmaps, machine);RB_CLEAR_NODE (&machine->rb_node);
@@ -79,7 +81,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)char comm[64 ];if (thread ==NULL ) -return -ENOMEM ; + goto out;snprintf (comm,sizeof (comm),"[guest/%d]" , pid);thread__set_comm (thread, comm,0 );
@@ -87,7 +89,11 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid) } machine->current_tid =NULL ; + err =0 ; +out: +if (err) +free (machine->root_dir);return 0 ; }
Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 07/17] perf tools: Free root_dir in machine__init error path
Em Tue, Feb 06, 2018 at 07:18:03PM +0100, Jiri Olsa escreveu:
Quoted 31 lines
Freeing
root_dirinmachine__initerror path.Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/util/machine.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)tools/perf/util/machine.c +3
-1@@ -50,6 +50,8 @@ static void machine__threads_init(struct machine *machine)int machine__init (struct machine *machine,const char *root_dir,pid_t pid) { +int err = -ENOMEM ; +memset (machine,0 ,sizeof (*machine));map_groups__init (&machine->kmaps, machine);RB_CLEAR_NODE (&machine->rb_node);@@ -79,7 +81,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)char comm[64 ];if (thread ==NULL ) -return -ENOMEM ; + goto out;snprintf (comm,sizeof (comm),"[guest/%d]" , pid);thread__set_comm (thread, comm,0 );
tools/perf/util/machine.c int machine__init(struct machine *machine, const char *root_dir, pid_t pid) · line 96
Quoted 8 lines
tools/perf/util/machine.c +4
-0@@ -87,7 +89,11 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid) } machine->current_tid =NULL ; + err =0 ; +out: +if (err) +free (machine->root_dir);
In these cases, i.e. freeing something inside some other struct, its better to use:
zfree(&machine->root_dir);
Quoted 5 lines
tools/perf/util/machine.c +0
-0return 0 ; }Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 8/17 perf tools: Move kernel mmap name into struct machine
In reply to Jiri OlsaIt simplifies and centralizes the code. The kernel mmap name is set for machine type, which we know from the beginning, so there's no reason to generate it every time we need it.
--- tools/perf/util/build-id.c | 10 +++---- tools/perf/util/event.c | 5 +--- tools/perf/util/machine.c | 67 +++++++++++++++++++++++----------------------- tools/perf/util/machine.h | 3 +-- tools/perf/util/symbol.c | 3 +-- 5 files changed, 39 insertions(+), 49 deletions(-)
tools/perf/util/build-id.c +3 -7
@@ -316,7 +316,6 @@ static int machine__write_buildid_table(struct machine *machine, struct feat_fd *fd) {int err =0 ; -char nm[PATH_MAX ];struct dso *pos;u16 kmisc =PERF_RECORD_MISC_KERNEL , umisc =PERF_RECORD_MISC_USER ;
@@ -338,9 +337,8 @@ static int machine__write_buildid_table(struct machine *machine, name = pos->short_name; name_len = pos->short_name_len; }else if (dso__is_kcore (pos)) { -machine__mmap_name (machine, nm,sizeof (nm)); - name = nm; - name_len =strlen (nm); + name = machine->mmap_name; + name_len =strlen (name); }else { name = pos->long_name; name_len = pos->long_name_len;
@@ -813,12 +811,10 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)bool is_kallsyms =dso__is_kallsyms (dso);bool is_vdso =dso__is_vdso (dso);const char *name = dso->long_name; -char nm[PATH_MAX ];if (dso__is_kcore (dso)) { is_kallsyms = true; -machine__mmap_name (machine, nm,sizeof (nm)); - name = nm; + name = machine->mmap_name; }return build_id_cache__add_b (dso->build_id,sizeof (dso->build_id), name, dso->nsinfo, is_kallsyms, is_vdso);
tools/perf/util/event.c +1 -4
@@ -894,8 +894,6 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool, struct machine *machine) {size_t size; -const char *mmap_name; -char name_buff[PATH_MAX ];struct map *map =machine__kernel_map (machine);struct kmap *kmap;int err;
@@ -918,7 +916,6 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,return -1 ; } - mmap_name =machine__mmap_name (machine, name_buff,sizeof (name_buff));if (machine__is_host (machine)) {/* * kernel uses PERF_RECORD_MISC_USER for user space maps,
@@ -931,7 +928,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool, kmap =map__kmap (map); size =snprintf (event->mmap.filename,sizeof (event->mmap.filename), -"%s%s" , mmap_name, kmap->ref_reloc_sym->name) +1 ; +"%s%s" , machine->mmap_name, kmap->ref_reloc_sym->name) +1 ; size =PERF_ALIGN (size,sizeof (u64)); event->mmap.header.type =PERF_RECORD_MMAP ; event->mmap.header.size = (sizeof(event->mmap) -
tools/perf/util/machine.c +33 -34
140 lines not on this page · raw message
tools/perf/util/machine.h +1 -2
@@ -43,6 +43,7 @@ struct machine {bool comm_exec;bool kptr_restrict_warned;char *root_dir; +char *mmap_name;struct threads threads[THREADS__TABLE_SIZE ];struct vdso_info *vdso_info;struct perf_env *env;
@@ -142,8 +143,6 @@ struct machine *machines__find(struct machines *machines, pid_t pid);struct machine *machines__findnew (struct machines *machines,pid_t pid);void machines__set_id_hdr_size (struct machines *machines,u16 id_hdr_size); -char *machine__mmap_name (struct machine *machine,char *bf,size_t size); -void machines__set_comm_exec (struct machines *machines,bool comm_exec);struct machine *machine__new_host (void );
tools/perf/util/symbol.c +1 -2
@@ -1960,8 +1960,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)pr_debug ("Using %s for symbols\n" , kallsyms_filename);if (err >0 && !dso__is_kcore (dso)) { dso->binary_type =DSO_BINARY_TYPE__GUEST_KALLSYMS ; -machine__mmap_name (machine, path,sizeof (path)); -dso__set_long_name (dso,strdup (path), true); +dso__set_long_name (dso, machine->mmap_name, false);map__fixup_start (map);map__fixup_end (map); }
Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 08/17] perf tools: Move kernel mmap name into struct machine
Em Tue, Feb 06, 2018 at 07:18:04PM +0100, Jiri Olsa escreveu:
Quoted 88 lines
It simplifies and centralizes the code. The kernel mmap name is set for machine type, which we know from the beginning, so there's no reason to generate it every time we need it.
Signed-off-by Jiri Olsa <[email protected]>--- tools/perf/util/build-id.c | 10 +++---- tools/perf/util/event.c | 5 +--- tools/perf/util/machine.c | 67 +++++++++++++++++++++++----------------------- tools/perf/util/machine.h | 3 +-- tools/perf/util/symbol.c | 3 +-- 5 files changed, 39 insertions(+), 49 deletions(-)tools/perf/util/build-id.c +3
-7@@ -316,7 +316,6 @@ static int machine__write_buildid_table(struct machine *machine, struct feat_fd *fd) {int err =0 ; -char nm[PATH_MAX ];struct dso *pos;u16 kmisc =PERF_RECORD_MISC_KERNEL , umisc =PERF_RECORD_MISC_USER ;@@ -338,9 +337,8 @@ static int machine__write_buildid_table(struct machine *machine, name = pos->short_name; name_len = pos->short_name_len; }else if (dso__is_kcore (pos)) { -machine__mmap_name (machine, nm,sizeof (nm)); - name = nm; - name_len =strlen (nm); + name = machine->mmap_name; + name_len =strlen (name); }else { name = pos->long_name; name_len = pos->long_name_len;@@ -813,12 +811,10 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine)bool is_kallsyms =dso__is_kallsyms (dso);bool is_vdso =dso__is_vdso (dso);const char *name = dso->long_name; -char nm[PATH_MAX ];if (dso__is_kcore (dso)) { is_kallsyms = true; -machine__mmap_name (machine, nm,sizeof (nm)); - name = nm; + name = machine->mmap_name; }return build_id_cache__add_b (dso->build_id,sizeof (dso->build_id), name, dso->nsinfo, is_kallsyms, is_vdso);tools/perf/util/event.c +1
-4@@ -894,8 +894,6 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool, struct machine *machine) {size_t size; -const char *mmap_name; -char name_buff[PATH_MAX ];struct map *map =machine__kernel_map (machine);struct kmap *kmap;int err;@@ -918,7 +916,6 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,return -1 ; } - mmap_name =machine__mmap_name (machine, name_buff,sizeof (name_buff));if (machine__is_host (machine)) {/* * kernel uses PERF_RECORD_MISC_USER for user space maps, @@ -931,7 +928,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool, kmap =map__kmap (map); size =snprintf (event->mmap.filename,sizeof (event->mmap.filename), -"%s%s" , mmap_name, kmap->ref_reloc_sym->name) +1 ; +"%s%s" , machine->mmap_name, kmap->ref_reloc_sym->name) +1 ; size =PERF_ALIGN (size,sizeof (u64)); event->mmap.header.type =PERF_RECORD_MMAP ; event->mmap.header.size = (sizeof(event->mmap) -tools/perf/util/machine.c +0
-0
tools/perf/util/machine.c static void machine__threads_init(struct machine *machine) · line 51
Quoted 4 lines
tools/perf/util/machine.c +1
-0@@ -48,6 +48,27 @@ static void machine__threads_init(struct machine *machine) } } +static int machine__mmap_name (struct machine *machine)
Rename this to machine__set_mmap_name()?
Quoted 34 lines
tools/perf/util/machine.c +23
-0+{ +if (machine__is_host (machine)) { +if (symbol_conf.vmlinux_name) + machine->mmap_name =strdup (symbol_conf.vmlinux_name); +else + machine->mmap_name =strdup ("[kernel.kallsyms]" ); + }else if (machine__is_default_guest (machine)) { +if (symbol_conf.default_guest_vmlinux_name) + machine->mmap_name =strdup (symbol_conf.default_guest_vmlinux_name); +else + machine->mmap_name =strdup ("[guest.kernel.kallsyms]" ); + }else { +if (asprintf (&machine->mmap_name,"[guest.kernel.kallsyms.%d]" , + machine->pid) <0 ) + machine->mmap_name =NULL ; + } + +return machine->mmap_name ?0 : -ENOMEM ; +} +int machine__init (struct machine *machine,const char *root_dir,pid_t pid) {int err = -ENOMEM ;@@ -75,6 +96,9 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)if (machine->root_dir ==NULL )return -ENOMEM ; +if (machine__mmap_name (machine)) + goto out; +if (pid !=HOST_KERNEL_ID ) {struct thread *thread =machine__findnew_thread (machine,-1 , pid);
tools/perf/util/machine.c int machine__init(struct machine *machine, const char *root_dir, pid_t pid) · line 121
Quoted 7 lines
tools/perf/util/machine.c +2
-1@@ -92,8 +116,10 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid) err =0 ; out: -if (err) +if (err) {free (machine->root_dir); +free (machine->mmap_name);
zfree()
Quoted 141 lines
tools/perf/util/machine.c +7
-33+ }return 0 ; }@@ -186,6 +212,7 @@ void machine__exit(struct machine *machine)dsos__exit (&machine->dsos);machine__exit_vdso (machine);zfree (&machine->root_dir); +zfree (&machine->mmap_name);zfree (&machine->current_tid);for (i =0 ; i <THREADS__TABLE_SIZE ; i++) {@@ -328,20 +355,6 @@ void machines__process_guests(struct machines *machines, } } -char *machine__mmap_name (struct machine *machine,char *bf,size_t size) -{ -if (machine__is_host (machine)) -snprintf (bf, size,"[%s]" ,"kernel.kallsyms" ); -else if (machine__is_default_guest (machine)) -snprintf (bf, size,"[%s]" ,"guest.kernel.kallsyms" ); -else { -snprintf (bf, size,"[%s.%d]" ,"guest.kernel.kallsyms" , - machine->pid); - } - -return bf; -} -void machines__set_id_hdr_size (struct machines *machines,u16 id_hdr_size) {struct rb_node *node;@@ -777,25 +790,13 @@ size_t machine__fprintf(struct machine *machine, FILE *fp)static struct dso *machine__get_kernel (struct machine *machine) { -const char *vmlinux_name =NULL ; +const char *vmlinux_name = machine->mmap_name;struct dso *kernel;if (machine__is_host (machine)) { - vmlinux_name = symbol_conf.vmlinux_name; -if (!vmlinux_name) - vmlinux_name =DSO__NAME_KALLSYMS ; - kernel =machine__findnew_kernel (machine, vmlinux_name,"[kernel]" ,DSO_TYPE_KERNEL ); }else { -char bf[PATH_MAX ]; - -if (machine__is_default_guest (machine)) - vmlinux_name = symbol_conf.default_guest_vmlinux_name; -if (!vmlinux_name) - vmlinux_name =machine__mmap_name (machine, bf, -sizeof (bf)); - kernel =machine__findnew_kernel (machine, vmlinux_name,"[guest.kernel]" ,DSO_TYPE_GUEST_KERNEL );@@ -1295,7 +1296,6 @@ static int machine__process_kernel_mmap_event(struct machine *machine, union perf_event *event) {struct map *map; -char kmmap_prefix[PATH_MAX ];enum dso_kernel_type kernel_type;bool is_kernel_mmap;@@ -1303,15 +1303,14 @@ static int machine__process_kernel_mmap_event(struct machine *machine,if (machine__uses_kcore (machine))return 0 ; -machine__mmap_name (machine, kmmap_prefix,sizeof (kmmap_prefix));if (machine__is_host (machine)) kernel_type =DSO_TYPE_KERNEL ;else kernel_type =DSO_TYPE_GUEST_KERNEL ; is_kernel_mmap =memcmp (event->mmap.filename, - kmmap_prefix, -strlen (kmmap_prefix) -1 ) ==0 ; + machine->mmap_name, +strlen (machine->mmap_name) -1 ) ==0 ;if (event->mmap.filename[0 ] =='/' || (!is_kernel_mmap && event->mmap.filename[0 ] =='[' )) { map = machine__findnew_module_map(machine, event->mmap.start,@@ -1322,7 +1321,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine, map->end = map->start + event->mmap.len; }else if (is_kernel_mmap) {const char *symbol_name = (event->mmap.filename + -strlen (kmmap_prefix)); +strlen (machine->mmap_name));/* * Should be there already, from the build-id table in * the header. @@ -1363,7 +1362,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,up_read (&machine->dsos.lock);if (kernel ==NULL ) - kernel =machine__findnew_dso (machine, kmmap_prefix); + kernel =machine__findnew_dso (machine, machine->mmap_name);if (kernel ==NULL ) goto out_problem;tools/perf/util/machine.h +1
-2@@ -43,6 +43,7 @@ struct machine {bool comm_exec;bool kptr_restrict_warned;char *root_dir; +char *mmap_name;struct threads threads[THREADS__TABLE_SIZE ];struct vdso_info *vdso_info;struct perf_env *env;@@ -142,8 +143,6 @@ struct machine *machines__find(struct machines *machines, pid_t pid);struct machine *machines__findnew (struct machines *machines,pid_t pid);void machines__set_id_hdr_size (struct machines *machines,u16 id_hdr_size); -char *machine__mmap_name (struct machine *machine,char *bf,size_t size); -void machines__set_comm_exec (struct machines *machines,bool comm_exec);struct machine *machine__new_host (void );tools/perf/util/symbol.c +1
-2@@ -1960,8 +1960,7 @@ static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map)pr_debug ("Using %s for symbols\n" , kallsyms_filename);if (err >0 && !dso__is_kcore (dso)) { dso->binary_type =DSO_BINARY_TYPE__GUEST_KALLSYMS ; -machine__mmap_name (machine, path,sizeof (path)); -dso__set_long_name (dso,strdup (path), true); +dso__set_long_name (dso, machine->mmap_name, false);map__fixup_start (map);map__fixup_end (map); }Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 9/17 perf tools: Don't search for active kernel start in __machine__create_kernel_maps
In reply to Jiri OlsaWe should not search for kernel start address in __machine__create_kernel_maps function, because it's being used in 'report' code path, where we are interested in kernel MMAP data address instead of in current kernel address
The __machine__create_kernel_maps serves purely for creating the machines kernel maps and setting up the kmap group. The report code path then sets the address based on the data from kernel MMAP event in machine__set_kernel_mmap function.
The kallsyms search address logic is used for test code, that calls machine__create_kernel_maps to get current maps and calls machine__get_running_kernel_start to get kernel starting address.
Also making __machine__create_kernel_maps static, because there's no external user.
--- tools/perf/util/machine.c | 6 ++---- tools/perf/util/machine.h | 1 - 2 files changed, 2 insertions(+), 5 deletions(-)
tools/perf/util/machine.c +2 -4
@@ -856,14 +856,12 @@ static int machine__get_running_kernel_start(struct machine *machine,return 0 ; } -int __machine__create_kernel_maps (struct machine *machine,struct dso *kernel) +static int +__machine__create_kernel_maps (struct machine *machine,struct dso *kernel) {int type;u64 start =0 ; -if (machine__get_running_kernel_start (machine,NULL , &start)) -return -1 ; -/* In case of renewal the kernel map, destroy previous one */ machine__destroy_kernel_maps (machine);
tools/perf/util/machine.h +0 -1
@@ -238,7 +238,6 @@ size_t machines__fprintf_dsos_buildid(struct machines *machines, FILE *fp, bool (skip)(struct dso *dso, int parm), int parm);void machine__destroy_kernel_maps (struct machine *machine); -int __machine__create_kernel_maps (struct machine *machine,struct dso *kernel);int machine__create_kernel_maps (struct machine *machine);int machines__create_kernel_maps (struct machines *machines,pid_t pid);
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 10/17 perf tools: Generalize machine__set_kernel_mmap function
In reply to Jiri OlsaSo it could be called without event object, just with start and end values. It will be used in following patch.
--- tools/perf/util/machine.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
tools/perf/util/machine.c +7 -6
@@ -1260,15 +1260,15 @@ int machine__create_kernel_maps(struct machine *machine)return 0 ; } -static void machine__set_kernel_mmap_len (struct machine *machine, -union perf_event *event) +static void machine__set_kernel_mmap (struct machine *machine, +u64 start,u64 end) {int i;for (i =0 ; i <MAP__NR_TYPES ; i++) { - machine->vmlinux_maps[i]->start = event->mmap.start; - machine->vmlinux_maps[i]->end = (event->mmap.start + - event->mmap.len); + machine->vmlinux_maps[i]->start = start; + machine->vmlinux_maps[i]->end = end; +/* * Be a bit paranoid here, some perf.data file came with * a zero sized synthesized MMAP event for the kernel.
@@ -1373,7 +1373,8 @@ static int machine__process_kernel_mmap_event(struct machine *machine,if (strstr (kernel->long_name,"vmlinux" ))dso__set_short_name (kernel,"[kernel.vmlinux]" , false); -machine__set_kernel_mmap_len (machine, event); +machine__set_kernel_mmap (machine, event->mmap.start, + event->mmap.start + event->mmap.len);/* * Avoid using a zero address (kptr_restrict) for the ref reloc
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 11/17 perf tools: Use machine__set_kernel_mmap instead of map_groups__fixup_end
In reply to Jiri OlsaThe machine__set_kernel_mmap does the same job as map_groups__fixup_end when used on kernel maps within machine__create_kernel_maps call.
This way we can also remove map_groups__fixup_end function, because there's no user to it. Also moving machine__set_kernel_mmap up in code, so we don't need forward declaration.
--- tools/perf/util/machine.c | 49 ++++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 30 deletions(-)
tools/perf/util/machine.c +19 -30
@@ -1028,13 +1028,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)return ret; } -static void map_groups__fixup_end (struct map_groups *mg) -{ -int i; -for (i =0 ; i <MAP__NR_TYPES ; ++i) -__map_groups__fixup_end (mg, i); -} -static char *get_kernel_version (const char *root_dir) {char version[PATH_MAX ];
@@ -1220,6 +1213,24 @@ static int machine__create_modules(struct machine *machine)return 0 ; } +static void machine__set_kernel_mmap (struct machine *machine, +u64 start,u64 end) +{ +int i; + +for (i =0 ; i <MAP__NR_TYPES ; i++) { + machine->vmlinux_maps[i]->start = start; + machine->vmlinux_maps[i]->end = end; + +/* +* Be a bit paranoid here, some perf.data file came with +* a zero sized synthesized MMAP event for the kernel. +*/ +if (machine->vmlinux_maps[i]->end ==0 ) + machine->vmlinux_maps[i]->end = ~0ULL ; + } +} +int machine__create_kernel_maps (struct machine *machine) {struct dso *kernel =machine__get_kernel (machine);
@@ -1244,11 +1255,6 @@ int machine__create_kernel_maps(struct machine *machine) "continuing anyway...\n", machine->pid); } -/* -* Now that we have all the maps created, just set the ->end of them: -*/ -map_groups__fixup_end (&machine->kmaps); -if (!machine__get_running_kernel_start (machine, &name, &addr)) {if (name &&maps__set_kallsyms_ref_reloc_sym (machine->vmlinux_maps, name, addr)) {
@@ -1257,27 +1263,10 @@ int machine__create_kernel_maps(struct machine *machine) } } +machine__set_kernel_mmap (machine, addr,0 );return 0 ; } -static void machine__set_kernel_mmap (struct machine *machine, -u64 start,u64 end) -{ -int i; - -for (i =0 ; i <MAP__NR_TYPES ; i++) { - machine->vmlinux_maps[i]->start = start; - machine->vmlinux_maps[i]->end = end; - -/* -* Be a bit paranoid here, some perf.data file came with -* a zero sized synthesized MMAP event for the kernel. -*/ -if (machine->vmlinux_maps[i]->end ==0 ) - machine->vmlinux_maps[i]->end = ~0ULL ; - } -} -static bool machine__uses_kcore (struct machine *machine) {struct dso *dso;
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 12/17 perf tools: Rename __map_groups__fixup_end to map_groups__fixup_end
In reply to Jiri OlsaThere's no need to keep the '__' prefix now when there's map_groups__fixup_end function gone.
--- tools/perf/util/machine.c | 2 +- tools/perf/util/symbol-elf.c | 2 +- tools/perf/util/symbol.c | 2 +- tools/perf/util/symbol.h | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)
tools/perf/util/machine.c +1 -1
@@ -1005,7 +1005,7 @@ int __machine__load_kallsyms(struct machine *machine, const char *filename,* kernel, with modules between them, fixup the end of all * sections. */ -__map_groups__fixup_end (&machine->kmaps, type); +map_groups__fixup_end (&machine->kmaps, type); }return ret;
tools/perf/util/symbol-elf.c +1 -1
@@ -1124,7 +1124,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,* We need to fixup this here too because we create new * maps here, for things like vsyscall sections. */ -__map_groups__fixup_end (kmaps, map->type); +map_groups__fixup_end (kmaps, map->type); } } err = nr;
tools/perf/util/symbol.c +1 -1
@@ -228,7 +228,7 @@ void symbols__fixup_end(struct rb_root *symbols) curr->end =roundup (curr->start,4096 ) +4096 ; } -void __map_groups__fixup_end (struct map_groups *mg,enum map_type type) +void map_groups__fixup_end (struct map_groups *mg,enum map_type type) {struct maps *maps = &mg->maps[type];struct map *next, *curr;
tools/perf/util/symbol.h +1 -1
@@ -316,7 +316,7 @@ void __symbols__insert(struct rb_root *symbols, struct symbol *sym, bool kernel)void symbols__insert (struct rb_root *symbols,struct symbol *sym);void symbols__fixup_duplicate (struct rb_root *symbols);void symbols__fixup_end (struct rb_root *symbols); -void __map_groups__fixup_end (struct map_groups *mg,enum map_type type); +void map_groups__fixup_end (struct map_groups *mg,enum map_type type);typedef int (*mapfn_t )(u64 start,u64 len,u64 pgoff,void *data); int file__read_maps(int fd, bool exe, mapfn_t mapfn, void *data,
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 13/17 perf tools: Remove machine__load_kallsyms function
In reply to Jiri OlsaAnd replacing it with __machine__load_kallsyms function.
The current machine__load_kallsyms function has no caller, so replacing it directly with __machine__load_kallsyms. Also removing the no_kcore argument as it was always called with true value.
--- tools/perf/tests/vmlinux-kallsyms.c | 2 +- tools/perf/util/machine.c | 14 ++++---------- tools/perf/util/machine.h | 2 -- 3 files changed, 5 insertions(+), 13 deletions(-)
tools/perf/tests/vmlinux-kallsyms.c +1 -1
@@ -56,7 +56,7 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest* be compacted against the list of modules found in the "vmlinux" * code and with the one got from /proc/modules from the "kallsyms" code. */ -if (__machine__load_kallsyms (&kallsyms,"/proc/kallsyms" , type, true) <=0 ) { +if (machine__load_kallsyms (&kallsyms,"/proc/kallsyms" , type) <=0 ) {pr_debug ("dso__load_kallsyms " ); goto out; }
tools/perf/util/machine.c +4 -10
@@ -151,7 +151,7 @@ struct machine *machine__new_kallsyms(void)* ask for not using the kcore parsing code, once this one is fixed * to create a map per module. */ -if (machine &&__machine__load_kallsyms (machine,"/proc/kallsyms" ,MAP__FUNCTION , true) <=0 ) { +if (machine &&machine__load_kallsyms (machine,"/proc/kallsyms" ,MAP__FUNCTION ) <=0 ) {machine__delete (machine); machine =NULL ; }
@@ -992,11 +992,11 @@ int machines__create_kernel_maps(struct machines *machines, pid_t pid)return machine__create_kernel_maps (machine); } -int __machine__load_kallsyms (struct machine *machine,const char *filename, -enum map_type type,bool no_kcore) +int machine__load_kallsyms (struct machine *machine,const char *filename, +enum map_type type) {struct map *map =machine__kernel_map (machine); -int ret =__dso__load_kallsyms (map->dso, filename, map, no_kcore); +int ret =__dso__load_kallsyms (map->dso, filename, map, true);if (ret >0 ) {dso__set_loaded (map->dso, type);
@@ -1011,12 +1011,6 @@ int __machine__load_kallsyms(struct machine *machine, const char *filename,return ret; } -int machine__load_kallsyms (struct machine *machine,const char *filename, -enum map_type type) -{ -return __machine__load_kallsyms (machine, filename, type, false); -} -int machine__load_vmlinux_path (struct machine *machine,enum map_type type) {struct map *map =machine__kernel_map (machine);
tools/perf/util/machine.h +0 -2
@@ -225,8 +225,6 @@ struct map *machine__findnew_module_map(struct machine *machine, u64 start, const char *filename);int arch__fix_module_text_start (u64 *start,const char *name); -int __machine__load_kallsyms (struct machine *machine,const char *filename, -enum map_type type,bool no_kcore);int machine__load_kallsyms (struct machine *machine,const char *filename,enum map_type type);int machine__load_vmlinux_path (struct machine *machine,enum map_type type);
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 14/17 perf tools: Do not create kernel maps in sample__resolve
In reply to Jiri OlsaThere's no need for kernel maps to be allocated at this point - sample processing.
We search for kernel maps using the kernel map_groups in machine::kmaps which is static. If vmlinux maps for any reason still don't exist, the search correctly fails because they are not in the map group.
--- tools/perf/util/event.c | 11 ----------- 1 file changed, 11 deletions(-)
tools/perf/util/event.c +0 -11
@@ -1588,17 +1588,6 @@ int machine__resolve(struct machine *machine, struct addr_location *al,return -1 ;dump_printf (" ... thread: %s:%d\n" ,thread__comm_str (thread), thread->tid); -/* -* Have we already created the kernel maps for this machine? -* -* This should have happened earlier, when we processed the kernel MMAP -* events, but for older perf.data files there was no such thing, so do -* it now. -*/ -if (sample->cpumode ==PERF_RECORD_MISC_KERNEL && -machine__kernel_map (machine) ==NULL ) -machine__create_kernel_maps (machine); -thread__find_addr_map (thread, sample->cpumode,MAP__FUNCTION , sample->ip, al); dump_printf(" ...... dso: %s\n", al->map ? al->map->dso->long_name :
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 15/17 perf tools: Check if we read regular file in dso__load
In reply to Jiri OlsaCurrent code in dso__load calls the is_regular_file function, but it checks its return value only after calling symsrc__init.
That can make symsrc__init block in elf_* functions on reading the file if the file happens to be device and not regular one.
Make the check before calling symsrc__init.
--- tools/perf/util/symbol.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
tools/perf/util/symbol.c +5 -10
@@ -1580,9 +1580,7 @@ int dso__load(struct dso *dso, struct map *map)for (i =0 ; i <DSO_BINARY_TYPE__SYMTAB_CNT ; i++) {struct symsrc *ss = &ss_[ss_pos];bool next_slot = false; -bool is_reg;bool nsexit; -int sirc;enum dso_binary_type symtab_type = binary_type_symtab[i];
@@ -1599,18 +1597,15 @@ int dso__load(struct dso *dso, struct map *map)if (nsexit)nsinfo__mountns_exit (&nsc); - is_reg =is_regular_file (name); - sirc =symsrc__init (ss, dso, name, symtab_type); +if (!is_regular_file (name)) +continue ; + +if (symsrc__init (ss, dso, name, symtab_type) <0 ) +continue ;if (nsexit)nsinfo__mountns_enter (dso->nsinfo, &nsc); -if (!is_reg || sirc <0 ) { -if (sirc >=0 ) -symsrc__destroy (ss); -continue ; - } -if (!syms_ss &&symsrc__has_symtab (ss)) { syms_ss = ss; next_slot = true;
Signature · 1 line
2.13.6
Jiri Olsa
reviewerPATCH 16/17 perf tests: Fix dwarf unwind for stripped binaries
In reply to Jiri OlsaWhen we strip the perf binary, dwarf unwind test stop to work. The reason is that strip will remove static function symbols, which we need to check for unwind.
This change will keep this test working in cases where the global symbols are put into dynamic symbol table, which is the case on x86. It still won't work on powerpc.
Making those 5 local functions global, and adding 'test_dwarf_unwind__' to their names.
--- tools/perf/tests/dwarf-unwind.c | 46 +++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-)
tools/perf/tests/dwarf-unwind.c +30 -16
@@ -37,6 +37,19 @@ static int init_live_machine(struct machine *machine) mmap_handler, machine, true, 500); } +/* +* We need to keep these functions global, despite the +* fact that they are used only locally in this object, +* in order to keep them around even if the binary is +* stripped. If they are gone, the unwind check for +* symbol fails. +*/ +int test_dwarf_unwind__thread (struct thread *thread); +int test_dwarf_unwind__compare (void *p1,void *p2); +int test_dwarf_unwind__krava_3 (struct thread *thread); +int test_dwarf_unwind__krava_2 (struct thread *thread); +int test_dwarf_unwind__krava_1 (struct thread *thread); +#define MAX_STACK 8static int unwind_entry (struct unwind_entry *entry,void *arg)
@@ -45,12 +58,12 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)char *symbol = entry->sym ? entry->sym->name :NULL ;static const char *funcs[MAX_STACK ] = {"test__arch_unwind_sample" , -"unwind_thread" , -"compare" , +"test_dwarf_unwind__thread" , +"test_dwarf_unwind__compare" ,"bsearch" , -"krava_3" , -"krava_2" , -"krava_1" , +"test_dwarf_unwind__krava_3" , +"test_dwarf_unwind__krava_2" , +"test_dwarf_unwind__krava_1" ,"test__dwarf_unwind" };/*
@@ -77,7 +90,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)return strcmp ((const char *) symbol, funcs[idx]); } -static noinline int unwind_thread(struct thread *thread) +noinline int test_dwarf_unwind__thread(struct thread *thread) {struct perf_sample sample;unsigned long cnt =0 ;
@@ -108,7 +121,7 @@ static noinline int unwind_thread(struct thread *thread)static int global_unwind_retval = -INT_MAX ; -static noinline int compare(void *p1, void *p2) +noinline int test_dwarf_unwind__compare(void *p1, void *p2) {/* Any possible value should be 'thread' */ struct thread *thread = *(struct thread **)p1;
@@ -117,17 +130,17 @@ static noinline int compare(void *p1, void *p2)/* Call unwinder twice for both callchain orders. */ callchain_param.order =ORDER_CALLER ; - global_unwind_retval =unwind_thread (thread); + global_unwind_retval =test_dwarf_unwind__thread (thread);if (!global_unwind_retval) { callchain_param.order =ORDER_CALLEE ; - global_unwind_retval =unwind_thread (thread); + global_unwind_retval =test_dwarf_unwind__thread (thread); } }return p1 - p2; } -static noinline int krava_3(struct thread *thread) +noinline int test_dwarf_unwind__krava_3(struct thread *thread) {struct thread *array[2 ] = {thread, thread};void *fp = &bsearch;
@@ -141,18 +154,19 @@ static noinline int krava_3(struct thread *thread) size_t, int (*)(void *, void *)); _bsearch = fp; -_bsearch (array, &thread,2 ,sizeof (struct thread **), compare); +_bsearch (array, &thread,2 ,sizeof (struct thread **), + test_dwarf_unwind__compare);return global_unwind_retval; } -static noinline int krava_2(struct thread *thread) +noinline int test_dwarf_unwind__krava_2(struct thread *thread) { -return krava_3 (thread); +return test_dwarf_unwind__krava_3 (thread); } -static noinline int krava_1(struct thread *thread) +noinline int test_dwarf_unwind__krava_1(struct thread *thread) { -return krava_2 (thread); +return test_dwarf_unwind__krava_2 (thread); }int test__dwarf_unwind (struct test *test__maybe_unused ,int subtest__maybe_unused )
@@ -189,7 +203,7 @@ int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unu goto out; } - err =krava_1 (thread); + err =test_dwarf_unwind__krava_1 (thread);thread__put (thread); out:
Signature · 1 line
2.13.6
Arnaldo Carvalho de Melo
maintainerRe: [PATCH 16/17] perf tests: Fix dwarf unwind for stripped binaries
Em Tue, Feb 06, 2018 at 07:18:12PM +0100, Jiri Olsa escreveu:
Quoted 10 lines
When we strip the perf binary, dwarf unwind test stop to work. The reason is that strip will remove static function symbols, which we need to check for unwind.
This change will keep this test working in cases where the global symbols are put into dynamic symbol table, which is the case on x86. It still won't work on powerpc.
Making those 5 local functions global, and adding '
test_dwarf_unwind__' to their names.
Tested, applied.
- Arnaldo
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tests: Fix dwarf unwind for stripped binaries
In reply to Jiri Olsaperf tests: Fix dwarf unwind for stripped binaries
Commit-ID: fdf7c49c200d1b9909e2204cec5bd68b48605c71 Gitweb: https://git.kernel.org/tip/fdf7c49c200d1b9909e2204cec5bd68b48605c71 Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:18:12 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
perf tests: Fix dwarf unwind for stripped binaries
When we strip the perf binary, dwarf unwind test stop to work. The reason is that strip will remove static function symbols, which we need to check for unwind.
This change will keep this test working in cases where the global symbols are put into dynamic symbol table, which is the case on x86. It still won't work on powerpc.
Making those 5 local functions global, and adding 'test_dwarf_unwind__' to their names.
Committer testing:
Before:
# perf test dwarf 58: DWARF unwind : Ok # strip ~/bin/perf # perf test dwarf 58: DWARF unwind : FAILED! # perf test -v dwarf 58: DWARF unwind : --- start --- test child forked, pid 6590 unwind: thread map already set, dso=/home/acme/bin/perf <SNIP> unwind: access_mem addr 0x7ffce6c48098 val 48563f, offset 1144 unwind: test__dwarf_unwind:ip = 0x4a54e5 (0xa54e5) got: test__dwarf_unwind 0xa54e5, expecting test__dwarf_unwind unwind: '':ip = 0x4a50bb (0xa50bb) failed: got unresolved address 0xa50bb unwind failed test child finished with -1 ---- end ---- DWARF unwind: FAILED! #
After:
# perf test dwarf 58: DWARF unwind : Ok # strip ~/bin/perf # perf test dwarf 58: DWARF unwind : Ok # # perf test -v dwarf 58: DWARF unwind : --- start --- test child forked, pid 7219 unwind: thread map already set, dso=/home/acme/bin/perf <SNIP> unwind: access_mem addr 0x7fff007da2c8 val 48575f, offset 1144 unwind: test__arch_unwind_sample:ip = 0x589044 (0x189044) got: test__arch_unwind_sample 0x189044, expecting test__arch_unwind_sample unwind: test_dwarf_unwind__thread:ip = 0x4a52f7 (0xa52f7) got: test_dwarf_unwind__thread 0xa52f7, expecting test_dwarf_unwind__thread unwind: test_dwarf_unwind__compare:ip = 0x4a5468 (0xa5468) got: test_dwarf_unwind__compare 0xa5468, expecting test_dwarf_unwind__compare unwind: bsearch:ip = 0x7f6608ae94d8 (0x394d8) got: bsearch 0x394d8, expecting bsearch unwind: test_dwarf_unwind__krava_3:ip = 0x4a54d1 (0xa54d1) got: test_dwarf_unwind__krava_3 0xa54d1, expecting test_dwarf_unwind__krava_3 unwind: test_dwarf_unwind__krava_2:ip = 0x4a550b (0xa550b) got: test_dwarf_unwind__krava_2 0xa550b, expecting test_dwarf_unwind__krava_2 unwind: test_dwarf_unwind__krava_1:ip = 0x4a554b (0xa554b) got: test_dwarf_unwind__krava_1 0xa554b, expecting test_dwarf_unwind__krava_1 unwind: test__dwarf_unwind:ip = 0x4a5605 (0xa5605) got: test__dwarf_unwind 0xa5605, expecting test__dwarf_unwind test child finished with 0 ---- end ---- DWARF unwind: Ok #
--- tools/perf/tests/dwarf-unwind.c | 46 +++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 16 deletions(-)
tools/perf/tests/dwarf-unwind.c +30 -16
@@ -37,6 +37,19 @@ static int init_live_machine(struct machine *machine) mmap_handler, machine, true, 500); } +/* +* We need to keep these functions global, despite the +* fact that they are used only locally in this object, +* in order to keep them around even if the binary is +* stripped. If they are gone, the unwind check for +* symbol fails. +*/ +int test_dwarf_unwind__thread (struct thread *thread); +int test_dwarf_unwind__compare (void *p1,void *p2); +int test_dwarf_unwind__krava_3 (struct thread *thread); +int test_dwarf_unwind__krava_2 (struct thread *thread); +int test_dwarf_unwind__krava_1 (struct thread *thread); +#define MAX_STACK 8static int unwind_entry (struct unwind_entry *entry,void *arg)
@@ -45,12 +58,12 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)char *symbol = entry->sym ? entry->sym->name :NULL ;static const char *funcs[MAX_STACK ] = {"test__arch_unwind_sample" , -"unwind_thread" , -"compare" , +"test_dwarf_unwind__thread" , +"test_dwarf_unwind__compare" ,"bsearch" , -"krava_3" , -"krava_2" , -"krava_1" , +"test_dwarf_unwind__krava_3" , +"test_dwarf_unwind__krava_2" , +"test_dwarf_unwind__krava_1" ,"test__dwarf_unwind" };/*
@@ -77,7 +90,7 @@ static int unwind_entry(struct unwind_entry *entry, void *arg)return strcmp ((const char *) symbol, funcs[idx]); } -static noinline int unwind_thread(struct thread *thread) +noinline int test_dwarf_unwind__thread(struct thread *thread) {struct perf_sample sample;unsigned long cnt =0 ;
@@ -108,7 +121,7 @@ static noinline int unwind_thread(struct thread *thread)static int global_unwind_retval = -INT_MAX ; -static noinline int compare(void *p1, void *p2) +noinline int test_dwarf_unwind__compare(void *p1, void *p2) {/* Any possible value should be 'thread' */ struct thread *thread = *(struct thread **)p1;
@@ -117,17 +130,17 @@ static noinline int compare(void *p1, void *p2)/* Call unwinder twice for both callchain orders. */ callchain_param.order =ORDER_CALLER ; - global_unwind_retval =unwind_thread (thread); + global_unwind_retval =test_dwarf_unwind__thread (thread);if (!global_unwind_retval) { callchain_param.order =ORDER_CALLEE ; - global_unwind_retval =unwind_thread (thread); + global_unwind_retval =test_dwarf_unwind__thread (thread); } }return p1 - p2; } -static noinline int krava_3(struct thread *thread) +noinline int test_dwarf_unwind__krava_3(struct thread *thread) {struct thread *array[2 ] = {thread, thread};void *fp = &bsearch;
@@ -141,18 +154,19 @@ static noinline int krava_3(struct thread *thread) size_t, int (*)(void *, void *)); _bsearch = fp; -_bsearch (array, &thread,2 ,sizeof (struct thread **), compare); +_bsearch (array, &thread,2 ,sizeof (struct thread **), + test_dwarf_unwind__compare);return global_unwind_retval; } -static noinline int krava_2(struct thread *thread) +noinline int test_dwarf_unwind__krava_2(struct thread *thread) { -return krava_3 (thread); +return test_dwarf_unwind__krava_3 (thread); } -static noinline int krava_1(struct thread *thread) +noinline int test_dwarf_unwind__krava_1(struct thread *thread) { -return krava_2 (thread); +return test_dwarf_unwind__krava_2 (thread); }int test__dwarf_unwind (struct test *test__maybe_unused ,int subtest__maybe_unused )
@@ -189,7 +203,7 @@ int test__dwarf_unwind(struct test *test __maybe_unused, int subtest __maybe_unu goto out; } - err =krava_1 (thread); + err =test_dwarf_unwind__krava_1 (thread);thread__put (thread); out:
Jiri Olsa
reviewerPATCH 17/17 perf tools: Fix comment for sort__* compare functions
In reply to Jiri OlsaIn commit 2f15bd8c6c6e ("perf tools: Fix "Command" sort_entry's cmp and collapse function") we switched from pointer to string comparison.
But failed to remove related comments. Removing them and adding another one to warn before pointer comparison in here.
--- tools/perf/util/sort.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
tools/perf/util/sort.c +5 -2
@@ -111,17 +111,20 @@ struct sort_entry sort_thread = {/* --sort comm */ +/* +* We can't use pointer comparison in functions below, +* because it gives different results based on pointer +* values, which could break some sorting assumptions. +*/ static int64_t sort__comm_cmp (struct hist_entry *left,struct hist_entry *right) { -/* Compare the addr that should be unique among comm */ return strcmp (comm__str (right->comm),comm__str (left->comm)); }static int64_t sort__comm_collapse (struct hist_entry *left,struct hist_entry *right) { -/* Compare the addr that should be unique among comm */ return strcmp (comm__str (right->comm),comm__str (left->comm)); }
Signature · 1 line
2.13.6
Re: [PATCH 17/17] perf tools: Fix comment for sort__* compare functions
Em Tue, Feb 06, 2018 at 07:18:13PM +0100, Jiri Olsa escreveu:
Quoted 5 lines
In commit
2f15bd8c6c6e("perf tools: Fix "Command" sort_entry's cmp and collapse function") we switched from pointer to string comparison.But failed to remove related comments. Removing them and adding another one to warn before pointer comparison in here.
Applied.
Pushing perf/core out, please take a look at the others and rebase,
- Arnaldo
tip-bot for Jiri Olsa
merge bot[tip:perf/core] perf tools: Fix comment for sort__* compare functions
In reply to Jiri Olsaperf tools: Fix comment for sort__* compare functions
Commit-ID: a7402c943bb4657cc0b44453177803fbead70990 Gitweb: https://git.kernel.org/tip/a7402c943bb4657cc0b44453177803fbead70990 Author: Jiri Olsa <[email protected]> AuthorDate: Tue, 6 Feb 2018 19:18:13 +0100 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Fri, 16 Feb 2018 10:09:23 -0300
perf tools: Fix comment for sort__* compare functions
In commit 2f15bd8c6c6e ("perf tools: Fix "Command" sort_entry's cmp and collapse function") we switched from pointer to string comparison.
But failed to remove related comments. Removing them and adding another one to warn before pointer comparison in here.
--- tools/perf/util/sort.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
tools/perf/util/sort.c +5 -2
@@ -111,17 +111,20 @@ struct sort_entry sort_thread = {/* --sort comm */ +/* +* We can't use pointer comparison in functions below, +* because it gives different results based on pointer +* values, which could break some sorting assumptions. +*/ static int64_t sort__comm_cmp (struct hist_entry *left,struct hist_entry *right) { -/* Compare the addr that should be unique among comm */ return strcmp (comm__str (right->comm),comm__str (left->comm)); }static int64_t sort__comm_collapse (struct hist_entry *left,struct hist_entry *right) { -/* Compare the addr that should be unique among comm */ return strcmp (comm__str (right->comm),comm__str (left->comm)); }