Skip to contentSkip to conversation

perf tools: Assorted fixes

PATCH Applied Jiri Olsa 4 participants 35 messages Feb 6, 2018 – Feb 17, 2018 perf events

v1 of 3 Applied Latest is v3 →

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover 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(-)
author reviewer
#1 lore raw

PATCH 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
index 4ad5dc649716..8ef71669e7a0 100644
@@ -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
#2 lore raw

Re: [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
---
 tools/perf/builtin-report.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/builtin-report.c +1 -0
index 4ad5dc649716..8ef71669e7a0 100644
@@ -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
#3 lore raw

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 |          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]#

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

#4 lore raw

Re: [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 |          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]#

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

#5 lore raw

[tip:perf/core] perf report: Ask for ordered events for --tasks option

In reply to Jiri Olsa
perf 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
index 4ad5dc6..8ef7166 100644
@@ -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;
author reviewer
#6 lore raw

PATCH 2/17 perf record: Put new line after target override warning

In reply to Jiri Olsa

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
index bf4ca749d1ac..907267206973 100644
@@ -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
#7 lore raw

[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) ]
Cc Alexander Shishkin <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Fixes 16ad2ffb822c ("perf tools: Introduce perf_target__strerror()")
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/builtin-record.c +1 -1
index bf4ca74..9072672 100644
@@ -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);
author reviewer
#8 lore raw

PATCH 3/17 perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND

In reply to Jiri Olsa

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
index 7730c1d2b5d3..36ec0257f8d3 100644
@@ -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
index ab19a6ee4093..cce926aeb0c0 100644
@@ -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
#9 lore raw

[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
index 7730c1d..36ec025 100644
@@ -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
index ab19a6e..cce926a 100644
@@ -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"),
author reviewer
#10 lore raw

PATCH 4/17 tools lib api fs: Add filename__read_xll function

In reply to Jiri Olsa

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
index b24afc0e6e81..8b0e4a4315bd 100644
@@ -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
index dda49deefb52..8ebee35a6395 100644
@@ -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
#11 lore raw

[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
index b24afc0..8b0e4a4 100644
@@ -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
index dda49de..8ebee35 100644
@@ -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);
author reviewer
#12 lore raw

PATCH 5/17 tools lib api fs: Add sysfs__read_xll function

In reply to Jiri Olsa

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
index 8b0e4a4315bd..6a12bbf39f7b 100644
@@ -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
index 8ebee35a6395..92d03b8396b1 100644
@@ -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
#13 lore raw

Re: [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_xll function 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
---
 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
index 8b0e4a4315bd..6a12bbf39f7b 100644
@@ -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
index 8ebee35a6395..92d03b8396b1 100644
@@ -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
#14 lore raw

[tip:perf/core] tools lib api fs: Add sysfs__read_xll function

In reply to Jiri Olsa
tools 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
index 8b0e4a4..6a12bbf 100644
@@ -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
index 8ebee35..92d03b8 100644
@@ -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);
 
author reviewer
#15 lore raw

PATCH 6/17 tools lib symbol: Skip non-address kallsyms line

In reply to Jiri Olsa

Adding 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
index 914cb8e3d40b..689b6a130dd7 100644
@@ -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
#16 lore raw

Re: [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
---
 tools/lib/symbol/kallsyms.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/lib/symbol/kallsyms.c +4 -0
index 914cb8e3d40b..689b6a130dd7 100644
@@ -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
#17 lore raw

Re: [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

author reviewer
#18 lore raw

PATCH 7/17 perf tools: Free root_dir in machine__init error path

In reply to Jiri Olsa

Freeing 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
index b05a67464c03..e300a643f65b 100644
@@ -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
#19 lore raw

Re: [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_dir in machine__init error path.

---
 tools/perf/util/machine.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
tools/perf/util/machine.c +3 -1
index b05a67464c03..e300a643f65b 100644
@@ -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 -0
 	return 0;
 }
 
Signature · 1 line
2.13.6
author reviewer
#20 lore raw

PATCH 8/17 perf tools: Move kernel mmap name into struct machine

In reply to Jiri Olsa

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.

---
 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
index 7f8553630c4d..537eadd81914 100644
@@ -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
index 44e603c27944..4644e751a3e3 100644
@@ -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
index 5ce860b64c74..cb0a20f3a96b 100644
@@ -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
index cc065d4bfafc..dcb81176669a 100644
@@ -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
#21 lore raw

Re: [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.

---
 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
index 7f8553630c4d..537eadd81914 100644
@@ -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
index 44e603c27944..4644e751a3e3 100644
@@ -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
index e300a643f65b..447f10e1323e 100644

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
index 5ce860b64c74..cb0a20f3a96b 100644
@@ -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
index cc065d4bfafc..dcb81176669a 100644
@@ -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
author reviewer
#22 lore raw

PATCH 9/17 perf tools: Don't search for active kernel start in __machine__create_kernel_maps

In reply to Jiri Olsa

We 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
index 447f10e1323e..f5c79110f0d1 100644
@@ -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
index cb0a20f3a96b..50d587d34459 100644
@@ -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
author reviewer
#23 lore raw

PATCH 10/17 perf tools: Generalize machine__set_kernel_mmap function

In reply to Jiri Olsa

So 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
index f5c79110f0d1..7838fe56b0c9 100644
@@ -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
author reviewer
#24 lore raw

PATCH 11/17 perf tools: Use machine__set_kernel_mmap instead of map_groups__fixup_end

In reply to Jiri Olsa

The 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
index 7838fe56b0c9..279eeb87c594 100644
@@ -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
author reviewer
#25 lore raw

PATCH 12/17 perf tools: Rename __map_groups__fixup_end to map_groups__fixup_end

In reply to Jiri Olsa

There'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
index 279eeb87c594..441d09d97f3d 100644
@@ -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
index 2de770511e70..6d1e4dc2709f 100644
@@ -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
index dcb81176669a..fe15dafaef66 100644
@@ -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
index 0563f33c1eb3..cf955f1d1718 100644
@@ -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
author reviewer
#26 lore raw

PATCH 13/17 perf tools: Remove machine__load_kallsyms function

In reply to Jiri Olsa

And 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
index f6789fb029d6..58349297f9fb 100644
@@ -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
index 441d09d97f3d..a9e470139722 100644
@@ -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
index 50d587d34459..66cc200ef86f 100644
@@ -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
author reviewer
#27 lore raw

PATCH 14/17 perf tools: Do not create kernel maps in sample__resolve

In reply to Jiri Olsa

There'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
index 4644e751a3e3..f0a6cbd033cc 100644
@@ -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
author reviewer
#28 lore raw

PATCH 15/17 perf tools: Check if we read regular file in dso__load

In reply to Jiri Olsa

Current 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
index fe15dafaef66..ea002b27b39d 100644
@@ -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
author reviewer
#29 lore raw

PATCH 16/17 perf tests: Fix dwarf unwind for stripped binaries

In reply to Jiri Olsa

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.

---
 tools/perf/tests/dwarf-unwind.c | 46 +++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 16 deletions(-)
tools/perf/tests/dwarf-unwind.c +30 -16
index 260418969120..2f008067d989 100644
@@ -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 8
 
 static 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
#30 lore raw

Re: [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

#31 lore raw

[tip:perf/core] perf tests: Fix dwarf unwind for stripped binaries

In reply to Jiri Olsa
perf 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
index 26041896..2f00806 100644
@@ -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 8
 
 static 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:
author reviewer
#32 lore raw

PATCH 17/17 perf tools: Fix comment for sort__* compare functions

In reply to Jiri Olsa

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
index 2da4d0456a03..e8514f651865 100644
@@ -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
maintainer Applied
#33 lore raw

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

#34 lore raw

[tip:perf/core] perf tools: Fix comment for sort__* compare functions

In reply to Jiri Olsa
perf 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
index 2da4d04..e8514f6 100644
@@ -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));
 }