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

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

[Adrian Hunter]

Hi

Here are V2 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.


Changes in V2:
	Use fifo: prefix in --control option


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 V2 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

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

[Alexey Budankov]

On 01.09.2020 12:37, Adrian Hunter wrote:
> 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(-)

Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Regards,
Alexei

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

[Arnaldo Carvalho de Melo]

Em Wed, Sep 02, 2020 at 07:06:51PM +0300, Alexey Budankov escreveu:
> 
> On 01.09.2020 12:37, Adrian Hunter wrote:
> > 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(-)
> 
> Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Thanks, applied.

- Arnaldo

[PATCH V2 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

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

[Alexey Budankov]

On 01.09.2020 12:37, Adrian Hunter wrote:
> 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(-)

Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Regards,
Alexei

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

[Arnaldo Carvalho de Melo]

Em Wed, Sep 02, 2020 at 07:11:14PM +0300, Alexey Budankov escreveu:
> 
> On 01.09.2020 12:37, Adrian Hunter wrote:
> > 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(-)
> 
> Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Thanks, applied.

- Arnaldo

[PATCH V2 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

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

[Alexey Budankov]

On 01.09.2020 12:37, Adrian Hunter wrote:
> 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(-)

Thanks Adrian for all the formating corrections.

Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Regards,
Alexei

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

[Arnaldo Carvalho de Melo]

Em Wed, Sep 02, 2020 at 07:12:43PM +0300, Alexey Budankov escreveu:
> 
> On 01.09.2020 12:37, Adrian Hunter wrote:
> > 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(-)
> 
> Thanks Adrian for all the formating corrections.
> 
> Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Thanks, applied.

- Arnaldo

[PATCH V2 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 fifo: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 fifo: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                 | 53 +++++++++++++++++++++++-
 tools/perf/util/evlist.h                 |  2 +-
 tools/perf/util/record.h                 |  1 +
 tools/perf/util/stat.h                   |  1 +
 8 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 07c4734f1c7a..2ffc1196d2b8 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=fifo: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..d33ffd11bb85 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=fifo: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..117dd180f780 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 fifo: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..6170226d44f9 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 fifo: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..b74e85bc683e 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1727,12 +1727,61 @@ 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 (strncmp(str, "fifo:", 5))
+		return -EINVAL;
+
+	str += 5;
+	if (!*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 V2 4/6] perf tools: Add FIFO file names as alternative options to --control

[Jiri Olsa]

On Tue, Sep 01, 2020 at 12:37:56PM +0300, Adrian Hunter wrote:

SNIP

> +	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);

could you please proactively set *ctl_fd_close = false in here,
so it's obvious.. also evlist__parse_control_fifo might be better
name, other that that it all looks great

we were recently asked to prvide faster intel_pt snapshot control
and we were thinking of putting input_pt support to libperf, but
your change might just fix this

for the patchset:

Acked-by: Jiri Olsa <jolsa@redhat.com>

Alexey, could you please check on this?

thanks,
jirka

>  
>  	*ctl_fd = strtoul(&str[3], &endptr, 0);
>  	if (endptr == &str[3])

SNIP

[PATCH V3 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 fifo: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 fifo:perf.control,perf.ack -- sleep 300
 $

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
---


Changes in V3:

	Rename evlist__parse_control_names to evlist__parse_control_fifo
	Explicitly initialize *ctl_fd_close = false


 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                 | 55 +++++++++++++++++++++++-
 tools/perf/util/evlist.h                 |  2 +-
 tools/perf/util/record.h                 |  1 +
 tools/perf/util/stat.h                   |  1 +
 8 files changed, 101 insertions(+), 14 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 07c4734f1c7a..2ffc1196d2b8 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=fifo: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..d33ffd11bb85 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=fifo: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..117dd180f780 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 fifo: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..6170226d44f9 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 fifo: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..00593e5f2a9d 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1727,12 +1727,63 @@ 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_fifo(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
+{
+	char *s, *p;
+	int ret = 0, fd;
+
+	if (strncmp(str, "fifo:", 5))
+		return -EINVAL;
+
+	str += 5;
+	if (!*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;
 
+	*ctl_fd_close = false;
+
 	if (strncmp(str, "fd:", 3))
-		return -EINVAL;
+		return evlist__parse_control_fifo(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 V3 4/6] perf tools: Add FIFO file names as alternative options to --control

[Alexey Budankov]

On 02.09.2020 13:57, Adrian Hunter wrote:
> 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 fifo: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 fifo:perf.control,perf.ack -- sleep 300
>  $
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> ---
> 
> 
> Changes in V3:
> 
> 	Rename evlist__parse_control_names to evlist__parse_control_fifo
> 	Explicitly initialize *ctl_fd_close = false
> 
> 
>  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                 | 55 +++++++++++++++++++++++-
>  tools/perf/util/evlist.h                 |  2 +-
>  tools/perf/util/record.h                 |  1 +
>  tools/perf/util/stat.h                   |  1 +
>  8 files changed, 101 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 07c4734f1c7a..2ffc1196d2b8 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=fifo: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..d33ffd11bb85 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=fifo: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..117dd180f780 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)

This function above and the same one for stat mode below look duplicating.
It could possibly be combined.

Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Regards,
Alexei

> +{
> +	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 fifo: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..6170226d44f9 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 fifo: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..00593e5f2a9d 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1727,12 +1727,63 @@ 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_fifo(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
> +{
> +	char *s, *p;
> +	int ret = 0, fd;
> +
> +	if (strncmp(str, "fifo:", 5))
> +		return -EINVAL;
> +
> +	str += 5;
> +	if (!*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;
>  
> +	*ctl_fd_close = false;
> +
>  	if (strncmp(str, "fd:", 3))
> -		return -EINVAL;
> +		return evlist__parse_control_fifo(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);
> 

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

[Adrian Hunter]

On 2/09/20 8:03 pm, Alexey Budankov wrote:
> 
> On 02.09.2020 13:57, Adrian Hunter wrote:
>> 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 fifo: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 fifo:perf.control,perf.ack -- sleep 300
>>  $
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>> ---
>>
>>
>> Changes in V3:
>>
>> 	Rename evlist__parse_control_names to evlist__parse_control_fifo
>> 	Explicitly initialize *ctl_fd_close = false
>>
>>
>>  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                 | 55 +++++++++++++++++++++++-
>>  tools/perf/util/evlist.h                 |  2 +-
>>  tools/perf/util/record.h                 |  1 +
>>  tools/perf/util/stat.h                   |  1 +
>>  8 files changed, 101 insertions(+), 14 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
>> index 07c4734f1c7a..2ffc1196d2b8 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=fifo: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..d33ffd11bb85 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=fifo: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..117dd180f780 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)
> 
> This function above and the same one for stat mode below look duplicating.
> It could possibly be combined.

Good idea.  I will send a separate patch for that though.

> 
> Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>
> 
> Regards,
> Alexei
> 
>> +{
>> +	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 fifo: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..6170226d44f9 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 fifo: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..00593e5f2a9d 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1727,12 +1727,63 @@ 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_fifo(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close)
>> +{
>> +	char *s, *p;
>> +	int ret = 0, fd;
>> +
>> +	if (strncmp(str, "fifo:", 5))
>> +		return -EINVAL;
>> +
>> +	str += 5;
>> +	if (!*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;
>>  
>> +	*ctl_fd_close = false;
>> +
>>  	if (strncmp(str, "fd:", 3))
>> -		return -EINVAL;
>> +		return evlist__parse_control_fifo(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);
>>

[PATCH] perf tools: Consolidate close_control_option()'s into one function

[Adrian Hunter]

Consolidate control option fifo closing into one function.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


This patch is on top of patch:
"perf tools: Add FIFO file names as alternative options to --control"


 tools/perf/builtin-record.c | 12 +-----------
 tools/perf/builtin-stat.c   | 12 +-----------
 tools/perf/util/evlist.c    | 10 ++++++++++
 tools/perf/util/evlist.h    |  1 +
 4 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 476b34ff3152..9db3901e6561 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2253,16 +2253,6 @@ static int parse_control_option(const struct option *opt,
 	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)
 {
 	u64 wakeup_size = evlist__mmap_size(rec->opts.mmap_pages);
@@ -2849,7 +2839,7 @@ int cmd_record(int argc, const char **argv)
 	symbol__exit();
 	auxtrace_record__free(rec->itr);
 out_opts:
-	close_control_option(&rec->opts);
+	evlist__close_control(rec->opts.ctl_fd, rec->opts.ctl_fd_ack, &rec->opts.ctl_fd_close);
 	return err;
 }
 
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 21424ed0734b..68d4bdf15635 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1051,16 +1051,6 @@ static int parse_control_option(const struct option *opt,
 	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[] = {
 	OPT_BOOLEAN('T', "transaction", &transaction_run,
 		    "hardware transaction statistics"),
@@ -2408,7 +2398,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);
+	evlist__close_control(stat_config.ctl_fd, stat_config.ctl_fd_ack, &stat_config.ctl_fd_close);
 
 	return status;
 }
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index e72ff7e78dec..ee7b576d3b12 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1802,6 +1802,16 @@ int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *c
 	return 0;
 }
 
+void evlist__close_control(int ctl_fd, int ctl_fd_ack, bool *ctl_fd_close)
+{
+	if (*ctl_fd_close) {
+		*ctl_fd_close = false;
+		close(ctl_fd);
+		if (ctl_fd_ack >= 0)
+			close(ctl_fd_ack);
+	}
+}
+
 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 91d1da6e1fe3..bc38a53f6a1a 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -376,6 +376,7 @@ enum evlist_ctl_cmd {
 };
 
 int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close);
+void evlist__close_control(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);
-- 
2.17.1

Re: [PATCH] perf tools: Consolidate close_control_option()'s into one function

[Arnaldo Carvalho de Melo]

Em Thu, Sep 03, 2020 at 03:29:37PM +0300, Adrian Hunter escreveu:
> Consolidate control option fifo closing into one function.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---

Applied and added this:

Suggested-by: Alexey Budankov <alexey.budankov@linux.intel.com>

Thanks,

- Arnaldo
 
> 
> This patch is on top of patch:
> "perf tools: Add FIFO file names as alternative options to --control"
> 
> 
>  tools/perf/builtin-record.c | 12 +-----------
>  tools/perf/builtin-stat.c   | 12 +-----------
>  tools/perf/util/evlist.c    | 10 ++++++++++
>  tools/perf/util/evlist.h    |  1 +
>  4 files changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 476b34ff3152..9db3901e6561 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2253,16 +2253,6 @@ static int parse_control_option(const struct option *opt,
>  	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)
>  {
>  	u64 wakeup_size = evlist__mmap_size(rec->opts.mmap_pages);
> @@ -2849,7 +2839,7 @@ int cmd_record(int argc, const char **argv)
>  	symbol__exit();
>  	auxtrace_record__free(rec->itr);
>  out_opts:
> -	close_control_option(&rec->opts);
> +	evlist__close_control(rec->opts.ctl_fd, rec->opts.ctl_fd_ack, &rec->opts.ctl_fd_close);
>  	return err;
>  }
>  
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 21424ed0734b..68d4bdf15635 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1051,16 +1051,6 @@ static int parse_control_option(const struct option *opt,
>  	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[] = {
>  	OPT_BOOLEAN('T', "transaction", &transaction_run,
>  		    "hardware transaction statistics"),
> @@ -2408,7 +2398,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);
> +	evlist__close_control(stat_config.ctl_fd, stat_config.ctl_fd_ack, &stat_config.ctl_fd_close);
>  
>  	return status;
>  }
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index e72ff7e78dec..ee7b576d3b12 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1802,6 +1802,16 @@ int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *c
>  	return 0;
>  }
>  
> +void evlist__close_control(int ctl_fd, int ctl_fd_ack, bool *ctl_fd_close)
> +{
> +	if (*ctl_fd_close) {
> +		*ctl_fd_close = false;
> +		close(ctl_fd);
> +		if (ctl_fd_ack >= 0)
> +			close(ctl_fd_ack);
> +	}
> +}
> +
>  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 91d1da6e1fe3..bc38a53f6a1a 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -376,6 +376,7 @@ enum evlist_ctl_cmd {
>  };
>  
>  int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close);
> +void evlist__close_control(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);
> -- 
> 2.17.1
> 

-- 

- Arnaldo

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

[Arnaldo Carvalho de Melo]

Em Wed, Sep 02, 2020 at 08:03:08PM +0300, Alexey Budankov escreveu:
> 
> On 02.09.2020 13:57, Adrian Hunter wrote:
> > 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 fifo: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 fifo:perf.control,perf.ack -- sleep 300
> >  $
> > 
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > Acked-by: Jiri Olsa <jolsa@redhat.com>

Its great when things come with the exact sequence of commands to
test, the output, etc, keep it that way, please.

Thanks, tested, applied,

- Arnaldo

[PATCH V2 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 fifo: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 2ffc1196d2b8..588e6191bf39 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 117dd180f780..476b34ff3152 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 fifo: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 6170226d44f9..21424ed0734b 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 b74e85bc683e..8cd53dbb0357 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1885,13 +1885,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;
 
@@ -1927,13 +1931,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 V2 5/6] perf record: Add 'snapshot' control command

[Alexey Budankov]

On 01.09.2020 12:37, 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.
> 
> Example:
> 
>  $ mkfifo perf.control
>  $ mkfifo perf.ack
>  $ cat perf.ack &
>  [1] 15235
>  $ sudo ~/bin/perf record --control fifo: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 2ffc1196d2b8..588e6191bf39 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 117dd180f780..476b34ff3152 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);

Could possibly print a messages to console that snapshot taking has started,
similar to enabling and disabling of events.

Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Regards,
Alexei

> +				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 fifo: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 6170226d44f9..21424ed0734b 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 b74e85bc683e..8cd53dbb0357 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1885,13 +1885,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;
>  
> @@ -1927,13 +1931,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"
> 

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

[Adrian Hunter]

On 2/09/20 8:03 pm, Alexey Budankov wrote:
> 
> On 01.09.2020 12:37, 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.
>>
>> Example:
>>
>>  $ mkfifo perf.control
>>  $ mkfifo perf.ack
>>  $ cat perf.ack &
>>  [1] 15235
>>  $ sudo ~/bin/perf record --control fifo: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 2ffc1196d2b8..588e6191bf39 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 117dd180f780..476b34ff3152 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);
> 
> Could possibly print a messages to console that snapshot taking has started,
> similar to enabling and disabling of events.

There is a an existing debug print for snapshots, and the ack fifo of
course.  So I am going to leave that for now.

> 
> Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>
> 
> Regards,
> Alexei
> 
>> +				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 fifo: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 6170226d44f9..21424ed0734b 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 b74e85bc683e..8cd53dbb0357 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -1885,13 +1885,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;
>>  
>> @@ -1927,13 +1931,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"
>>

[PATCH V2 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..cb637e0d0743 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 fifo: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

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

[Alexey Budankov]

On 01.09.2020 12:37, Adrian Hunter wrote:
> 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(-)

Acked-by: Alexei Budankov <alexey.budankov@linux.intel.com>

Regards,
Alexei