From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>,
lkml <linux-kernel@vger.kernel.org>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
Ingo Molnar <mingo@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Michael Petlan <mpetlan@redhat.com>,
Ian Rogers <irogers@google.com>,
Stephane Eranian <eranian@google.com>,
Alexei Budankov <abudankov@huawei.com>
Subject: Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file
Date: Tue, 15 Dec 2020 12:14:13 -0300 [thread overview]
Message-ID: <20201215151413.GE252952@kernel.org> (raw)
In-Reply-To: <20201210204330.233864-3-jolsa@kernel.org>
Em Thu, Dec 10, 2020 at 09:43:29PM +0100, Jiri Olsa escreveu:
> Adding new control events to enable/disable specific event.
> The interface string for control file are:
>
> 'enable-<EVENT NAME>'
> 'disable-<EVENT NAME>'
Wwy do we have "enable-" as the "tag" for this?
Also is it possible to use "enable sched:*" and have that match what is
in the evlist and enable (or disable, if using "disable sched:*") what
matches?
This second suggestion can be done on top of this, i.e. as an
enhancement, but mixing up the command (enable, disable) with its
arguments looks strange.
- Arnaldo
> when received the command, perf will scan the current evlist
> for <EVENT NAME> and if found it's enabled/disabled.
>
> Example session:
>
> terminal 1:
> # mkfifo control ack perf.pipe
> # perf record --control=fifo:control,ack -D -1 --no-buffering -e 'sched:*' -o - > perf.pipe
> Events disabled
>
> terminal 2:
> # cat perf.pipe | ./perf --no-pager script -i -
>
> terminal 3:
> # echo enable-sched:sched_process_fork > control
>
> terminal 1:
> # mkfifo control ack perf.pipe
> # perf record --control=fifo:control,ack -D -1 --no-buffering -e 'sched:*' -o - > perf.pipe
> ...
> event sched:sched_process_fork enabled
>
> terminal 2:
> # cat perf.pipe | ./perf --no-pager script -i -
> bash 33349 [034] 149587.674295: sched:sched_process_fork: comm=bash pid=33349 child_comm=bash child_pid=34056
> bash 33349 [034] 149588.239521: sched:sched_process_fork: comm=bash pid=33349 child_comm=bash child_pid=34057
>
> terminal 3:
> # echo enable-sched:sched_wakeup_new > control
>
> terminal 1:
> # mkfifo control ack perf.pipe
> # perf record --control=fifo:control,ack -D -1 --no-buffering -e 'sched:*' -o - > perf.pipe
> ...
> event sched:sched_wakeup_new enabled
>
> terminal 2:
> # cat perf.pipe | ./perf --no-pager script -i -
> ...
> bash 33349 [034] 149632.228023: sched:sched_process_fork: comm=bash pid=33349 child_comm=bash child_pid=34059
> bash 33349 [034] 149632.228050: sched:sched_wakeup_new: bash:34059 [120] success=1 CPU:036
> bash 33349 [034] 149633.950005: sched:sched_process_fork: comm=bash pid=33349 child_comm=bash child_pid=34060
> bash 33349 [034] 149633.950030: sched:sched_wakeup_new: bash:34060 [120] success=1 CPU:036
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/perf/builtin-record.c | 2 ++
> tools/perf/builtin-stat.c | 2 ++
> tools/perf/util/evlist.c | 30 +++++++++++++++++++++++++++++-
> tools/perf/util/evlist.h | 4 ++++
> 4 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index d832c108a1ca..582b8fba012c 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1949,6 +1949,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> break;
> case EVLIST_CTL_CMD_ACK:
> case EVLIST_CTL_CMD_UNSUPPORTED:
> + case EVLIST_CTL_CMD_ENABLE_EVSEL:
> + case EVLIST_CTL_CMD_DISABLE_EVSEL:
> default:
> break;
> }
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 89c32692f40c..6a21fb665008 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -590,6 +590,8 @@ static void process_evlist(struct evlist *evlist, unsigned int interval)
> case EVLIST_CTL_CMD_SNAPSHOT:
> case EVLIST_CTL_CMD_ACK:
> case EVLIST_CTL_CMD_UNSUPPORTED:
> + case EVLIST_CTL_CMD_ENABLE_EVSEL:
> + case EVLIST_CTL_CMD_DISABLE_EVSEL:
> default:
> break;
> }
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 70aff26612a9..729c98d10628 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1915,7 +1915,13 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
> bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0");
>
> if (bytes_read > 0) {
> - if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG,
> + if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_EVSEL_TAG,
> + (sizeof(EVLIST_CTL_CMD_ENABLE_EVSEL_TAG)-1))) {
> + *cmd = EVLIST_CTL_CMD_ENABLE_EVSEL;
> + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_EVSEL_TAG,
> + (sizeof(EVLIST_CTL_CMD_DISABLE_EVSEL_TAG)-1))) {
> + *cmd = EVLIST_CTL_CMD_DISABLE_EVSEL;
> + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_ENABLE_TAG,
> (sizeof(EVLIST_CTL_CMD_ENABLE_TAG)-1))) {
> *cmd = EVLIST_CTL_CMD_ENABLE;
> } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_DISABLE_TAG,
> @@ -1952,6 +1958,8 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
> char cmd_data[EVLIST_CTL_CMD_MAX_LEN];
> int ctlfd_pos = evlist->ctl_fd.pos;
> struct pollfd *entries = evlist->core.pollfd.entries;
> + struct evsel *evsel;
> + char *evsel_name;
>
> if (!evlist__ctlfd_initialized(evlist) || !entries[ctlfd_pos].revents)
> return 0;
> @@ -1967,6 +1975,26 @@ 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_ENABLE_EVSEL:
> + evsel_name = cmd_data + sizeof(EVLIST_CTL_CMD_ENABLE_EVSEL_TAG) - 1;
> + evsel = evlist__find_evsel_by_str(evlist, evsel_name);
> + if (evsel) {
> + evlist__enable_evsel(evlist, evsel_name);
> + pr_info("event %s enabled\n", evsel->name);
> + } else {
> + pr_info("failed: can't find '%s' event\n", evsel_name);
> + }
> + break;
> + case EVLIST_CTL_CMD_DISABLE_EVSEL:
> + evsel_name = cmd_data + sizeof(EVLIST_CTL_CMD_DISABLE_EVSEL_TAG) - 1;
> + evsel = evlist__find_evsel_by_str(evlist, evsel_name);
> + if (evsel) {
> + evlist__disable_evsel(evlist, evsel_name);
> + pr_info("event %s disabled\n", evsel->name);
> + } else {
> + pr_info("failed: can't find '%s' event\n", evsel_name);
> + }
> + break;
> case EVLIST_CTL_CMD_SNAPSHOT:
> break;
> case EVLIST_CTL_CMD_ACK:
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 1aae75895dea..e4e8ff8831a3 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -330,6 +330,8 @@ struct evsel *evlist__reset_weak_group(struct evlist *evlist, struct evsel *evse
> #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_ENABLE_EVSEL_TAG "enable-"
> +#define EVLIST_CTL_CMD_DISABLE_EVSEL_TAG "disable-"
>
> #define EVLIST_CTL_CMD_MAX_LEN 64
>
> @@ -337,6 +339,8 @@ enum evlist_ctl_cmd {
> EVLIST_CTL_CMD_UNSUPPORTED = 0,
> EVLIST_CTL_CMD_ENABLE,
> EVLIST_CTL_CMD_DISABLE,
> + EVLIST_CTL_CMD_ENABLE_EVSEL,
> + EVLIST_CTL_CMD_DISABLE_EVSEL,
> EVLIST_CTL_CMD_ACK,
> EVLIST_CTL_CMD_SNAPSHOT,
> };
> --
> 2.26.2
>
--
- Arnaldo
next prev parent reply other threads:[~2020-12-15 15:15 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-10 20:43 [PATCHv2 0/3] perf tools: Allow to enable/disable events via control pipe Jiri Olsa
2020-12-10 20:43 ` [PATCH 1/3] perf tools: Add evlist__disable_evsel/evlist__enable_evsel Jiri Olsa
2020-12-15 15:17 ` Arnaldo Carvalho de Melo
2020-12-10 20:43 ` [PATCH 2/3] perf tools: Allow to enable/disable events via control file Jiri Olsa
2020-12-15 15:14 ` Arnaldo Carvalho de Melo [this message]
2020-12-15 15:24 ` Jiri Olsa
2020-12-15 16:03 ` Arnaldo Carvalho de Melo
2020-12-15 16:18 ` Jiri Olsa
2020-12-15 16:27 ` Arnaldo Carvalho de Melo
2020-12-10 20:43 ` [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands Jiri Olsa
2020-12-11 3:27 ` Namhyung Kim
2020-12-15 15:23 ` Arnaldo Carvalho de Melo
2020-12-15 15:59 ` Jiri Olsa
2020-12-15 16:09 ` Arnaldo Carvalho de Melo
2020-12-15 16:27 ` Jiri Olsa
-- strict thread matches above, loose matches on Subject: below --
2020-12-06 17:05 [PATCH 0/3] perf tools: Allow to enable/disable events via control pipe Jiri Olsa
2020-12-06 17:05 ` [PATCH 2/3] perf tools: Allow to enable/disable events via control file Jiri Olsa
2020-12-07 17:02 ` Alexei Budankov
2020-12-10 16:24 ` Jiri Olsa
2020-12-10 17:15 ` Arnaldo Carvalho de Melo
2020-12-10 17:19 ` Arnaldo Carvalho de Melo
2020-12-10 18:06 ` Jiri Olsa
2020-12-10 18:20 ` Alexei Budankov
2020-12-10 18:27 ` Arnaldo Carvalho de Melo
2020-12-10 18:32 ` Alexei 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=20201215151413.GE252952@kernel.org \
--to=acme@kernel.org \
--cc=a.p.zijlstra@chello.nl \
--cc=abudankov@huawei.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=mpetlan@redhat.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.