Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1620702 > unrolled thread

[PATCH 0/7] perf tool: pipe-mode fixes

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] perf tool: pipe-mode fixes David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
    [PATCH 1/7] perf inject: don't proceed if perf_session__process_event fails David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
      [tip:perf/core] perf inject: Don't proceed if  perf_session__process_event() fails tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-04-12 07:50 +0200
    [PATCH 2/7] perf inject: copy events when reordering events in pipe mode David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
      Re: [PATCH 2/7] perf inject: copy events when reordering events in  pipe mode Jiri Olsa <jolsa@redhat.com> - 2017-04-11 11:50 +0200
        Re: [PATCH 2/7] perf inject: copy events when reordering events in  pipe mode David Carrillo-Cisneros <davidcc@google.com> - 2017-04-11 21:00 +0200
      [tip:perf/core] perf inject: Copy events when reordering events in  pipe mode tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-04-12 07:50 +0200
    [PATCH 4/7] perf annotate: process attr and build_id records David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
      [tip:perf/core] perf annotate: Process attr and build_id records tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-04-12 07:50 +0200
    [PATCH 6/7] perf tool: protect empty evlists David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
      Re: [PATCH 6/7] perf tool: protect empty evlists Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-11 20:20 +0200
        Re: [PATCH 6/7] perf tool: protect empty evlists David Carrillo-Cisneros <davidcc@google.com> - 2017-04-11 20:40 +0200
    [PATCH 7/7] perf tool: do not print missing features in pipe-mode David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
      [tip:perf/core] perf tools: Do not print missing features in  pipe-mode tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-04-12 07:50 +0200
    [PATCH 5/7] perf session: don't rely on evlist in pipe mode David Carrillo-Cisneros <davidcc@google.com> - 2017-04-10 22:20 +0200
      Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-11 20:50 +0200
        Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode David Carrillo-Cisneros <davidcc@google.com> - 2017-04-11 21:00 +0200
          Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-11 21:50 +0200
      Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-11 20:50 +0200
      Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-11 20:50 +0200
      [tip:perf/core] perf session: Don't rely on evlist in pipe mode tip-bot for David Carrillo-Cisneros <tipbot@zytor.com> - 2017-04-12 07:50 +0200
    Re: [PATCH 0/7] perf tool: pipe-mode fixes Jiri Olsa <jolsa@redhat.com> - 2017-04-11 11:50 +0200
      Re: [PATCH 0/7] perf tool: pipe-mode fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-04-11 20:20 +0200

Page 1 of 2  [1] 2  Next page →

#1620702 — [PATCH 0/7] perf tool: pipe-mode fixes

Various fixes for perf tool pipe-mode for bugs that arose while trying
to make this work:

    perf record -o - noploop | perf inject -b | perf annotate -i -

David Carrillo-Cisneros (7):
  perf inject: don't proceed if perf_session__process_event fails
  perf inject: copy events when reordering events in pipe mode
  perf tool: describe pipe mode in perf.data-file-fomat.txt
  perf annotate: process attr and build_id records
  perf session: don't rely on evlist in pipe mode
  perf tool: protect empty evlists
  perf tool: do not print missing features in pipe-mode

 tools/perf/Documentation/perf.data-file-format.txt | 19 +++++++++++++++++--
 tools/perf/builtin-annotate.c                      |  2 ++
 tools/perf/builtin-inject.c                        |  2 ++
 tools/perf/util/evlist.h                           |  4 ++++
 tools/perf/util/header.c                           | 13 +++++++------
 tools/perf/util/ordered-events.c                   |  3 ++-
 tools/perf/util/session.c                          | 17 ++++++++++++++---
 7 files changed, 48 insertions(+), 12 deletions(-)

-- 
2.12.2.715.g7642488e1d-goog

[toc] | [next] | [standalone]

#1620704 — [PATCH 1/7] perf inject: don't proceed if perf_session__process_event fails

All paths following perf_session__process_event in __cmd_inject are
useless if __cmd_inject is to fail, some depend on a correct
session->evlist.

First commit to add code that depends on session->evlist without checking
error was commmit e558a5bd8b ("perf inject: Work with files"). It has
grown since then.

Change __cmd_inject to fail immediately after perf_session__process_event
fails.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/builtin-inject.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 42dff0b1375a..65e1c026a2f0 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -694,6 +694,8 @@ static int __cmd_inject(struct perf_inject *inject)
 		lseek(fd, output_data_offset, SEEK_SET);
 
 	ret = perf_session__process_events(session);
+	if (ret)
+		return ret;
 
 	if (!file_out->is_pipe) {
 		if (inject->build_ids)
-- 
2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621808 — [tip:perf/core] perf inject: Don't proceed if perf_session__process_event() fails

Commit-ID:  bb8d521f77f3e68a713456b7fb1e99f52ff3342c
Gitweb:     http://git.kernel.org/tip/bb8d521f77f3e68a713456b7fb1e99f52ff3342c
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Mon, 10 Apr 2017 13:14:26 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 11 Apr 2017 15:23:40 -0300

perf inject: Don't proceed if perf_session__process_event() fails

All paths following perf_session__process_event() in __cmd_inject() are
useless if __cmd_inject() is to fail, some depend on a correct
session->evlist.

First commit to add code that depends on session->evlist without checking
error was commmit e558a5bd8b ("perf inject: Work with files"). It has
grown since then.

Change __cmd_inject() to fail immediately after
perf_session__process_event() fails.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrew Vagin <avagin@openvz.org>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Simon Que <sque@chromium.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Fixes: e558a5bd8b74 ("perf inject: Work with files")
Link: http://lkml.kernel.org/r/20170410201432.24807-2-davidcc@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-inject.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 42dff0b..65e1c02 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -694,6 +694,8 @@ static int __cmd_inject(struct perf_inject *inject)
 		lseek(fd, output_data_offset, SEEK_SET);
 
 	ret = perf_session__process_events(session);
+	if (ret)
+		return ret;
 
 	if (!file_out->is_pipe) {
 		if (inject->build_ids)

[toc] | [prev] | [next] | [standalone]

#1620706 — [PATCH 2/7] perf inject: copy events when reordering events in pipe mode

__perf_session__process_pipe_events reuses the same memory buffer to
process all events in the pipe.

When reordering is needed (e.g. -b option), events are not immediately
flushed, but kept around until reordering is possible, causing
memory corruption.

The problem is usually observed by a "Unknown sample error" output. It
can easily be reproduced by:

  perf record -o - noploop | perf inject -b > output

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/ordered-events.c | 3 ++-
 tools/perf/util/session.c        | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index fe84df1875aa..e70e935b1841 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -79,7 +79,7 @@ static union perf_event *dup_event(struct ordered_events *oe,
 
 static void free_dup_event(struct ordered_events *oe, union perf_event *event)
 {
-	if (oe->copy_on_queue) {
+	if (event && oe->copy_on_queue) {
 		oe->cur_alloc_size -= event->header.size;
 		free(event);
 	}
@@ -150,6 +150,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
 	list_move(&event->list, &oe->cache);
 	oe->nr_events--;
 	free_dup_event(oe, event->event);
+	event->event = NULL;
 }
 
 int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 24259bc2c598..a25302bc55a8 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1656,6 +1656,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
 	buf = malloc(cur_size);
 	if (!buf)
 		return -errno;
+	ordered_events__set_copy_on_queue(oe, true);
 more:
 	event = buf;
 	err = readn(fd, event, sizeof(struct perf_event_header));
-- 
2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621100 — Re: [PATCH 2/7] perf inject: copy events when reordering events in pipe mode

On Mon, Apr 10, 2017 at 01:14:27PM -0700, David Carrillo-Cisneros wrote:
> __perf_session__process_pipe_events reuses the same memory buffer to
> process all events in the pipe.
> 
> When reordering is needed (e.g. -b option), events are not immediately
> flushed, but kept around until reordering is possible, causing
> memory corruption.
> 
> The problem is usually observed by a "Unknown sample error" output. It
> can easily be reproduced by:
> 
>   perf record -o - noploop | perf inject -b > output
> 
> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> ---
>  tools/perf/util/ordered-events.c | 3 ++-
>  tools/perf/util/session.c        | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
> index fe84df1875aa..e70e935b1841 100644
> --- a/tools/perf/util/ordered-events.c
> +++ b/tools/perf/util/ordered-events.c
> @@ -79,7 +79,7 @@ static union perf_event *dup_event(struct ordered_events *oe,
>  
>  static void free_dup_event(struct ordered_events *oe, union perf_event *event)
>  {
> -	if (oe->copy_on_queue) {
> +	if (event && oe->copy_on_queue) {
>  		oe->cur_alloc_size -= event->header.size;
>  		free(event);
>  	}
> @@ -150,6 +150,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
>  	list_move(&event->list, &oe->cache);
>  	oe->nr_events--;
>  	free_dup_event(oe, event->event);
> +	event->event = NULL;

based on the changelog I understand the need for the change below,
but I dont get why do we need this cleanu and check above

thanks,
jirka

>  }
>  
>  int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 24259bc2c598..a25302bc55a8 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1656,6 +1656,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
>  	buf = malloc(cur_size);
>  	if (!buf)
>  		return -errno;
> +	ordered_events__set_copy_on_queue(oe, true);
>  more:
>  	event = buf;
>  	err = readn(fd, event, sizeof(struct perf_event_header));
> -- 
> 2.12.2.715.g7642488e1d-goog
> 

[toc] | [prev] | [next] | [standalone]

#1621553 — Re: [PATCH 2/7] perf inject: copy events when reordering events in pipe mode

free_dup_event is called in two places
  1) ordered_events__free
  2) ordered_events__delete

without this patch, calling 1) after 2) would cause a double
de-allocation, but that's exactly what happens in
__perf_session__process_pipe_events. It wasn't triggered before
because events where not copied when queued.

With this patch, the event->event = NULL in ordered_events__delete
will clear event pointer that is checked in free_dup_event, avoiding
de-allocating the event a second time.

Thanks,
David

On Tue, Apr 11, 2017 at 2:40 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Mon, Apr 10, 2017 at 01:14:27PM -0700, David Carrillo-Cisneros wrote:
>> __perf_session__process_pipe_events reuses the same memory buffer to
>> process all events in the pipe.
>>
>> When reordering is needed (e.g. -b option), events are not immediately
>> flushed, but kept around until reordering is possible, causing
>> memory corruption.
>>
>> The problem is usually observed by a "Unknown sample error" output. It
>> can easily be reproduced by:
>>
>>   perf record -o - noploop | perf inject -b > output
>>
>> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>> ---
>>  tools/perf/util/ordered-events.c | 3 ++-
>>  tools/perf/util/session.c        | 1 +
>>  2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
>> index fe84df1875aa..e70e935b1841 100644
>> --- a/tools/perf/util/ordered-events.c
>> +++ b/tools/perf/util/ordered-events.c
>> @@ -79,7 +79,7 @@ static union perf_event *dup_event(struct ordered_events *oe,
>>
>>  static void free_dup_event(struct ordered_events *oe, union perf_event *event)
>>  {
>> -     if (oe->copy_on_queue) {
>> +     if (event && oe->copy_on_queue) {
>>               oe->cur_alloc_size -= event->header.size;
>>               free(event);
>>       }
>> @@ -150,6 +150,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
>>       list_move(&event->list, &oe->cache);
>>       oe->nr_events--;
>>       free_dup_event(oe, event->event);
>> +     event->event = NULL;
>
> based on the changelog I understand the need for the change below,
> but I dont get why do we need this cleanu and check above
>
> thanks,
> jirka
>
>>  }
>>
>>  int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
>> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> index 24259bc2c598..a25302bc55a8 100644
>> --- a/tools/perf/util/session.c
>> +++ b/tools/perf/util/session.c
>> @@ -1656,6 +1656,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
>>       buf = malloc(cur_size);
>>       if (!buf)
>>               return -errno;
>> +     ordered_events__set_copy_on_queue(oe, true);
>>  more:
>>       event = buf;
>>       err = readn(fd, event, sizeof(struct perf_event_header));
>> --
>> 2.12.2.715.g7642488e1d-goog
>>

[toc] | [prev] | [next] | [standalone]

#1621813 — [tip:perf/core] perf inject: Copy events when reordering events in pipe mode

Commit-ID:  1e0d4f0200e4dbdfc38d818f329d8a0955f7c6f5
Gitweb:     http://git.kernel.org/tip/1e0d4f0200e4dbdfc38d818f329d8a0955f7c6f5
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Mon, 10 Apr 2017 13:14:27 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 11 Apr 2017 15:23:41 -0300

perf inject: Copy events when reordering events in pipe mode

__perf_session__process_pipe_events reuses the same memory buffer to
process all events in the pipe.

When reordering is needed (e.g. -b option), events are not immediately
flushed, but kept around until reordering is possible, causing
memory corruption.

The problem is usually observed by a "Unknown sample error" output. It
can easily be reproduced by:

  perf record -o - noploop | perf inject -b > output

Committer testing:

Before:

  $ perf record -o - stress -t 2 -c 2 | perf inject -b > /dev/null
  stress: info: [8297] dispatching hogs: 2 cpu, 0 io, 0 vm, 0 hdd
  stress: info: [8297] successful run completed in 2s
  [ perf record: Woken up 3 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  Warning:
  Found 1 unknown events!

  Is this an older tool processing a perf.data file generated by a more recent tool?

  If that is not the case, consider reporting to linux-kernel@vger.kernel.org.

  $

After:

  $ perf record -o - stress -t 2 -c 2 | perf inject -b > /dev/null
  stress: info: [9027] dispatching hogs: 2 cpu, 0 io, 0 vm, 0 hdd
  stress: info: [9027] successful run completed in 2s
  [ perf record: Woken up 3 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  no symbols found in /usr/bin/stress, maybe install a debug package?
  no symbols found in /usr/bin/stress, maybe install a debug package?
  $

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Simon Que <sque@chromium.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170410201432.24807-3-davidcc@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ordered-events.c | 3 ++-
 tools/perf/util/session.c        | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index fe84df1..e70e935 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -79,7 +79,7 @@ static union perf_event *dup_event(struct ordered_events *oe,
 
 static void free_dup_event(struct ordered_events *oe, union perf_event *event)
 {
-	if (oe->copy_on_queue) {
+	if (event && oe->copy_on_queue) {
 		oe->cur_alloc_size -= event->header.size;
 		free(event);
 	}
@@ -150,6 +150,7 @@ void ordered_events__delete(struct ordered_events *oe, struct ordered_event *eve
 	list_move(&event->list, &oe->cache);
 	oe->nr_events--;
 	free_dup_event(oe, event->event);
+	event->event = NULL;
 }
 
 int ordered_events__queue(struct ordered_events *oe, union perf_event *event,
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 24259bc2..a25302b 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1656,6 +1656,7 @@ static int __perf_session__process_pipe_events(struct perf_session *session)
 	buf = malloc(cur_size);
 	if (!buf)
 		return -errno;
+	ordered_events__set_copy_on_queue(oe, true);
 more:
 	event = buf;
 	err = readn(fd, event, sizeof(struct perf_event_header));

[toc] | [prev] | [next] | [standalone]

#1620707 — [PATCH 4/7] perf annotate: process attr and build_id records

perf annotate did not get some love for pipe-mode, and did not have
.attr and .buil_id setup (while record and inject did. Fix that.

It can easily be reproduced by:

  perf record -o - noploop | perf annotate

that in my system shows:
    0xd8 [0x28]: failed to process type: 9

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/builtin-annotate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 56a7c8d210b9..b2b2722f6bb7 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -394,6 +394,8 @@ int cmd_annotate(int argc, const char **argv)
 			.exit	= perf_event__process_exit,
 			.fork	= perf_event__process_fork,
 			.namespaces = perf_event__process_namespaces,
+			.attr	= perf_event__process_attr,
+			.build_id = perf_event__process_build_id,
 			.ordered_events = true,
 			.ordering_requires_timestamps = true,
 		},
-- 
2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621810 — [tip:perf/core] perf annotate: Process attr and build_id records

Commit-ID:  6ab11f3a35aa07be2ff167b9de37e6c1eb58396b
Gitweb:     http://git.kernel.org/tip/6ab11f3a35aa07be2ff167b9de37e6c1eb58396b
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Mon, 10 Apr 2017 13:14:29 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 11 Apr 2017 15:23:42 -0300

perf annotate: Process attr and build_id records

perf annotate did not get some love for pipe-mode, and did not have
.attr and .buil_id setup (while record and inject did. Fix that.

It can easily be reproduced by:

  perf record -o - noploop | perf annotate

that in my system shows:
    0xd8 [0x28]: failed to process type: 9

Committer Testing:

Before:

  $ perf record -o - stress -t 2 -c 2 | perf annotate --stdio
  stress: info: [11060] dispatching hogs: 2 cpu, 0 io, 0 vm, 0 hdd
  0x4470 [0x28]: failed to process type: 9
  $ stress: info: [11060] successful run completed in 2s

  $

After:

  $ perf record -o - stress -t 2 -c 2 | perf annotate --stdio
  stress: info: [11871] dispatching hogs: 2 cpu, 0 io, 0 vm, 0 hdd
  stress: info: [11871] successful run completed in 2s
  [ perf record: Woken up 2 times to write data ]
  [ perf record: Captured and wrote 0.000 MB - ]
  no symbols found in /usr/bin/stress, maybe install a debug package?
   Percent |      Source code & Disassembly of libc-2.24.so for cycles:uhH (6117 samples)
  ---------------------------------------------------------------------------------------
           :
           :      Disassembly of section .text:
           :
           :      000000000003b050 <random_r>:
           :      __random_r():
     10.56 :        3b050:       test   %rdi,%rdi
      0.00 :        3b053:       je     3b0d0 <random_r+0x80>
      0.34 :        3b055:       test   %rsi,%rsi
      0.00 :        3b058:       je     3b0d0 <random_r+0x80>
      0.46 :        3b05a:       mov    0x18(%rdi),%eax
     12.44 :        3b05d:       mov    0x10(%rdi),%r8
      0.18 :        3b061:       test   %eax,%eax
      0.00 :        3b063:       je     3b0b0 <random_r+0x60>
<SNIP>

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Simon Que <sque@chromium.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170410201432.24807-5-davidcc@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 56a7c8d..b2b2722 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -394,6 +394,8 @@ int cmd_annotate(int argc, const char **argv)
 			.exit	= perf_event__process_exit,
 			.fork	= perf_event__process_fork,
 			.namespaces = perf_event__process_namespaces,
+			.attr	= perf_event__process_attr,
+			.build_id = perf_event__process_build_id,
 			.ordered_events = true,
 			.ordering_requires_timestamps = true,
 		},

[toc] | [prev] | [next] | [standalone]

#1620708 — [PATCH 6/7] perf tool: protect empty evlists

A common pattern in in pipe-mode bugs is accessing an empty evlist.
Return NULL to make it easier to catch this problems.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/evlist.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 39942995f537..ba4788462325 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -257,11 +257,15 @@ void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
 
 static inline struct perf_evsel *perf_evlist__first(struct perf_evlist *evlist)
 {
+	if (list_empty(&evlist->entries))
+		return NULL;
 	return list_entry(evlist->entries.next, struct perf_evsel, node);
 }
 
 static inline struct perf_evsel *perf_evlist__last(struct perf_evlist *evlist)
 {
+	if (list_empty(&evlist->entries))
+		return NULL;
 	return list_entry(evlist->entries.prev, struct perf_evsel, node);
 }
 
-- 
2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621522 — Re: [PATCH 6/7] perf tool: protect empty evlists

Em Mon, Apr 10, 2017 at 01:14:31PM -0700, David Carrillo-Cisneros escreveu:
> A common pattern in in pipe-mode bugs is accessing an empty evlist.
> Return NULL to make it easier to catch this problems.

This one is far reaching, we need to take care handling all places using
these functions, albeit probably most cases will always have an evsel, I
looked at builtin-top.c for instance, and I think we need to be more
careful here, will leave this for later.

- Arnaldo
 
> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> ---
>  tools/perf/util/evlist.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 39942995f537..ba4788462325 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -257,11 +257,15 @@ void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
>  
>  static inline struct perf_evsel *perf_evlist__first(struct perf_evlist *evlist)
>  {
> +	if (list_empty(&evlist->entries))
> +		return NULL;
>  	return list_entry(evlist->entries.next, struct perf_evsel, node);
>  }
>  
>  static inline struct perf_evsel *perf_evlist__last(struct perf_evlist *evlist)
>  {
> +	if (list_empty(&evlist->entries))
> +		return NULL;
>  	return list_entry(evlist->entries.prev, struct perf_evsel, node);
>  }
>  
> -- 
> 2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621544 — Re: [PATCH 6/7] perf tool: protect empty evlists

I did my best to fix those issues in perf inject and perf report, but
yeah, other commands may have issues.

On Tue, Apr 11, 2017 at 11:12 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Mon, Apr 10, 2017 at 01:14:31PM -0700, David Carrillo-Cisneros escreveu:
>> A common pattern in in pipe-mode bugs is accessing an empty evlist.
>> Return NULL to make it easier to catch this problems.
>
> This one is far reaching, we need to take care handling all places using
> these functions, albeit probably most cases will always have an evsel, I
> looked at builtin-top.c for instance, and I think we need to be more
> careful here, will leave this for later.
>
> - Arnaldo
>
>> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>> ---
>>  tools/perf/util/evlist.h | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>> index 39942995f537..ba4788462325 100644
>> --- a/tools/perf/util/evlist.h
>> +++ b/tools/perf/util/evlist.h
>> @@ -257,11 +257,15 @@ void perf_evlist__splice_list_tail(struct perf_evlist *evlist,
>>
>>  static inline struct perf_evsel *perf_evlist__first(struct perf_evlist *evlist)
>>  {
>> +     if (list_empty(&evlist->entries))
>> +             return NULL;
>>       return list_entry(evlist->entries.next, struct perf_evsel, node);
>>  }
>>
>>  static inline struct perf_evsel *perf_evlist__last(struct perf_evlist *evlist)
>>  {
>> +     if (list_empty(&evlist->entries))
>> +             return NULL;
>>       return list_entry(evlist->entries.prev, struct perf_evsel, node);
>>  }
>>
>> --
>> 2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1620709 — [PATCH 7/7] perf tool: do not print missing features in pipe-mode

Pipe-mode has no perf.data header, hence no upfront knowledge
of presend and missing features, hence, do not print missing
features in pipe-mode.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/header.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ef09f26e67da..2ccc7f06db79 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2270,6 +2270,9 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
 	perf_header__process_sections(header, fd, &hd,
 				      perf_file_section__fprintf_info);
 
+	if (session->file->is_pipe)
+		return 0;
+
 	fprintf(fp, "# missing features: ");
 	for_each_clear_bit(bit, header->adds_features, HEADER_LAST_FEATURE) {
 		if (bit)
-- 
2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621819 — [tip:perf/core] perf tools: Do not print missing features in pipe-mode

Commit-ID:  c9d1c93421e3b3c7051b193c9cf648a3bc55cb3e
Gitweb:     http://git.kernel.org/tip/c9d1c93421e3b3c7051b193c9cf648a3bc55cb3e
Author:     David Carrillo-Cisneros <davidcc@google.com>
AuthorDate: Mon, 10 Apr 2017 13:14:32 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 11 Apr 2017 16:22:22 -0300

perf tools: Do not print missing features in pipe-mode

Pipe-mode has no perf.data header, hence no upfront knowledge of presend
and missing features, hence, do not print missing features in pipe-mode.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Paul Turner <pjt@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Simon Que <sque@chromium.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20170410201432.24807-8-davidcc@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ef09f26..2ccc7f0 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2270,6 +2270,9 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
 	perf_header__process_sections(header, fd, &hd,
 				      perf_file_section__fprintf_info);
 
+	if (session->file->is_pipe)
+		return 0;
+
 	fprintf(fp, "# missing features: ");
 	for_each_clear_bit(bit, header->adds_features, HEADER_LAST_FEATURE) {
 		if (bit)

[toc] | [prev] | [next] | [standalone]

#1620710 — [PATCH 5/7] perf session: don't rely on evlist in pipe mode

Session sets a number parameters that rely on evlist. These parameters
are not used in pipe-mode and should not be set, since evlist is
unavailable. Fix that.

Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
---
 tools/perf/util/session.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index a25302bc55a8..db554b7461b8 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -140,8 +140,14 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 			if (perf_session__open(session) < 0)
 				goto out_close;
 
-			perf_session__set_id_hdr_size(session);
-			perf_session__set_comm_exec(session);
+			/*
+			 * set session attributes that are present in perf.data
+			 * but not in pipe-mode.
+			 */
+			if (!file->is_pipe) {
+				perf_session__set_id_hdr_size(session);
+				perf_session__set_comm_exec(session);
+			}
 		}
 	} else  {
 		session->machines.host.env = &perf_env;
@@ -156,7 +162,11 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 			pr_warning("Cannot read kernel map\n");
 	}
 
-	if (tool && tool->ordering_requires_timestamps &&
+	/*
+	 * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
+	 * processed, so perf_evlist__sample_id_all is not meaningful here.
+	 */
+	if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
 	    tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
 		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
 		tool->ordered_events = false;
-- 
2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621545 — Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode

Em Tue, Apr 11, 2017 at 03:40:50PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Apr 11, 2017 at 03:40:14PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Mon, Apr 10, 2017 at 01:14:30PM -0700, David Carrillo-Cisneros escreveu:
> > > Session sets a number parameters that rely on evlist. These parameters
> > > are not used in pipe-mode and should not be set, since evlist is
> > > unavailable. Fix that.
> > 
> > Ditching this one as well, until further investigation, well, file must
> > be null in this case...

This should be enough, right?

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index db554b7461b8..37cf4e6abf9f 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -144,7 +144,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 			 * set session attributes that are present in perf.data
 			 * but not in pipe-mode.
 			 */
-			if (!file->is_pipe) {
+			if (!file || !file->is_pipe) {
 				perf_session__set_id_hdr_size(session);
 				perf_session__set_comm_exec(session);
 			}
@@ -166,7 +166,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 	 * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
 	 * processed, so perf_evlist__sample_id_all is not meaningful here.
 	 */
-	if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
+	if ((!file || !file->is_pipe) && tool && tool->ordering_requires_timestamps &&
 	    tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
 		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
 		tool->ordered_events = false;
 
> Ah, this is the problem:
> 
> [root@jouet ~]# perf top
> perf: Segmentation fault
> -------- backtrace --------
> perf[0x57eddb]
> /lib64/libc.so.6(+0x35990)[0x7fb435d06990]
> perf(perf_session__new+0xb7)[0x4eb547]
> perf(cmd_top+0xde9)[0x44ce89]
> perf[0x49953f]
> perf[0x499831]
> perf(main+0x252)[0x428dd2]
> /lib64/libc.so.6(__libc_start_main+0xf1)[0x7fb435cf1401]
> perf(_start+0x2a)[0x4292da]
> [0x0]
> [root@jouet ~]#
>  
> > [acme@jouet linux]$ git bisect bad
> > 5d71c27204d51492abe9fc69064fd36a0bda410b is the first bad commit
> > commit 5d71c27204d51492abe9fc69064fd36a0bda410b
> > Author: David Carrillo-Cisneros <davidcc@google.com>
> > Date:   Mon Apr 10 13:14:30 2017 -0700
> > 
> >     perf session: Don't rely on evlist in pipe mode
> >     
> >     Session sets a number parameters that rely on evlist. These parameters
> >     are not used in pipe-mode and should not be set, since evlist is
> >     unavailable. Fix that.
> >     
> >     Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> >     Acked-by: Jiri Olsa <jolsa@kernel.org>
> >     Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >     Cc: Andi Kleen <ak@linux.intel.com>
> >     Cc: He Kuang <hekuang@huawei.com>
> >     Cc: Masami Hiramatsu <mhiramat@kernel.org>
> >     Cc: Paul Turner <pjt@google.com>
> >     Cc: Peter Zijlstra <peterz@infradead.org>
> >     Cc: Simon Que <sque@chromium.org>
> >     Cc: Stephane Eranian <eranian@google.com>
> >     Cc: Wang Nan <wangnan0@huawei.com>
> >     Link: http://lkml.kernel.org/r/20170410201432.24807-6-davidcc@google.com
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > :040000 040000 7df28fe9d7c9d962b78c0532f4961caea2737b59 34e4fed0042b1401c1a1a247d8ae40df17f9d272 M	tools
> > [acme@jouet linux]$
> >  
> > > Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> > > ---
> > >  tools/perf/util/session.c | 16 +++++++++++++---
> > >  1 file changed, 13 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > > index a25302bc55a8..db554b7461b8 100644
> > > --- a/tools/perf/util/session.c
> > > +++ b/tools/perf/util/session.c
> > > @@ -140,8 +140,14 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
> > >  			if (perf_session__open(session) < 0)
> > >  				goto out_close;
> > >  
> > > -			perf_session__set_id_hdr_size(session);
> > > -			perf_session__set_comm_exec(session);
> > > +			/*
> > > +			 * set session attributes that are present in perf.data
> > > +			 * but not in pipe-mode.
> > > +			 */
> > > +			if (!file->is_pipe) {
> > > +				perf_session__set_id_hdr_size(session);
> > > +				perf_session__set_comm_exec(session);
> > > +			}
> > >  		}
> > >  	} else  {
> > >  		session->machines.host.env = &perf_env;
> > > @@ -156,7 +162,11 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
> > >  			pr_warning("Cannot read kernel map\n");
> > >  	}
> > >  
> > > -	if (tool && tool->ordering_requires_timestamps &&
> > > +	/*
> > > +	 * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
> > > +	 * processed, so perf_evlist__sample_id_all is not meaningful here.
> > > +	 */
> > > +	if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
> > >  	    tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
> > >  		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
> > >  		tool->ordered_events = false;
> > > -- 
> > > 2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621559 — Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode

Only the second one is needed. file is already checked for the first one.

On Tue, Apr 11, 2017 at 11:44 AM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Tue, Apr 11, 2017 at 03:40:50PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Apr 11, 2017 at 03:40:14PM -0300, Arnaldo Carvalho de Melo escreveu:
>> > Em Mon, Apr 10, 2017 at 01:14:30PM -0700, David Carrillo-Cisneros escreveu:
>> > > Session sets a number parameters that rely on evlist. These parameters
>> > > are not used in pipe-mode and should not be set, since evlist is
>> > > unavailable. Fix that.
>> >
>> > Ditching this one as well, until further investigation, well, file must
>> > be null in this case...
>
> This should be enough, right?
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index db554b7461b8..37cf4e6abf9f 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -144,7 +144,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
>                          * set session attributes that are present in perf.data
>                          * but not in pipe-mode.
>                          */
> -                       if (!file->is_pipe) {
> +                       if (!file || !file->is_pipe) {
>                                 perf_session__set_id_hdr_size(session);
>                                 perf_session__set_comm_exec(session);
>                         }
> @@ -166,7 +166,7 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
>          * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
>          * processed, so perf_evlist__sample_id_all is not meaningful here.
>          */
> -       if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
> +       if ((!file || !file->is_pipe) && tool && tool->ordering_requires_timestamps &&
>             tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
>                 dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
>                 tool->ordered_events = false;
>
>> Ah, this is the problem:
>>
>> [root@jouet ~]# perf top
>> perf: Segmentation fault
>> -------- backtrace --------
>> perf[0x57eddb]
>> /lib64/libc.so.6(+0x35990)[0x7fb435d06990]
>> perf(perf_session__new+0xb7)[0x4eb547]
>> perf(cmd_top+0xde9)[0x44ce89]
>> perf[0x49953f]
>> perf[0x499831]
>> perf(main+0x252)[0x428dd2]
>> /lib64/libc.so.6(__libc_start_main+0xf1)[0x7fb435cf1401]
>> perf(_start+0x2a)[0x4292da]
>> [0x0]
>> [root@jouet ~]#
>>
>> > [acme@jouet linux]$ git bisect bad
>> > 5d71c27204d51492abe9fc69064fd36a0bda410b is the first bad commit
>> > commit 5d71c27204d51492abe9fc69064fd36a0bda410b
>> > Author: David Carrillo-Cisneros <davidcc@google.com>
>> > Date:   Mon Apr 10 13:14:30 2017 -0700
>> >
>> >     perf session: Don't rely on evlist in pipe mode
>> >
>> >     Session sets a number parameters that rely on evlist. These parameters
>> >     are not used in pipe-mode and should not be set, since evlist is
>> >     unavailable. Fix that.
>> >
>> >     Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>> >     Acked-by: Jiri Olsa <jolsa@kernel.org>
>> >     Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> >     Cc: Andi Kleen <ak@linux.intel.com>
>> >     Cc: He Kuang <hekuang@huawei.com>
>> >     Cc: Masami Hiramatsu <mhiramat@kernel.org>
>> >     Cc: Paul Turner <pjt@google.com>
>> >     Cc: Peter Zijlstra <peterz@infradead.org>
>> >     Cc: Simon Que <sque@chromium.org>
>> >     Cc: Stephane Eranian <eranian@google.com>
>> >     Cc: Wang Nan <wangnan0@huawei.com>
>> >     Link: http://lkml.kernel.org/r/20170410201432.24807-6-davidcc@google.com
>> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>> >
>> > :040000 040000 7df28fe9d7c9d962b78c0532f4961caea2737b59 34e4fed0042b1401c1a1a247d8ae40df17f9d272 M  tools
>> > [acme@jouet linux]$
>> >
>> > > Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>> > > ---
>> > >  tools/perf/util/session.c | 16 +++++++++++++---
>> > >  1 file changed, 13 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
>> > > index a25302bc55a8..db554b7461b8 100644
>> > > --- a/tools/perf/util/session.c
>> > > +++ b/tools/perf/util/session.c
>> > > @@ -140,8 +140,14 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
>> > >                   if (perf_session__open(session) < 0)
>> > >                           goto out_close;
>> > >
>> > > -                 perf_session__set_id_hdr_size(session);
>> > > -                 perf_session__set_comm_exec(session);
>> > > +                 /*
>> > > +                  * set session attributes that are present in perf.data
>> > > +                  * but not in pipe-mode.
>> > > +                  */
>> > > +                 if (!file->is_pipe) {
>> > > +                         perf_session__set_id_hdr_size(session);
>> > > +                         perf_session__set_comm_exec(session);
>> > > +                 }
>> > >           }
>> > >   } else  {
>> > >           session->machines.host.env = &perf_env;
>> > > @@ -156,7 +162,11 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
>> > >                   pr_warning("Cannot read kernel map\n");
>> > >   }
>> > >
>> > > - if (tool && tool->ordering_requires_timestamps &&
>> > > + /*
>> > > +  * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
>> > > +  * processed, so perf_evlist__sample_id_all is not meaningful here.
>> > > +  */
>> > > + if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
>> > >       tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
>> > >           dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
>> > >           tool->ordered_events = false;
>> > > --
>> > > 2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621587 — Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode

Em Tue, Apr 11, 2017 at 11:59:18AM -0700, David Carrillo-Cisneros escreveu:
> Only the second one is needed. file is already checked for the first one.

Right, fixed, tested.

- Arnaldo
 

[toc] | [prev] | [next] | [standalone]

#1621547 — Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode

Em Tue, Apr 11, 2017 at 03:40:14PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Apr 10, 2017 at 01:14:30PM -0700, David Carrillo-Cisneros escreveu:
> > Session sets a number parameters that rely on evlist. These parameters
> > are not used in pipe-mode and should not be set, since evlist is
> > unavailable. Fix that.
> 
> Ditching this one as well, until further investigation, well, file must
> be null in this case...

Ah, this is the problem:

[root@jouet ~]# perf top
perf: Segmentation fault
-------- backtrace --------
perf[0x57eddb]
/lib64/libc.so.6(+0x35990)[0x7fb435d06990]
perf(perf_session__new+0xb7)[0x4eb547]
perf(cmd_top+0xde9)[0x44ce89]
perf[0x49953f]
perf[0x499831]
perf(main+0x252)[0x428dd2]
/lib64/libc.so.6(__libc_start_main+0xf1)[0x7fb435cf1401]
perf(_start+0x2a)[0x4292da]
[0x0]
[root@jouet ~]#
 
> [acme@jouet linux]$ git bisect bad
> 5d71c27204d51492abe9fc69064fd36a0bda410b is the first bad commit
> commit 5d71c27204d51492abe9fc69064fd36a0bda410b
> Author: David Carrillo-Cisneros <davidcc@google.com>
> Date:   Mon Apr 10 13:14:30 2017 -0700
> 
>     perf session: Don't rely on evlist in pipe mode
>     
>     Session sets a number parameters that rely on evlist. These parameters
>     are not used in pipe-mode and should not be set, since evlist is
>     unavailable. Fix that.
>     
>     Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
>     Acked-by: Jiri Olsa <jolsa@kernel.org>
>     Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>     Cc: Andi Kleen <ak@linux.intel.com>
>     Cc: He Kuang <hekuang@huawei.com>
>     Cc: Masami Hiramatsu <mhiramat@kernel.org>
>     Cc: Paul Turner <pjt@google.com>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Simon Que <sque@chromium.org>
>     Cc: Stephane Eranian <eranian@google.com>
>     Cc: Wang Nan <wangnan0@huawei.com>
>     Link: http://lkml.kernel.org/r/20170410201432.24807-6-davidcc@google.com
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> :040000 040000 7df28fe9d7c9d962b78c0532f4961caea2737b59 34e4fed0042b1401c1a1a247d8ae40df17f9d272 M	tools
> [acme@jouet linux]$
>  
> > Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> > ---
> >  tools/perf/util/session.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index a25302bc55a8..db554b7461b8 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -140,8 +140,14 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
> >  			if (perf_session__open(session) < 0)
> >  				goto out_close;
> >  
> > -			perf_session__set_id_hdr_size(session);
> > -			perf_session__set_comm_exec(session);
> > +			/*
> > +			 * set session attributes that are present in perf.data
> > +			 * but not in pipe-mode.
> > +			 */
> > +			if (!file->is_pipe) {
> > +				perf_session__set_id_hdr_size(session);
> > +				perf_session__set_comm_exec(session);
> > +			}
> >  		}
> >  	} else  {
> >  		session->machines.host.env = &perf_env;
> > @@ -156,7 +162,11 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
> >  			pr_warning("Cannot read kernel map\n");
> >  	}
> >  
> > -	if (tool && tool->ordering_requires_timestamps &&
> > +	/*
> > +	 * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
> > +	 * processed, so perf_evlist__sample_id_all is not meaningful here.
> > +	 */
> > +	if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
> >  	    tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
> >  		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
> >  		tool->ordered_events = false;
> > -- 
> > 2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

#1621552 — Re: [PATCH 5/7] perf session: don't rely on evlist in pipe mode

Em Mon, Apr 10, 2017 at 01:14:30PM -0700, David Carrillo-Cisneros escreveu:
> Session sets a number parameters that rely on evlist. These parameters
> are not used in pipe-mode and should not be set, since evlist is
> unavailable. Fix that.

Ditching this one as well, until further investigation, well, file must
be null in this case...

[acme@jouet linux]$ git bisect bad
5d71c27204d51492abe9fc69064fd36a0bda410b is the first bad commit
commit 5d71c27204d51492abe9fc69064fd36a0bda410b
Author: David Carrillo-Cisneros <davidcc@google.com>
Date:   Mon Apr 10 13:14:30 2017 -0700

    perf session: Don't rely on evlist in pipe mode
    
    Session sets a number parameters that rely on evlist. These parameters
    are not used in pipe-mode and should not be set, since evlist is
    unavailable. Fix that.
    
    Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
    Acked-by: Jiri Olsa <jolsa@kernel.org>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Andi Kleen <ak@linux.intel.com>
    Cc: He Kuang <hekuang@huawei.com>
    Cc: Masami Hiramatsu <mhiramat@kernel.org>
    Cc: Paul Turner <pjt@google.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Simon Que <sque@chromium.org>
    Cc: Stephane Eranian <eranian@google.com>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: http://lkml.kernel.org/r/20170410201432.24807-6-davidcc@google.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

:040000 040000 7df28fe9d7c9d962b78c0532f4961caea2737b59 34e4fed0042b1401c1a1a247d8ae40df17f9d272 M	tools
[acme@jouet linux]$
 
> Signed-off-by: David Carrillo-Cisneros <davidcc@google.com>
> ---
>  tools/perf/util/session.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index a25302bc55a8..db554b7461b8 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -140,8 +140,14 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
>  			if (perf_session__open(session) < 0)
>  				goto out_close;
>  
> -			perf_session__set_id_hdr_size(session);
> -			perf_session__set_comm_exec(session);
> +			/*
> +			 * set session attributes that are present in perf.data
> +			 * but not in pipe-mode.
> +			 */
> +			if (!file->is_pipe) {
> +				perf_session__set_id_hdr_size(session);
> +				perf_session__set_comm_exec(session);
> +			}
>  		}
>  	} else  {
>  		session->machines.host.env = &perf_env;
> @@ -156,7 +162,11 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
>  			pr_warning("Cannot read kernel map\n");
>  	}
>  
> -	if (tool && tool->ordering_requires_timestamps &&
> +	/*
> +	 * In pipe-mode, evlist is empty until PERF_RECORD_HEADER_ATTR is
> +	 * processed, so perf_evlist__sample_id_all is not meaningful here.
> +	 */
> +	if (!file->is_pipe && tool && tool->ordering_requires_timestamps &&
>  	    tool->ordered_events && !perf_evlist__sample_id_all(session->evlist)) {
>  		dump_printf("WARNING: No sample_id_all support, falling back to unordered processing\n");
>  		tool->ordered_events = false;
> -- 
> 2.12.2.715.g7642488e1d-goog

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web