[PATCH 0/5] perf tools: Pipe fixes

[PATCH 0/5] perf tools: Pipe fixes

[Jiri Olsa]

hi,
sending changes that allows callchain detection in pipe mode,
fixing followign use case:

  # perf record --no-buffering --call-graph dwarf -e sdt_rtld:init_start -a -o - | perf --no-pager script -i -

plus change that allows to read pipe data from file:

  # perf record -o - sleep 1 > /tmp/perf.pipe.data
  # perf report -i /tmp/perf.pipe.data

plus unrelated build fix.

Also reachable in here:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/pipe

thanks,
jirka


---
Jiri Olsa (5):
      perf tools: Do not display extra info when there is nothing to build
      perf tools: Do not seek in pipe fd during tracing data processing
      perf session: Try to read pipe data from file
      perf tools: Setup callchain properly in pipe mode
      perf script: Enable IP fields for callchains

 tools/perf/Makefile.perf    |  8 +++++---
 tools/perf/builtin-report.c | 33 ++++++++++++++++++++++-----------
 tools/perf/builtin-script.c | 23 +++++++++++++++++++----
 tools/perf/util/callchain.c | 14 ++++++++++++++
 tools/perf/util/callchain.h |  1 +
 tools/perf/util/header.c    | 34 ++++++++++++++++++++++++++--------
 tools/perf/util/session.c   |  9 +++++++--
 7 files changed, 94 insertions(+), 28 deletions(-)

[PATCH 1/5] perf tools: Do not display extra info when there is nothing to build

[Jiri Olsa]

Even with fully built tree, we still display extra output
when make is invoked, like:

  $ make
    BUILD:   Doing 'make -j8' parallel build
    DESCEND  plugins
  make[3]: Nothing to be done for 'plugins/libtraceevent-dynamic-list'.

Changing the make descend directly to plugins directory,
which quiets those messages down.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/Makefile.perf | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index 94a495594e99..30e41dcd4095 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -278,6 +278,7 @@ strip-libs = $(filter-out -l%,$(1))
 
 ifneq ($(OUTPUT),)
   TE_PATH=$(OUTPUT)
+  PLUGINS_PATH=$(OUTPUT)
   BPF_PATH=$(OUTPUT)
   SUBCMD_PATH=$(OUTPUT)
   LIBPERF_PATH=$(OUTPUT)
@@ -288,6 +289,7 @@ else
 endif
 else
   TE_PATH=$(TRACE_EVENT_DIR)
+  PLUGINS_PATH=$(TRACE_EVENT_DIR)plugins/
   API_PATH=$(LIB_DIR)
   BPF_PATH=$(BPF_DIR)
   SUBCMD_PATH=$(SUBCMD_DIR)
@@ -297,7 +299,7 @@ endif
 LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
 export LIBTRACEEVENT
 
-LIBTRACEEVENT_DYNAMIC_LIST = $(TE_PATH)plugins/libtraceevent-dynamic-list
+LIBTRACEEVENT_DYNAMIC_LIST = $(PLUGINS_PATH)libtraceevent-dynamic-list
 
 #
 # The static build has no dynsym table, so this does not work for
@@ -756,10 +758,10 @@ $(LIBTRACEEVENT): FORCE
 	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
 
 libtraceevent_plugins: FORCE
-	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
+	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR)plugins $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
 
 $(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
-	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)plugins/libtraceevent-dynamic-list
+	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR)plugins $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
 
 $(LIBTRACEEVENT)-clean:
 	$(call QUIET_CLEAN, libtraceevent)
-- 
2.25.4

Re: [PATCH 1/5] perf tools: Do not display extra info when there is nothing to build

[Arnaldo Carvalho de Melo]

Em Thu, May 07, 2020 at 11:50:20AM +0200, Jiri Olsa escreveu:
> Even with fully built tree, we still display extra output
> when make is invoked, like:
> 
>   $ make
>     BUILD:   Doing 'make -j8' parallel build
>     DESCEND  plugins
>   make[3]: Nothing to be done for 'plugins/libtraceevent-dynamic-list'.
> 
> Changing the make descend directly to plugins directory,
> which quiets those messages down.

Thanks a lot for getting rid of that nuisance :-)

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/Makefile.perf | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
> index 94a495594e99..30e41dcd4095 100644
> --- a/tools/perf/Makefile.perf
> +++ b/tools/perf/Makefile.perf
> @@ -278,6 +278,7 @@ strip-libs = $(filter-out -l%,$(1))
>  
>  ifneq ($(OUTPUT),)
>    TE_PATH=$(OUTPUT)
> +  PLUGINS_PATH=$(OUTPUT)
>    BPF_PATH=$(OUTPUT)
>    SUBCMD_PATH=$(OUTPUT)
>    LIBPERF_PATH=$(OUTPUT)
> @@ -288,6 +289,7 @@ else
>  endif
>  else
>    TE_PATH=$(TRACE_EVENT_DIR)
> +  PLUGINS_PATH=$(TRACE_EVENT_DIR)plugins/
>    API_PATH=$(LIB_DIR)
>    BPF_PATH=$(BPF_DIR)
>    SUBCMD_PATH=$(SUBCMD_DIR)
> @@ -297,7 +299,7 @@ endif
>  LIBTRACEEVENT = $(TE_PATH)libtraceevent.a
>  export LIBTRACEEVENT
>  
> -LIBTRACEEVENT_DYNAMIC_LIST = $(TE_PATH)plugins/libtraceevent-dynamic-list
> +LIBTRACEEVENT_DYNAMIC_LIST = $(PLUGINS_PATH)libtraceevent-dynamic-list
>  
>  #
>  # The static build has no dynsym table, so this does not work for
> @@ -756,10 +758,10 @@ $(LIBTRACEEVENT): FORCE
>  	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent.a
>  
>  libtraceevent_plugins: FORCE
> -	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
> +	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR)plugins $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) plugins
>  
>  $(LIBTRACEEVENT_DYNAMIC_LIST): libtraceevent_plugins
> -	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR) $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)plugins/libtraceevent-dynamic-list
> +	$(Q)$(MAKE) -C $(TRACE_EVENT_DIR)plugins $(LIBTRACEEVENT_FLAGS) O=$(OUTPUT) $(OUTPUT)libtraceevent-dynamic-list
>  
>  $(LIBTRACEEVENT)-clean:
>  	$(call QUIET_CLEAN, libtraceevent)
> -- 
> 2.25.4
> 

-- 

- Arnaldo

[PATCH 2/5] perf tools: Do not seek in pipe fd during tracing data processing

[Jiri Olsa]

There's no need to set 'fd' position in pipe mode, the file
descriptor is already in proper place. Moreover the lseek will
fail on pipe descriptor and that's why it's been working properly.

I was tempted to remove the lseek calls completely, because it seems
that tracing data event was always synthesized only in pipe mode, so
there's no need for 'file' mode handling. But I guess there was a reason
behind this and there might (however unlikely) be a perf.data that we
could break processing for.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/header.c  | 18 ++++++++++++++----
 tools/perf/util/session.c |  9 +++++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 0ce47283a8a1..13a1fe4ac0c0 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3947,12 +3947,22 @@ int perf_event__process_tracing_data(struct perf_session *session,
 {
 	ssize_t size_read, padding, size = event->tracing_data.size;
 	int fd = perf_data__fd(session->data);
-	off_t offset = lseek(fd, 0, SEEK_CUR);
 	char buf[BUFSIZ];
 
-	/* setup for reading amidst mmap */
-	lseek(fd, offset + sizeof(struct perf_record_header_tracing_data),
-	      SEEK_SET);
+	/*
+	 * The pipe fd is already in proper place and in any case
+	 * we can't move it, and we'd screw the case where we read
+	 * 'pipe' data from regular file. The trace_report reads
+	 * data from 'fd' so we need to set it directly behind the
+	 * event, where the tracing data starts.
+	 */
+	if (!perf_data__is_pipe(session->data)) {
+		off_t offset = lseek(fd, 0, SEEK_CUR);
+
+		/* setup for reading amidst mmap */
+		lseek(fd, offset + sizeof(struct perf_record_header_tracing_data),
+		      SEEK_SET);
+	}
 
 	size_read = trace_report(fd, &session->tevent,
 				 session->repipe);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index c11d89e0ee55..b860f9f1b09e 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1542,8 +1542,13 @@ static s64 perf_session__process_user_event(struct perf_session *session,
 		 */
 		return 0;
 	case PERF_RECORD_HEADER_TRACING_DATA:
-		/* setup for reading amidst mmap */
-		lseek(fd, file_offset, SEEK_SET);
+		/*
+		 * Setup for reading amidst mmap, but only when we
+		 * are in 'file' mode. The 'pipe' fd is in proper
+		 * place already.
+		 */
+		if (!perf_data__is_pipe(session->data))
+			lseek(fd, file_offset, SEEK_SET);
 		return tool->tracing_data(session, event);
 	case PERF_RECORD_HEADER_BUILD_ID:
 		return tool->build_id(session, event);
-- 
2.25.4

[PATCH 3/5] perf session: Try to read pipe data from file

[Jiri Olsa]

Ian came with the idea of having support to read the pipe
data also from file. Currently pipe mode files fail like:

  $ perf record -o - sleep 1 > /tmp/perf.pipe.data
  $ perf report -i /tmp/perf.pipe.data
  incompatible file format (rerun with -v to learn more)

This patch adds the support to do that by trying the pipe
header first, and if its successfully detected, switching
the perf data to pipe mode.

Original-patch-by: Ian Rogers <irogers@google.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/util/header.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 13a1fe4ac0c0..7a67d017d72c 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3574,7 +3574,7 @@ static int perf_header__read_pipe(struct perf_session *session)
 		return -EINVAL;
 	}
 
-	return 0;
+	return f_header.size == sizeof(f_header) ? 0 : -1;
 }
 
 static int read_attr(int fd, struct perf_header *ph,
@@ -3676,7 +3676,7 @@ int perf_session__read_header(struct perf_session *session)
 	struct perf_file_header	f_header;
 	struct perf_file_attr	f_attr;
 	u64			f_id;
-	int nr_attrs, nr_ids, i, j;
+	int nr_attrs, nr_ids, i, j, err;
 	int fd = perf_data__fd(data);
 
 	session->evlist = evlist__new();
@@ -3685,8 +3685,16 @@ int perf_session__read_header(struct perf_session *session)
 
 	session->evlist->env = &header->env;
 	session->machines.host.env = &header->env;
-	if (perf_data__is_pipe(data))
-		return perf_header__read_pipe(session);
+
+	/*
+	 * We can read 'pipe' data event from regular file,
+	 * check for the pipe header regardless of source.
+	 */
+	err = perf_header__read_pipe(session);
+	if (!err || (err && perf_data__is_pipe(data))) {
+		data->is_pipe = true;
+		return err;
+	}
 
 	if (perf_file_header__read(&f_header, header, fd) < 0)
 		return -EINVAL;
-- 
2.25.4

[PATCH 4/5] perf tools: Setup callchain properly in pipe mode

[Jiri Olsa]

Callchains are automatically initialized by checking
on event's sample_type. For pipe mode we need to put
this check into attr event code.

Moving the callchains setup code into callchain_param_setup
function and calling it from attr event process code.

This enables pipe output having callchains, like:

  # perf record -g -e 'raw_syscalls:sys_enter' true | perf script
  # perf record -g -e 'raw_syscalls:sys_enter' true | perf report

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-report.c | 33 ++++++++++++++++++++++-----------
 tools/perf/builtin-script.c | 14 ++++++++++++--
 tools/perf/util/callchain.c | 14 ++++++++++++++
 tools/perf/util/callchain.h |  1 +
 4 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index ba63390246c2..5b7c6db4c930 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -402,16 +402,7 @@ static int report__setup_sample_type(struct report *rep)
 		}
 	}
 
-	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain) {
-		if ((sample_type & PERF_SAMPLE_REGS_USER) &&
-		    (sample_type & PERF_SAMPLE_STACK_USER)) {
-			callchain_param.record_mode = CALLCHAIN_DWARF;
-			dwarf_callchain_users = true;
-		} else if (sample_type & PERF_SAMPLE_BRANCH_STACK)
-			callchain_param.record_mode = CALLCHAIN_LBR;
-		else
-			callchain_param.record_mode = CALLCHAIN_FP;
-	}
+	callchain_param_setup(sample_type);
 
 	if (rep->stitch_lbr && (callchain_param.record_mode != CALLCHAIN_LBR)) {
 		ui__warning("Can't find LBR callchain. Switch off --stitch-lbr.\n"
@@ -1090,6 +1081,26 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
+static int process_attr(struct perf_tool *tool __maybe_unused,
+			union perf_event *event,
+			struct evlist **pevlist)
+{
+	u64 sample_type;
+	int err;
+
+	err = perf_event__process_attr(tool, event, pevlist);
+	if (err)
+		return err;
+
+	/*
+	 * Check if we need to enable callchains based
+	 * on events sample_type.
+	 */
+	sample_type = perf_evlist__combined_sample_type(*pevlist);
+	callchain_param_setup(sample_type);
+	return 0;
+}
+
 int cmd_report(int argc, const char **argv)
 {
 	struct perf_session *session;
@@ -1120,7 +1131,7 @@ int cmd_report(int argc, const char **argv)
 			.fork		 = perf_event__process_fork,
 			.lost		 = perf_event__process_lost,
 			.read		 = process_read_event,
-			.attr		 = perf_event__process_attr,
+			.attr		 = process_attr,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
 			.id_index	 = perf_event__process_id_index,
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 56d7bcd12671..5c4a580c048a 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2085,6 +2085,7 @@ static int process_attr(struct perf_tool *tool, union perf_event *event,
 	struct perf_script *scr = container_of(tool, struct perf_script, tool);
 	struct evlist *evlist;
 	struct evsel *evsel, *pos;
+	u64 sample_type;
 	int err;
 	static struct evsel_script *es;
 
@@ -2119,10 +2120,19 @@ static int process_attr(struct perf_tool *tool, union perf_event *event,
 
 	set_print_ip_opts(&evsel->core.attr);
 
-	if (evsel->core.attr.sample_type)
+	if (evsel->core.attr.sample_type) {
 		err = perf_evsel__check_attr(evsel, scr->session);
+		if (err)
+			return err;
+	}
 
-	return err;
+	/*
+	 * Check if we need to enable callchains based
+	 * on events sample_type.
+	 */
+	sample_type = perf_evlist__combined_sample_type(evlist);
+	callchain_param_setup(sample_type);
+	return 0;
 }
 
 static int print_event_with_time(struct perf_tool *tool,
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 818aa4efd386..2775b752f2fa 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1599,3 +1599,17 @@ void callchain_cursor_reset(struct callchain_cursor *cursor)
 	for (node = cursor->first; node != NULL; node = node->next)
 		map__zput(node->ms.map);
 }
+
+void callchain_param_setup(u64 sample_type)
+{
+	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain) {
+		if ((sample_type & PERF_SAMPLE_REGS_USER) &&
+		    (sample_type & PERF_SAMPLE_STACK_USER)) {
+			callchain_param.record_mode = CALLCHAIN_DWARF;
+			dwarf_callchain_users = true;
+		} else if (sample_type & PERF_SAMPLE_BRANCH_STACK)
+			callchain_param.record_mode = CALLCHAIN_LBR;
+		else
+			callchain_param.record_mode = CALLCHAIN_FP;
+	}
+}
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 8f668ee29f25..fe36a9e5ccd1 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -297,4 +297,5 @@ int callchain_branch_counts(struct callchain_root *root,
 			    u64 *branch_count, u64 *predicted_count,
 			    u64 *abort_count, u64 *cycles_count);
 
+void callchain_param_setup(u64 sample_type);
 #endif	/* __PERF_CALLCHAIN_H */
-- 
2.25.4

[PATCH 5/5] perf script: Enable IP fields for callchains

[Jiri Olsa]

In case the callchains were deleted in pipe mode,
we need to ensure that the IP fields are enabled,
otherwise the callchain is not displayed.

Enabling IP and SYM, which should be enough for
callchains.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/builtin-script.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 5c4a580c048a..ecc8bd4c5e57 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2118,8 +2118,6 @@ static int process_attr(struct perf_tool *tool, union perf_event *event,
 			return 0;
 	}
 
-	set_print_ip_opts(&evsel->core.attr);
-
 	if (evsel->core.attr.sample_type) {
 		err = perf_evsel__check_attr(evsel, scr->session);
 		if (err)
@@ -2132,6 +2130,13 @@ static int process_attr(struct perf_tool *tool, union perf_event *event,
 	 */
 	sample_type = perf_evlist__combined_sample_type(evlist);
 	callchain_param_setup(sample_type);
+
+	/* Enable fields for callchain entries, if it got enabled. */
+	if (callchain_param.record_mode != CALLCHAIN_NONE) {
+		output[output_type(evsel->core.attr.type)].fields |= PERF_OUTPUT_IP |
+								     PERF_OUTPUT_SYM;
+	}
+	set_print_ip_opts(&evsel->core.attr);
 	return 0;
 }
 
-- 
2.25.4

Re: [PATCH 0/5] perf tools: Pipe fixes

[Arnaldo Carvalho de Melo]

Em Thu, May 07, 2020 at 11:50:19AM +0200, Jiri Olsa escreveu:
> hi,
> sending changes that allows callchain detection in pipe mode,
> fixing followign use case:
> 
>   # perf record --no-buffering --call-graph dwarf -e sdt_rtld:init_start -a -o - | perf --no-pager script -i -
> 
> plus change that allows to read pipe data from file:
> 
>   # perf record -o - sleep 1 > /tmp/perf.pipe.data
>   # perf report -i /tmp/perf.pipe.data
> 
> plus unrelated build fix.
> 
> Also reachable in here:
>   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>   perf/pipe

Thanks a lot, all tested, great improvements for pipe mode!

- Arnaldo
 
> thanks,
> jirka
> 
> 
> ---
> Jiri Olsa (5):
>       perf tools: Do not display extra info when there is nothing to build
>       perf tools: Do not seek in pipe fd during tracing data processing
>       perf session: Try to read pipe data from file
>       perf tools: Setup callchain properly in pipe mode
>       perf script: Enable IP fields for callchains
> 
>  tools/perf/Makefile.perf    |  8 +++++---
>  tools/perf/builtin-report.c | 33 ++++++++++++++++++++++-----------
>  tools/perf/builtin-script.c | 23 +++++++++++++++++++----
>  tools/perf/util/callchain.c | 14 ++++++++++++++
>  tools/perf/util/callchain.h |  1 +
>  tools/perf/util/header.c    | 34 ++++++++++++++++++++++++++--------
>  tools/perf/util/session.c   |  9 +++++++--
>  7 files changed, 94 insertions(+), 28 deletions(-)
> 

-- 

- Arnaldo