All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] perf record: Add an option to take an AUX snapshot on exit
@ 2019-08-06 14:41 Alexander Shishkin
  2019-08-07  7:40 ` Adrian Hunter
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Alexander Shishkin @ 2019-08-06 14:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Alexander Shishkin,
	Adrian Hunter

It is sometimes useful to generate a snapshot when perf record exits;
I've been using a wrapper script around the workload that would do a
killall -USR2 perf when the workload exits.

This patch makes it easier and also works when perf record is attached
to a pre-existing task. A new snapshot option 'e' can be specified in
-S to enable this behavior:

root@elsewhere:~# perf record -e intel_pt// -Se sleep 1
[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.085 MB perf.data ]

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Co-developed-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt | 11 +++++---
 tools/perf/builtin-record.c              | 34 +++++++++++++++++++++---
 tools/perf/perf.h                        |  1 +
 tools/perf/util/auxtrace.c               | 14 ++++++++--
 tools/perf/util/auxtrace.h               |  2 +-
 5 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 15e0fa87241b..d5e58e0a2bca 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -422,9 +422,14 @@ CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
 -S::
 --snapshot::
 Select AUX area tracing Snapshot Mode. This option is valid only with an
-AUX area tracing event. Optionally the number of bytes to capture per
-snapshot can be specified. In Snapshot Mode, trace data is captured only when
-signal SIGUSR2 is received.
+AUX area tracing event. Optionally, certain snapshot capturing parameters
+can be specified in a string that follows this option:
+  'e': take one last snapshot on exit; guarantees that there is at least one
+       snapshot in the output file;
+  <size>: if the PMU supports this, specify the desired snapshot size.
+
+In Snapshot Mode trace data is captured only when signal SIGUSR2 is received
+and on exit if the above 'e' option is given.
 
 --proc-map-timeout::
 When processing pre-existing threads /proc/XXX/mmap, it may take a long time,
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d31d7a5a1be3..e9a2525ecfcc 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -613,19 +613,35 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
 	return rc;
 }
 
-static void record__read_auxtrace_snapshot(struct record *rec)
+static void record__read_auxtrace_snapshot(struct record *rec, bool on_exit)
 {
 	pr_debug("Recording AUX area tracing snapshot\n");
 	if (record__auxtrace_read_snapshot_all(rec) < 0) {
 		trigger_error(&auxtrace_snapshot_trigger);
 	} else {
-		if (auxtrace_record__snapshot_finish(rec->itr))
+		if (auxtrace_record__snapshot_finish(rec->itr, on_exit))
 			trigger_error(&auxtrace_snapshot_trigger);
 		else
 			trigger_ready(&auxtrace_snapshot_trigger);
 	}
 }
 
+static int record__auxtrace_snapshot_exit(struct record *rec)
+{
+	if (trigger_is_error(&auxtrace_snapshot_trigger))
+		return 0;
+
+	if (!auxtrace_record__snapshot_started &&
+	    auxtrace_record__snapshot_start(rec->itr))
+		return -1;
+
+	record__read_auxtrace_snapshot(rec, true);
+	if (trigger_is_error(&auxtrace_snapshot_trigger))
+		return -1;
+
+	return 0;
+}
+
 static int record__auxtrace_init(struct record *rec)
 {
 	int err;
@@ -654,7 +670,7 @@ int record__auxtrace_mmap_read(struct record *rec __maybe_unused,
 }
 
 static inline
-void record__read_auxtrace_snapshot(struct record *rec __maybe_unused)
+void record__read_auxtrace_snapshot(struct record *rec __maybe_unused, bool on_exit)
 {
 }
 
@@ -664,6 +680,12 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
 	return 0;
 }
 
+static inline
+int record__auxtrace_snapshot_exit(struct record *rec)
+{
+	return 0;
+}
+
 static int record__auxtrace_init(struct record *rec __maybe_unused)
 {
 	return 0;
@@ -1536,7 +1558,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		if (auxtrace_record__snapshot_started) {
 			auxtrace_record__snapshot_started = 0;
 			if (!trigger_is_error(&auxtrace_snapshot_trigger))
-				record__read_auxtrace_snapshot(rec);
+				record__read_auxtrace_snapshot(rec, false);
 			if (trigger_is_error(&auxtrace_snapshot_trigger)) {
 				pr_err("AUX area tracing snapshot failed\n");
 				err = -1;
@@ -1609,9 +1631,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 			disabled = true;
 		}
 	}
+
 	trigger_off(&auxtrace_snapshot_trigger);
 	trigger_off(&switch_output_trigger);
 
+	if (opts->auxtrace_snapshot_on_exit)
+		record__auxtrace_snapshot_exit(rec);
+
 	if (forks && workload_exec_errno) {
 		char msg[STRERR_BUFSIZE];
 		const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 74d0124d38f3..dc0a7a237887 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -57,6 +57,7 @@ struct record_opts {
 	bool	     running_time;
 	bool	     full_auxtrace;
 	bool	     auxtrace_snapshot_mode;
+	bool	     auxtrace_snapshot_on_exit;
 	bool	     record_namespaces;
 	bool	     record_switch_events;
 	bool	     all_kernel;
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 65728cdeefb6..72ce4c5e7c78 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -539,9 +539,9 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr)
 	return 0;
 }
 
-int auxtrace_record__snapshot_finish(struct auxtrace_record *itr)
+int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit)
 {
-	if (itr && itr->snapshot_finish)
+	if (!on_exit && itr && itr->snapshot_finish)
 		return itr->snapshot_finish(itr);
 	return 0;
 }
@@ -577,6 +577,16 @@ int auxtrace_parse_snapshot_options(struct auxtrace_record *itr,
 	if (!str)
 		return 0;
 
+	/* PMU-agnostic options */
+	switch (*str) {
+	case 'e':
+		opts->auxtrace_snapshot_on_exit = true;
+		str++;
+		break;
+	default:
+		break;
+	}
+
 	if (itr)
 		return itr->parse_snapshot_options(itr, opts, str);
 
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 17eb04a1da4d..8ccabacd0b11 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -499,7 +499,7 @@ int auxtrace_record__info_fill(struct auxtrace_record *itr,
 			       size_t priv_size);
 void auxtrace_record__free(struct auxtrace_record *itr);
 int auxtrace_record__snapshot_start(struct auxtrace_record *itr);
-int auxtrace_record__snapshot_finish(struct auxtrace_record *itr);
+int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit);
 int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx,
 				   struct auxtrace_mmap *mm,
 				   unsigned char *data, u64 *head, u64 *old);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] perf record: Add an option to take an AUX snapshot on exit
  2019-08-06 14:41 [PATCH v1] perf record: Add an option to take an AUX snapshot on exit Alexander Shishkin
@ 2019-08-07  7:40 ` Adrian Hunter
  2019-08-07 13:03   ` Arnaldo Carvalho de Melo
  2019-08-13 15:33 ` Arnaldo Carvalho de Melo
  2019-08-15  9:29 ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2 siblings, 1 reply; 5+ messages in thread
From: Adrian Hunter @ 2019-08-07  7:40 UTC (permalink / raw)
  To: Alexander Shishkin, Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel

On 6/08/19 5:41 PM, Alexander Shishkin wrote:
> It is sometimes useful to generate a snapshot when perf record exits;
> I've been using a wrapper script around the workload that would do a
> killall -USR2 perf when the workload exits.
> 
> This patch makes it easier and also works when perf record is attached
> to a pre-existing task. A new snapshot option 'e' can be specified in
> -S to enable this behavior:
> 
> root@elsewhere:~# perf record -e intel_pt// -Se sleep 1
> [ perf record: Woken up 2 times to write data ]
> [ perf record: Captured and wrote 0.085 MB perf.data ]
> 
> Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Co-developed-by: Adrian Hunter <adrian.hunter@intel.com>

checkpatch says:
WARNING: Co-developed-by: must be immediately followed by Signed-off-by

> ---
>  tools/perf/Documentation/perf-record.txt | 11 +++++---
>  tools/perf/builtin-record.c              | 34 +++++++++++++++++++++---
>  tools/perf/perf.h                        |  1 +
>  tools/perf/util/auxtrace.c               | 14 ++++++++--
>  tools/perf/util/auxtrace.h               |  2 +-
>  5 files changed, 52 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 15e0fa87241b..d5e58e0a2bca 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -422,9 +422,14 @@ CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
>  -S::
>  --snapshot::
>  Select AUX area tracing Snapshot Mode. This option is valid only with an
> -AUX area tracing event. Optionally the number of bytes to capture per
> -snapshot can be specified. In Snapshot Mode, trace data is captured only when
> -signal SIGUSR2 is received.
> +AUX area tracing event. Optionally, certain snapshot capturing parameters
> +can be specified in a string that follows this option:
> +  'e': take one last snapshot on exit; guarantees that there is at least one
> +       snapshot in the output file;
> +  <size>: if the PMU supports this, specify the desired snapshot size.
> +
> +In Snapshot Mode trace data is captured only when signal SIGUSR2 is received
> +and on exit if the above 'e' option is given.
>  
>  --proc-map-timeout::
>  When processing pre-existing threads /proc/XXX/mmap, it may take a long time,
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index d31d7a5a1be3..e9a2525ecfcc 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -613,19 +613,35 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
>  	return rc;
>  }
>  
> -static void record__read_auxtrace_snapshot(struct record *rec)
> +static void record__read_auxtrace_snapshot(struct record *rec, bool on_exit)
>  {
>  	pr_debug("Recording AUX area tracing snapshot\n");
>  	if (record__auxtrace_read_snapshot_all(rec) < 0) {
>  		trigger_error(&auxtrace_snapshot_trigger);
>  	} else {
> -		if (auxtrace_record__snapshot_finish(rec->itr))
> +		if (auxtrace_record__snapshot_finish(rec->itr, on_exit))
>  			trigger_error(&auxtrace_snapshot_trigger);
>  		else
>  			trigger_ready(&auxtrace_snapshot_trigger);
>  	}
>  }
>  
> +static int record__auxtrace_snapshot_exit(struct record *rec)
> +{
> +	if (trigger_is_error(&auxtrace_snapshot_trigger))
> +		return 0;
> +
> +	if (!auxtrace_record__snapshot_started &&
> +	    auxtrace_record__snapshot_start(rec->itr))
> +		return -1;
> +
> +	record__read_auxtrace_snapshot(rec, true);

Buffers can get un-mapped earlier as tasks exit.  Refer
perf_evlist__filter_pollfd() -> perf_evlist__munmap_filtered().
Maybe we should prevent that for this case.
e.g. do perf_mmap__get()'s on the mmaps at the start, and then 'put' them
all here.

> +	if (trigger_is_error(&auxtrace_snapshot_trigger))
> +		return -1;
> +
> +	return 0;
> +}
> +
>  static int record__auxtrace_init(struct record *rec)
>  {
>  	int err;
> @@ -654,7 +670,7 @@ int record__auxtrace_mmap_read(struct record *rec __maybe_unused,
>  }
>  
>  static inline
> -void record__read_auxtrace_snapshot(struct record *rec __maybe_unused)
> +void record__read_auxtrace_snapshot(struct record *rec __maybe_unused, bool on_exit)
>  {
>  }
>  
> @@ -664,6 +680,12 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
>  	return 0;
>  }
>  
> +static inline
> +int record__auxtrace_snapshot_exit(struct record *rec)
> +{
> +	return 0;
> +}
> +
>  static int record__auxtrace_init(struct record *rec __maybe_unused)
>  {
>  	return 0;
> @@ -1536,7 +1558,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  		if (auxtrace_record__snapshot_started) {
>  			auxtrace_record__snapshot_started = 0;
>  			if (!trigger_is_error(&auxtrace_snapshot_trigger))
> -				record__read_auxtrace_snapshot(rec);
> +				record__read_auxtrace_snapshot(rec, false);
>  			if (trigger_is_error(&auxtrace_snapshot_trigger)) {
>  				pr_err("AUX area tracing snapshot failed\n");
>  				err = -1;
> @@ -1609,9 +1631,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  			disabled = true;
>  		}
>  	}
> +
>  	trigger_off(&auxtrace_snapshot_trigger);
>  	trigger_off(&switch_output_trigger);
>  
> +	if (opts->auxtrace_snapshot_on_exit)
> +		record__auxtrace_snapshot_exit(rec);
> +
>  	if (forks && workload_exec_errno) {
>  		char msg[STRERR_BUFSIZE];
>  		const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 74d0124d38f3..dc0a7a237887 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -57,6 +57,7 @@ struct record_opts {
>  	bool	     running_time;
>  	bool	     full_auxtrace;
>  	bool	     auxtrace_snapshot_mode;
> +	bool	     auxtrace_snapshot_on_exit;
>  	bool	     record_namespaces;
>  	bool	     record_switch_events;
>  	bool	     all_kernel;
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index 65728cdeefb6..72ce4c5e7c78 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -539,9 +539,9 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr)
>  	return 0;
>  }
>  
> -int auxtrace_record__snapshot_finish(struct auxtrace_record *itr)
> +int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit)
>  {
> -	if (itr && itr->snapshot_finish)
> +	if (!on_exit && itr && itr->snapshot_finish)

You are assuming you know what every ->snapshot_finish() does.  It would be
better to pass on_exit to ->snapshot_finish().

>  		return itr->snapshot_finish(itr);
>  	return 0;
>  }
> @@ -577,6 +577,16 @@ int auxtrace_parse_snapshot_options(struct auxtrace_record *itr,
>  	if (!str)
>  		return 0;
>  
> +	/* PMU-agnostic options */
> +	switch (*str) {
> +	case 'e':
> +		opts->auxtrace_snapshot_on_exit = true;
> +		str++;
> +		break;
> +	default:
> +		break;
> +	}
> +
>  	if (itr)
>  		return itr->parse_snapshot_options(itr, opts, str);
>  
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 17eb04a1da4d..8ccabacd0b11 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -499,7 +499,7 @@ int auxtrace_record__info_fill(struct auxtrace_record *itr,
>  			       size_t priv_size);
>  void auxtrace_record__free(struct auxtrace_record *itr);
>  int auxtrace_record__snapshot_start(struct auxtrace_record *itr);
> -int auxtrace_record__snapshot_finish(struct auxtrace_record *itr);
> +int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit);
>  int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx,
>  				   struct auxtrace_mmap *mm,
>  				   unsigned char *data, u64 *head, u64 *old);
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] perf record: Add an option to take an AUX snapshot on exit
  2019-08-07  7:40 ` Adrian Hunter
@ 2019-08-07 13:03   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-07 13:03 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Alexander Shishkin, Peter Zijlstra, Ingo Molnar, linux-kernel

Em Wed, Aug 07, 2019 at 10:40:39AM +0300, Adrian Hunter escreveu:
> On 6/08/19 5:41 PM, Alexander Shishkin wrote:
> > It is sometimes useful to generate a snapshot when perf record exits;
> > I've been using a wrapper script around the workload that would do a
> > killall -USR2 perf when the workload exits.
> > 
> > This patch makes it easier and also works when perf record is attached
> > to a pre-existing task. A new snapshot option 'e' can be specified in
> > -S to enable this behavior:
> > 
> > root@elsewhere:~# perf record -e intel_pt// -Se sleep 1
> > [ perf record: Woken up 2 times to write data ]
> > [ perf record: Captured and wrote 0.085 MB perf.data ]
> > 
> > Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Co-developed-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> checkpatch says:
> WARNING: Co-developed-by: must be immediately followed by Signed-off-by

I can fix this.

- Arnaldo
 
> > ---
> >  tools/perf/Documentation/perf-record.txt | 11 +++++---
> >  tools/perf/builtin-record.c              | 34 +++++++++++++++++++++---
> >  tools/perf/perf.h                        |  1 +
> >  tools/perf/util/auxtrace.c               | 14 ++++++++--
> >  tools/perf/util/auxtrace.h               |  2 +-
> >  5 files changed, 52 insertions(+), 10 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 15e0fa87241b..d5e58e0a2bca 100644
> > --- a/tools/perf/Documentation/perf-record.txt
> > +++ b/tools/perf/Documentation/perf-record.txt
> > @@ -422,9 +422,14 @@ CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
> >  -S::
> >  --snapshot::
> >  Select AUX area tracing Snapshot Mode. This option is valid only with an
> > -AUX area tracing event. Optionally the number of bytes to capture per
> > -snapshot can be specified. In Snapshot Mode, trace data is captured only when
> > -signal SIGUSR2 is received.
> > +AUX area tracing event. Optionally, certain snapshot capturing parameters
> > +can be specified in a string that follows this option:
> > +  'e': take one last snapshot on exit; guarantees that there is at least one
> > +       snapshot in the output file;
> > +  <size>: if the PMU supports this, specify the desired snapshot size.
> > +
> > +In Snapshot Mode trace data is captured only when signal SIGUSR2 is received
> > +and on exit if the above 'e' option is given.
> >  
> >  --proc-map-timeout::
> >  When processing pre-existing threads /proc/XXX/mmap, it may take a long time,
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index d31d7a5a1be3..e9a2525ecfcc 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -613,19 +613,35 @@ static int record__auxtrace_read_snapshot_all(struct record *rec)
> >  	return rc;
> >  }
> >  
> > -static void record__read_auxtrace_snapshot(struct record *rec)
> > +static void record__read_auxtrace_snapshot(struct record *rec, bool on_exit)
> >  {
> >  	pr_debug("Recording AUX area tracing snapshot\n");
> >  	if (record__auxtrace_read_snapshot_all(rec) < 0) {
> >  		trigger_error(&auxtrace_snapshot_trigger);
> >  	} else {
> > -		if (auxtrace_record__snapshot_finish(rec->itr))
> > +		if (auxtrace_record__snapshot_finish(rec->itr, on_exit))
> >  			trigger_error(&auxtrace_snapshot_trigger);
> >  		else
> >  			trigger_ready(&auxtrace_snapshot_trigger);
> >  	}
> >  }
> >  
> > +static int record__auxtrace_snapshot_exit(struct record *rec)
> > +{
> > +	if (trigger_is_error(&auxtrace_snapshot_trigger))
> > +		return 0;
> > +
> > +	if (!auxtrace_record__snapshot_started &&
> > +	    auxtrace_record__snapshot_start(rec->itr))
> > +		return -1;
> > +
> > +	record__read_auxtrace_snapshot(rec, true);
> 
> Buffers can get un-mapped earlier as tasks exit.  Refer
> perf_evlist__filter_pollfd() -> perf_evlist__munmap_filtered().
> Maybe we should prevent that for this case.
> e.g. do perf_mmap__get()'s on the mmaps at the start, and then 'put' them
> all here.
> 
> > +	if (trigger_is_error(&auxtrace_snapshot_trigger))
> > +		return -1;
> > +
> > +	return 0;
> > +}
> > +
> >  static int record__auxtrace_init(struct record *rec)
> >  {
> >  	int err;
> > @@ -654,7 +670,7 @@ int record__auxtrace_mmap_read(struct record *rec __maybe_unused,
> >  }
> >  
> >  static inline
> > -void record__read_auxtrace_snapshot(struct record *rec __maybe_unused)
> > +void record__read_auxtrace_snapshot(struct record *rec __maybe_unused, bool on_exit)
> >  {
> >  }
> >  
> > @@ -664,6 +680,12 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
> >  	return 0;
> >  }
> >  
> > +static inline
> > +int record__auxtrace_snapshot_exit(struct record *rec)
> > +{
> > +	return 0;
> > +}
> > +
> >  static int record__auxtrace_init(struct record *rec __maybe_unused)
> >  {
> >  	return 0;
> > @@ -1536,7 +1558,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >  		if (auxtrace_record__snapshot_started) {
> >  			auxtrace_record__snapshot_started = 0;
> >  			if (!trigger_is_error(&auxtrace_snapshot_trigger))
> > -				record__read_auxtrace_snapshot(rec);
> > +				record__read_auxtrace_snapshot(rec, false);
> >  			if (trigger_is_error(&auxtrace_snapshot_trigger)) {
> >  				pr_err("AUX area tracing snapshot failed\n");
> >  				err = -1;
> > @@ -1609,9 +1631,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >  			disabled = true;
> >  		}
> >  	}
> > +
> >  	trigger_off(&auxtrace_snapshot_trigger);
> >  	trigger_off(&switch_output_trigger);
> >  
> > +	if (opts->auxtrace_snapshot_on_exit)
> > +		record__auxtrace_snapshot_exit(rec);
> > +
> >  	if (forks && workload_exec_errno) {
> >  		char msg[STRERR_BUFSIZE];
> >  		const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));
> > diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> > index 74d0124d38f3..dc0a7a237887 100644
> > --- a/tools/perf/perf.h
> > +++ b/tools/perf/perf.h
> > @@ -57,6 +57,7 @@ struct record_opts {
> >  	bool	     running_time;
> >  	bool	     full_auxtrace;
> >  	bool	     auxtrace_snapshot_mode;
> > +	bool	     auxtrace_snapshot_on_exit;
> >  	bool	     record_namespaces;
> >  	bool	     record_switch_events;
> >  	bool	     all_kernel;
> > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> > index 65728cdeefb6..72ce4c5e7c78 100644
> > --- a/tools/perf/util/auxtrace.c
> > +++ b/tools/perf/util/auxtrace.c
> > @@ -539,9 +539,9 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr)
> >  	return 0;
> >  }
> >  
> > -int auxtrace_record__snapshot_finish(struct auxtrace_record *itr)
> > +int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit)
> >  {
> > -	if (itr && itr->snapshot_finish)
> > +	if (!on_exit && itr && itr->snapshot_finish)
> 
> You are assuming you know what every ->snapshot_finish() does.  It would be
> better to pass on_exit to ->snapshot_finish().
> 
> >  		return itr->snapshot_finish(itr);
> >  	return 0;
> >  }
> > @@ -577,6 +577,16 @@ int auxtrace_parse_snapshot_options(struct auxtrace_record *itr,
> >  	if (!str)
> >  		return 0;
> >  
> > +	/* PMU-agnostic options */
> > +	switch (*str) {
> > +	case 'e':
> > +		opts->auxtrace_snapshot_on_exit = true;
> > +		str++;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> >  	if (itr)
> >  		return itr->parse_snapshot_options(itr, opts, str);
> >  
> > diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> > index 17eb04a1da4d..8ccabacd0b11 100644
> > --- a/tools/perf/util/auxtrace.h
> > +++ b/tools/perf/util/auxtrace.h
> > @@ -499,7 +499,7 @@ int auxtrace_record__info_fill(struct auxtrace_record *itr,
> >  			       size_t priv_size);
> >  void auxtrace_record__free(struct auxtrace_record *itr);
> >  int auxtrace_record__snapshot_start(struct auxtrace_record *itr);
> > -int auxtrace_record__snapshot_finish(struct auxtrace_record *itr);
> > +int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit);
> >  int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx,
> >  				   struct auxtrace_mmap *mm,
> >  				   unsigned char *data, u64 *head, u64 *old);
> > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v1] perf record: Add an option to take an AUX snapshot on exit
  2019-08-06 14:41 [PATCH v1] perf record: Add an option to take an AUX snapshot on exit Alexander Shishkin
  2019-08-07  7:40 ` Adrian Hunter
@ 2019-08-13 15:33 ` Arnaldo Carvalho de Melo
  2019-08-15  9:29 ` [tip:perf/core] " tip-bot for Alexander Shishkin
  2 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-08-13 15:33 UTC (permalink / raw)
  To: Alexander Shishkin
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, Adrian Hunter, acme

Em Tue, Aug 06, 2019 at 05:41:01PM +0300, Alexander Shishkin escreveu:
> It is sometimes useful to generate a snapshot when perf record exits;
> I've been using a wrapper script around the workload that would do a
> killall -USR2 perf when the workload exits.
> 
> @@ -654,7 +670,7 @@ int record__auxtrace_mmap_read(struct record *rec __maybe_unused,
>  }
>  
>  static inline
> -void record__read_auxtrace_snapshot(struct record *rec __maybe_unused)
> +void record__read_auxtrace_snapshot(struct record *rec __maybe_unused, bool on_exit)

You forgot to add the __maybe_unused for the on_exit and later for the
'rec' in record__auxtrace_snapshot_exit, which causes the build to fail
when auxtrace isn't being built, I've fixed those.

- Arnaldo

>  {
>  }
>  
> @@ -664,6 +680,12 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
>  	return 0;
>  }
>  
> +static inline
> +int record__auxtrace_snapshot_exit(struct record *rec)
> +{
> +	return 0;
> +}
> +
>  static int record__auxtrace_init(struct record *rec __maybe_unused)
>  {
>  	return 0;
> @@ -1536,7 +1558,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [tip:perf/core] perf record: Add an option to take an AUX snapshot on exit
  2019-08-06 14:41 [PATCH v1] perf record: Add an option to take an AUX snapshot on exit Alexander Shishkin
  2019-08-07  7:40 ` Adrian Hunter
  2019-08-13 15:33 ` Arnaldo Carvalho de Melo
@ 2019-08-15  9:29 ` tip-bot for Alexander Shishkin
  2 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Alexander Shishkin @ 2019-08-15  9:29 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: hpa, tglx, acme, adrian.hunter, mingo, linux-kernel,
	alexander.shishkin, peterz

Commit-ID:  ce7b0e426ef359ee1d4a6126314ee3547a8eed87
Gitweb:     https://git.kernel.org/tip/ce7b0e426ef359ee1d4a6126314ee3547a8eed87
Author:     Alexander Shishkin <alexander.shishkin@linux.intel.com>
AuthorDate: Tue, 6 Aug 2019 17:41:01 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 14 Aug 2019 10:59:59 -0300

perf record: Add an option to take an AUX snapshot on exit

It is sometimes useful to generate a snapshot when perf record exits;
I've been using a wrapper script around the workload that would do a
killall -USR2 perf when the workload exits.

This patch makes it easier and also works when perf record is attached
to a pre-existing task. A new snapshot option 'e' can be specified in
-S to enable this behavior:

root@elsewhere:~# perf record -e intel_pt// -Se sleep 1
[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.085 MB perf.data ]

Co-developed-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20190806144101.62892-1-alexander.shishkin@linux.intel.com
[ Fixed up !HAVE_AUXTRACE_SUPPORT build in builtin-record.c, adding 2 missing __maybe_unused ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt | 11 +++++++---
 tools/perf/builtin-record.c              | 35 ++++++++++++++++++++++++++++----
 tools/perf/perf.h                        |  1 +
 tools/perf/util/auxtrace.c               | 14 +++++++++++--
 tools/perf/util/auxtrace.h               |  2 +-
 5 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 15e0fa87241b..d5e58e0a2bca 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -422,9 +422,14 @@ CLOCK_BOOTTIME, CLOCK_REALTIME and CLOCK_TAI.
 -S::
 --snapshot::
 Select AUX area tracing Snapshot Mode. This option is valid only with an
-AUX area tracing event. Optionally the number of bytes to capture per
-snapshot can be specified. In Snapshot Mode, trace data is captured only when
-signal SIGUSR2 is received.
+AUX area tracing event. Optionally, certain snapshot capturing parameters
+can be specified in a string that follows this option:
+  'e': take one last snapshot on exit; guarantees that there is at least one
+       snapshot in the output file;
+  <size>: if the PMU supports this, specify the desired snapshot size.
+
+In Snapshot Mode trace data is captured only when signal SIGUSR2 is received
+and on exit if the above 'e' option is given.
 
 --proc-map-timeout::
 When processing pre-existing threads /proc/XXX/mmap, it may take a long time,
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d31d7a5a1be3..f71631f2bcb5 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -613,19 +613,35 @@ out:
 	return rc;
 }
 
-static void record__read_auxtrace_snapshot(struct record *rec)
+static void record__read_auxtrace_snapshot(struct record *rec, bool on_exit)
 {
 	pr_debug("Recording AUX area tracing snapshot\n");
 	if (record__auxtrace_read_snapshot_all(rec) < 0) {
 		trigger_error(&auxtrace_snapshot_trigger);
 	} else {
-		if (auxtrace_record__snapshot_finish(rec->itr))
+		if (auxtrace_record__snapshot_finish(rec->itr, on_exit))
 			trigger_error(&auxtrace_snapshot_trigger);
 		else
 			trigger_ready(&auxtrace_snapshot_trigger);
 	}
 }
 
+static int record__auxtrace_snapshot_exit(struct record *rec)
+{
+	if (trigger_is_error(&auxtrace_snapshot_trigger))
+		return 0;
+
+	if (!auxtrace_record__snapshot_started &&
+	    auxtrace_record__snapshot_start(rec->itr))
+		return -1;
+
+	record__read_auxtrace_snapshot(rec, true);
+	if (trigger_is_error(&auxtrace_snapshot_trigger))
+		return -1;
+
+	return 0;
+}
+
 static int record__auxtrace_init(struct record *rec)
 {
 	int err;
@@ -654,7 +670,8 @@ int record__auxtrace_mmap_read(struct record *rec __maybe_unused,
 }
 
 static inline
-void record__read_auxtrace_snapshot(struct record *rec __maybe_unused)
+void record__read_auxtrace_snapshot(struct record *rec __maybe_unused,
+				    bool on_exit __maybe_unused)
 {
 }
 
@@ -664,6 +681,12 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr __maybe_unused)
 	return 0;
 }
 
+static inline
+int record__auxtrace_snapshot_exit(struct record *rec __maybe_unused)
+{
+	return 0;
+}
+
 static int record__auxtrace_init(struct record *rec __maybe_unused)
 {
 	return 0;
@@ -1536,7 +1559,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		if (auxtrace_record__snapshot_started) {
 			auxtrace_record__snapshot_started = 0;
 			if (!trigger_is_error(&auxtrace_snapshot_trigger))
-				record__read_auxtrace_snapshot(rec);
+				record__read_auxtrace_snapshot(rec, false);
 			if (trigger_is_error(&auxtrace_snapshot_trigger)) {
 				pr_err("AUX area tracing snapshot failed\n");
 				err = -1;
@@ -1609,9 +1632,13 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 			disabled = true;
 		}
 	}
+
 	trigger_off(&auxtrace_snapshot_trigger);
 	trigger_off(&switch_output_trigger);
 
+	if (opts->auxtrace_snapshot_on_exit)
+		record__auxtrace_snapshot_exit(rec);
+
 	if (forks && workload_exec_errno) {
 		char msg[STRERR_BUFSIZE];
 		const char *emsg = str_error_r(workload_exec_errno, msg, sizeof(msg));
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 74d0124d38f3..dc0a7a237887 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -57,6 +57,7 @@ struct record_opts {
 	bool	     running_time;
 	bool	     full_auxtrace;
 	bool	     auxtrace_snapshot_mode;
+	bool	     auxtrace_snapshot_on_exit;
 	bool	     record_namespaces;
 	bool	     record_switch_events;
 	bool	     all_kernel;
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 65728cdeefb6..72ce4c5e7c78 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -539,9 +539,9 @@ int auxtrace_record__snapshot_start(struct auxtrace_record *itr)
 	return 0;
 }
 
-int auxtrace_record__snapshot_finish(struct auxtrace_record *itr)
+int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit)
 {
-	if (itr && itr->snapshot_finish)
+	if (!on_exit && itr && itr->snapshot_finish)
 		return itr->snapshot_finish(itr);
 	return 0;
 }
@@ -577,6 +577,16 @@ int auxtrace_parse_snapshot_options(struct auxtrace_record *itr,
 	if (!str)
 		return 0;
 
+	/* PMU-agnostic options */
+	switch (*str) {
+	case 'e':
+		opts->auxtrace_snapshot_on_exit = true;
+		str++;
+		break;
+	default:
+		break;
+	}
+
 	if (itr)
 		return itr->parse_snapshot_options(itr, opts, str);
 
diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
index 17eb04a1da4d..8ccabacd0b11 100644
--- a/tools/perf/util/auxtrace.h
+++ b/tools/perf/util/auxtrace.h
@@ -499,7 +499,7 @@ int auxtrace_record__info_fill(struct auxtrace_record *itr,
 			       size_t priv_size);
 void auxtrace_record__free(struct auxtrace_record *itr);
 int auxtrace_record__snapshot_start(struct auxtrace_record *itr);
-int auxtrace_record__snapshot_finish(struct auxtrace_record *itr);
+int auxtrace_record__snapshot_finish(struct auxtrace_record *itr, bool on_exit);
 int auxtrace_record__find_snapshot(struct auxtrace_record *itr, int idx,
 				   struct auxtrace_mmap *mm,
 				   unsigned char *data, u64 *head, u64 *old);

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-08-15  9:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-06 14:41 [PATCH v1] perf record: Add an option to take an AUX snapshot on exit Alexander Shishkin
2019-08-07  7:40 ` Adrian Hunter
2019-08-07 13:03   ` Arnaldo Carvalho de Melo
2019-08-13 15:33 ` Arnaldo Carvalho de Melo
2019-08-15  9:29 ` [tip:perf/core] " tip-bot for Alexander Shishkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.