From: Adrian Hunter <adrian.hunter@intel.com>
To: Alexey Budankov <alexey.budankov@linux.intel.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Jiri Olsa <jolsa@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>,
Namhyung Kim <namhyung@kernel.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2 5/6] perf record: Add 'snapshot' control command
Date: Thu, 3 Sep 2020 13:44:13 +0300 [thread overview]
Message-ID: <aa7a2be1-9944-2def-03af-cec559e633e4@intel.com> (raw)
In-Reply-To: <3b080496-7357-04a3-ec9b-bf531b8ec75a@linux.intel.com>
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"
>>
next prev parent reply other threads:[~2020-09-03 10:44 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 9:37 [PATCH V2 0/6] perf tools: Enable AUX area tracing snapshots using a FIFO Adrian Hunter
2020-09-01 9:37 ` [PATCH V2 1/6] perf tools: Consolidate --control option parsing into one function Adrian Hunter
2020-09-02 16:06 ` Alexey Budankov
2020-09-03 20:15 ` Arnaldo Carvalho de Melo
2020-09-01 9:37 ` [PATCH V2 2/6] perf tools: Handle read errors from ctl_fd Adrian Hunter
2020-09-02 16:11 ` Alexey Budankov
2020-09-03 20:16 ` Arnaldo Carvalho de Melo
2020-09-01 9:37 ` [PATCH V2 3/6] perf tools: Use AsciiDoc formatting for --control option documentation Adrian Hunter
2020-09-02 16:12 ` Alexey Budankov
2020-09-03 20:17 ` Arnaldo Carvalho de Melo
2020-09-01 9:37 ` [PATCH V2 4/6] perf tools: Add FIFO file names as alternative options to --control Adrian Hunter
2020-09-01 20:06 ` Jiri Olsa
2020-09-02 10:57 ` [PATCH V3 " Adrian Hunter
2020-09-02 17:03 ` Alexey Budankov
2020-09-03 10:38 ` Adrian Hunter
2020-09-03 12:29 ` [PATCH] perf tools: Consolidate close_control_option()'s into one function Adrian Hunter
2020-09-04 18:56 ` Arnaldo Carvalho de Melo
2020-09-03 20:21 ` [PATCH V3 4/6] perf tools: Add FIFO file names as alternative options to --control Arnaldo Carvalho de Melo
2020-09-01 9:37 ` [PATCH V2 5/6] perf record: Add 'snapshot' control command Adrian Hunter
2020-09-02 17:03 ` Alexey Budankov
2020-09-03 10:44 ` Adrian Hunter [this message]
2020-09-01 9:37 ` [PATCH V2 6/6] perf intel-pt: Document snapshot " Adrian Hunter
2020-09-02 16:53 ` Alexey Budankov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aa7a2be1-9944-2def-03af-cec559e633e4@intel.com \
--to=adrian.hunter@intel.com \
--cc=acme@kernel.org \
--cc=ak@linux.intel.com \
--cc=alexey.budankov@linux.intel.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=namhyung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome