[PATCH 0/7] perf tools: Add switch-output size and time threshold options

[PATCH 0/7] perf tools: Add switch-output size and time threshold options

[Jiri Olsa]

hi,
adding a way to configure switch data output
for size and time, like:

  $ sudo perf record -e 'sched:*' --switch-output=10M -avg
  callchain: type FP
  switch-output with 10M size threshold
  mmap size 528384B
  [ perf record: dump data: Woken up 37 times ]
  [ perf record: Dump perf.data.2017010309135512 ]
  [ perf record: dump data: Woken up 39 times ]
  [ perf record: Dump perf.data.2017010309135771 ]
  [ perf record: dump data: Woken up 38 times ]
  [ perf record: Dump perf.data.2017010309140005 ]
  ^C[ perf record: Woken up 16 times to write data ]
  [ perf record: Dump perf.data.2017010309140111 ]
  [ perf record: Captured and wrote 4.748 MB perf.data.<timestamp> ]
  ...

the default for switch-output option stays
and does the SIGUSR2 output switch

Also available in:
  git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  perf/fixes

thanks,
jirka


Cc: Wang Nan <wangnan0@huawei.com>
---
Jiri Olsa (7):
      tools lib subcmd: Add OPT_STRING_OPTARG_SET option
      perf record: Make __record_options static
      perf record: Fix --switch-output documentation and comment
      perf record: Add struct switch_output
      perf record: Change switch-output option to take optional argument
      perf record: Add switch-output size option argument
      perf record: Add switch-output time option argument

 tools/lib/subcmd/parse-options.c         |   3 ++
 tools/lib/subcmd/parse-options.h         |   5 ++++
 tools/perf/Documentation/perf-record.txt |  13 +++++++--
 tools/perf/builtin-record.c              | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
 4 files changed, 139 insertions(+), 16 deletions(-)

[PATCH 1/7] tools lib subcmd: Add OPT_STRING_OPTARG_SET option

[Jiri Olsa]

To allow string options with default argument and
variable set when the option is used.

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: Josh Poimboeuf <jpoimboe@redhat.com>
Link: http://lkml.kernel.org/n/tip-xqtj0dy6t4bzah3jqi20tz43@git.kernel.org
---
 tools/lib/subcmd/parse-options.c | 3 +++
 tools/lib/subcmd/parse-options.h | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index 3284bb14ae78..8aad81151d50 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -213,6 +213,9 @@ static int get_value(struct parse_opt_ctx_t *p,
 		else
 			err = get_arg(p, opt, flags, (const char **)opt->value);
 
+		if (opt->set)
+			*(bool *)opt->set = true;
+
 		/* PARSE_OPT_NOEMPTY: Allow NULL but disallow empty string. */
 		if (opt->flags & PARSE_OPT_NOEMPTY) {
 			const char *val = *(const char **)opt->value;
diff --git a/tools/lib/subcmd/parse-options.h b/tools/lib/subcmd/parse-options.h
index 8866ac438b34..11c3be3bcce7 100644
--- a/tools/lib/subcmd/parse-options.h
+++ b/tools/lib/subcmd/parse-options.h
@@ -137,6 +137,11 @@ struct option {
 	{ .type = OPTION_STRING,  .short_name = (s), .long_name = (l), \
 	  .value = check_vtype(v, const char **), (a), .help = (h), \
 	  .flags = PARSE_OPT_OPTARG, .defval = (intptr_t)(d) }
+#define OPT_STRING_OPTARG_SET(s, l, v, os, a, h, d) \
+	{ .type = OPTION_STRING, .short_name = (s), .long_name = (l), \
+	  .value = check_vtype(v, const char **), (a), .help = (h), \
+	  .flags = PARSE_OPT_OPTARG, .defval = (intptr_t)(d), \
+	  .set = check_vtype(os, bool *)}
 #define OPT_STRING_NOEMPTY(s, l, v, a, h)   { .type = OPTION_STRING,  .short_name = (s), .long_name = (l), .value = check_vtype(v, const char **), (a), .help = (h), .flags = PARSE_OPT_NOEMPTY}
 #define OPT_DATE(s, l, v, h) \
 	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb }
-- 
2.7.4

[tip:perf/urgent] tools lib subcmd: Add OPT_STRING_OPTARG_SET option

[tip-bot for Jiri Olsa]

Commit-ID:  b66fb1da5a8cac3f5c3cdbe41937c91efc4e76a4
Gitweb:     http://git.kernel.org/tip/b66fb1da5a8cac3f5c3cdbe41937c91efc4e76a4
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 3 Jan 2017 09:19:54 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 3 Jan 2017 11:10:38 -0300

tools lib subcmd: Add OPT_STRING_OPTARG_SET option

To allow string options with a default argument and variable set when
the option is used.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Wang Nan <wangnan0@huawei.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1483431600-19887-2-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/lib/subcmd/parse-options.c | 3 +++
 tools/lib/subcmd/parse-options.h | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index 3284bb1..8aad811 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -213,6 +213,9 @@ static int get_value(struct parse_opt_ctx_t *p,
 		else
 			err = get_arg(p, opt, flags, (const char **)opt->value);
 
+		if (opt->set)
+			*(bool *)opt->set = true;
+
 		/* PARSE_OPT_NOEMPTY: Allow NULL but disallow empty string. */
 		if (opt->flags & PARSE_OPT_NOEMPTY) {
 			const char *val = *(const char **)opt->value;
diff --git a/tools/lib/subcmd/parse-options.h b/tools/lib/subcmd/parse-options.h
index 8866ac4..11c3be3 100644
--- a/tools/lib/subcmd/parse-options.h
+++ b/tools/lib/subcmd/parse-options.h
@@ -137,6 +137,11 @@ struct option {
 	{ .type = OPTION_STRING,  .short_name = (s), .long_name = (l), \
 	  .value = check_vtype(v, const char **), (a), .help = (h), \
 	  .flags = PARSE_OPT_OPTARG, .defval = (intptr_t)(d) }
+#define OPT_STRING_OPTARG_SET(s, l, v, os, a, h, d) \
+	{ .type = OPTION_STRING, .short_name = (s), .long_name = (l), \
+	  .value = check_vtype(v, const char **), (a), .help = (h), \
+	  .flags = PARSE_OPT_OPTARG, .defval = (intptr_t)(d), \
+	  .set = check_vtype(os, bool *)}
 #define OPT_STRING_NOEMPTY(s, l, v, a, h)   { .type = OPTION_STRING,  .short_name = (s), .long_name = (l), .value = check_vtype(v, const char **), (a), .help = (h), .flags = PARSE_OPT_NOEMPTY}
 #define OPT_DATE(s, l, v, h) \
 	{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb }

[PATCH 2/7] perf record: Make __record_options static

[Jiri Olsa]

There's no need for this one to be global.

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>
Link: http://lkml.kernel.org/n/tip-wbfnc3deti65q8ntw30ljzh8@git.kernel.org
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 74d6a035133a..31cf0ce12a65 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1405,7 +1405,7 @@ static bool dry_run;
  * perf_evlist__prepare_workload, etc instead of fork+exec'in 'perf record',
  * using pipes, etc.
  */
-struct option __record_options[] = {
+static struct option __record_options[] = {
 	OPT_CALLBACK('e', "event", &record.evlist, "event",
 		     "event selector. use 'perf list' to list available events",
 		     parse_events_option),
-- 
2.7.4

[tip:perf/urgent] perf record: Make __record_options static

[tip-bot for Jiri Olsa]

Commit-ID:  efd21307119d5a23ac83ae8fd5a39a5c7aeb8493
Gitweb:     http://git.kernel.org/tip/efd21307119d5a23ac83ae8fd5a39a5c7aeb8493
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 3 Jan 2017 09:19:55 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 3 Jan 2017 11:11:10 -0300

perf record: Make __record_options static

There's no need for this one to be global.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Wang Nan <wangnan0@huawei.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1483431600-19887-3-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 74d6a03..31cf0ce 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1405,7 +1405,7 @@ static bool dry_run;
  * perf_evlist__prepare_workload, etc instead of fork+exec'in 'perf record',
  * using pipes, etc.
  */
-struct option __record_options[] = {
+static struct option __record_options[] = {
 	OPT_CALLBACK('e', "event", &record.evlist, "event",
 		     "event selector. use 'perf list' to list available events",
 		     parse_events_option),

[PATCH 3/7] perf record: Fix --switch-output documentation and comment

[Jiri Olsa]

There's no --signal-trigger option, also adding
the code comment into record man page.

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-wbfnc3deti65q8ntw30ljzh8@git.kernel.org
---
 tools/perf/Documentation/perf-record.txt | 4 ++++
 tools/perf/builtin-record.c              | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27fc3617c6a4..5054d9147f0f 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -430,6 +430,10 @@ that gets then processed, possibly via a perf script, to decide if that
 particular perf.data snapshot should be kept or not.
 
 Implies --timestamp-filename, --no-buildid and --no-buildid-cache.
+The reason for the latter two is to reduce the data file switching
+overhead. You can still switch them on with:
+
+  --switch-output --no-no-buildid  --no-no-buildid-cache
 
 --dry-run::
 Parse options then exit. --dry-run can be used to detect errors in cmdline
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 31cf0ce12a65..4ec10e9427d9 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1636,7 +1636,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		 * overhead. Still generate buildid if they are required
 		 * explicitly using
 		 *
-		 *  perf record --signal-trigger --no-no-buildid \
+		 *  perf record --switch-output --no-no-buildid \
 		 *              --no-no-buildid-cache
 		 *
 		 * Following code equals to:
-- 
2.7.4

[tip:perf/urgent] perf record: Fix --switch-output documentation and comment

[tip-bot for Jiri Olsa]

Commit-ID:  60437ac02f398e0ee0927748d4798dd5534ac90d
Gitweb:     http://git.kernel.org/tip/60437ac02f398e0ee0927748d4798dd5534ac90d
Author:     Jiri Olsa <jolsa@kernel.org>
AuthorDate: Tue, 3 Jan 2017 09:19:56 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 3 Jan 2017 11:11:38 -0300

perf record: Fix --switch-output documentation and comment

There's no --signal-trigger option, also adding the code comment into
record man page.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Tested-by: Wang Nan <wangnan0@huawei.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1483431600-19887-4-git-send-email-jolsa@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt | 4 ++++
 tools/perf/builtin-record.c              | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 27fc361..5054d91 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -430,6 +430,10 @@ that gets then processed, possibly via a perf script, to decide if that
 particular perf.data snapshot should be kept or not.
 
 Implies --timestamp-filename, --no-buildid and --no-buildid-cache.
+The reason for the latter two is to reduce the data file switching
+overhead. You can still switch them on with:
+
+  --switch-output --no-no-buildid  --no-no-buildid-cache
 
 --dry-run::
 Parse options then exit. --dry-run can be used to detect errors in cmdline
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 31cf0ce..4ec10e9 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1636,7 +1636,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		 * overhead. Still generate buildid if they are required
 		 * explicitly using
 		 *
-		 *  perf record --signal-trigger --no-no-buildid \
+		 *  perf record --switch-output --no-no-buildid \
 		 *              --no-no-buildid-cache
 		 *
 		 * Following code equals to:

[PATCH 4/7] perf record: Add struct switch_output

[Jiri Olsa]

Next patches will add more --switch-output option arguments,
so preparing the data holder.

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-tibmeq1qw3kgrfzrswt5xypx@git.kernel.org
---
 tools/perf/builtin-record.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4ec10e9427d9..f7e805b30527 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -46,6 +46,10 @@
 #include <asm/bug.h>
 #include <linux/time64.h>
 
+struct switch_output {
+	bool		 signal;
+};
+
 struct record {
 	struct perf_tool	tool;
 	struct record_opts	opts;
@@ -62,7 +66,7 @@ struct record {
 	bool			no_buildid_cache_set;
 	bool			buildid_all;
 	bool			timestamp_filename;
-	bool			switch_output;
+	struct switch_output	switch_output;
 	unsigned long long	samples;
 };
 
@@ -842,11 +846,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) {
+	if (rec->opts.auxtrace_snapshot_mode || rec->switch_output.signal) {
 		signal(SIGUSR2, snapshot_sig_handler);
 		if (rec->opts.auxtrace_snapshot_mode)
 			trigger_on(&auxtrace_snapshot_trigger);
-		if (rec->switch_output)
+		if (rec->switch_output.signal)
 			trigger_on(&switch_output_trigger);
 	} else {
 		signal(SIGUSR2, SIG_IGN);
@@ -1519,7 +1523,7 @@ static struct option __record_options[] = {
 		    "Record build-id of all DSOs regardless of hits"),
 	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
 		    "append timestamp to output filename"),
-	OPT_BOOLEAN(0, "switch-output", &record.switch_output,
+	OPT_BOOLEAN(0, "switch-output", &record.switch_output.signal,
 		    "Switch output when receive SIGUSR2"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "Parse options then exit"),
@@ -1578,7 +1582,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		return -EINVAL;
 	}
 
-	if (rec->switch_output)
+	if (rec->switch_output.signal)
 		rec->timestamp_filename = true;
 
 	if (!rec->itr) {
@@ -1629,7 +1633,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) {
+	} else if (rec->switch_output.signal) {
 		/*
 		 * In 'perf record --switch-output', disable buildid
 		 * generation by default to reduce data file switching
-- 
2.7.4

[PATCH 5/7] perf record: Change switch-output option to take optional argument

[Jiri Olsa]

Next patches will add --switch-output option arguments,
changing the option to allow that and adding its default
value to 'signal'.

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-d3qy0tnc4m2dyt8yd6x5phyv@git.kernel.org
---
 tools/perf/builtin-record.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f7e805b30527..b503e5ebc1e7 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -48,6 +48,8 @@
 
 struct switch_output {
 	bool		 signal;
+	const char	*str;
+	bool		 set;
 };
 
 struct record {
@@ -1356,6 +1358,17 @@ static int record__parse_mmap_pages(const struct option *opt,
 	return ret;
 }
 
+static int switch_output_setup(struct switch_output *s)
+{
+	if (!strcmp(s->str, "signal")) {
+		s->signal = true;
+		pr_debug("switch-output with SIGUSR2 signal\n");
+		return 0;
+	}
+
+	return -1;
+}
+
 static const char * const __record_usage[] = {
 	"perf record [<options>] [<command>]",
 	"perf record [<options>] -- <command> [<options>]",
@@ -1523,8 +1536,9 @@ static struct option __record_options[] = {
 		    "Record build-id of all DSOs regardless of hits"),
 	OPT_BOOLEAN(0, "timestamp-filename", &record.timestamp_filename,
 		    "append timestamp to output filename"),
-	OPT_BOOLEAN(0, "switch-output", &record.switch_output.signal,
-		    "Switch output when receive SIGUSR2"),
+	OPT_STRING_OPTARG_SET(0, "switch-output", &record.switch_output.str,
+			  &record.switch_output.set, "signal",
+			  "Switch output when receive SIGUSR2", "signal"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "Parse options then exit"),
 	OPT_END()
@@ -1582,6 +1596,12 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 		return -EINVAL;
 	}
 
+	if (record.switch_output.set &&
+	    switch_output_setup(&rec->switch_output)) {
+		parse_options_usage(record_usage, record_options, "switch-output", 0);
+		return -EINVAL;
+	}
+
 	if (rec->switch_output.signal)
 		rec->timestamp_filename = true;
 
-- 
2.7.4

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

[Jiri Olsa]

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

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

[Arnaldo Carvalho de Melo]

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

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

[Arnaldo Carvalho de Melo]

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

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

[Jiri Olsa]

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

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

[Arnaldo Carvalho de Melo]

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

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

[David Ahern]

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?

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

[Arnaldo Carvalho de Melo]

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

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

[David Ahern]

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?

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

[Jiri Olsa]

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

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

[Jiri Olsa]

It's now possible to specify the threshold time for
perf.data like:

  $ perf record --switch-output=30s ...

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

  $ perf record --switch-output=30s ...
  [ perf record: dump data: Woken up 44 times ]
  [ perf record: Dump perf.data.2017010213043746 ]
  ...

The time is expected to be a number with appended unit
character - s/m/h/d.

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-2u6y3h89c0guejpn1nwv32lc@git.kernel.org
---
 tools/perf/Documentation/perf-record.txt |  2 ++
 tools/perf/builtin-record.c              | 44 ++++++++++++++++++++++++++++++--
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index d838354df417..176f85e89eaa 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -427,6 +427,8 @@ 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
+  <time>   - when reaching the time threshold, size is expected to
+             be a number with appended unit character - s/m/h/d
 
 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 42321a0fa80e..972272efba47 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -50,6 +50,7 @@ struct switch_output {
 	bool		 enabled;
 	bool		 signal;
 	unsigned long	 size;
+	unsigned long	 time;
 	const char	*str;
 	bool		 set;
 };
@@ -91,6 +92,12 @@ static bool switch_output_size(struct record *rec)
 	       (rec->bytes_written >= rec->switch_output.size);
 }
 
+static bool switch_output_time(struct record *rec)
+{
+	return rec->switch_output.time &&
+	       trigger_is_ready(&switch_output_trigger);
+}
+
 static int record__write(struct record *rec, void *bf, size_t size)
 {
 	if (perf_data_file__write(rec->session->file, bf, size) < 0) {
@@ -737,6 +744,7 @@ static void workload_exec_failed_signal(int signo __maybe_unused,
 }
 
 static void snapshot_sig_handler(int sig);
+static void alarm_sig_handler(int sig);
 
 int __weak
 perf_event__synth_time_conv(const struct perf_event_mmap_page *pc __maybe_unused,
@@ -1068,6 +1076,10 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 				err = fd;
 				goto out_child;
 			}
+
+			/* re-arm the alarm */
+			if (rec->switch_output.time)
+				alarm(rec->switch_output.time);
 		}
 
 		if (hits == rec->samples) {
@@ -1387,6 +1399,13 @@ static int switch_output_setup(struct switch_output *s)
 		{ .tag  = 'G', .mult = 1 << 30 },
 		{ .tag  = 0 },
 	};
+	static struct parse_tag tags_time[] = {
+		{ .tag  = 's', .mult = 1        },
+		{ .tag  = 'm', .mult = 60       },
+		{ .tag  = 'h', .mult = 60*60    },
+		{ .tag  = 'd', .mult = 60*60*24 },
+		{ .tag  = 0 },
+	};
 
 	if (!strcmp(s->str, "signal")) {
 		s->signal = true;
@@ -1401,6 +1420,14 @@ static int switch_output_setup(struct switch_output *s)
 		goto enabled;
 	}
 
+	val = parse_tag_value(s->str, tags_time);
+	if (val != (unsigned long) -1) {
+		s->time = val;
+		pr_debug("switch-output with %s time threshold (%lu seconds)\n",
+			 s->str, s->time);
+		goto enabled;
+	}
+
 	return -1;
 
 enabled:
@@ -1576,8 +1603,8 @@ 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,size",
-			  "Switch output when receive SIGUSR2 or cross size threshold",
+			  &record.switch_output.set, "signal,size,time",
+			  "Switch output when receive SIGUSR2 or cross size,time threshold",
 			  "signal"),
 	OPT_BOOLEAN(0, "dry-run", &dry_run,
 		    "Parse options then exit"),
@@ -1645,6 +1672,11 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (rec->switch_output.enabled)
 		rec->timestamp_filename = true;
 
+	if (rec->switch_output.time) {
+		signal(SIGALRM, alarm_sig_handler);
+		alarm(rec->switch_output.time);
+	}
+
 	if (!rec->itr) {
 		rec->itr = auxtrace_record__init(rec->evlist, &err);
 		if (err)
@@ -1797,3 +1829,11 @@ static void snapshot_sig_handler(int sig __maybe_unused)
 	if (switch_output_signal(rec))
 		trigger_hit(&switch_output_trigger);
 }
+
+static void alarm_sig_handler(int sig __maybe_unused)
+{
+	struct record *rec = &record;
+
+	if (switch_output_time(rec))
+		trigger_hit(&switch_output_trigger);
+}
-- 
2.7.4

Re: [PATCH 0/7] perf tools: Add switch-output size and time threshold options

[Wangnan (F)]

On 2017/1/3 16:19, Jiri Olsa wrote:
> hi,
> adding a way to configure switch data output
> for size and time, like:
>
>    $ sudo perf record -e 'sched:*' --switch-output=10M -avg
>    callchain: type FP
>    switch-output with 10M size threshold
>    mmap size 528384B
>    [ perf record: dump data: Woken up 37 times ]
>    [ perf record: Dump perf.data.2017010309135512 ]
>    [ perf record: dump data: Woken up 39 times ]
>    [ perf record: Dump perf.data.2017010309135771 ]
>    [ perf record: dump data: Woken up 38 times ]
>    [ perf record: Dump perf.data.2017010309140005 ]
>    ^C[ perf record: Woken up 16 times to write data ]
>    [ perf record: Dump perf.data.2017010309140111 ]
>    [ perf record: Captured and wrote 4.748 MB perf.data.<timestamp> ]
>    ...
>
> the default for switch-output option stays
> and does the SIGUSR2 output switch
>
> Also available in:
>    git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
>    perf/fixes
>
> thanks,
> jirka
>
>
> Cc: Wang Nan <wangnan0@huawei.com>
> ---

Good functions, and thank you for fixing documentations.

You didn't cc patch 1 and 2 to me.

Tested-by: Wang Nan <wangnan0@huawei.com>


> Jiri Olsa (7):
>        tools lib subcmd: Add OPT_STRING_OPTARG_SET option
>        perf record: Make __record_options static
>        perf record: Fix --switch-output documentation and comment
>        perf record: Add struct switch_output
>        perf record: Change switch-output option to take optional argument
>        perf record: Add switch-output size option argument
>        perf record: Add switch-output time option argument
>
>   tools/lib/subcmd/parse-options.c         |   3 ++
>   tools/lib/subcmd/parse-options.h         |   5 ++++
>   tools/perf/Documentation/perf-record.txt |  13 +++++++--
>   tools/perf/builtin-record.c              | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
>   4 files changed, 139 insertions(+), 16 deletions(-)

Re: [PATCH 0/7] perf tools: Add switch-output size and time threshold options

[Jiri Olsa]

On Tue, Jan 03, 2017 at 05:51:46PM +0800, Wangnan (F) wrote:
> 
> 
> On 2017/1/3 16:19, Jiri Olsa wrote:
> > hi,
> > adding a way to configure switch data output
> > for size and time, like:
> > 
> >    $ sudo perf record -e 'sched:*' --switch-output=10M -avg
> >    callchain: type FP
> >    switch-output with 10M size threshold
> >    mmap size 528384B
> >    [ perf record: dump data: Woken up 37 times ]
> >    [ perf record: Dump perf.data.2017010309135512 ]
> >    [ perf record: dump data: Woken up 39 times ]
> >    [ perf record: Dump perf.data.2017010309135771 ]
> >    [ perf record: dump data: Woken up 38 times ]
> >    [ perf record: Dump perf.data.2017010309140005 ]
> >    ^C[ perf record: Woken up 16 times to write data ]
> >    [ perf record: Dump perf.data.2017010309140111 ]
> >    [ perf record: Captured and wrote 4.748 MB perf.data.<timestamp> ]
> >    ...
> > 
> > the default for switch-output option stays
> > and does the SIGUSR2 output switch
> > 
> > Also available in:
> >    git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >    perf/fixes
> > 
> > thanks,
> > jirka
> > 
> > 
> > Cc: Wang Nan <wangnan0@huawei.com>
> > ---
> 
> Good functions, and thank you for fixing documentations.
> 
> You didn't cc patch 1 and 2 to me.

sry, will do next time..  thanks ;-)

jirka

Re: [PATCH 0/7] perf tools: Add switch-output size and time threshold options

[Arnaldo Carvalho de Melo]

Em Tue, Jan 03, 2017 at 05:51:46PM +0800, Wangnan (F) escreveu:
> On 2017/1/3 16:19, Jiri Olsa wrote:
> > Cc: Wang Nan <wangnan0@huawei.com>
> > ---
> 
> Good functions, and thank you for fixing documentations.
> 
> You didn't cc patch 1 and 2 to me.

> Tested-by: Wang Nan <wangnan0@huawei.com>

I'm tentatively switching this to an Acked-by for the latest version
from Jiri, that should have no major changes, right?

- Arnaldo

Re: [PATCH 0/7] perf tools: Add switch-output size and time threshold options

[Jiri Olsa]

On Tue, Jan 10, 2017 at 10:35:57AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Jan 03, 2017 at 05:51:46PM +0800, Wangnan (F) escreveu:
> > On 2017/1/3 16:19, Jiri Olsa wrote:
> > > Cc: Wang Nan <wangnan0@huawei.com>
> > > ---
> > 
> > Good functions, and thank you for fixing documentations.
> > 
> > You didn't cc patch 1 and 2 to me.
> 
> > Tested-by: Wang Nan <wangnan0@huawei.com>
> 
> I'm tentatively switching this to an Acked-by for the latest version
> from Jiri, that should have no major changes, right?

well, not major.. but some for sure ;-)

jirka