* [PATCHv2 0/3] perf tools: Allow to enable/disable events via control pipe @ 2020-12-10 20:43 Jiri Olsa 2020-12-10 20:43 ` [PATCH 1/3] perf tools: Add evlist__disable_evsel/evlist__enable_evsel Jiri Olsa ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Jiri Olsa @ 2020-12-10 20:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov hi, adding support to enable/disable specific events via control file via following commands: # echo enable-sched:sched_process_fork > control # echo disabled-sched:sched_process_fork > control v2 changes: - added acks - change list to evlist [Arnaldo] - add evlist-verbose command [Arnaldo] - add '' to enale-/disable- error message The code is available in here: git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git perf/control thanks, jirka --- Jiri Olsa (3): perf tools: Add evlist__disable_evsel/evlist__enable_evsel perf tools: Allow to enable/disable events via control file perf tools: Add evlist/evlist-verbose control commands tools/perf/builtin-record.c | 4 ++++ tools/perf/builtin-stat.c | 4 ++++ tools/perf/util/evlist.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- tools/perf/util/evlist.h | 10 ++++++++++ 4 files changed, 128 insertions(+), 4 deletions(-) ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/3] perf tools: Add evlist__disable_evsel/evlist__enable_evsel 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 ` 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-10 20:43 ` [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands Jiri Olsa 2 siblings, 1 reply; 15+ messages in thread From: Jiri Olsa @ 2020-12-10 20:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, Alexei Budankov, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian Adding interface to enable/disable single event in the evlist based on its name. It will be used later in new control enable/disable interface. Keeping the evlist::enabled true when one or more events are enabled so the toggle can work properly and toggle evlist to disabled state. Acked-by: Namhyung Kim <namhyung@kernel.org> Acked-by: Alexei Budankov <abudankov@huawei.com> Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/util/evlist.c | 69 ++++++++++++++++++++++++++++++++++++++-- tools/perf/util/evlist.h | 2 ++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 493819173a8e..70aff26612a9 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -370,7 +370,30 @@ bool evsel__cpu_iter_skip(struct evsel *ev, int cpu) return true; } -void evlist__disable(struct evlist *evlist) +static int evsel__strcmp(struct evsel *pos, char *evsel_name) +{ + if (!evsel_name) + return 0; + if (evsel__is_dummy_event(pos)) + return 1; + return strcmp(pos->name, evsel_name); +} + +static int evlist__is_enabled(struct evlist *evlist) +{ + struct evsel *pos; + + evlist__for_each_entry(evlist, pos) { + if (!evsel__is_group_leader(pos) || !pos->core.fd) + continue; + /* If at least one event is enabled, evlist is enabled. */ + if (!pos->disabled) + return true; + } + return false; +} + +static void __evlist__disable(struct evlist *evlist, char *evsel_name) { struct evsel *pos; struct affinity affinity; @@ -386,6 +409,8 @@ void evlist__disable(struct evlist *evlist) affinity__set(&affinity, cpu); evlist__for_each_entry(evlist, pos) { + if (evsel__strcmp(pos, evsel_name)) + continue; if (evsel__cpu_iter_skip(pos, cpu)) continue; if (pos->disabled || !evsel__is_group_leader(pos) || !pos->core.fd) @@ -403,15 +428,34 @@ void evlist__disable(struct evlist *evlist) affinity__cleanup(&affinity); evlist__for_each_entry(evlist, pos) { + if (evsel__strcmp(pos, evsel_name)) + continue; if (!evsel__is_group_leader(pos) || !pos->core.fd) continue; pos->disabled = true; } - evlist->enabled = false; + /* + * If we disabled only single event, we need to check + * the enabled state of the evlist manually. + */ + if (evsel_name) + evlist->enabled = evlist__is_enabled(evlist); + else + evlist->enabled = false; +} + +void evlist__disable(struct evlist *evlist) +{ + __evlist__disable(evlist, NULL); +} + +void evlist__disable_evsel(struct evlist *evlist, char *evsel_name) +{ + __evlist__disable(evlist, evsel_name); } -void evlist__enable(struct evlist *evlist) +static void __evlist__enable(struct evlist *evlist, char *evsel_name) { struct evsel *pos; struct affinity affinity; @@ -424,6 +468,8 @@ void evlist__enable(struct evlist *evlist) affinity__set(&affinity, cpu); evlist__for_each_entry(evlist, pos) { + if (evsel__strcmp(pos, evsel_name)) + continue; if (evsel__cpu_iter_skip(pos, cpu)) continue; if (!evsel__is_group_leader(pos) || !pos->core.fd) @@ -433,14 +479,31 @@ void evlist__enable(struct evlist *evlist) } affinity__cleanup(&affinity); evlist__for_each_entry(evlist, pos) { + if (evsel__strcmp(pos, evsel_name)) + continue; if (!evsel__is_group_leader(pos) || !pos->core.fd) continue; pos->disabled = false; } + /* + * Even single event sets the 'enabled' for evlist, + * so the toggle can work properly and toggle to + * 'disabled' state. + */ evlist->enabled = true; } +void evlist__enable(struct evlist *evlist) +{ + __evlist__enable(evlist, NULL); +} + +void evlist__enable_evsel(struct evlist *evlist, char *evsel_name) +{ + __evlist__enable(evlist, evsel_name); +} + void evlist__toggle_enable(struct evlist *evlist) { (evlist->enabled ? evlist__disable : evlist__enable)(evlist); diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 9b0c795736bb..1aae75895dea 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -186,6 +186,8 @@ size_t evlist__mmap_size(unsigned long pages); void evlist__disable(struct evlist *evlist); void evlist__enable(struct evlist *evlist); void evlist__toggle_enable(struct evlist *evlist); +void evlist__disable_evsel(struct evlist *evlist, char *evsel_name); +void evlist__enable_evsel(struct evlist *evlist, char *evsel_name); int evlist__enable_event_idx(struct evlist *evlist, struct evsel *evsel, int idx); -- 2.26.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] perf tools: Add evlist__disable_evsel/evlist__enable_evsel 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 0 siblings, 0 replies; 15+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-12-15 15:17 UTC (permalink / raw) To: Jiri Olsa Cc: Namhyung Kim, Alexei Budankov, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian Em Thu, Dec 10, 2020 at 09:43:28PM +0100, Jiri Olsa escreveu: > Adding interface to enable/disable single event in the > evlist based on its name. It will be used later in new > control enable/disable interface. > > Keeping the evlist::enabled true when one or more events > are enabled so the toggle can work properly and toggle > evlist to disabled state. Thanks, applied. - Arnaldo > Acked-by: Namhyung Kim <namhyung@kernel.org> > Acked-by: Alexei Budankov <abudankov@huawei.com> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/util/evlist.c | 69 ++++++++++++++++++++++++++++++++++++++-- > tools/perf/util/evlist.h | 2 ++ > 2 files changed, 68 insertions(+), 3 deletions(-) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 493819173a8e..70aff26612a9 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -370,7 +370,30 @@ bool evsel__cpu_iter_skip(struct evsel *ev, int cpu) > return true; > } > > -void evlist__disable(struct evlist *evlist) > +static int evsel__strcmp(struct evsel *pos, char *evsel_name) > +{ > + if (!evsel_name) > + return 0; > + if (evsel__is_dummy_event(pos)) > + return 1; > + return strcmp(pos->name, evsel_name); > +} > + > +static int evlist__is_enabled(struct evlist *evlist) > +{ > + struct evsel *pos; > + > + evlist__for_each_entry(evlist, pos) { > + if (!evsel__is_group_leader(pos) || !pos->core.fd) > + continue; > + /* If at least one event is enabled, evlist is enabled. */ > + if (!pos->disabled) > + return true; > + } > + return false; > +} > + > +static void __evlist__disable(struct evlist *evlist, char *evsel_name) > { > struct evsel *pos; > struct affinity affinity; > @@ -386,6 +409,8 @@ void evlist__disable(struct evlist *evlist) > affinity__set(&affinity, cpu); > > evlist__for_each_entry(evlist, pos) { > + if (evsel__strcmp(pos, evsel_name)) > + continue; > if (evsel__cpu_iter_skip(pos, cpu)) > continue; > if (pos->disabled || !evsel__is_group_leader(pos) || !pos->core.fd) > @@ -403,15 +428,34 @@ void evlist__disable(struct evlist *evlist) > > affinity__cleanup(&affinity); > evlist__for_each_entry(evlist, pos) { > + if (evsel__strcmp(pos, evsel_name)) > + continue; > if (!evsel__is_group_leader(pos) || !pos->core.fd) > continue; > pos->disabled = true; > } > > - evlist->enabled = false; > + /* > + * If we disabled only single event, we need to check > + * the enabled state of the evlist manually. > + */ > + if (evsel_name) > + evlist->enabled = evlist__is_enabled(evlist); > + else > + evlist->enabled = false; > +} > + > +void evlist__disable(struct evlist *evlist) > +{ > + __evlist__disable(evlist, NULL); > +} > + > +void evlist__disable_evsel(struct evlist *evlist, char *evsel_name) > +{ > + __evlist__disable(evlist, evsel_name); > } > > -void evlist__enable(struct evlist *evlist) > +static void __evlist__enable(struct evlist *evlist, char *evsel_name) > { > struct evsel *pos; > struct affinity affinity; > @@ -424,6 +468,8 @@ void evlist__enable(struct evlist *evlist) > affinity__set(&affinity, cpu); > > evlist__for_each_entry(evlist, pos) { > + if (evsel__strcmp(pos, evsel_name)) > + continue; > if (evsel__cpu_iter_skip(pos, cpu)) > continue; > if (!evsel__is_group_leader(pos) || !pos->core.fd) > @@ -433,14 +479,31 @@ void evlist__enable(struct evlist *evlist) > } > affinity__cleanup(&affinity); > evlist__for_each_entry(evlist, pos) { > + if (evsel__strcmp(pos, evsel_name)) > + continue; > if (!evsel__is_group_leader(pos) || !pos->core.fd) > continue; > pos->disabled = false; > } > > + /* > + * Even single event sets the 'enabled' for evlist, > + * so the toggle can work properly and toggle to > + * 'disabled' state. > + */ > evlist->enabled = true; > } > > +void evlist__enable(struct evlist *evlist) > +{ > + __evlist__enable(evlist, NULL); > +} > + > +void evlist__enable_evsel(struct evlist *evlist, char *evsel_name) > +{ > + __evlist__enable(evlist, evsel_name); > +} > + > void evlist__toggle_enable(struct evlist *evlist) > { > (evlist->enabled ? evlist__disable : evlist__enable)(evlist); > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index 9b0c795736bb..1aae75895dea 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -186,6 +186,8 @@ size_t evlist__mmap_size(unsigned long pages); > void evlist__disable(struct evlist *evlist); > void evlist__enable(struct evlist *evlist); > void evlist__toggle_enable(struct evlist *evlist); > +void evlist__disable_evsel(struct evlist *evlist, char *evsel_name); > +void evlist__enable_evsel(struct evlist *evlist, char *evsel_name); > > int evlist__enable_event_idx(struct evlist *evlist, struct evsel *evsel, int idx); > > -- > 2.26.2 > -- - Arnaldo ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] perf tools: Allow to enable/disable events via control file 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-10 20:43 ` Jiri Olsa 2020-12-15 15:14 ` Arnaldo Carvalho de Melo 2020-12-10 20:43 ` [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands Jiri Olsa 2 siblings, 1 reply; 15+ messages in thread From: Jiri Olsa @ 2020-12-10 20:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Adding new control events to enable/disable specific event. The interface string for control file are: 'enable-<EVENT NAME>' 'disable-<EVENT NAME>' 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file 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 2020-12-15 15:24 ` Jiri Olsa 0 siblings, 1 reply; 15+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-12-15 15:14 UTC (permalink / raw) To: Jiri Olsa Cc: Namhyung Kim, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file 2020-12-15 15:14 ` Arnaldo Carvalho de Melo @ 2020-12-15 15:24 ` Jiri Olsa 2020-12-15 16:03 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 15+ messages in thread From: Jiri Olsa @ 2020-12-15 15:24 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Namhyung Kim, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov On Tue, Dec 15, 2020 at 12:14:13PM -0300, Arnaldo Carvalho de Melo wrote: > 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? the whole 'enable-' is prefix for command that enables specific event following '-' starts the event name > > 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? yep, that should be possible to add > > 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. the '-' determines that there's event name following, pure 'enable' switches on everything jirka ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file 2020-12-15 15:24 ` Jiri Olsa @ 2020-12-15 16:03 ` Arnaldo Carvalho de Melo 2020-12-15 16:18 ` Jiri Olsa 0 siblings, 1 reply; 15+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-12-15 16:03 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Namhyung Kim, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Em Tue, Dec 15, 2020 at 04:24:20PM +0100, Jiri Olsa escreveu: > On Tue, Dec 15, 2020 at 12:14:13PM -0300, Arnaldo Carvalho de Melo wrote: > > 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? > > the whole 'enable-' is prefix for command that enables specific event > following '-' starts the event name > > > > > 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? > > yep, that should be possible to add > > > > > 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. > > the '-' determines that there's event name following, > pure 'enable' switches on everything I see it, but why not use the more natural ' ' space to separate the command from its arguments? Just like in a bash command line, say? I.e. why not: enable to enable everything, and: enable sched:sched_switch To enable just the "sched:sched_switch" event? - Arnaldo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file 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 0 siblings, 1 reply; 15+ messages in thread From: Jiri Olsa @ 2020-12-15 16:18 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Namhyung Kim, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov On Tue, Dec 15, 2020 at 01:03:32PM -0300, Arnaldo Carvalho de Melo wrote: > Em Tue, Dec 15, 2020 at 04:24:20PM +0100, Jiri Olsa escreveu: > > On Tue, Dec 15, 2020 at 12:14:13PM -0300, Arnaldo Carvalho de Melo wrote: > > > 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? > > > > the whole 'enable-' is prefix for command that enables specific event > > following '-' starts the event name > > > > > > > > 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? > > > > yep, that should be possible to add > > > > > > > > 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. > > > > the '-' determines that there's event name following, > > pure 'enable' switches on everything > > I see it, but why not use the more natural ' ' space to separate the > command from its arguments? Just like in a bash command line, say? > > I.e. why not: > > enable > > to enable everything, and: > > enable sched:sched_switch > > To enable just the "sched:sched_switch" event? right, that's we discussed in the other patch thread, I'll make the change thanks, jirka ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] perf tools: Allow to enable/disable events via control file 2020-12-15 16:18 ` Jiri Olsa @ 2020-12-15 16:27 ` Arnaldo Carvalho de Melo 0 siblings, 0 replies; 15+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-12-15 16:27 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, Namhyung Kim, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Em Tue, Dec 15, 2020 at 05:18:38PM +0100, Jiri Olsa escreveu: > On Tue, Dec 15, 2020 at 01:03:32PM -0300, Arnaldo Carvalho de Melo wrote: > > I see it, but why not use the more natural ' ' space to separate the > > command from its arguments? Just like in a bash command line, say? > > I.e. why not: > > enable > > to enable everything, and: > > enable sched:sched_switch > > To enable just the "sched:sched_switch" event? > right, that's we discussed in the other patch thread, > I'll make the change This is a new way to control perf, its important that we try to reuse the same concepts as in the pre-existing forms of interaction, so as to reduce the learning curve for using this control mode. I.e. this 'enable' should be as equivalent to the -e argument as possible, for what makes sense for a pre-existing, already configured event. For _adding_ new ones, that we probably will want next, then its even more important to reuse the same -e parser :-) Thanks! - Arnaldo ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands 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-10 20:43 ` [PATCH 2/3] perf tools: Allow to enable/disable events via control file Jiri Olsa @ 2020-12-10 20:43 ` Jiri Olsa 2020-12-11 3:27 ` Namhyung Kim 2020-12-15 15:23 ` Arnaldo Carvalho de Melo 2 siblings, 2 replies; 15+ messages in thread From: Jiri Olsa @ 2020-12-10 20:43 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Adding new control events to display all evlist events. The interface string for control file is 'evlist' and 'evlist-verbose'. When evlist is received, perf will scan and print current evlist into perf record terminal. When evlist-verbose is received perf will scan and print current evlist details (like perf evlist -v) into perf record terminal. Example session: terminal 1: # mkfifo control ack perf.pipe # perf record --control=fifo:control,ack -e 'sched:*' terminal 2: # echo evlist > control terminal 1: # perf record --control=fifo:control,ack -e 'sched:*' ... sched:sched_kthread_stop sched:sched_kthread_stop_ret sched:sched_waking sched:sched_wakeup sched:sched_wakeup_new sched:sched_switch sched:sched_migrate_task sched:sched_process_free sched:sched_process_exit ... terminal 2: # echo evlist-vebose > control terminal 1: ... sched:sched_kthread_stop: type: 2, size: 120, config: 0x145, \ { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: \ 1, sample_id_all: 1, exclude_guest: 1 sched:sched_kthread_stop_ret: type: 2, size: 120, config: 0x144 \ , { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, \ sample_id_all: 1, exclude_guest: 1 ... This new evlist command is handy to get real event names when wildcards are used. The evlist-verbose is handy to check on actually enabled perf_event_attr values. 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 | 15 +++++++++++++++ tools/perf/util/evlist.h | 4 ++++ 4 files changed, 23 insertions(+) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 582b8fba012c..d40406880722 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -1951,6 +1951,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) case EVLIST_CTL_CMD_UNSUPPORTED: case EVLIST_CTL_CMD_ENABLE_EVSEL: case EVLIST_CTL_CMD_DISABLE_EVSEL: + case EVLIST_CTL_CMD_EVLIST: + case EVLIST_CTL_CMD_EVLIST_VERBOSE: default: break; } diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 6a21fb665008..425e2a8ebde6 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -592,6 +592,8 @@ static void process_evlist(struct evlist *evlist, unsigned int interval) case EVLIST_CTL_CMD_UNSUPPORTED: case EVLIST_CTL_CMD_ENABLE_EVSEL: case EVLIST_CTL_CMD_DISABLE_EVSEL: + case EVLIST_CTL_CMD_EVLIST: + case EVLIST_CTL_CMD_EVLIST_VERBOSE: default: break; } diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 729c98d10628..571d2ad61f4a 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -24,6 +24,7 @@ #include "bpf-event.h" #include "util/string2.h" #include "util/perf_api_probe.h" +#include "util/evsel_fprintf.h" #include <signal.h> #include <unistd.h> #include <sched.h> @@ -1931,6 +1932,12 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd, (sizeof(EVLIST_CTL_CMD_SNAPSHOT_TAG)-1))) { *cmd = EVLIST_CTL_CMD_SNAPSHOT; pr_debug("is snapshot\n"); + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG, + (sizeof(EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG)-1))) { + *cmd = EVLIST_CTL_CMD_EVLIST_VERBOSE; + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_EVLIST_TAG, + (sizeof(EVLIST_CTL_CMD_EVLIST_TAG)-1))) { + *cmd = EVLIST_CTL_CMD_EVLIST; } } @@ -1954,6 +1961,7 @@ int evlist__ctlfd_ack(struct evlist *evlist) int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) { + struct perf_attr_details details = { .verbose = false, }; int err = 0; char cmd_data[EVLIST_CTL_CMD_MAX_LEN]; int ctlfd_pos = evlist->ctl_fd.pos; @@ -1995,6 +2003,13 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) pr_info("failed: can't find '%s' event\n", evsel_name); } break; + case EVLIST_CTL_CMD_EVLIST_VERBOSE: + details.verbose = true; + __fallthrough; + case EVLIST_CTL_CMD_EVLIST: + evlist__for_each_entry(evlist, evsel) + evsel__fprintf(evsel, &details, stderr); + 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 e4e8ff8831a3..7892f084632d 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -332,6 +332,8 @@ struct evsel *evlist__reset_weak_group(struct evlist *evlist, struct evsel *evse #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_EVLIST_TAG "evlist" +#define EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG "evlist-verbose" #define EVLIST_CTL_CMD_MAX_LEN 64 @@ -343,6 +345,8 @@ enum evlist_ctl_cmd { EVLIST_CTL_CMD_DISABLE_EVSEL, EVLIST_CTL_CMD_ACK, EVLIST_CTL_CMD_SNAPSHOT, + EVLIST_CTL_CMD_EVLIST, + EVLIST_CTL_CMD_EVLIST_VERBOSE, }; int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close); -- 2.26.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands 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 1 sibling, 0 replies; 15+ messages in thread From: Namhyung Kim @ 2020-12-11 3:27 UTC (permalink / raw) To: Jiri Olsa Cc: Arnaldo Carvalho de Melo, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Hi Jiri, On Fri, Dec 11, 2020 at 5:43 AM Jiri Olsa <jolsa@kernel.org> wrote: > > Adding new control events to display all evlist events. > > The interface string for control file is 'evlist' and > 'evlist-verbose'. > > When evlist is received, perf will scan and print current > evlist into perf record terminal. > > When evlist-verbose is received perf will scan and print > current evlist details (like perf evlist -v) into perf > record terminal. > > Example session: > > terminal 1: > # mkfifo control ack perf.pipe > # perf record --control=fifo:control,ack -e 'sched:*' > > terminal 2: > # echo evlist > control > > terminal 1: > # perf record --control=fifo:control,ack -e 'sched:*' > ... > sched:sched_kthread_stop > sched:sched_kthread_stop_ret > sched:sched_waking > sched:sched_wakeup > sched:sched_wakeup_new > sched:sched_switch > sched:sched_migrate_task > sched:sched_process_free > sched:sched_process_exit > ... > > terminal 2: > # echo evlist-vebose > control > > terminal 1: > ... > sched:sched_kthread_stop: type: 2, size: 120, config: 0x145, \ > { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: \ > 1, sample_id_all: 1, exclude_guest: 1 > sched:sched_kthread_stop_ret: type: 2, size: 120, config: 0x144 \ > , { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, \ > sample_id_all: 1, exclude_guest: 1 > ... > > This new evlist command is handy to get real event names when > wildcards are used. > > The evlist-verbose is handy to check on actually enabled perf_event_attr > values. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> Acked-by: Namhyung Kim <namhyung@kernel.org> Thanks, Namhyung > --- > tools/perf/builtin-record.c | 2 ++ > tools/perf/builtin-stat.c | 2 ++ > tools/perf/util/evlist.c | 15 +++++++++++++++ > tools/perf/util/evlist.h | 4 ++++ > 4 files changed, 23 insertions(+) > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 582b8fba012c..d40406880722 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -1951,6 +1951,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) > case EVLIST_CTL_CMD_UNSUPPORTED: > case EVLIST_CTL_CMD_ENABLE_EVSEL: > case EVLIST_CTL_CMD_DISABLE_EVSEL: > + case EVLIST_CTL_CMD_EVLIST: > + case EVLIST_CTL_CMD_EVLIST_VERBOSE: > default: > break; > } > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index 6a21fb665008..425e2a8ebde6 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -592,6 +592,8 @@ static void process_evlist(struct evlist *evlist, unsigned int interval) > case EVLIST_CTL_CMD_UNSUPPORTED: > case EVLIST_CTL_CMD_ENABLE_EVSEL: > case EVLIST_CTL_CMD_DISABLE_EVSEL: > + case EVLIST_CTL_CMD_EVLIST: > + case EVLIST_CTL_CMD_EVLIST_VERBOSE: > default: > break; > } > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 729c98d10628..571d2ad61f4a 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -24,6 +24,7 @@ > #include "bpf-event.h" > #include "util/string2.h" > #include "util/perf_api_probe.h" > +#include "util/evsel_fprintf.h" > #include <signal.h> > #include <unistd.h> > #include <sched.h> > @@ -1931,6 +1932,12 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd, > (sizeof(EVLIST_CTL_CMD_SNAPSHOT_TAG)-1))) { > *cmd = EVLIST_CTL_CMD_SNAPSHOT; > pr_debug("is snapshot\n"); > + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG, > + (sizeof(EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG)-1))) { > + *cmd = EVLIST_CTL_CMD_EVLIST_VERBOSE; > + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_EVLIST_TAG, > + (sizeof(EVLIST_CTL_CMD_EVLIST_TAG)-1))) { > + *cmd = EVLIST_CTL_CMD_EVLIST; > } > } > > @@ -1954,6 +1961,7 @@ int evlist__ctlfd_ack(struct evlist *evlist) > > int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) > { > + struct perf_attr_details details = { .verbose = false, }; > int err = 0; > char cmd_data[EVLIST_CTL_CMD_MAX_LEN]; > int ctlfd_pos = evlist->ctl_fd.pos; > @@ -1995,6 +2003,13 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) > pr_info("failed: can't find '%s' event\n", evsel_name); > } > break; > + case EVLIST_CTL_CMD_EVLIST_VERBOSE: > + details.verbose = true; > + __fallthrough; > + case EVLIST_CTL_CMD_EVLIST: > + evlist__for_each_entry(evlist, evsel) > + evsel__fprintf(evsel, &details, stderr); > + 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 e4e8ff8831a3..7892f084632d 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -332,6 +332,8 @@ struct evsel *evlist__reset_weak_group(struct evlist *evlist, struct evsel *evse > #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_EVLIST_TAG "evlist" > +#define EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG "evlist-verbose" > > #define EVLIST_CTL_CMD_MAX_LEN 64 > > @@ -343,6 +345,8 @@ enum evlist_ctl_cmd { > EVLIST_CTL_CMD_DISABLE_EVSEL, > EVLIST_CTL_CMD_ACK, > EVLIST_CTL_CMD_SNAPSHOT, > + EVLIST_CTL_CMD_EVLIST, > + EVLIST_CTL_CMD_EVLIST_VERBOSE, > }; > > int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close); > -- > 2.26.2 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands 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 1 sibling, 1 reply; 15+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-12-15 15:23 UTC (permalink / raw) To: Jiri Olsa Cc: lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Em Thu, Dec 10, 2020 at 09:43:30PM +0100, Jiri Olsa escreveu: > Adding new control events to display all evlist events. > > The interface string for control file is 'evlist' and > 'evlist-verbose'. Can't we pass args to such commands? Then its just one event, i.e. "evlist", and -v can be passed to it. i.e.: The commands would be: evlist That produces: terminal 2: # echo evlist > control terminal 1: # perf record --control=fifo:control,ack -e 'sched:*' ... sched:sched_kthread_stop sched:sched_kthread_stop_ret sched:sched_waking And 'evlist -v', that produces: terminal 2: # echo "evlist -v" > control terminal 1: ... sched:sched_kthread_stop: type: 2, size: 120, config: 0x145, \ { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: \ 1, sample_id_all: 1, exclude_guest: 1 sched:sched_kthread_stop_ret: type: 2, size: 120, config: 0x144 \ , { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, \ sample_id_all: 1, exclude_guest: 1 ... I think we could even change things such that we pass a file descriptor for cmd_evlist to use, passing the argv received from the control file, etc. With this in place we could reuse more stuff and allow using this control file to obtain information such as 'perf report --header-only', etc. echo "report --header-only" > control would get us the same thing as 'perf report --header-only' for an existing perf.data file: # perf report --header-only # ======== # captured on : Tue Dec 15 12:21:23 2020 # header version : 1 # data offset : 432 # data size : 1648 # feat offset : 2080 # hostname : five # os release : 5.10.0-rc7+ # perf version : 5.10.rc6.gc56d2601b5d0 # arch : x86_64 # nrcpus online : 24 # nrcpus avail : 24 # cpudesc : AMD Ryzen 9 3900X 12-Core Processor # cpuid : AuthenticAMD,23,113,0 # total memory : 32884432 kB # cmdline : /home/acme/bin/perf record ls # event : name = cycles:u, , id = { 85540, 85541, 85542, 85543, 85544, 85545, 85546, 85547, 85548, 85549, 85550, 85551, 85552, 85553, 85554, 85555, 85556, 85557, 85558, 85559, 85560, 85561, 85562, 85563 }, size = 120, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, read_format = ID, disabled = 1, inherit = 1, exclude_kernel = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, ksymbol = 1, bpf_event = 1 # CPU_TOPOLOGY info available, use -I to display # NUMA_TOPOLOGY info available, use -I to display # pmu mappings: amd_df = 8, software = 1, ibs_op = 11, power = 14, ibs_fetch = 10, uprobe = 7, cpu = 4, amd_iommu_0 = 12, breakpoint = 5, amd_l3 = 9, tracepoint = 2, kprobe = 6, msr = 13 # CACHE info available, use -I to display # time of first sample : 12184.494971 # time of last sample : 12184.495496 # sample duration : 0.525 ms # MEM_TOPOLOGY info available, use -I to display # cpu pmu capabilities: max_precise=0 # missing features: TRACING_DATA BRANCH_STACK GROUP_DESC AUXTRACE STAT CLOCKID DIR_FORMAT COMPRESSED CLOCK_DATA # ======== # I.e. users would discover that using this control file is as easy as working with perf.data files or with the pipe mode, all the three ways of interacting with perf would use the same command interface arguments. - Arnaldo > When evlist is received, perf will scan and print current > evlist into perf record terminal. > > When evlist-verbose is received perf will scan and print > current evlist details (like perf evlist -v) into perf > record terminal. > > Example session: > > terminal 1: > # mkfifo control ack perf.pipe > # perf record --control=fifo:control,ack -e 'sched:*' > > terminal 2: > # echo evlist > control > > terminal 1: > # perf record --control=fifo:control,ack -e 'sched:*' > ... > sched:sched_kthread_stop > sched:sched_kthread_stop_ret > sched:sched_waking > sched:sched_wakeup > sched:sched_wakeup_new > sched:sched_switch > sched:sched_migrate_task > sched:sched_process_free > sched:sched_process_exit > ... > > terminal 2: > # echo evlist-vebose > control > > terminal 1: > ... > sched:sched_kthread_stop: type: 2, size: 120, config: 0x145, \ > { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: \ > 1, sample_id_all: 1, exclude_guest: 1 > sched:sched_kthread_stop_ret: type: 2, size: 120, config: 0x144 \ > , { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, \ > sample_id_all: 1, exclude_guest: 1 > ... > > This new evlist command is handy to get real event names when > wildcards are used. > > The evlist-verbose is handy to check on actually enabled perf_event_attr > values. > > 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 | 15 +++++++++++++++ > tools/perf/util/evlist.h | 4 ++++ > 4 files changed, 23 insertions(+) > > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index 582b8fba012c..d40406880722 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -1951,6 +1951,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) > case EVLIST_CTL_CMD_UNSUPPORTED: > case EVLIST_CTL_CMD_ENABLE_EVSEL: > case EVLIST_CTL_CMD_DISABLE_EVSEL: > + case EVLIST_CTL_CMD_EVLIST: > + case EVLIST_CTL_CMD_EVLIST_VERBOSE: > default: > break; > } > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index 6a21fb665008..425e2a8ebde6 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -592,6 +592,8 @@ static void process_evlist(struct evlist *evlist, unsigned int interval) > case EVLIST_CTL_CMD_UNSUPPORTED: > case EVLIST_CTL_CMD_ENABLE_EVSEL: > case EVLIST_CTL_CMD_DISABLE_EVSEL: > + case EVLIST_CTL_CMD_EVLIST: > + case EVLIST_CTL_CMD_EVLIST_VERBOSE: > default: > break; > } > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 729c98d10628..571d2ad61f4a 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -24,6 +24,7 @@ > #include "bpf-event.h" > #include "util/string2.h" > #include "util/perf_api_probe.h" > +#include "util/evsel_fprintf.h" > #include <signal.h> > #include <unistd.h> > #include <sched.h> > @@ -1931,6 +1932,12 @@ static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd, > (sizeof(EVLIST_CTL_CMD_SNAPSHOT_TAG)-1))) { > *cmd = EVLIST_CTL_CMD_SNAPSHOT; > pr_debug("is snapshot\n"); > + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG, > + (sizeof(EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG)-1))) { > + *cmd = EVLIST_CTL_CMD_EVLIST_VERBOSE; > + } else if (!strncmp(cmd_data, EVLIST_CTL_CMD_EVLIST_TAG, > + (sizeof(EVLIST_CTL_CMD_EVLIST_TAG)-1))) { > + *cmd = EVLIST_CTL_CMD_EVLIST; > } > } > > @@ -1954,6 +1961,7 @@ int evlist__ctlfd_ack(struct evlist *evlist) > > int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) > { > + struct perf_attr_details details = { .verbose = false, }; > int err = 0; > char cmd_data[EVLIST_CTL_CMD_MAX_LEN]; > int ctlfd_pos = evlist->ctl_fd.pos; > @@ -1995,6 +2003,13 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd) > pr_info("failed: can't find '%s' event\n", evsel_name); > } > break; > + case EVLIST_CTL_CMD_EVLIST_VERBOSE: > + details.verbose = true; > + __fallthrough; > + case EVLIST_CTL_CMD_EVLIST: > + evlist__for_each_entry(evlist, evsel) > + evsel__fprintf(evsel, &details, stderr); > + 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 e4e8ff8831a3..7892f084632d 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -332,6 +332,8 @@ struct evsel *evlist__reset_weak_group(struct evlist *evlist, struct evsel *evse > #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_EVLIST_TAG "evlist" > +#define EVLIST_CTL_CMD_EVLIST_VERBOSE_TAG "evlist-verbose" > > #define EVLIST_CTL_CMD_MAX_LEN 64 > > @@ -343,6 +345,8 @@ enum evlist_ctl_cmd { > EVLIST_CTL_CMD_DISABLE_EVSEL, > EVLIST_CTL_CMD_ACK, > EVLIST_CTL_CMD_SNAPSHOT, > + EVLIST_CTL_CMD_EVLIST, > + EVLIST_CTL_CMD_EVLIST_VERBOSE, > }; > > int evlist__parse_control(const char *str, int *ctl_fd, int *ctl_fd_ack, bool *ctl_fd_close); > -- > 2.26.2 > -- - Arnaldo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands 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 0 siblings, 1 reply; 15+ messages in thread From: Jiri Olsa @ 2020-12-15 15:59 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov On Tue, Dec 15, 2020 at 12:23:43PM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Dec 10, 2020 at 09:43:30PM +0100, Jiri Olsa escreveu: > > Adding new control events to display all evlist events. > > > > The interface string for control file is 'evlist' and > > 'evlist-verbose'. > > Can't we pass args to such commands? > > Then its just one event, i.e. "evlist", and -v can be passed to it. it's possible but it adds another processing to the single record thread where we want to be fast but I agree it could be helpful to get the other output that evsel__fprintf can print, and we already call evsel__fprintf, so it's just a matter of setting 'struct perf_attr_details' properly I need to check, but perhaps we could use ' ' instead of '-' and have: echo 'evlist -v' > control echo 'evlist -g' > control echo 'evlist -F' > control and have: echo 'enable cycles' > control instead of: echo 'enable-cycles' > control I'd like to avoid any elaborate parsing logic.. how about that? > > i.e.: > > The commands would be: > > evlist > > That produces: > > > terminal 2: > # echo evlist > control > > terminal 1: > # perf record --control=fifo:control,ack -e 'sched:*' > ... > sched:sched_kthread_stop > sched:sched_kthread_stop_ret > sched:sched_waking > > And 'evlist -v', that produces: > > terminal 2: > # echo "evlist -v" > control > > terminal 1: > ... > sched:sched_kthread_stop: type: 2, size: 120, config: 0x145, \ > { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: \ > 1, sample_id_all: 1, exclude_guest: 1 > sched:sched_kthread_stop_ret: type: 2, size: 120, config: 0x144 \ > , { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, \ > sample_id_all: 1, exclude_guest: 1 > ... > > I think we could even change things such that we pass a file descriptor > for cmd_evlist to use, passing the argv received from the control file, > etc. hum, so perf evlist opens/reads perf.data, which we do not have ready at that point in perf record > > With this in place we could reuse more stuff and allow using this > control file to obtain information such as 'perf report --header-only', > etc. > > echo "report --header-only" > control would get us the same thing as > 'perf report --header-only' for an existing perf.data file: all those header data are written when record is exiting or dumped for switch output, so we don't have that data at that moment control command is received > > > # perf report --header-only > # ======== > # captured on : Tue Dec 15 12:21:23 2020 > # header version : 1 > # data offset : 432 > # data size : 1648 > # feat offset : 2080 > # hostname : five > # os release : 5.10.0-rc7+ > # perf version : 5.10.rc6.gc56d2601b5d0 > # arch : x86_64 > # nrcpus online : 24 > # nrcpus avail : 24 > # cpudesc : AMD Ryzen 9 3900X 12-Core Processor > # cpuid : AuthenticAMD,23,113,0 > # total memory : 32884432 kB > # cmdline : /home/acme/bin/perf record ls > # event : name = cycles:u, , id = { 85540, 85541, 85542, 85543, 85544, 85545, 85546, 85547, 85548, 85549, 85550, 85551, 85552, 85553, 85554, 85555, 85556, 85557, 85558, 85559, 85560, 85561, 85562, 85563 }, size = 120, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, read_format = ID, disabled = 1, inherit = 1, exclude_kernel = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, ksymbol = 1, bpf_event = 1 > # CPU_TOPOLOGY info available, use -I to display > # NUMA_TOPOLOGY info available, use -I to display > # pmu mappings: amd_df = 8, software = 1, ibs_op = 11, power = 14, ibs_fetch = 10, uprobe = 7, cpu = 4, amd_iommu_0 = 12, breakpoint = 5, amd_l3 = 9, tracepoint = 2, kprobe = 6, msr = 13 > # CACHE info available, use -I to display > # time of first sample : 12184.494971 > # time of last sample : 12184.495496 > # sample duration : 0.525 ms > # MEM_TOPOLOGY info available, use -I to display > # cpu pmu capabilities: max_precise=0 > # missing features: TRACING_DATA BRANCH_STACK GROUP_DESC AUXTRACE STAT CLOCKID DIR_FORMAT COMPRESSED CLOCK_DATA > # ======== > # > > I.e. users would discover that using this control file is as easy as > working with perf.data files or with the pipe mode, all the three ways > of interacting with perf would use the same command interface arguments. yep, I agree we can mimic the similar arguments, but I doubt we can easily reuse the same code for that jirka ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands 2020-12-15 15:59 ` Jiri Olsa @ 2020-12-15 16:09 ` Arnaldo Carvalho de Melo 2020-12-15 16:27 ` Jiri Olsa 0 siblings, 1 reply; 15+ messages in thread From: Arnaldo Carvalho de Melo @ 2020-12-15 16:09 UTC (permalink / raw) To: Jiri Olsa Cc: Jiri Olsa, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov Em Tue, Dec 15, 2020 at 04:59:11PM +0100, Jiri Olsa escreveu: > On Tue, Dec 15, 2020 at 12:23:43PM -0300, Arnaldo Carvalho de Melo wrote: > > Em Thu, Dec 10, 2020 at 09:43:30PM +0100, Jiri Olsa escreveu: > > > Adding new control events to display all evlist events. > > > > > > The interface string for control file is 'evlist' and > > > 'evlist-verbose'. > > > > Can't we pass args to such commands? > > > > Then its just one event, i.e. "evlist", and -v can be passed to it. > > it's possible but it adds another processing to the single record thread > where we want to be fast > but I agree it could be helpful to get the other output that evsel__fprintf > can print, and we already call evsel__fprintf, so it's just a matter of > setting 'struct perf_attr_details' properly > I need to check, but perhaps we could use ' ' instead of '-' and have: > > echo 'evlist -v' > control > echo 'evlist -g' > control > echo 'evlist -F' > control > and have: > echo 'enable cycles' > control > instead of: > echo 'enable-cycles' > control > I'd like to avoid any elaborate parsing logic.. > how about that? Use space to separate command from its arguments, keep the same experience as: [acme@quaco ~]$ perf evlist -h Usage: perf evlist [<options>] -f, --force don't complain, do it -F, --freq Show the sample frequency -g, --group Show event group information -i, --input <file> Input file name -v, --verbose Show all event attr details --trace-fields Show tracepoint fields [acme@quaco ~]$ and: echo "evlist arguments" ? Fits the bill :-) The experience users have on existing commands, the same arguments, etc. Even -h can have its uses, i.e. has this daemon support for some option or is it an old version? - Arnaldo > > > > i.e.: > > > > The commands would be: > > > > evlist > > > > That produces: > > > > > > terminal 2: > > # echo evlist > control > > > > terminal 1: > > # perf record --control=fifo:control,ack -e 'sched:*' > > ... > > sched:sched_kthread_stop > > sched:sched_kthread_stop_ret > > sched:sched_waking > > > > And 'evlist -v', that produces: > > > > terminal 2: > > # echo "evlist -v" > control > > > > terminal 1: > > ... > > sched:sched_kthread_stop: type: 2, size: 120, config: 0x145, \ > > { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: \ > > 1, sample_id_all: 1, exclude_guest: 1 > > sched:sched_kthread_stop_ret: type: 2, size: 120, config: 0x144 \ > > , { sample_period, sample_freq }: 1, sample_type: IP|TID|TIME|CPU \ > > |PERIOD|RAW|IDENTIFIER, read_format: ID, disabled: 1, inherit: 1, \ > > sample_id_all: 1, exclude_guest: 1 > > ... > > > > I think we could even change things such that we pass a file descriptor > > for cmd_evlist to use, passing the argv received from the control file, > > etc. > > hum, so perf evlist opens/reads perf.data, which we do not have ready > at that point in perf record > > > > > With this in place we could reuse more stuff and allow using this > > control file to obtain information such as 'perf report --header-only', > > etc. > > > > echo "report --header-only" > control would get us the same thing as > > 'perf report --header-only' for an existing perf.data file: > > all those header data are written when record is exiting or dumped > for switch output, so we don't have that data at that moment control > command is received > > > > > > > # perf report --header-only > > # ======== > > # captured on : Tue Dec 15 12:21:23 2020 > > # header version : 1 > > # data offset : 432 > > # data size : 1648 > > # feat offset : 2080 > > # hostname : five > > # os release : 5.10.0-rc7+ > > # perf version : 5.10.rc6.gc56d2601b5d0 > > # arch : x86_64 > > # nrcpus online : 24 > > # nrcpus avail : 24 > > # cpudesc : AMD Ryzen 9 3900X 12-Core Processor > > # cpuid : AuthenticAMD,23,113,0 > > # total memory : 32884432 kB > > # cmdline : /home/acme/bin/perf record ls > > # event : name = cycles:u, , id = { 85540, 85541, 85542, 85543, 85544, 85545, 85546, 85547, 85548, 85549, 85550, 85551, 85552, 85553, 85554, 85555, 85556, 85557, 85558, 85559, 85560, 85561, 85562, 85563 }, size = 120, { sample_period, sample_freq } = 4000, sample_type = IP|TID|TIME|PERIOD, read_format = ID, disabled = 1, inherit = 1, exclude_kernel = 1, mmap = 1, comm = 1, freq = 1, enable_on_exec = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1, ksymbol = 1, bpf_event = 1 > > # CPU_TOPOLOGY info available, use -I to display > > # NUMA_TOPOLOGY info available, use -I to display > > # pmu mappings: amd_df = 8, software = 1, ibs_op = 11, power = 14, ibs_fetch = 10, uprobe = 7, cpu = 4, amd_iommu_0 = 12, breakpoint = 5, amd_l3 = 9, tracepoint = 2, kprobe = 6, msr = 13 > > # CACHE info available, use -I to display > > # time of first sample : 12184.494971 > > # time of last sample : 12184.495496 > > # sample duration : 0.525 ms > > # MEM_TOPOLOGY info available, use -I to display > > # cpu pmu capabilities: max_precise=0 > > # missing features: TRACING_DATA BRANCH_STACK GROUP_DESC AUXTRACE STAT CLOCKID DIR_FORMAT COMPRESSED CLOCK_DATA > > # ======== > > # > > > > I.e. users would discover that using this control file is as easy as > > working with perf.data files or with the pipe mode, all the three ways > > of interacting with perf would use the same command interface arguments. > > yep, I agree we can mimic the similar arguments, but I doubt we > can easily reuse the same code for that > > jirka > -- - Arnaldo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] perf tools: Add evlist/evlist-verbose control commands 2020-12-15 16:09 ` Arnaldo Carvalho de Melo @ 2020-12-15 16:27 ` Jiri Olsa 0 siblings, 0 replies; 15+ messages in thread From: Jiri Olsa @ 2020-12-15 16:27 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, lkml, Peter Zijlstra, Ingo Molnar, Mark Rutland, Namhyung Kim, Alexander Shishkin, Michael Petlan, Ian Rogers, Stephane Eranian, Alexei Budankov On Tue, Dec 15, 2020 at 01:09:33PM -0300, Arnaldo Carvalho de Melo wrote: SNIP > > instead of: > > > echo 'enable-cycles' > control > > > I'd like to avoid any elaborate parsing logic.. > > > how about that? > > Use space to separate command from its arguments, keep the same > experience as: > > [acme@quaco ~]$ perf evlist -h > > Usage: perf evlist [<options>] > > -f, --force don't complain, do it > -F, --freq Show the sample frequency > -g, --group Show event group information > -i, --input <file> Input file name > -v, --verbose Show all event attr details > --trace-fields Show tracepoint fields > > [acme@quaco ~]$ > > and: > > echo "evlist arguments" > > ? > > Fits the bill :-) ok > > The experience users have on existing commands, the same arguments, etc. > > Even -h can have its uses, i.e. has this daemon support for some option > or is it an old version? Alexei just suggested to add 'stop' control command and use it instead of kill signal, which is good idea but other than that daemon just opens the control file and shows its path jirka ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2020-12-15 16:29 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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.