[PATCH 0/6] perf tools: Enable AUX area tracing snapshots using a FIFO

[PATCH 0/6] perf tools: Enable AUX area tracing snapshots using a FIFO

[Adrian Hunter]

Hi

Here are some patches to enable AUX area tracing snapshots using a FIFO.
Presently SIGUSR2 can be used but the advantage of the FIFO is that access
is governed by access to the FIFO.  Refer to the example in patch 5.

The first 4 patches are preparation, including patch 4 which enhances the
--control option to accept file names.

Patch 5 adds snapshot control command.

The final patch updates Intel PT documentation.


Adrian Hunter (6):
      perf tools: Consolidate --control option parsing into one function
      perf tools: Handle read errors from ctl_fd
      perf tools: Use AsciiDoc formatting for --control option documentation
      perf tools: Add FIFO file names as alternative options to --control
      perf record: Add 'snapshot' control command
      perf intel-pt: Document snapshot control command

 tools/perf/Documentation/perf-intel-pt.txt | 23 ++++++-
 tools/perf/Documentation/perf-record.txt   | 56 ++++++++---------
 tools/perf/Documentation/perf-stat.txt     | 48 ++++++++-------
 tools/perf/builtin-record.c                | 74 +++++++++++++----------
 tools/perf/builtin-stat.c                  | 35 +++++------
 tools/perf/util/evlist.c                   | 96 +++++++++++++++++++++++++++---
 tools/perf/util/evlist.h                   |  6 +-
 tools/perf/util/record.h                   |  1 +
 tools/perf/util/stat.h                     |  1 +
 9 files changed, 227 insertions(+), 113 deletions(-)


Regards
Adrian

[PATCH 1/6] perf tools: Consolidate --control option parsing into one function

[Adrian Hunter]

Consolidate --control option parsing into one function, in preparation for
adding FIFO file name options.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 22 ++--------------------
 tools/perf/builtin-stat.c   | 22 ++--------------------
 tools/perf/util/evlist.c    | 24 ++++++++++++++++++++++++
 tools/perf/util/evlist.h    |  1 +
 4 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f91352f847c0..f2ab5bd7e2ba 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2234,27 +2234,9 @@ static int parse_control_option(const struct option *opt,
 				const char *str,
 				int unset __maybe_unused)
 {
-	char *comma = NULL, *endptr = NULL;
-	struct record_opts *config = (struct record_opts *)opt->value;
-
-	if (strncmp(str, "fd:", 3))
-		return -EINVAL;
-
-	config->ctl_fd = strtoul(&str[3], &endptr, 0);
-	if (endptr == &str[3])
-		return -EINVAL;
-
-	comma = strchr(str, ',');
-	if (comma) {
-		if (endptr != comma)
-			return -EINVAL;
-
-		config->ctl_fd_ack = strtoul(comma + 1, &endptr, 0);
-		if (endptr == comma + 1 || *endptr != '\0')
-			return -EINVAL;
-	}
+	struct record_opts *opts = opt->value;
 
-	return 0;
+	return evlist__parse_control(str, &opts->ctl_fd, &opts->ctl_fd_ack);
 }
 
 static void switch_output_size_warn(struct record *rec)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 483a28ef4ec4..12ce5cf2b10e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1045,27 +1045,9 @@ static int parse_control_option(const struct option *opt,
 				const char *str,
 				int unset __maybe_unused)
 {
-	char *comma = NULL, *endptr = NULL;
-	struct perf_stat_config *config = (struct perf_stat_config *)opt->value;
+	struct perf_stat_config *config = opt->value;
 
-	if (strncmp(str, "fd:", 3))
-		return -EINVAL;
-
-	config->ctl_fd = strtoul(&str[3], &endptr, 0);
-	if (endptr == &str[3])
-		return -EINVAL;
-
-	comma = strchr(str, ',');
-	if (comma) {
-		if (endptr != comma)
-			return -EINVAL;
-
-		config->ctl_fd_ack = strtoul(comma + 1, &endptr, 0);
-		if (endptr == comma + 1 || *endptr != '\0')
-			return -EINVAL;
-	}
-
-	return 0;
+	return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack);
 }
 
 static struct option stat_options[] = {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e3fa3bf7498a..62e3f87547ce 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1727,6 +1727,30 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
 	return leader;
 }
 
+int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack)
+{
+	char *comma = NULL, *endptr = NULL;
+
+	if (strncmp(str, "fd:", 3))
+		return -EINVAL;
+
+	*ctl_fd = strtoul(&str[3], &endptr, 0);
+	if (endptr == &str[3])
+		return -EINVAL;
+
+	comma = strchr(str, ',');
+	if (comma) {
+		if (endptr != comma)
+			return -EINVAL;
+
+		*ctl_fd_ack = strtoul(comma + 1, &endptr, 0);
+		if (endptr == comma + 1 || *endptr != '\0')
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 int evlist__initialize_ctlfd(struct evlist *evlist, int fd, int ack)
 {
 	if (fd == -1) {
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index c73f7f7f120b..a5a5a07d5c55 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -373,6 +373,7 @@ enum evlist_ctl_cmd {
 	EVLIST_CTL_CMD_ACK
 };
 
+int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack);
 int evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
 int evlist__finalize_ctlfd(struct evlist *evlist);
 bool evlist__ctlfd_initialized(struct evlist *evlist);
-- 
2.17.1

[PATCH 2/6] perf tools: Handle read errors from ctl_fd

[Adrian Hunter]

Handle read errors from ctl_fd such as EINTR, EAGAIN and EWOULDBLOCK.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 62e3f87547ce..47d1045a19af 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1802,6 +1802,7 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
 	char c;
 	size_t bytes_read = 0;
 
+	*cmd = EVLIST_CTL_CMD_UNSUPPORTED;
 	memset(cmd_data, 0, data_size);
 	data_size--;
 
@@ -1813,17 +1814,22 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
 			cmd_data[bytes_read++] = c;
 			if (bytes_read == data_size)
 				break;
-		} else {
-			if (err == -1)
+			continue;
+		} else if (err == -1) {
+			if (errno == EINTR)
+				continue;
+			if (errno == EAGAIN || errno == EWOULDBLOCK)
+				err = 0;
+			else
 				pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd.fd);
-			break;
 		}
+		break;
 	} while (1);
 
 	pr_debug("Message from ctl_fd: \"%s%s\"\n", cmd_data,
 		 bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0");
 
-	if (err > 0) {
+	if (bytes_read > 0) {
 		if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG,
 			     (sizeof(EVLIST_CTL_CMD_ENABLE_TAG)-1))) {
 			*cmd = EVLIST_CTL_CMD_ENABLE;
@@ -1833,7 +1839,7 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
 		}
 	}
 
-	return err;
+	return bytes_read ? (int)bytes_read : err;
 }
 
 static int evlist__ctlfd_ack(struct evlist *evlist)
-- 
2.17.1

[PATCH 3/6] perf tools: Use AsciiDoc formatting for --control option documentation

[Adrian Hunter]

The --control option does not display well in man pages unless AsciiDoc
formatting is used.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt | 46 ++++++++++++------------
 tools/perf/Documentation/perf-stat.txt   | 46 ++++++++++++------------
 2 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 3f72d8e261f3..07c4734f1c7a 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -627,43 +627,43 @@ option. The -e option and this one can be mixed and matched.  Events
 can be grouped using the {} notation.
 endif::HAVE_LIBPFM[]
 
---control fd:ctl-fd[,ack-fd]
+--control=fd:ctl-fd[,ack-fd]::
 Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
 'disable': disable events). Measurements can be started with events disabled using
 --delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
 to synchronize with the controlling process. Example of bash shell script to enable and
 disable events during measurements:
 
-#!/bin/bash
+ #!/bin/bash
 
-ctl_dir=/tmp/
+ ctl_dir=/tmp/
 
-ctl_fifo=${ctl_dir}perf_ctl.fifo
-test -p ${ctl_fifo} && unlink ${ctl_fifo}
-mkfifo ${ctl_fifo}
-exec {ctl_fd}<>${ctl_fifo}
+ ctl_fifo=${ctl_dir}perf_ctl.fifo
+ test -p ${ctl_fifo} && unlink ${ctl_fifo}
+ mkfifo ${ctl_fifo}
+ exec {ctl_fd}<>${ctl_fifo}
 
-ctl_ack_fifo=${ctl_dir}perf_ctl_ack.fifo
-test -p ${ctl_ack_fifo} && unlink ${ctl_ack_fifo}
-mkfifo ${ctl_ack_fifo}
-exec {ctl_fd_ack}<>${ctl_ack_fifo}
+ ctl_ack_fifo=${ctl_dir}perf_ctl_ack.fifo
+ test -p ${ctl_ack_fifo} && unlink ${ctl_ack_fifo}
+ mkfifo ${ctl_ack_fifo}
+ exec {ctl_fd_ack}<>${ctl_ack_fifo}
 
-perf record -D -1 -e cpu-cycles -a               \
-            --control fd:${ctl_fd},${ctl_fd_ack} \
-            -- sleep 30 &
-perf_pid=$!
+ perf record -D -1 -e cpu-cycles -a               \
+             --control fd:${ctl_fd},${ctl_fd_ack} \
+             -- sleep 30 &
+ perf_pid=$!
 
-sleep 5  && echo 'enable' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
-sleep 10 && echo 'disable' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
+ sleep 5  && echo 'enable' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
+ sleep 10 && echo 'disable' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
 
-exec {ctl_fd_ack}>&-
-unlink ${ctl_ack_fifo}
+ exec {ctl_fd_ack}>&-
+ unlink ${ctl_ack_fifo}
 
-exec {ctl_fd}>&-
-unlink ${ctl_fifo}
+ exec {ctl_fd}>&-
+ unlink ${ctl_fifo}
 
-wait -n ${perf_pid}
-exit $?
+ wait -n ${perf_pid}
+ exit $?
 
 
 SEE ALSO
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index c9bfefc051fb..7fb7368cc2d9 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -176,43 +176,43 @@ with it.  --append may be used here.  Examples:
      3>results  perf stat --log-fd 3          -- $cmd
      3>>results perf stat --log-fd 3 --append -- $cmd
 
---control fd:ctl-fd[,ack-fd]
+--control=fd:ctl-fd[,ack-fd]::
 Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
 'disable': disable events). Measurements can be started with events disabled using
 --delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
 to synchronize with the controlling process. Example of bash shell script to enable and
 disable events during measurements:
 
-#!/bin/bash
+ #!/bin/bash
 
-ctl_dir=/tmp/
+ ctl_dir=/tmp/
 
-ctl_fifo=${ctl_dir}perf_ctl.fifo
-test -p ${ctl_fifo} && unlink ${ctl_fifo}
-mkfifo ${ctl_fifo}
-exec {ctl_fd}<>${ctl_fifo}
+ ctl_fifo=${ctl_dir}perf_ctl.fifo
+ test -p ${ctl_fifo} && unlink ${ctl_fifo}
+ mkfifo ${ctl_fifo}
+ exec {ctl_fd}<>${ctl_fifo}
 
-ctl_ack_fifo=${ctl_dir}perf_ctl_ack.fifo
-test -p ${ctl_ack_fifo} && unlink ${ctl_ack_fifo}
-mkfifo ${ctl_ack_fifo}
-exec {ctl_fd_ack}<>${ctl_ack_fifo}
+ ctl_ack_fifo=${ctl_dir}perf_ctl_ack.fifo
+ test -p ${ctl_ack_fifo} && unlink ${ctl_ack_fifo}
+ mkfifo ${ctl_ack_fifo}
+ exec {ctl_fd_ack}<>${ctl_ack_fifo}
 
-perf stat -D -1 -e cpu-cycles -a -I 1000       \
-          --control fd:${ctl_fd},${ctl_fd_ack} \
-          -- sleep 30 &
-perf_pid=$!
+ perf stat -D -1 -e cpu-cycles -a -I 1000       \
+           --control fd:${ctl_fd},${ctl_fd_ack} \
+           -- sleep 30 &
+ perf_pid=$!
 
-sleep 5  && echo 'enable' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
-sleep 10 && echo 'disable' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
+ sleep 5  && echo 'enable' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
+ sleep 10 && echo 'disable' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
 
-exec {ctl_fd_ack}>&-
-unlink ${ctl_ack_fifo}
+ exec {ctl_fd_ack}>&-
+ unlink ${ctl_ack_fifo}
 
-exec {ctl_fd}>&-
-unlink ${ctl_fifo}
+ exec {ctl_fd}>&-
+ unlink ${ctl_fifo}
 
-wait -n ${perf_pid}
-exit $?
+ wait -n ${perf_pid}
+ exit $?
 
 
 --pre::
-- 
2.17.1

[PATCH 4/6] perf tools: Add FIFO file names as alternative options to --control

[Adrian Hunter]

Enable the --control option to accept file names as an alternative to
file descriptors.

Example:

 $ mkfifo perf.control
 $ mkfifo perf.ack
 $ cat perf.ack &
 [1] 6808
 $ perf record --control perf.control,perf.ack -- sleep 300 &
 [2] 6810
 $ echo disable > perf.control
 $ Events disabled
 ack

 $ echo enable > perf.control
 $ Events enabled
 ack

 $ echo disable > perf.control
 $ Events disabled
 ack

 $ kill %2
 [ perf record: Woken up 4 times to write data ]
 $ [ perf record: Captured and wrote 0.018 MB perf.data (7 samples) ]

 [1]-  Done                    cat perf.ack
 [2]+  Terminated              perf record --control perf.control,perf.ack -- sleep 300
 $

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt |  2 +
 tools/perf/Documentation/perf-stat.txt   |  2 +
 tools/perf/builtin-record.c              | 34 ++++++++++++----
 tools/perf/builtin-stat.c                | 18 +++++++--
 tools/perf/util/evlist.c                 | 49 +++++++++++++++++++++++-
 tools/perf/util/evlist.h                 |  2 +-
 tools/perf/util/record.h                 |  1 +
 tools/perf/util/stat.h                   |  1 +
 8 files changed, 95 insertions(+), 14 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 07c4734f1c7a..74fa3e905bf1 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -627,7 +627,9 @@ option. The -e option and this one can be mixed and matched.  Events
 can be grouped using the {} notation.
 endif::HAVE_LIBPFM[]
 
+--control=ctl-fifo[,ack-fifo]::
 --control=fd:ctl-fd[,ack-fd]::
+ctl-fifo / ack-fifo are opened and used as ctl-fd / ack-fd as follows.
 Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
 'disable': disable events). Measurements can be started with events disabled using
 --delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 7fb7368cc2d9..bd7e52f09048 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -176,7 +176,9 @@ with it.  --append may be used here.  Examples:
      3>results  perf stat --log-fd 3          -- $cmd
      3>>results perf stat --log-fd 3 --append -- $cmd
 
+--control=ctl-fifo[,ack-fifo]::
 --control=fd:ctl-fd[,ack-fd]::
+ctl-fifo / ack-fifo are opened and used as ctl-fd / ack-fd as follows.
 Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
 'disable': disable events). Measurements can be started with events disabled using
 --delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index f2ab5bd7e2ba..af7238b1356f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2236,7 +2236,17 @@ static int parse_control_option(const struct option *opt,
 {
 	struct record_opts *opts = opt->value;
 
-	return evlist__parse_control(str, &opts->ctl_fd, &opts->ctl_fd_ack);
+	return evlist__parse_control(str, &opts->ctl_fd, &opts->ctl_fd_ack, &opts->ctl_fd_close);
+}
+
+static void close_control_option(struct record_opts *opts)
+{
+	if (opts->ctl_fd_close) {
+		opts->ctl_fd_close = false;
+		close(opts->ctl_fd);
+		if (opts->ctl_fd_ack >= 0)
+			close(opts->ctl_fd_ack);
+	}
 }
 
 static void switch_output_size_warn(struct record *rec)
@@ -2578,9 +2588,10 @@ static struct option __record_options[] = {
 		"libpfm4 event selector. use 'perf list' to list available events",
 		parse_libpfm_events_option),
 #endif
-	OPT_CALLBACK(0, "control", &record.opts, "fd:ctl-fd[,ack-fd]",
+	OPT_CALLBACK(0, "control", &record.opts, "fd:ctl-fd[,ack-fd] or ctl-fifo[,ack-fifo]",
 		     "Listen on ctl-fd descriptor for command to control measurement ('enable': enable events, 'disable': disable events).\n"
-		     "\t\t\t  Optionally send control command completion ('ack\\n') to ack-fd descriptor.",
+		     "\t\t\t  Optionally send control command completion ('ack\\n') to ack-fd descriptor.\n"
+		     "\t\t\t  Alternatively, ctl-fifo / ack-fifo will be opened and used as ctl-fd / ack-fd.",
 		      parse_control_option),
 	OPT_END()
 };
@@ -2653,12 +2664,14 @@ int cmd_record(int argc, const char **argv)
 	    !perf_can_record_switch_events()) {
 		ui__error("kernel does not support recording context switch events\n");
 		parse_options_usage(record_usage, record_options, "switch-events", 0);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out_opts;
 	}
 
 	if (switch_output_setup(rec)) {
 		parse_options_usage(record_usage, record_options, "switch-output", 0);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out_opts;
 	}
 
 	if (rec->switch_output.time) {
@@ -2669,8 +2682,10 @@ int cmd_record(int argc, const char **argv)
 	if (rec->switch_output.num_files) {
 		rec->switch_output.filenames = calloc(sizeof(char *),
 						      rec->switch_output.num_files);
-		if (!rec->switch_output.filenames)
-			return -EINVAL;
+		if (!rec->switch_output.filenames) {
+			err = -EINVAL;
+			goto out_opts;
+		}
 	}
 
 	/*
@@ -2686,7 +2701,8 @@ int cmd_record(int argc, const char **argv)
 		rec->affinity_mask.bits = bitmap_alloc(rec->affinity_mask.nbits);
 		if (!rec->affinity_mask.bits) {
 			pr_err("Failed to allocate thread mask for %zd cpus\n", rec->affinity_mask.nbits);
-			return -ENOMEM;
+			err = -ENOMEM;
+			goto out_opts;
 		}
 		pr_debug2("thread mask[%zd]: empty\n", rec->affinity_mask.nbits);
 	}
@@ -2817,6 +2833,8 @@ int cmd_record(int argc, const char **argv)
 	evlist__delete(rec->evlist);
 	symbol__exit();
 	auxtrace_record__free(rec->itr);
+out_opts:
+	close_control_option(&rec->opts);
 	return err;
 }
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 12ce5cf2b10e..0d4495bace13 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1047,7 +1047,17 @@ static int parse_control_option(const struct option *opt,
 {
 	struct perf_stat_config *config = opt->value;
 
-	return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack);
+	return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack, &config->ctl_fd_close);
+}
+
+static void close_control_option(struct perf_stat_config *config)
+{
+	if (config->ctl_fd_close) {
+		config->ctl_fd_close = false;
+		close(config->ctl_fd);
+		if (config->ctl_fd_ack >= 0)
+			close(config->ctl_fd_ack);
+	}
 }
 
 static struct option stat_options[] = {
@@ -1151,9 +1161,10 @@ static struct option stat_options[] = {
 		"libpfm4 event selector. use 'perf list' to list available events",
 		parse_libpfm_events_option),
 #endif
-	OPT_CALLBACK(0, "control", &stat_config, "fd:ctl-fd[,ack-fd]",
+	OPT_CALLBACK(0, "control", &stat_config, "fd:ctl-fd[,ack-fd] or ctl-fifo[,ack-fifo]",
 		     "Listen on ctl-fd descriptor for command to control measurement ('enable': enable events, 'disable': disable events).\n"
-		     "\t\t\t  Optionally send control command completion ('ack\\n') to ack-fd descriptor.",
+		     "\t\t\t  Optionally send control command completion ('ack\\n') to ack-fd descriptor.\n"
+		     "\t\t\t  Alternatively, ctl-fifo / ack-fifo will be opened and used as ctl-fd / ack-fd.",
 		      parse_control_option),
 	OPT_END()
 };
@@ -2396,6 +2407,7 @@ int cmd_stat(int argc, const char **argv)
 
 	metricgroup__rblist_exit(&stat_config.metric_events);
 	runtime_stat_delete(&stat_config);
+	close_control_option(&stat_config);
 
 	return status;
 }
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 47d1045a19af..cb61c223927f 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1727,12 +1727,57 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
 	return leader;
 }
 
-int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack)
+static int evlist__parse_control_names(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
+{
+	char *s, *p;
+	int ret = 0, fd;
+
+	if (!str || !*str || *str == ',')
+		return -EINVAL;
+
+	s = strdup(str);
+	if (!s)
+		return -ENOMEM;
+
+	p = strchr(s, ',');
+	if (p)
+		*p = '\0';
+
+	/*
+	 * O_RDWR avoids POLLHUPs which is necessary to allow the other
+	 * end of a FIFO to be repeatedly opened and closed.
+	 */
+	fd = open(s, O_RDWR | O_NONBLOCK | O_CLOEXEC);
+	if (fd < 0) {
+		pr_err("Failed to open '%s'\n", s);
+		ret = -errno;
+		goto out_free;
+	}
+	*ctl_fd = fd;
+	*ctl_fd_close = true;
+
+	if (p && *++p) {
+		/* O_RDWR | O_NONBLOCK means the other end need not be open */
+		fd = open(p, O_RDWR | O_NONBLOCK | O_CLOEXEC);
+		if (fd < 0) {
+			pr_err("Failed to open '%s'\n", p);
+			ret = -errno;
+			goto out_free;
+		}
+		*ctl_fd_ack = fd;
+	}
+
+out_free:
+	free(s);
+	return ret;
+}
+
+int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
 {
 	char *comma = NULL, *endptr = NULL;
 
 	if (strncmp(str, "fd:", 3))
-		return -EINVAL;
+		return evlist__parse_control_names(str, ctl_fd, ctl_fd_ack, ctl_fd_close);
 
 	*ctl_fd = strtoul(&str[3], &endptr, 0);
 	if (endptr == &str[3])
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index a5a5a07d5c55..a5678eb5ee60 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -373,7 +373,7 @@ enum evlist_ctl_cmd {
 	EVLIST_CTL_CMD_ACK
 };
 
-int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack);
+int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close);
 int evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
 int evlist__finalize_ctlfd(struct evlist *evlist);
 bool evlist__ctlfd_initialized(struct evlist *evlist);
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 03678ff25539..266760ac9143 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -73,6 +73,7 @@ struct record_opts {
 	unsigned int  nr_threads_synthesize;
 	int	      ctl_fd;
 	int	      ctl_fd_ack;
+	bool	      ctl_fd_close;
 };
 
 extern const char * const *record_usage;
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index f8778cffd941..65402e13b704 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -135,6 +135,7 @@ struct perf_stat_config {
 	struct rblist		 metric_events;
 	int			 ctl_fd;
 	int			 ctl_fd_ack;
+	bool			 ctl_fd_close;
 };
 
 void perf_stat__set_big_num(int set);
-- 
2.17.1

Re: [PATCH 4/6] perf tools: Add FIFO file names as alternative options to --control

[Jiri Olsa]

On Sat, Aug 29, 2020 at 01:50:13PM +0300, Adrian Hunter wrote:

SNIP

> +		*p = '\0';
> +
> +	/*
> +	 * O_RDWR avoids POLLHUPs which is necessary to allow the other
> +	 * end of a FIFO to be repeatedly opened and closed.
> +	 */
> +	fd = open(s, O_RDWR | O_NONBLOCK | O_CLOEXEC);
> +	if (fd < 0) {
> +		pr_err("Failed to open '%s'\n", s);
> +		ret = -errno;
> +		goto out_free;
> +	}
> +	*ctl_fd = fd;
> +	*ctl_fd_close = true;
> +
> +	if (p && *++p) {
> +		/* O_RDWR | O_NONBLOCK means the other end need not be open */
> +		fd = open(p, O_RDWR | O_NONBLOCK | O_CLOEXEC);
> +		if (fd < 0) {
> +			pr_err("Failed to open '%s'\n", p);
> +			ret = -errno;
> +			goto out_free;
> +		}
> +		*ctl_fd_ack = fd;
> +	}
> +
> +out_free:
> +	free(s);
> +	return ret;
> +}
> +
> +int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
>  {
>  	char *comma = NULL, *endptr = NULL;
>  
>  	if (strncmp(str, "fd:", 3))
> -		return -EINVAL;
> +		return evlist__parse_control_names(str, ctl_fd, ctl_fd_ack, ctl_fd_close);

do we want to mention somewhere that the fifo name is everything
except for 'fd:' ?

also how likely is that we will add another channel type that
will need another keyword (likd 'fd:')? I originaly thought
we'd use 'fifo:filename' for this ... would be great to somehow
avoid future confusions

thanks,
jirka

Re: [PATCH 4/6] perf tools: Add FIFO file names as alternative options to --control

[Adrian Hunter]

On 31/08/20 4:19 pm, Jiri Olsa wrote:
> On Sat, Aug 29, 2020 at 01:50:13PM +0300, Adrian Hunter wrote:
> 
> SNIP
> 
>> +		*p = '\0';
>> +
>> +	/*
>> +	 * O_RDWR avoids POLLHUPs which is necessary to allow the other
>> +	 * end of a FIFO to be repeatedly opened and closed.
>> +	 */
>> +	fd = open(s, O_RDWR | O_NONBLOCK | O_CLOEXEC);
>> +	if (fd < 0) {
>> +		pr_err("Failed to open '%s'\n", s);
>> +		ret = -errno;
>> +		goto out_free;
>> +	}
>> +	*ctl_fd = fd;
>> +	*ctl_fd_close = true;
>> +
>> +	if (p && *++p) {
>> +		/* O_RDWR | O_NONBLOCK means the other end need not be open */
>> +		fd = open(p, O_RDWR | O_NONBLOCK | O_CLOEXEC);
>> +		if (fd < 0) {
>> +			pr_err("Failed to open '%s'\n", p);
>> +			ret = -errno;
>> +			goto out_free;
>> +		}
>> +		*ctl_fd_ack = fd;
>> +	}
>> +
>> +out_free:
>> +	free(s);
>> +	return ret;
>> +}
>> +
>> +int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
>>  {
>>  	char *comma = NULL, *endptr = NULL;
>>  
>>  	if (strncmp(str, "fd:", 3))
>> -		return -EINVAL;
>> +		return evlist__parse_control_names(str, ctl_fd, ctl_fd_ack, ctl_fd_close);
> 
> do we want to mention somewhere that the fifo name is everything
> except for 'fd:' ?

It is only mentioned in the documentation i.e.

--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -627,7 +627,9 @@ option. The -e option and this one can be mixed and matched.  Events
 can be grouped using the {} notation.
 endif::HAVE_LIBPFM[]
 
+--control=ctl-fifo[,ack-fifo]::
 --control=fd:ctl-fd[,ack-fd]::
+ctl-fifo / ack-fifo are opened and used as ctl-fd / ack-fd as follows.
 Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
 'disable': disable events). Measurements can be started with events disabled using
 --delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor

> 
> also how likely is that we will add another channel type that
> will need another keyword (likd 'fd:')? I originaly thought
> we'd use 'fifo:filename' for this ... would be great to somehow
> avoid future confusions

Sure, I will add fifo: in V2

[PATCH 5/6] perf record: Add 'snapshot' control command

[Adrian Hunter]

Add 'snapshot' control command to create an AUX area tracing snapshot the
same as if sending SIGUSR2. The advantage of the FIFO is that access is
governed by access to the FIFO.

Example:

 $ mkfifo perf.control
 $ mkfifo perf.ack
 $ cat perf.ack &
 [1] 15235
 $ sudo ~/bin/perf record --control perf.control,perf.ack -S -e intel_pt//u -- sleep 60 &
 [2] 15243
 $ ps -e | grep perf
  15244 pts/1    00:00:00 perf
 $ kill -USR2 15244
 bash: kill: (15244) - Operation not permitted
 $ echo snapshot > perf.control
 ack

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt |  8 ++++----
 tools/perf/builtin-record.c              | 24 +++++++++++++++++-------
 tools/perf/builtin-stat.c                |  1 +
 tools/perf/util/evlist.c                 | 11 +++++++++--
 tools/perf/util/evlist.h                 |  5 ++++-
 5 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 74fa3e905bf1..0c028d48ebd0 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -631,10 +631,10 @@ endif::HAVE_LIBPFM[]
 --control=fd:ctl-fd[,ack-fd]::
 ctl-fifo / ack-fifo are opened and used as ctl-fd / ack-fd as follows.
 Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
-'disable': disable events). Measurements can be started with events disabled using
---delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
-to synchronize with the controlling process. Example of bash shell script to enable and
-disable events during measurements:
+'disable': disable events, 'snapshot': AUX area tracing snapshot). Measurements can be
+started with events disabled using --delay=-1 option. Optionally send control command
+completion ('ack\n') to ack-fd descriptor to synchronize with the controlling process.
+Example of bash shell script to enable and disable events during measurements:
 
  #!/bin/bash
 
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index af7238b1356f..77d86a3a8553 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1593,6 +1593,16 @@ static int record__init_clock(struct record *rec)
 	return 0;
 }
 
+static void hit_auxtrace_snapshot_trigger(struct record *rec)
+{
+	if (trigger_is_ready(&auxtrace_snapshot_trigger)) {
+		trigger_hit(&auxtrace_snapshot_trigger);
+		auxtrace_record__snapshot_started = 1;
+		if (auxtrace_record__snapshot_start(rec->itr))
+			trigger_error(&auxtrace_snapshot_trigger);
+	}
+}
+
 static int __cmd_record(struct record *rec, int argc, const char **argv)
 {
 	int err;
@@ -1937,6 +1947,10 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 			case EVLIST_CTL_CMD_DISABLE:
 				pr_info(EVLIST_DISABLED_MSG);
 				break;
+			case EVLIST_CTL_CMD_SNAPSHOT:
+				hit_auxtrace_snapshot_trigger(rec);
+				evlist__ctlfd_ack(rec->evlist);
+				break;
 			case EVLIST_CTL_CMD_ACK:
 			case EVLIST_CTL_CMD_UNSUPPORTED:
 			default:
@@ -2589,7 +2603,8 @@ static struct option __record_options[] = {
 		parse_libpfm_events_option),
 #endif
 	OPT_CALLBACK(0, "control", &record.opts, "fd:ctl-fd[,ack-fd] or ctl-fifo[,ack-fifo]",
-		     "Listen on ctl-fd descriptor for command to control measurement ('enable': enable events, 'disable': disable events).\n"
+		     "Listen on ctl-fd descriptor for command to control measurement ('enable': enable events, 'disable': disable events,\n"
+		     "\t\t\t  'snapshot': AUX area tracing snapshot).\n"
 		     "\t\t\t  Optionally send control command completion ('ack\\n') to ack-fd descriptor.\n"
 		     "\t\t\t  Alternatively, ctl-fifo / ack-fifo will be opened and used as ctl-fd / ack-fd.",
 		      parse_control_option),
@@ -2842,12 +2857,7 @@ 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;
-		if (auxtrace_record__snapshot_start(record.itr))
-			trigger_error(&auxtrace_snapshot_trigger);
-	}
+	hit_auxtrace_snapshot_trigger(rec);
 
 	if (switch_output_signal(rec))
 		trigger_hit(&switch_output_trigger);
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 0d4495bace13..3536299496a8 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -578,6 +578,7 @@ static void process_evlist(struct evlist *evlist, unsigned int interval)
 				process_interval();
 			pr_info(EVLIST_DISABLED_MSG);
 			break;
+		case EVLIST_CTL_CMD_SNAPSHOT:
 		case EVLIST_CTL_CMD_ACK:
 		case EVLIST_CTL_CMD_UNSUPPORTED:
 		default:
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index cb61c223927f..bd88521a01a5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1881,13 +1881,17 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
 		} else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_TAG,
 				    (sizeof(EVLIST_CTL_CMD_DISABLE_TAG)-1))) {
 			*cmd = EVLIST_CTL_CMD_DISABLE;
+		} else if (!strncmp(cmd_data, EVLIST_CTL_CMD_SNAPSHOT_TAG,
+				    (sizeof(EVLIST_CTL_CMD_SNAPSHOT_TAG)-1))) {
+			*cmd = EVLIST_CTL_CMD_SNAPSHOT;
+			pr_debug("is snapshot\n");
 		}
 	}
 
 	return bytes_read ? (int)bytes_read : err;
 }
 
-static int evlist__ctlfd_ack(struct evlist *evlist)
+int evlist__ctlfd_ack(struct evlist *evlist)
 {
 	int err;
 
@@ -1923,13 +1927,16 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
 			case EVLIST_CTL_CMD_DISABLE:
 				evlist__disable(evlist);
 				break;
+			case EVLIST_CTL_CMD_SNAPSHOT:
+				break;
 			case EVLIST_CTL_CMD_ACK:
 			case EVLIST_CTL_CMD_UNSUPPORTED:
 			default:
 				pr_debug("ctlfd: unsupported %d\n", *cmd);
 				break;
 			}
-			if (!(*cmd == EVLIST_CTL_CMD_ACK || *cmd == EVLIST_CTL_CMD_UNSUPPORTED))
+			if (!(*cmd == EVLIST_CTL_CMD_ACK || *cmd == EVLIST_CTL_CMD_UNSUPPORTED ||
+			      *cmd == EVLIST_CTL_CMD_SNAPSHOT))
 				evlist__ctlfd_ack(evlist);
 		}
 	}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index a5678eb5ee60..91d1da6e1fe3 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -363,6 +363,7 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
 #define EVLIST_CTL_CMD_ENABLE_TAG  "enable"
 #define EVLIST_CTL_CMD_DISABLE_TAG "disable"
 #define EVLIST_CTL_CMD_ACK_TAG     "ack\n"
+#define EVLIST_CTL_CMD_SNAPSHOT_TAG "snapshot"
 
 #define EVLIST_CTL_CMD_MAX_LEN 64
 
@@ -370,7 +371,8 @@ enum evlist_ctl_cmd {
 	EVLIST_CTL_CMD_UNSUPPORTED = 0,
 	EVLIST_CTL_CMD_ENABLE,
 	EVLIST_CTL_CMD_DISABLE,
-	EVLIST_CTL_CMD_ACK
+	EVLIST_CTL_CMD_ACK,
+	EVLIST_CTL_CMD_SNAPSHOT,
 };
 
 int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close);
@@ -378,6 +380,7 @@ int evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
 int evlist__finalize_ctlfd(struct evlist *evlist);
 bool evlist__ctlfd_initialized(struct evlist *evlist);
 int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
+int evlist__ctlfd_ack(struct evlist *evlist);
 
 #define EVLIST_ENABLED_MSG "Events enabled\n"
 #define EVLIST_DISABLED_MSG "Events disabled\n"
-- 
2.17.1

Re: [PATCH 5/6] perf record: Add 'snapshot' control command

[Andi Kleen]

On Sat, Aug 29, 2020 at 01:50:14PM +0300, Adrian Hunter wrote:
> Add 'snapshot' control command to create an AUX area tracing snapshot the
> same as if sending SIGUSR2. The advantage of the FIFO is that access is
> governed by access to the FIFO.

How about the --switch-output SIGUSR2 switch? Could that be handled too?

Other than that it looks great. Thanks.

-Andi

Re: [PATCH 5/6] perf record: Add 'snapshot' control command

[Adrian Hunter]

On 31/08/20 9:23 am, Andi Kleen wrote:
> On Sat, Aug 29, 2020 at 01:50:14PM +0300, Adrian Hunter wrote:
>> Add 'snapshot' control command to create an AUX area tracing snapshot the
>> same as if sending SIGUSR2. The advantage of the FIFO is that access is
>> governed by access to the FIFO.
> 
> How about the --switch-output SIGUSR2 switch? Could that be handled too?

Presumably, but I have never used --switch-out because Intel PT does not yet
support it (it is on the list of things to do).  Perhaps someone more
familiar with it can add that :-)

> 
> Other than that it looks great. Thanks.
> 
> -Andi
> 

[PATCH 6/6] perf intel-pt: Document snapshot control command

[Adrian Hunter]

The documentation descibes snapshot mode.  Update it to include the new
snapshot control command.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-intel-pt.txt | 23 +++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-intel-pt.txt b/tools/perf/Documentation/perf-intel-pt.txt
index d5a266d7f15b..b4414f75ad0b 100644
--- a/tools/perf/Documentation/perf-intel-pt.txt
+++ b/tools/perf/Documentation/perf-intel-pt.txt
@@ -558,7 +558,7 @@ The mmap size and auxtrace mmap size are displayed if the -vv option is used e.g
 Intel PT modes of operation
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Intel PT can be used in 2 modes:
+Intel PT can be used in 3 modes:
 	full-trace mode
 	sample mode
 	snapshot mode
@@ -571,7 +571,8 @@ Sample mode attaches a Intel PT sample to other events e.g.
 
 	perf record --aux-sample -e intel_pt//u -e branch-misses:u
 
-Snapshot mode captures the available data when a signal is sent e.g.
+Snapshot mode captures the available data when a signal is sent or "snapshot"
+control command is issued. e.g. using a signal
 
 	perf record -v -e intel_pt//u -S ./loopy 1000000000 &
 	[1] 11435
@@ -582,7 +583,23 @@ Note that the signal sent is SIGUSR2.
 Note that "Recording AUX area tracing snapshot" is displayed because the -v
 option is used.
 
-The 2 modes cannot be used together.
+The advantage of using "snapshot" control command is that the access is
+controlled by access to a FIFO e.g.
+
+	$ mkfifo perf.control
+	$ mkfifo perf.ack
+	$ cat perf.ack &
+	[1] 15235
+	$ sudo ~/bin/perf record --control perf.control,perf.ack -S -e intel_pt//u -- sleep 60 &
+	[2] 15243
+	$ ps -e | grep perf
+	15244 pts/1    00:00:00 perf
+	$ kill -USR2 15244
+	bash: kill: (15244) - Operation not permitted
+	$ echo snapshot > perf.control
+	ack
+
+The 3 Intel PT modes of operation cannot be used together.
 
 
 Buffer handling
-- 
2.17.1