[PATCH 00/17] perf tools: Assorted fixes

[PATCH 00/17] perf tools: Assorted fixes

[Jiri Olsa]

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(-)

[PATCH 01/17] perf report: Ask ordered events for --tasks option

[Jiri Olsa]

If we have the time in, keep the events in time order.

Link: http://lkml.kernel.org/n/tip-3wcrngoibk5l96nqyhp0nbkm@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-report.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4ad5dc649716..8ef71669e7a0 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -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;
-- 
2.13.6

Re: [PATCH 01/17] perf report: Ask ordered events for --tasks option

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:17:57PM +0100, Jiri Olsa escreveu:
> 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/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

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
 
> Link: http://lkml.kernel.org/n/tip-3wcrngoibk5l96nqyhp0nbkm@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/builtin-report.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 4ad5dc649716..8ef71669e7a0 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -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;
> -- 
> 2.13.6

Re: [PATCH 01/17] perf report: Ask ordered events for --tasks option

[Jiri Olsa]

On Tue, Feb 06, 2018 at 03:48:20PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 06, 2018 at 07:17:57PM +0100, Jiri Olsa escreveu:
> > 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/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
> 
> 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?

right ;-) time sorted is always better..

thanks,
jirka

Re: [PATCH 01/17] perf report: Ask ordered events for --tasks option

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:59:51PM +0100, Jiri Olsa escreveu:
> On Tue, Feb 06, 2018 at 03:48:20PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Feb 06, 2018 at 07:17:57PM +0100, Jiri Olsa escreveu:
> > > 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/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
> > 
> > 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?
> 
> right ;-) time sorted is always better..

Sure ;-) Adding the comments and applying...

- Arnaldo

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

[tip-bot for Jiri Olsa]

Commit-ID:  8614ada0be7d7be84b85c006d526a9c8f76484fa
Gitweb:     https://git.kernel.org/tip/8614ada0be7d7be84b85c006d526a9c8f76484fa
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:17:57 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180206181813.10943-2-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4ad5dc6..8ef7166 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -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;

[PATCH 02/17] perf record: Put new line after target override warning

[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) ]

Link: http://lkml.kernel.org/n/tip-62dn6rrcb2nf2a26kxj9xphr@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bf4ca749d1ac..907267206973 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -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);
-- 
2.13.6

[tip:perf/core] perf record: Put new line after target override warning

[tip-bot for Jiri Olsa]

Commit-ID:  c3dec27b7f70a9ad5f777d943d51ecdfcd9824d0
Gitweb:     https://git.kernel.org/tip/c3dec27b7f70a9ad5f777d943d51ecdfcd9824d0
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:17:58 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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) ]

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 16ad2ffb822c ("perf tools: Introduce perf_target__strerror()")
Link: http://lkml.kernel.org/r/20180206181813.10943-3-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index bf4ca74..9072672 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -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);

[PATCH 03/17] perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND

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

Link: http://lkml.kernel.org/n/tip-b89ggszz9x26hm7ztor29b0s@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Documentation/perf-script.txt |  3 +++
 tools/perf/builtin-script.c              | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 7730c1d2b5d3..36ec0257f8d3 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -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.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index ab19a6ee4093..cce926aeb0c0 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -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"),
-- 
2.13.6

[tip:perf/core] perf script: Add --show-round-event to display PERF_RECORD_FINISHED_ROUND

[tip-bot for Jiri Olsa]

Commit-ID:  3233b37a71c794e25a0a794185df8d6abd9f277e
Gitweb:     https://git.kernel.org/tip/3233b37a71c794e25a0a794185df8d6abd9f277e
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:17:59 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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...

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180206181813.10943-4-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-script.txt |  3 +++
 tools/perf/builtin-script.c              | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 7730c1d..36ec025 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -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.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index ab19a6e..cce926a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -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"),

[PATCH 04/17] tools lib api fs: Add filename__read_xll function

[Jiri Olsa]

Adding filename__read_xll function to be able to read files
with hex numbers in, which do not have 0x prefix.

Link: http://lkml.kernel.org/n/tip-d9qv8n8xlmkywsb9vdrcj796@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/api/fs/fs.c | 29 ++++++++++++++++++++++-------
 tools/lib/api/fs/fs.h |  1 +
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index b24afc0e6e81..8b0e4a4315bd 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -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)
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index dda49deefb52..8ebee35a6395 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -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);
-- 
2.13.6

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

[tip-bot for Jiri Olsa]

Commit-ID:  6baddfc6900eca7f6b360c91ff737890ab4f1d55
Gitweb:     https://git.kernel.org/tip/6baddfc6900eca7f6b360c91ff737890ab4f1d55
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:18:00 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180206181813.10943-5-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 29 ++++++++++++++++++++++-------
 tools/lib/api/fs/fs.h |  1 +
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index b24afc0..8b0e4a4 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -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)
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index dda49de..8ebee35 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -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);

[PATCH 05/17] tools lib api fs: Add sysfs__read_xll function

[Jiri Olsa]

Adding sysfs__read_xll function to be able to read sysfs
files with hex numbers in, which do not have 0x prefix.

Link: http://lkml.kernel.org/n/tip-j5ullvrcli5ga3hn6692t2aw@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/api/fs/fs.c | 15 +++++++++++++--
 tools/lib/api/fs/fs.h |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 8b0e4a4315bd..6a12bbf39f7b 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -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)
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index 8ebee35a6395..92d03b8396b1 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -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);
 
-- 
2.13.6

Re: [PATCH 05/17] tools lib api fs: Add sysfs__read_xll function

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:18:01PM +0100, Jiri Olsa escreveu:
> 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
 
> Link: http://lkml.kernel.org/n/tip-j5ullvrcli5ga3hn6692t2aw@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/api/fs/fs.c | 15 +++++++++++++--
>  tools/lib/api/fs/fs.h |  1 +
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
> index 8b0e4a4315bd..6a12bbf39f7b 100644
> --- a/tools/lib/api/fs/fs.c
> +++ b/tools/lib/api/fs/fs.c
> @@ -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)
> diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
> index 8ebee35a6395..92d03b8396b1 100644
> --- a/tools/lib/api/fs/fs.h
> +++ b/tools/lib/api/fs/fs.h
> @@ -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);
>  
> -- 
> 2.13.6

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

[tip-bot for Jiri Olsa]

Commit-ID:  d9c5f32240f503481291a6d4e7246ee0a128d76d
Gitweb:     https://git.kernel.org/tip/d9c5f32240f503481291a6d4e7246ee0a128d76d
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:18:01 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180206181813.10943-6-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/api/fs/fs.c | 15 +++++++++++++--
 tools/lib/api/fs/fs.h |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/lib/api/fs/fs.c b/tools/lib/api/fs/fs.c
index 8b0e4a4..6a12bbf 100644
--- a/tools/lib/api/fs/fs.c
+++ b/tools/lib/api/fs/fs.c
@@ -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)
diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
index 8ebee35..92d03b8 100644
--- a/tools/lib/api/fs/fs.h
+++ b/tools/lib/api/fs/fs.h
@@ -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);
 

[PATCH 06/17] tools lib symbol: Skip non-address kallsyms line

[Jiri Olsa]

Adding check on failed attempt to parse the address
and skip the line parsing early in that case.

Link: http://lkml.kernel.org/n/tip-djqwni3p6lgctf6o7xhhwpmw@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/symbol/kallsyms.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c
index 914cb8e3d40b..689b6a130dd7 100644
--- a/tools/lib/symbol/kallsyms.c
+++ b/tools/lib/symbol/kallsyms.c
@@ -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;
-- 
2.13.6

Re: [PATCH 06/17] tools lib symbol: Skip non-address kallsyms line

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:18:02PM +0100, Jiri Olsa escreveu:
> 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
 
> Link: http://lkml.kernel.org/n/tip-djqwni3p6lgctf6o7xhhwpmw@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/symbol/kallsyms.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c
> index 914cb8e3d40b..689b6a130dd7 100644
> --- a/tools/lib/symbol/kallsyms.c
> +++ b/tools/lib/symbol/kallsyms.c
> @@ -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;
> -- 
> 2.13.6

Re: [PATCH 06/17] tools lib symbol: Skip non-address kallsyms line

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 04:06:49PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 06, 2018 at 07:18:02PM +0100, Jiri Olsa escreveu:
> > 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

[PATCH 07/17] perf tools: Free root_dir in machine__init error path

[Jiri Olsa]

Freeing root_dir in machine__init error path.

Link: http://lkml.kernel.org/n/tip-ng92slsanexqw7h1d6sadnj7@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/machine.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index b05a67464c03..e300a643f65b 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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;
 }
 
-- 
2.13.6

Re: [PATCH 07/17] perf tools: Free root_dir in machine__init error path

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:18:03PM +0100, Jiri Olsa escreveu:
> Freeing root_dir in machine__init error path.
> 
> Link: http://lkml.kernel.org/n/tip-ng92slsanexqw7h1d6sadnj7@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/util/machine.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index b05a67464c03..e300a643f65b 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -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);

In these cases, i.e. freeing something inside some other struct, its
better to use:

		zfree(&machine->root_dir);

>  	return 0;
>  }
>  
> -- 
> 2.13.6

[PATCH 08/17] perf tools: Move kernel mmap name into struct machine

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

Link: http://lkml.kernel.org/n/tip-2fx7kxxdc5zcm6990cq2mday@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 7f8553630c4d..537eadd81914 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -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);
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 44e603c27944..4644e751a3e3 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -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) -
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e300a643f65b..447f10e1323e 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -48,6 +48,27 @@ static void machine__threads_init(struct machine *machine)
 	}
 }
 
+static int machine__mmap_name(struct machine *machine)
+{
+	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);
@@ -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);
+	}
 	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;
 
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 5ce860b64c74..cb0a20f3a96b 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -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);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index cc065d4bfafc..dcb81176669a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -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);
 	}
-- 
2.13.6

Re: [PATCH 08/17] perf tools: Move kernel mmap name into struct machine

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:18:04PM +0100, Jiri Olsa escreveu:
> 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.
> 
> Link: http://lkml.kernel.org/n/tip-2fx7kxxdc5zcm6990cq2mday@git.kernel.org
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  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(-)
> 
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 7f8553630c4d..537eadd81914 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -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);
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 44e603c27944..4644e751a3e3 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -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) -
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index e300a643f65b..447f10e1323e 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -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()?

> +{
> +	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);
> @@ -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()

> +	}
>  	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;
>  
> diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> index 5ce860b64c74..cb0a20f3a96b 100644
> --- a/tools/perf/util/machine.h
> +++ b/tools/perf/util/machine.h
> @@ -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);
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index cc065d4bfafc..dcb81176669a 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -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);
>  	}
> -- 
> 2.13.6

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

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

Link: http://lkml.kernel.org/n/tip-37lmdjzh8dwq03golnk7hgkd@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/machine.c | 6 ++----
 tools/perf/util/machine.h | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 447f10e1323e..f5c79110f0d1 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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);
 
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index cb0a20f3a96b..50d587d34459 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -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);
-- 
2.13.6

[PATCH 10/17] perf tools: Generalize machine__set_kernel_mmap function

[Jiri Olsa]

So it could be called without event object, just with start
and end values. It will be used in following patch.

Link: http://lkml.kernel.org/n/tip-u4hu7m5fmwwsscy6ki70hhq6@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/machine.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f5c79110f0d1..7838fe56b0c9 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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
-- 
2.13.6

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

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

Link: http://lkml.kernel.org/n/tip-9kqrs3bsojwe4ktwpnkxc6g4@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/machine.c | 49 ++++++++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 30 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 7838fe56b0c9..279eeb87c594 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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;
-- 
2.13.6

[PATCH 12/17] perf tools: Rename __map_groups__fixup_end to map_groups__fixup_end

[Jiri Olsa]

There's no need to keep the '__' prefix now when there's
map_groups__fixup_end function gone.

Link: http://lkml.kernel.org/n/tip-xq9wpm97spnpaxfjhaz1a03n@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 279eeb87c594..441d09d97f3d 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 2de770511e70..6d1e4dc2709f 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -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;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index dcb81176669a..fe15dafaef66 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -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;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 0563f33c1eb3..cf955f1d1718 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -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,
-- 
2.13.6

[PATCH 13/17] perf tools: Remove machine__load_kallsyms function

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

Link: http://lkml.kernel.org/n/tip-kj9bpn6v213n6vaoe6ryxv2q@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 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(-)

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index f6789fb029d6..58349297f9fb 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -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;
 	}
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 441d09d97f3d..a9e470139722 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -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);
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 50d587d34459..66cc200ef86f 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -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);
-- 
2.13.6

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

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

Link: http://lkml.kernel.org/n/tip-f1swg55ooc3sihcvadgzkw1z@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/event.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 4644e751a3e3..f0a6cbd033cc 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -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 :
-- 
2.13.6

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

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

Link: http://lkml.kernel.org/n/tip-qm0sl764xzwvzgz0abqt6m46@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/symbol.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index fe15dafaef66..ea002b27b39d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -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;
-- 
2.13.6

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

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

Link: http://lkml.kernel.org/n/tip-y6ovucoyct8nimmzggi6tq9s@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/tests/dwarf-unwind.c | 46 +++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index 260418969120..2f008067d989 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -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:
-- 
2.13.6

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

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:18:12PM +0100, Jiri Olsa escreveu:
> 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:perf/core] perf tests: Fix dwarf unwind for stripped binaries

[tip-bot for Jiri Olsa]

Commit-ID:  fdf7c49c200d1b9909e2204cec5bd68b48605c71
Gitweb:     https://git.kernel.org/tip/fdf7c49c200d1b9909e2204cec5bd68b48605c71
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:18:12 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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
  #

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180206181813.10943-17-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/dwarf-unwind.c | 46 +++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index 26041896..2f00806 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -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:

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

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

Link: http://lkml.kernel.org/n/tip-8p85wxr2o5ia9uinxeea8853@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/sort.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 2da4d0456a03..e8514f651865 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -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));
 }
 
-- 
2.13.6

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

[Arnaldo Carvalho de Melo]

Em Tue, Feb 06, 2018 at 07:18:13PM +0100, Jiri Olsa escreveu:
> 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:perf/core] perf tools: Fix comment for sort__* compare functions

[tip-bot for Jiri Olsa]

Commit-ID:  a7402c943bb4657cc0b44453177803fbead70990
Gitweb:     https://git.kernel.org/tip/a7402c943bb4657cc0b44453177803fbead70990
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 6 Feb 2018 19:18:13 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180206181813.10943-18-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 2da4d04..e8514f6 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -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));
 }