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

Groups > linux.kernel > #1549600 > unrolled thread

[PATCH 6/7] perf record: Add switch-output size option argument

Started byJiri Olsa <jolsa@kernel.org>
First post2017-01-03 09:30 +0100
Last post2017-01-03 20:50 +0100
Articles 9 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 6/7] perf record: Add switch-output size option argument Jiri Olsa <jolsa@kernel.org> - 2017-01-03 09:30 +0100
    Re: [PATCH 6/7] perf record: Add switch-output size option argument Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-03 15:30 +0100
      Re: [PATCH 6/7] perf record: Add switch-output size option argument Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-03 15:30 +0100
      Re: [PATCH 6/7] perf record: Add switch-output size option argument Jiri Olsa <jolsa@redhat.com> - 2017-01-03 15:40 +0100
        Re: [PATCH 6/7] perf record: Add switch-output size option argument Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-03 16:00 +0100
    Re: [PATCH 6/7] perf record: Add switch-output size option argument David Ahern <dsahern@gmail.com> - 2017-01-03 16:40 +0100
      Re: [PATCH 6/7] perf record: Add switch-output size option argument Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-01-03 17:30 +0100
        Re: [PATCH 6/7] perf record: Add switch-output size option argument David Ahern <dsahern@gmail.com> - 2017-01-03 17:30 +0100
          Re: [PATCH 6/7] perf record: Add switch-output size option argument Jiri Olsa <jolsa@redhat.com> - 2017-01-03 20:50 +0100

#1549600 — [PATCH 6/7] perf record: Add switch-output size option argument

FromJiri Olsa <jolsa@kernel.org>
Date2017-01-03 09:30 +0100
Subject[PATCH 6/7] perf record: Add switch-output size option argument
Message-ID<sVsDM-3nL-23@gated-at.bofh.it>
It's now possible to specify the threshold size for
perf.data like:

  $ perf record --switch-output=2G ...

Once it's reached, the current data are dumped in to the
perf.data.<timestamp> file and session does on.

  $ perf record --switch-output=2G ...
  [ perf record: dump data: Woken up 7244 times ]
  [ perf record: Dump perf.data.2017010214093746 ]
  ...

The size is expected to be a number with appended unit
character - B/K/M/G.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-xw4x8qj6aojox378q9ley29e@git.kernel.org
---
 tools/perf/Documentation/perf-record.txt |  7 +++-
 tools/perf/builtin-record.c              | 66 ++++++++++++++++++++++++++------
 2 files changed, 59 insertions(+), 14 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 5054d9147f0f..d838354df417 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -421,9 +421,12 @@ Configure all used events to run in user space.
 --timestamp-filename
 Append timestamp to output file name.
 
---switch-output::
+--switch-output[=mode]::
 Generate multiple perf.data files, timestamp prefixed, switching to a new one
-when receiving a SIGUSR2.
+based on 'mode' value:
+  "signal" - when receiving a SIGUSR2 (default value) or
+  <size>   - when reaching the size threshold, size is expected to
+             be a number with appended unit character - B/K/M/G
 
 A possible use case is to, given an external event, slice the perf.data file
 that gets then processed, possibly via a perf script, to decide if that
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index b503e5ebc1e7..42321a0fa80e 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -47,7 +47,9 @@
 #include <linux/time64.h>
 
 struct switch_output {
+	bool		 enabled;
 	bool		 signal;
+	unsigned long	 size;
 	const char	*str;
 	bool		 set;
 };
@@ -72,6 +74,23 @@ struct record {
 	unsigned long long	samples;
 };
 
+static volatile int auxtrace_record__snapshot_started;
+static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
+static DEFINE_TRIGGER(switch_output_trigger);
+
+static bool switch_output_signal(struct record *rec)
+{
+	return rec->switch_output.signal &&
+	       trigger_is_ready(&switch_output_trigger);
+}
+
+static bool switch_output_size(struct record *rec)
+{
+	return rec->switch_output.size &&
+	       trigger_is_ready(&switch_output_trigger) &&
+	       (rec->bytes_written >= rec->switch_output.size);
+}
+
 static int record__write(struct record *rec, void *bf, size_t size)
 {
 	if (perf_data_file__write(rec->session->file, bf, size) < 0) {
@@ -80,6 +99,10 @@ static int record__write(struct record *rec, void *bf, size_t size)
 	}
 
 	rec->bytes_written += size;
+
+	if (switch_output_size(rec))
+		trigger_hit(&switch_output_trigger);
+
 	return 0;
 }
 
@@ -199,10 +222,6 @@ static volatile int done;
 static volatile int signr = -1;
 static volatile int child_finished;
 
-static volatile int auxtrace_record__snapshot_started;
-static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
-static DEFINE_TRIGGER(switch_output_trigger);
-
 static void sig_handler(int sig)
 {
 	if (sig == SIGCHLD)
@@ -848,11 +867,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	signal(SIGTERM, sig_handler);
 	signal(SIGSEGV, sigsegv_handler);
 
-	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.signal) {
+	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.enabled) {
 		signal(SIGUSR2, snapshot_sig_handler);
 		if (rec->opts.auxtrace_snapshot_mode)
 			trigger_on(&auxtrace_snapshot_trigger);
-		if (rec->switch_output.signal)
+		if (rec->switch_output.enabled)
 			trigger_on(&switch_output_trigger);
 	} else {
 		signal(SIGUSR2, SIG_IGN);
@@ -1360,13 +1379,33 @@ static int record__parse_mmap_pages(const struct option *opt,
 
 static int switch_output_setup(struct switch_output *s)
 {
+	unsigned long val;
+	static struct parse_tag tags_size[] = {
+		{ .tag  = 'B', .mult = 1       },
+		{ .tag  = 'K', .mult = 1 << 10 },
+		{ .tag  = 'M', .mult = 1 << 20 },
+		{ .tag  = 'G', .mult = 1 << 30 },
+		{ .tag  = 0 },
+	};
+
 	if (!strcmp(s->str, "signal")) {
 		s->signal = true;
 		pr_debug("switch-output with SIGUSR2 signal\n");
-		return 0;
+		goto enabled;
+	}
+
+	val = parse_tag_value(s->str, tags_size);
+	if (val != (unsigned long) -1) {
+		s->size = val;
+		pr_debug("switch-output with %s size threshold\n", s->str);
+		goto enabled;
 	}
 
 	return -1;
+
+enabled:
+	s->enabled = true;
+	return 0;
 }
 
 static const char * const __record_usage[] = {
@@ -1537,8 +1576,9 @@ static struct option __record_options[] = {
 	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
 		    "append timestamp to output filename"),
 	OPT_STRING_OPTARG_SET(0, "switch-output", &record.switch_output.str,
-			  &record.switch_output.set, "signal",
-			  "Switch output when receive SIGUSR2", "signal"),
+			  &record.switch_output.set, "signal,size",
+			  "Switch output when receive SIGUSR2 or cross size threshold",
+			  "signal"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "Parse options then exit"),
 	OPT_END()
@@ -1602,7 +1642,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		return -EINVAL;
 	}
 
-	if (rec->switch_output.signal)
+	if (rec->switch_output.enabled)
 		rec->timestamp_filename = true;
 
 	if (!rec->itr) {
@@ -1653,7 +1693,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	if (rec->no_buildid_cache || rec->no_buildid) {
 		disable_buildid_cache();
-	} else if (rec->switch_output.signal) {
+	} else if (rec->switch_output.enabled) {
 		/*
 		 * In 'perf record --switch-output', disable buildid
 		 * generation by default to reduce data file switching
@@ -1745,6 +1785,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 
 static void snapshot_sig_handler(int sig __maybe_unused)
 {
+	struct record *rec = &record;
+
 	if (trigger_is_ready(&auxtrace_snapshot_trigger)) {
 		trigger_hit(&auxtrace_snapshot_trigger);
 		auxtrace_record__snapshot_started = 1;
@@ -1752,6 +1794,6 @@ static void snapshot_sig_handler(int sig __maybe_unused)
 			trigger_error(&auxtrace_snapshot_trigger);
 	}
 
-	if (trigger_is_ready(&switch_output_trigger))
+	if (switch_output_signal(rec))
 		trigger_hit(&switch_output_trigger);
 }
-- 
2.7.4

[toc] | [next] | [standalone]

#1549805

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-01-03 15:30 +0100
Message-ID<sVyg9-7AS-1@gated-at.bofh.it>
In reply to#1549600
Em Tue, Jan 03, 2017 at 09:19:59AM +0100, Jiri Olsa escreveu:
> It's now possible to specify the threshold size for
> perf.data like:
> 
>   $ perf record --switch-output=2G ...
> 
> Once it's reached, the current data are dumped in to the
> perf.data.<timestamp> file and session does on.

  s/does/goes/g

But:

[root@jouet ~]# perf record -F9000 -a --switch-output=1K sleep 5
[ perf record: dump data: Woken up 0 times ]
[ perf record: Dump perf.data.2017010311185502 ]
[ perf record: dump data: Woken up 1 times ]
[ perf record: Dump perf.data.2017010311190003 ]
[ perf record: Woken up 0 times to write data ]
[ perf record: Dump perf.data.2017010311190020 ]
[ perf record: Captured and wrote 2.240 MB perf.data.<timestamp> ]
[root@jouet ~]# ls -larth perf.data.*
-rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311181984
-rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311182002
-rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311185502
-rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190003
-rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190020
[root@jouet ~]#

What is that I am missing?

- Arnaldo
 
>   $ perf record --switch-output=2G ...
>   [ perf record: dump data: Woken up 7244 times ]
>   [ perf record: Dump perf.data.2017010214093746 ]
>   ...
> 
> The size is expected to be a number with appended unit
> character - B/K/M/G.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Wang Nan <wangnan0@huawei.com>
> Link: http://lkml.kernel.org/n/tip-xw4x8qj6aojox378q9ley29e@git.kernel.org
> ---
>  tools/perf/Documentation/perf-record.txt |  7 +++-
>  tools/perf/builtin-record.c              | 66 ++++++++++++++++++++++++++------
>  2 files changed, 59 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 5054d9147f0f..d838354df417 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -421,9 +421,12 @@ Configure all used events to run in user space.
>  --timestamp-filename
>  Append timestamp to output file name.
>  
> ---switch-output::
> +--switch-output[=mode]::
>  Generate multiple perf.data files, timestamp prefixed, switching to a new one
> -when receiving a SIGUSR2.
> +based on 'mode' value:
> +  "signal" - when receiving a SIGUSR2 (default value) or
> +  <size>   - when reaching the size threshold, size is expected to
> +             be a number with appended unit character - B/K/M/G
>  
>  A possible use case is to, given an external event, slice the perf.data file
>  that gets then processed, possibly via a perf script, to decide if that
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index b503e5ebc1e7..42321a0fa80e 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -47,7 +47,9 @@
>  #include <linux/time64.h>
>  
>  struct switch_output {
> +	bool		 enabled;
>  	bool		 signal;
> +	unsigned long	 size;
>  	const char	*str;
>  	bool		 set;
>  };
> @@ -72,6 +74,23 @@ struct record {
>  	unsigned long long	samples;
>  };
>  
> +static volatile int auxtrace_record__snapshot_started;
> +static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
> +static DEFINE_TRIGGER(switch_output_trigger);
> +
> +static bool switch_output_signal(struct record *rec)
> +{
> +	return rec->switch_output.signal &&
> +	       trigger_is_ready(&switch_output_trigger);
> +}
> +
> +static bool switch_output_size(struct record *rec)
> +{
> +	return rec->switch_output.size &&
> +	       trigger_is_ready(&switch_output_trigger) &&
> +	       (rec->bytes_written >= rec->switch_output.size);
> +}
> +
>  static int record__write(struct record *rec, void *bf, size_t size)
>  {
>  	if (perf_data_file__write(rec->session->file, bf, size) < 0) {
> @@ -80,6 +99,10 @@ static int record__write(struct record *rec, void *bf, size_t size)
>  	}
>  
>  	rec->bytes_written += size;
> +
> +	if (switch_output_size(rec))
> +		trigger_hit(&switch_output_trigger);
> +
>  	return 0;
>  }
>  
> @@ -199,10 +222,6 @@ static volatile int done;
>  static volatile int signr = -1;
>  static volatile int child_finished;
>  
> -static volatile int auxtrace_record__snapshot_started;
> -static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
> -static DEFINE_TRIGGER(switch_output_trigger);
> -
>  static void sig_handler(int sig)
>  {
>  	if (sig == SIGCHLD)
> @@ -848,11 +867,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>  	signal(SIGTERM, sig_handler);
>  	signal(SIGSEGV, sigsegv_handler);
>  
> -	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.signal) {
> +	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.enabled) {
>  		signal(SIGUSR2, snapshot_sig_handler);
>  		if (rec->opts.auxtrace_snapshot_mode)
>  			trigger_on(&auxtrace_snapshot_trigger);
> -		if (rec->switch_output.signal)
> +		if (rec->switch_output.enabled)
>  			trigger_on(&switch_output_trigger);
>  	} else {
>  		signal(SIGUSR2, SIG_IGN);
> @@ -1360,13 +1379,33 @@ static int record__parse_mmap_pages(const struct option *opt,
>  
>  static int switch_output_setup(struct switch_output *s)
>  {
> +	unsigned long val;
> +	static struct parse_tag tags_size[] = {
> +		{ .tag  = 'B', .mult = 1       },
> +		{ .tag  = 'K', .mult = 1 << 10 },
> +		{ .tag  = 'M', .mult = 1 << 20 },
> +		{ .tag  = 'G', .mult = 1 << 30 },
> +		{ .tag  = 0 },
> +	};
> +
>  	if (!strcmp(s->str, "signal")) {
>  		s->signal = true;
>  		pr_debug("switch-output with SIGUSR2 signal\n");
> -		return 0;
> +		goto enabled;
> +	}
> +
> +	val = parse_tag_value(s->str, tags_size);
> +	if (val != (unsigned long) -1) {
> +		s->size = val;
> +		pr_debug("switch-output with %s size threshold\n", s->str);
> +		goto enabled;
>  	}
>  
>  	return -1;
> +
> +enabled:
> +	s->enabled = true;
> +	return 0;
>  }
>  
>  static const char * const __record_usage[] = {
> @@ -1537,8 +1576,9 @@ static struct option __record_options[] = {
>  	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
>  		    "append timestamp to output filename"),
>  	OPT_STRING_OPTARG_SET(0, "switch-output", &record.switch_output.str,
> -			  &record.switch_output.set, "signal",
> -			  "Switch output when receive SIGUSR2", "signal"),
> +			  &record.switch_output.set, "signal,size",
> +			  "Switch output when receive SIGUSR2 or cross size threshold",
> +			  "signal"),
>  	OPT_BOOLEAN(0, "dry-run", &dry_run,
>  		    "Parse options then exit"),
>  	OPT_END()
> @@ -1602,7 +1642,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  		return -EINVAL;
>  	}
>  
> -	if (rec->switch_output.signal)
> +	if (rec->switch_output.enabled)
>  		rec->timestamp_filename = true;
>  
>  	if (!rec->itr) {
> @@ -1653,7 +1693,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  
>  	if (rec->no_buildid_cache || rec->no_buildid) {
>  		disable_buildid_cache();
> -	} else if (rec->switch_output.signal) {
> +	} else if (rec->switch_output.enabled) {
>  		/*
>  		 * In 'perf record --switch-output', disable buildid
>  		 * generation by default to reduce data file switching
> @@ -1745,6 +1785,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
>  
>  static void snapshot_sig_handler(int sig __maybe_unused)
>  {
> +	struct record *rec = &record;
> +
>  	if (trigger_is_ready(&auxtrace_snapshot_trigger)) {
>  		trigger_hit(&auxtrace_snapshot_trigger);
>  		auxtrace_record__snapshot_started = 1;
> @@ -1752,6 +1794,6 @@ static void snapshot_sig_handler(int sig __maybe_unused)
>  			trigger_error(&auxtrace_snapshot_trigger);
>  	}
>  
> -	if (trigger_is_ready(&switch_output_trigger))
> +	if (switch_output_signal(rec))
>  		trigger_hit(&switch_output_trigger);
>  }
> -- 
> 2.7.4

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

#1549812

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-01-03 15:30 +0100
Message-ID<sVyga-7AS-29@gated-at.bofh.it>
In reply to#1549805
Em Tue, Jan 03, 2017 at 11:20:27AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jan 03, 2017 at 09:19:59AM +0100, Jiri Olsa escreveu:
> > It's now possible to specify the threshold size for
> > perf.data like:
> > 
> >   $ perf record --switch-output=2G ...
> > 
> > Once it's reached, the current data are dumped in to the
> > perf.data.<timestamp> file and session does on.

BTW, the first three patches are trivial and were merged, thanks,

- Arnaldo
 
>   s/does/goes/g
> 
> But:
> 
> [root@jouet ~]# perf record -F9000 -a --switch-output=1K sleep 5
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2017010311185502 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017010311190003 ]
> [ perf record: Woken up 0 times to write data ]
> [ perf record: Dump perf.data.2017010311190020 ]
> [ perf record: Captured and wrote 2.240 MB perf.data.<timestamp> ]
> [root@jouet ~]# ls -larth perf.data.*
> -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311181984
> -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311182002
> -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311185502
> -rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190003
> -rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190020
> [root@jouet ~]#
> 
> What is that I am missing?
> 
> - Arnaldo
>  
> >   $ perf record --switch-output=2G ...
> >   [ perf record: dump data: Woken up 7244 times ]
> >   [ perf record: Dump perf.data.2017010214093746 ]
> >   ...
> > 
> > The size is expected to be a number with appended unit
> > character - B/K/M/G.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > Cc: David Ahern <dsahern@gmail.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: Wang Nan <wangnan0@huawei.com>
> > Link: http://lkml.kernel.org/n/tip-xw4x8qj6aojox378q9ley29e@git.kernel.org
> > ---
> >  tools/perf/Documentation/perf-record.txt |  7 +++-
> >  tools/perf/builtin-record.c              | 66 ++++++++++++++++++++++++++------
> >  2 files changed, 59 insertions(+), 14 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> > index 5054d9147f0f..d838354df417 100644
> > --- a/tools/perf/Documentation/perf-record.txt
> > +++ b/tools/perf/Documentation/perf-record.txt
> > @@ -421,9 +421,12 @@ Configure all used events to run in user space.
> >  --timestamp-filename
> >  Append timestamp to output file name.
> >  
> > ---switch-output::
> > +--switch-output[=mode]::
> >  Generate multiple perf.data files, timestamp prefixed, switching to a new one
> > -when receiving a SIGUSR2.
> > +based on 'mode' value:
> > +  "signal" - when receiving a SIGUSR2 (default value) or
> > +  <size>   - when reaching the size threshold, size is expected to
> > +             be a number with appended unit character - B/K/M/G
> >  
> >  A possible use case is to, given an external event, slice the perf.data file
> >  that gets then processed, possibly via a perf script, to decide if that
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index b503e5ebc1e7..42321a0fa80e 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -47,7 +47,9 @@
> >  #include <linux/time64.h>
> >  
> >  struct switch_output {
> > +	bool		 enabled;
> >  	bool		 signal;
> > +	unsigned long	 size;
> >  	const char	*str;
> >  	bool		 set;
> >  };
> > @@ -72,6 +74,23 @@ struct record {
> >  	unsigned long long	samples;
> >  };
> >  
> > +static volatile int auxtrace_record__snapshot_started;
> > +static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
> > +static DEFINE_TRIGGER(switch_output_trigger);
> > +
> > +static bool switch_output_signal(struct record *rec)
> > +{
> > +	return rec->switch_output.signal &&
> > +	       trigger_is_ready(&switch_output_trigger);
> > +}
> > +
> > +static bool switch_output_size(struct record *rec)
> > +{
> > +	return rec->switch_output.size &&
> > +	       trigger_is_ready(&switch_output_trigger) &&
> > +	       (rec->bytes_written >= rec->switch_output.size);
> > +}
> > +
> >  static int record__write(struct record *rec, void *bf, size_t size)
> >  {
> >  	if (perf_data_file__write(rec->session->file, bf, size) < 0) {
> > @@ -80,6 +99,10 @@ static int record__write(struct record *rec, void *bf, size_t size)
> >  	}
> >  
> >  	rec->bytes_written += size;
> > +
> > +	if (switch_output_size(rec))
> > +		trigger_hit(&switch_output_trigger);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -199,10 +222,6 @@ static volatile int done;
> >  static volatile int signr = -1;
> >  static volatile int child_finished;
> >  
> > -static volatile int auxtrace_record__snapshot_started;
> > -static DEFINE_TRIGGER(auxtrace_snapshot_trigger);
> > -static DEFINE_TRIGGER(switch_output_trigger);
> > -
> >  static void sig_handler(int sig)
> >  {
> >  	if (sig == SIGCHLD)
> > @@ -848,11 +867,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >  	signal(SIGTERM, sig_handler);
> >  	signal(SIGSEGV, sigsegv_handler);
> >  
> > -	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.signal) {
> > +	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.enabled) {
> >  		signal(SIGUSR2, snapshot_sig_handler);
> >  		if (rec->opts.auxtrace_snapshot_mode)
> >  			trigger_on(&auxtrace_snapshot_trigger);
> > -		if (rec->switch_output.signal)
> > +		if (rec->switch_output.enabled)
> >  			trigger_on(&switch_output_trigger);
> >  	} else {
> >  		signal(SIGUSR2, SIG_IGN);
> > @@ -1360,13 +1379,33 @@ static int record__parse_mmap_pages(const struct option *opt,
> >  
> >  static int switch_output_setup(struct switch_output *s)
> >  {
> > +	unsigned long val;
> > +	static struct parse_tag tags_size[] = {
> > +		{ .tag  = 'B', .mult = 1       },
> > +		{ .tag  = 'K', .mult = 1 << 10 },
> > +		{ .tag  = 'M', .mult = 1 << 20 },
> > +		{ .tag  = 'G', .mult = 1 << 30 },
> > +		{ .tag  = 0 },
> > +	};
> > +
> >  	if (!strcmp(s->str, "signal")) {
> >  		s->signal = true;
> >  		pr_debug("switch-output with SIGUSR2 signal\n");
> > -		return 0;
> > +		goto enabled;
> > +	}
> > +
> > +	val = parse_tag_value(s->str, tags_size);
> > +	if (val != (unsigned long) -1) {
> > +		s->size = val;
> > +		pr_debug("switch-output with %s size threshold\n", s->str);
> > +		goto enabled;
> >  	}
> >  
> >  	return -1;
> > +
> > +enabled:
> > +	s->enabled = true;
> > +	return 0;
> >  }
> >  
> >  static const char * const __record_usage[] = {
> > @@ -1537,8 +1576,9 @@ static struct option __record_options[] = {
> >  	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
> >  		    "append timestamp to output filename"),
> >  	OPT_STRING_OPTARG_SET(0, "switch-output", &record.switch_output.str,
> > -			  &record.switch_output.set, "signal",
> > -			  "Switch output when receive SIGUSR2", "signal"),
> > +			  &record.switch_output.set, "signal,size",
> > +			  "Switch output when receive SIGUSR2 or cross size threshold",
> > +			  "signal"),
> >  	OPT_BOOLEAN(0, "dry-run", &dry_run,
> >  		    "Parse options then exit"),
> >  	OPT_END()
> > @@ -1602,7 +1642,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
> >  		return -EINVAL;
> >  	}
> >  
> > -	if (rec->switch_output.signal)
> > +	if (rec->switch_output.enabled)
> >  		rec->timestamp_filename = true;
> >  
> >  	if (!rec->itr) {
> > @@ -1653,7 +1693,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
> >  
> >  	if (rec->no_buildid_cache || rec->no_buildid) {
> >  		disable_buildid_cache();
> > -	} else if (rec->switch_output.signal) {
> > +	} else if (rec->switch_output.enabled) {
> >  		/*
> >  		 * In 'perf record --switch-output', disable buildid
> >  		 * generation by default to reduce data file switching
> > @@ -1745,6 +1785,8 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
> >  
> >  static void snapshot_sig_handler(int sig __maybe_unused)
> >  {
> > +	struct record *rec = &record;
> > +
> >  	if (trigger_is_ready(&auxtrace_snapshot_trigger)) {
> >  		trigger_hit(&auxtrace_snapshot_trigger);
> >  		auxtrace_record__snapshot_started = 1;
> > @@ -1752,6 +1794,6 @@ static void snapshot_sig_handler(int sig __maybe_unused)
> >  			trigger_error(&auxtrace_snapshot_trigger);
> >  	}
> >  
> > -	if (trigger_is_ready(&switch_output_trigger))
> > +	if (switch_output_signal(rec))
> >  		trigger_hit(&switch_output_trigger);
> >  }
> > -- 
> > 2.7.4

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

#1549814

FromJiri Olsa <jolsa@redhat.com>
Date2017-01-03 15:40 +0100
Message-ID<sVypP-7EJ-9@gated-at.bofh.it>
In reply to#1549805
On Tue, Jan 03, 2017 at 11:20:27AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 03, 2017 at 09:19:59AM +0100, Jiri Olsa escreveu:
> > It's now possible to specify the threshold size for
> > perf.data like:
> > 
> >   $ perf record --switch-output=2G ...
> > 
> > Once it's reached, the current data are dumped in to the
> > perf.data.<timestamp> file and session does on.
> 
>   s/does/goes/g
> 
> But:
> 
> [root@jouet ~]# perf record -F9000 -a --switch-output=1K sleep 5
> [ perf record: dump data: Woken up 0 times ]
> [ perf record: Dump perf.data.2017010311185502 ]
> [ perf record: dump data: Woken up 1 times ]
> [ perf record: Dump perf.data.2017010311190003 ]
> [ perf record: Woken up 0 times to write data ]
> [ perf record: Dump perf.data.2017010311190020 ]
> [ perf record: Captured and wrote 2.240 MB perf.data.<timestamp> ]
> [root@jouet ~]# ls -larth perf.data.*
> -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311181984
> -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311182002
> -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311185502
> -rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190003
> -rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190020
> [root@jouet ~]#
> 
> What is that I am missing?

hum, I think the size you configured is smaller then the size
of the shared kernel buffer and perf gets woken up by default
only when we cross some level of data that's in.. would need
to check

also I dont think 1K won't fit even the perf.data
header.. are you trying to break it? ;-)

jirka

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

#1549843

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-01-03 16:00 +0100
Message-ID<sVyJh-7Mp-11@gated-at.bofh.it>
In reply to#1549814
Em Tue, Jan 03, 2017 at 03:32:32PM +0100, Jiri Olsa escreveu:
> On Tue, Jan 03, 2017 at 11:20:27AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jan 03, 2017 at 09:19:59AM +0100, Jiri Olsa escreveu:
> > > It's now possible to specify the threshold size for
> > > perf.data like:
> > > 
> > >   $ perf record --switch-output=2G ...
> > > 
> > > Once it's reached, the current data are dumped in to the
> > > perf.data.<timestamp> file and session does on.
> > 
> >   s/does/goes/g
> > 
> > But:
> > 
> > [root@jouet ~]# perf record -F9000 -a --switch-output=1K sleep 5
> > [ perf record: dump data: Woken up 0 times ]
> > [ perf record: Dump perf.data.2017010311185502 ]
> > [ perf record: dump data: Woken up 1 times ]
> > [ perf record: Dump perf.data.2017010311190003 ]
> > [ perf record: Woken up 0 times to write data ]
> > [ perf record: Dump perf.data.2017010311190020 ]
> > [ perf record: Captured and wrote 2.240 MB perf.data.<timestamp> ]
> > [root@jouet ~]# ls -larth perf.data.*
> > -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311181984
> > -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311182002
> > -rw-------. 1 root root 2.3M Jan  3 11:18 perf.data.2017010311185502
> > -rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190003
> > -rw-------. 1 root root 2.3M Jan  3 11:19 perf.data.2017010311190020
> > [root@jouet ~]#
> > 
> > What is that I am missing?
> 
> hum, I think the size you configured is smaller then the size
> of the shared kernel buffer and perf gets woken up by default
> only when we cross some level of data that's in.. would need
> to check
> 
> also I dont think 1K won't fit even the perf.data
> header.. are you trying to break it? ;-)

I'm trying to follow the documentation ;-) Your explanation sounds
reasonable, could you get this code to check those constraints and emit
a sensible error message?

- Arnaldo

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

#1549888

FromDavid Ahern <dsahern@gmail.com>
Date2017-01-03 16:40 +0100
Message-ID<sVzlT-8kf-3@gated-at.bofh.it>
In reply to#1549600
On 1/3/17 1:19 AM, Jiri Olsa wrote:
> It's now possible to specify the threshold size for
> perf.data like:
> 
>   $ perf record --switch-output=2G ...
> 
> Once it's reached, the current data are dumped in to the
> perf.data.<timestamp> file and session does on.

How about something like max-file-size instead of switch-output?

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

#1549931

FromArnaldo Carvalho de Melo <acme@kernel.org>
Date2017-01-03 17:30 +0100
Message-ID<sVA8i-ua-9@gated-at.bofh.it>
In reply to#1549888
Em Tue, Jan 03, 2017 at 08:33:35AM -0700, David Ahern escreveu:
> On 1/3/17 1:19 AM, Jiri Olsa wrote:
> > It's now possible to specify the threshold size for
> > perf.data like:
> > 
> >   $ perf record --switch-output=2G ...
> > 
> > Once it's reached, the current data are dumped in to the
> > perf.data.<timestamp> file and session does on.
> 
> How about something like max-file-size instead of switch-output?

Well, I think he wants to use the "switch-output" semantic, which will
go on "slicing" the output into multiple files according to the
specified criteria, be it the existing signal one or a file size.

"max-file-size" looks like a hard limit, no hint about producing
multiple files.

- Arnaldo

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

#1549933

FromDavid Ahern <dsahern@gmail.com>
Date2017-01-03 17:30 +0100
Message-ID<sVA8i-ua-7@gated-at.bofh.it>
In reply to#1549931
On 1/3/17 9:12 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 03, 2017 at 08:33:35AM -0700, David Ahern escreveu:
>> On 1/3/17 1:19 AM, Jiri Olsa wrote:
>>> It's now possible to specify the threshold size for
>>> perf.data like:
>>>
>>>   $ perf record --switch-output=2G ...
>>>
>>> Once it's reached, the current data are dumped in to the
>>> perf.data.<timestamp> file and session does on.
>>
>> How about something like max-file-size instead of switch-output?
> 
> Well, I think he wants to use the "switch-output" semantic, which will
> go on "slicing" the output into multiple files according to the
> specified criteria, be it the existing signal one or a file size.
> 
> "max-file-size" looks like a hard limit, no hint about producing
> multiple files.

Sure, my point is that switch-output is an odd name for the option.

file-slice?

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

#1550132

FromJiri Olsa <jolsa@redhat.com>
Date2017-01-03 20:50 +0100
Message-ID<sVDfQ-2y2-35@gated-at.bofh.it>
In reply to#1549933
On Tue, Jan 03, 2017 at 09:23:18AM -0700, David Ahern wrote:
> On 1/3/17 9:12 AM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jan 03, 2017 at 08:33:35AM -0700, David Ahern escreveu:
> >> On 1/3/17 1:19 AM, Jiri Olsa wrote:
> >>> It's now possible to specify the threshold size for
> >>> perf.data like:
> >>>
> >>>   $ perf record --switch-output=2G ...
> >>>
> >>> Once it's reached, the current data are dumped in to the
> >>> perf.data.<timestamp> file and session does on.
> >>
> >> How about something like max-file-size instead of switch-output?
> > 
> > Well, I think he wants to use the "switch-output" semantic, which will
> > go on "slicing" the output into multiple files according to the
> > specified criteria, be it the existing signal one or a file size.

yea, I wanted to keep current option.. which I think
we will have to keep in any case

> > 
> > "max-file-size" looks like a hard limit, no hint about producing
> > multiple files.
> 
> Sure, my point is that switch-output is an odd name for the option.
> 
> file-slice?

I actually don't mind the current switch-output=signal/size/time,
because we change/switch the output file on various conditions:
  signal/size/time ;-)

jirka

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web