* [PATCH v2 01/11] perf evlist: introduce control file descriptors
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
@ 2020-05-06 18:18 ` Alexey Budankov
2020-05-06 18:19 ` [PATCH v2 02/11] perf evlist: implement control command handling functions Alexey Budankov
` (9 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Define and initialize control file descriptors.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/util/evlist.c | 3 +++
tools/perf/util/evlist.h | 3 +++
2 files changed, 6 insertions(+)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 0a0b760d6948..2db4bedc4f81 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -63,6 +63,9 @@ void evlist__init(struct evlist *evlist, struct perf_cpu_map *cpus,
perf_evlist__set_maps(&evlist->core, cpus, threads);
evlist->workload.pid = -1;
evlist->bkw_mmap_state = BKW_MMAP_NOTREADY;
+ evlist->ctl_fd = -1;
+ evlist->ctl_fd_ack = -1;
+ evlist->ctl_fd_pos = -1;
}
struct evlist *evlist__new(void)
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index b6f325dfb4d2..62f259d89b41 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -74,6 +74,9 @@ struct evlist {
pthread_t th;
volatile int done;
} thread;
+ int ctl_fd;
+ int ctl_fd_ack;
+ int ctl_fd_pos;
};
struct evsel_str_handler {
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 02/11] perf evlist: implement control command handling functions
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
2020-05-06 18:18 ` [PATCH v2 01/11] perf evlist: introduce control file descriptors Alexey Budankov
@ 2020-05-06 18:19 ` Alexey Budankov
2020-05-06 20:21 ` Arnaldo Carvalho de Melo
2020-05-06 18:20 ` [PATCH v2 03/11] perf stat: factor out event handling loop into a function Alexey Budankov
` (8 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:19 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Implement functions of initialization, finalization and processing
of control commands coming from control file descriptors.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/util/evlist.c | 100 +++++++++++++++++++++++++++++++++++++++
tools/perf/util/evlist.h | 12 +++++
2 files changed, 112 insertions(+)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 2db4bedc4f81..e086c846ef3a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1707,3 +1707,103 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
}
return leader;
}
+
+int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack)
+{
+ if (ctl_fd == -1) {
+ pr_debug("Control descriptor is not initialized\n");
+ return 0;
+ }
+
+ evlist->ctl_fd_pos = perf_evlist__add_pollfd(&evlist->core, ctl_fd, NULL, POLLIN);
+ if (evlist->ctl_fd_pos < 0) {
+ evlist->ctl_fd_pos = -1;
+ pr_err("Failed to add ctl fd entry: %m\n");
+ return -1;
+ }
+
+ evlist->ctl_fd = ctl_fd;
+ evlist->ctl_fd_ack = ctl_fd_ack;
+
+ return 0;
+}
+
+int perf_evlist__finalize_ctlfd(struct evlist *evlist)
+{
+ if (evlist->ctl_fd_pos == -1)
+ return 0;
+
+ evlist->core.pollfd.entries[evlist->ctl_fd_pos].fd = -1;
+ evlist->ctl_fd_pos = -1;
+ evlist->ctl_fd_ack = -1;
+ evlist->ctl_fd = -1;
+
+ return 0;
+}
+
+static int perf_evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
+{
+ int err;
+ char buf[2];
+
+ err = read(evlist->ctl_fd, &buf, sizeof(buf));
+ if (err > 0)
+ *cmd = buf[0];
+ else if (err == -1)
+ pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd);
+
+ return err;
+}
+
+static int perf_evlist__ctlfd_ack(struct evlist *evlist)
+{
+ int err;
+ char buf[2] = {CTL_CMD_ACK, '\n'};
+
+ if (evlist->ctl_fd_ack == -1)
+ return 0;
+
+ err = write(evlist->ctl_fd_ack, buf, sizeof(buf));
+ if (err == -1)
+ pr_err("failed to write to ctl_ack_fd %d: %m\n", evlist->ctl_fd_ack);
+
+ return err;
+}
+
+int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
+{
+ int err = 0;
+ int ctlfd_pos = evlist->ctl_fd_pos;
+ struct pollfd *entries = evlist->core.pollfd.entries;
+
+ if (!entries[ctlfd_pos].revents)
+ return 0;
+
+ if (entries[ctlfd_pos].revents & POLLIN) {
+ err = perf_evlist__ctlfd_recv(evlist, cmd);
+ if (err > 0) {
+ switch (*cmd) {
+ case CTL_CMD_ENABLE:
+ evlist__enable(evlist);
+ break;
+ case CTL_CMD_DISABLE:
+ evlist__disable(evlist);
+ break;
+ case CTL_CMD_ACK:
+ case CTL_CMD_UNSUPPORTED:
+ default:
+ pr_debug("ctlfd: unsupported %d\n", *cmd);
+ break;
+ }
+ if (!(*cmd == CTL_CMD_ACK || *cmd == CTL_CMD_UNSUPPORTED))
+ perf_evlist__ctlfd_ack(evlist);
+ }
+ }
+
+ if (entries[ctlfd_pos].revents & (POLLHUP | POLLERR))
+ perf_evlist__finalize_ctlfd(evlist);
+ else
+ entries[ctlfd_pos].revents = 0;
+
+ return err;
+}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 62f259d89b41..84386850c290 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -358,4 +358,16 @@ void perf_evlist__force_leader(struct evlist *evlist);
struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
struct evsel *evsel,
bool close);
+
+enum evlist_ctl_cmd {
+ CTL_CMD_UNSUPPORTED = 0,
+ CTL_CMD_ENABLE = 'e',
+ CTL_CMD_DISABLE = 'd',
+ CTL_CMD_ACK = 'a'
+};
+
+int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
+int perf_evlist__finalize_ctlfd(struct evlist *evlist);
+int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
+
#endif /* __PERF_EVLIST_H */
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 02/11] perf evlist: implement control command handling functions
2020-05-06 18:19 ` [PATCH v2 02/11] perf evlist: implement control command handling functions Alexey Budankov
@ 2020-05-06 20:21 ` Arnaldo Carvalho de Melo
2020-05-07 8:32 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-06 20:21 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Wed, May 06, 2020 at 09:19:22PM +0300, Alexey Budankov escreveu:
>
> Implement functions of initialization, finalization and processing
> of control commands coming from control file descriptors.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> tools/perf/util/evlist.c | 100 +++++++++++++++++++++++++++++++++++++++
> tools/perf/util/evlist.h | 12 +++++
> 2 files changed, 112 insertions(+)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 2db4bedc4f81..e086c846ef3a 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1707,3 +1707,103 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
> }
> return leader;
> }
> +
> +int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack)
> +{
> + if (ctl_fd == -1) {
> + pr_debug("Control descriptor is not initialized\n");
> + return 0;
> + }
> +
> + evlist->ctl_fd_pos = perf_evlist__add_pollfd(&evlist->core, ctl_fd, NULL, POLLIN);
> + if (evlist->ctl_fd_pos < 0) {
> + evlist->ctl_fd_pos = -1;
> + pr_err("Failed to add ctl fd entry: %m\n");
> + return -1;
> + }
> +
> + evlist->ctl_fd = ctl_fd;
> + evlist->ctl_fd_ack = ctl_fd_ack;
> +
> + return 0;
> +}
> +
> +int perf_evlist__finalize_ctlfd(struct evlist *evlist)
> +{
> + if (evlist->ctl_fd_pos == -1)
> + return 0;
> +
> + evlist->core.pollfd.entries[evlist->ctl_fd_pos].fd = -1;
> + evlist->ctl_fd_pos = -1;
> + evlist->ctl_fd_ack = -1;
> + evlist->ctl_fd = -1;
> +
> + return 0;
> +}
> +
> +static int perf_evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
> +{
> + int err;
> + char buf[2];
> +
> + err = read(evlist->ctl_fd, &buf, sizeof(buf));
> + if (err > 0)
> + *cmd = buf[0];
> + else if (err == -1)
> + pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd);
> +
> + return err;
> +}
> +
> +static int perf_evlist__ctlfd_ack(struct evlist *evlist)
> +{
> + int err;
> + char buf[2] = {CTL_CMD_ACK, '\n'};
> +
> + if (evlist->ctl_fd_ack == -1)
> + return 0;
> +
> + err = write(evlist->ctl_fd_ack, buf, sizeof(buf));
> + if (err == -1)
> + pr_err("failed to write to ctl_ack_fd %d: %m\n", evlist->ctl_fd_ack);
> +
> + return err;
> +}
> +
> +int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
> +{
> + int err = 0;
> + int ctlfd_pos = evlist->ctl_fd_pos;
> + struct pollfd *entries = evlist->core.pollfd.entries;
> +
> + if (!entries[ctlfd_pos].revents)
> + return 0;
> +
> + if (entries[ctlfd_pos].revents & POLLIN) {
> + err = perf_evlist__ctlfd_recv(evlist, cmd);
> + if (err > 0) {
> + switch (*cmd) {
> + case CTL_CMD_ENABLE:
> + evlist__enable(evlist);
> + break;
> + case CTL_CMD_DISABLE:
> + evlist__disable(evlist);
> + break;
> + case CTL_CMD_ACK:
> + case CTL_CMD_UNSUPPORTED:
> + default:
> + pr_debug("ctlfd: unsupported %d\n", *cmd);
> + break;
> + }
> + if (!(*cmd == CTL_CMD_ACK || *cmd == CTL_CMD_UNSUPPORTED))
> + perf_evlist__ctlfd_ack(evlist);
> + }
> + }
> +
> + if (entries[ctlfd_pos].revents & (POLLHUP | POLLERR))
> + perf_evlist__finalize_ctlfd(evlist);
> + else
> + entries[ctlfd_pos].revents = 0;
> +
> + return err;
> +}
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 62f259d89b41..84386850c290 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -358,4 +358,16 @@ void perf_evlist__force_leader(struct evlist *evlist);
> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
> struct evsel *evsel,
> bool close);
> +
> +enum evlist_ctl_cmd {
> + CTL_CMD_UNSUPPORTED = 0,
> + CTL_CMD_ENABLE = 'e',
> + CTL_CMD_DISABLE = 'd',
> + CTL_CMD_ACK = 'a'
> +};
Can we make this a string, I think we'll eventually ask for lots more
stuff, like asking for a --switch-output snapshot with --overwrite,
reconfiguring events to increase/decrease frequency, etc, interfacing
with PERF_EVENT_IOC_MODIFY_ATTRIBUTES, PERF_EVENT_IOC_SET_FILTER, etc.
This will also allow us to have parameters, etc, wdyt?
Also please since these are events that deal with 'struct evlist', name
them with the evlist__ prefix, not the perf_evlist__ one, as those
should be used with 'struct perf_evlist', i.e. the one in libperf
(tools/lib/perf/).
Right now this is inconsistent, we did it that way to minimize
disruption of the codebase when moving things from tools/perf/ to
tools/lib/perf/, but this confuses things and I just did a
s/perf_evsel__/evsel__) for things dealing with 'struct evsel', so lets
not add new ones with the wrong prefix, eventually we'll have perf_ only
for things in libperf.
> +
> +int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
> +int perf_evlist__finalize_ctlfd(struct evlist *evlist);
> +int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
> +
> #endif /* __PERF_EVLIST_H */
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 02/11] perf evlist: implement control command handling functions
2020-05-06 20:21 ` Arnaldo Carvalho de Melo
@ 2020-05-07 8:32 ` Alexey Budankov
2020-05-07 17:01 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-07 8:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 06.05.2020 23:21, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2020 at 09:19:22PM +0300, Alexey Budankov escreveu:
>>
>> Implement functions of initialization, finalization and processing
>> of control commands coming from control file descriptors.
>>
>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> ---
>> tools/perf/util/evlist.c | 100 +++++++++++++++++++++++++++++++++++++++
>> tools/perf/util/evlist.h | 12 +++++
>> 2 files changed, 112 insertions(+)
<SNIP>
>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>> index 62f259d89b41..84386850c290 100644
>> --- a/tools/perf/util/evlist.h
>> +++ b/tools/perf/util/evlist.h
>> @@ -358,4 +358,16 @@ void perf_evlist__force_leader(struct evlist *evlist);
>> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
>> struct evsel *evsel,
>> bool close);
>> +
>> +enum evlist_ctl_cmd {
>> + CTL_CMD_UNSUPPORTED = 0,
>> + CTL_CMD_ENABLE = 'e',
>> + CTL_CMD_DISABLE = 'd',
>> + CTL_CMD_ACK = 'a'
>> +};
>
> Can we make this a string, I think we'll eventually ask for lots more
Like this?
#define EVLIST__CTL_CMD_ENABLE "enable"
#define EVLIST__CTL_CMD_DISABLE "disable"
#define EVLIST__CTL_CMD_ACK "ack"
> stuff, like asking for a --switch-output snapshot with --overwrite,
> reconfiguring events to increase/decrease frequency, etc, interfacing
> with PERF_EVENT_IOC_MODIFY_ATTRIBUTES, PERF_EVENT_IOC_SET_FILTER, etc.
>
> This will also allow us to have parameters, etc, wdyt?
Being a part of this patch the extension will implement configurability
that potentially could never be used.
Switch from int to string commands of variable length belongs to
the patches also implementing usage of that string commands.
>
> Also please since these are events that deal with 'struct evlist', name
> them with the evlist__ prefix, not the perf_evlist__ one, as those
> should be used with 'struct perf_evlist', i.e. the one in libperf
> (tools/lib/perf/).
Accepted in v3.
>
> Right now this is inconsistent, we did it that way to minimize
> disruption of the codebase when moving things from tools/perf/ to
> tools/lib/perf/, but this confuses things and I just did a
> s/perf_evsel__/evsel__) for things dealing with 'struct evsel', so lets
> not add new ones with the wrong prefix, eventually we'll have perf_ only
> for things in libperf.
>
>> +
>> +int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
>> +int perf_evlist__finalize_ctlfd(struct evlist *evlist);
>> +int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
>> +
>> #endif /* __PERF_EVLIST_H */
>> --
>> 2.24.1
>>
>>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 02/11] perf evlist: implement control command handling functions
2020-05-07 8:32 ` Alexey Budankov
@ 2020-05-07 17:01 ` Arnaldo Carvalho de Melo
2020-05-07 17:51 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-07 17:01 UTC (permalink / raw)
To: Alexey Budankov
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Ingo Molnar, Andi Kleen,
linux-kernel
Em Thu, May 07, 2020 at 11:32:53AM +0300, Alexey Budankov escreveu:
>
> On 06.05.2020 23:21, Arnaldo Carvalho de Melo wrote:
> > Em Wed, May 06, 2020 at 09:19:22PM +0300, Alexey Budankov escreveu:
> >>
> >> Implement functions of initialization, finalization and processing
> >> of control commands coming from control file descriptors.
> >>
> >> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> >> ---
> >> tools/perf/util/evlist.c | 100 +++++++++++++++++++++++++++++++++++++++
> >> tools/perf/util/evlist.h | 12 +++++
> >> 2 files changed, 112 insertions(+)
>
> <SNIP>
>
> >> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> >> index 62f259d89b41..84386850c290 100644
> >> --- a/tools/perf/util/evlist.h
> >> +++ b/tools/perf/util/evlist.h
> >> @@ -358,4 +358,16 @@ void perf_evlist__force_leader(struct evlist *evlist);
> >> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
> >> struct evsel *evsel,
> >> bool close);
> >> +
> >> +enum evlist_ctl_cmd {
> >> + CTL_CMD_UNSUPPORTED = 0,
> >> + CTL_CMD_ENABLE = 'e',
> >> + CTL_CMD_DISABLE = 'd',
> >> + CTL_CMD_ACK = 'a'
> >> +};
> >
> > Can we make this a string, I think we'll eventually ask for lots more
>
> Like this?
>
> #define EVLIST__CTL_CMD_ENABLE "enable"
> #define EVLIST__CTL_CMD_DISABLE "disable"
> #define EVLIST__CTL_CMD_ACK "ack"
Yeah
> > stuff, like asking for a --switch-output snapshot with --overwrite,
> > reconfiguring events to increase/decrease frequency, etc, interfacing
> > with PERF_EVENT_IOC_MODIFY_ATTRIBUTES, PERF_EVENT_IOC_SET_FILTER, etc.
> >
> > This will also allow us to have parameters, etc, wdyt?
>
> Being a part of this patch the extension will implement configurability
> that potentially could never be used.
>
> Switch from int to string commands of variable length belongs to
> the patches also implementing usage of that string commands.
Well, at that point we would have to support both, i.e. the way you're
doing now with integers, and as strings, otherwise 3rd party tooling
(vtune? :)) using this interface would break.
I.e. this is like the syscall interface.
So if we have "enable" now we can go ahead and forever understand that
as "please enable this evlist", but in the future we can extend it and
pass parameters to it, to control how that enablement will take place,
perhaps with a delay, etc.
> > Also please since these are events that deal with 'struct evlist', name
> > them with the evlist__ prefix, not the perf_evlist__ one, as those
> > should be used with 'struct perf_evlist', i.e. the one in libperf
> > (tools/lib/perf/).
>
> Accepted in v3.
>
> >
> > Right now this is inconsistent, we did it that way to minimize
> > disruption of the codebase when moving things from tools/perf/ to
> > tools/lib/perf/, but this confuses things and I just did a
> > s/perf_evsel__/evsel__) for things dealing with 'struct evsel', so lets
> > not add new ones with the wrong prefix, eventually we'll have perf_ only
> > for things in libperf.
> >
> >> +
> >> +int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
> >> +int perf_evlist__finalize_ctlfd(struct evlist *evlist);
> >> +int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
> >> +
> >> #endif /* __PERF_EVLIST_H */
> >> --
> >> 2.24.1
> >>
> >>
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 02/11] perf evlist: implement control command handling functions
2020-05-07 17:01 ` Arnaldo Carvalho de Melo
@ 2020-05-07 17:51 ` Alexey Budankov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-07 17:51 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 07.05.2020 20:01, Arnaldo Carvalho de Melo wrote:
> Em Thu, May 07, 2020 at 11:32:53AM +0300, Alexey Budankov escreveu:
>>
>> On 06.05.2020 23:21, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, May 06, 2020 at 09:19:22PM +0300, Alexey Budankov escreveu:
>>>>
>>>> Implement functions of initialization, finalization and processing
>>>> of control commands coming from control file descriptors.
>>>>
>>>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>>>> ---
>>>> tools/perf/util/evlist.c | 100 +++++++++++++++++++++++++++++++++++++++
>>>> tools/perf/util/evlist.h | 12 +++++
>>>> 2 files changed, 112 insertions(+)
>>
>> <SNIP>
>>
>>>> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
>>>> index 62f259d89b41..84386850c290 100644
>>>> --- a/tools/perf/util/evlist.h
>>>> +++ b/tools/perf/util/evlist.h
>>>> @@ -358,4 +358,16 @@ void perf_evlist__force_leader(struct evlist *evlist);
>>>> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
>>>> struct evsel *evsel,
>>>> bool close);
>>>> +
>>>> +enum evlist_ctl_cmd {
>>>> + CTL_CMD_UNSUPPORTED = 0,
>>>> + CTL_CMD_ENABLE = 'e',
>>>> + CTL_CMD_DISABLE = 'd',
>>>> + CTL_CMD_ACK = 'a'
>>>> +};
>>>
>>> Can we make this a string, I think we'll eventually ask for lots more
>>
>> Like this?
>>
>> #define EVLIST__CTL_CMD_ENABLE "enable"
>> #define EVLIST__CTL_CMD_DISABLE "disable"
>> #define EVLIST__CTL_CMD_ACK "ack"
>
> Yeah
Well, ok. Accepted in v3.
Command becomes of variable length of chars + \n,
in comparison to current single char + \n.
>
>>> stuff, like asking for a --switch-output snapshot with --overwrite,
>>> reconfiguring events to increase/decrease frequency, etc, interfacing
>>> with PERF_EVENT_IOC_MODIFY_ATTRIBUTES, PERF_EVENT_IOC_SET_FILTER, etc.
>>>
>>> This will also allow us to have parameters, etc, wdyt?
>>
>> Being a part of this patch the extension will implement configurability
>> that potentially could never be used.
>>
>> Switch from int to string commands of variable length belongs to
>> the patches also implementing usage of that string commands.
>
> Well, at that point we would have to support both, i.e. the way you're
> doing now with integers, and as strings, otherwise 3rd party tooling
> (vtune? :)) using this interface would break.
>
> I.e. this is like the syscall interface.
>
> So if we have "enable" now we can go ahead and forever understand that
> as "please enable this evlist", but in the future we can extend it and
> pass parameters to it, to control how that enablement will take place,
> perhaps with a delay, etc.
>
>>> Also please since these are events that deal with 'struct evlist', name
>>> them with the evlist__ prefix, not the perf_evlist__ one, as those
>>> should be used with 'struct perf_evlist', i.e. the one in libperf
>>> (tools/lib/perf/).
>>
>> Accepted in v3.
>>
>>>
>>> Right now this is inconsistent, we did it that way to minimize
>>> disruption of the codebase when moving things from tools/perf/ to
>>> tools/lib/perf/, but this confuses things and I just did a
>>> s/perf_evsel__/evsel__) for things dealing with 'struct evsel', so lets
>>> not add new ones with the wrong prefix, eventually we'll have perf_ only
>>> for things in libperf.
>>>
>>>> +
>>>> +int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
>>>> +int perf_evlist__finalize_ctlfd(struct evlist *evlist);
>>>> +int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
>>>> +
>>>> #endif /* __PERF_EVLIST_H */
>>>> --
>>>> 2.24.1
>>>>
>>>>
>>>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 03/11] perf stat: factor out event handling loop into a function
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
2020-05-06 18:18 ` [PATCH v2 01/11] perf evlist: introduce control file descriptors Alexey Budankov
2020-05-06 18:19 ` [PATCH v2 02/11] perf evlist: implement control command handling functions Alexey Budankov
@ 2020-05-06 18:20 ` Alexey Budankov
2020-05-06 18:20 ` [PATCH v2 04/11] perf stat: extend -D,--delay option with -1 value Alexey Budankov
` (7 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:20 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Factor out event handling loop into handle_events() function.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-stat.c | 85 +++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 35 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e0c1ad23c768..9775b0905146 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -371,6 +371,16 @@ static void process_interval(void)
print_counters(&rs, 0, NULL);
}
+static bool print_interval_and_stop(struct perf_stat_config *config, int *times)
+{
+ if (config->interval) {
+ process_interval();
+ if (interval_count && !(--(*times)))
+ return true;
+ }
+ return false;
+}
+
static void enable_counters(void)
{
if (stat_config.initial_delay)
@@ -436,6 +446,42 @@ static bool is_target_alive(struct target *_target,
return false;
}
+static int handle_events(pid_t pid, struct perf_stat_config *config)
+{
+ pid_t child = 0;
+ bool res, stop = false;
+ struct timespec time_to_sleep;
+ int sleep_time, status = 0, times = config->times;
+
+ if (config->interval)
+ sleep_time = config->interval;
+ else if (config->timeout)
+ sleep_time = config->timeout;
+ else
+ sleep_time = 1000;
+
+ time_to_sleep.tv_sec = sleep_time / MSEC_PER_SEC;
+ time_to_sleep.tv_nsec = (sleep_time % MSEC_PER_SEC) * NSEC_PER_MSEC;
+
+ do {
+ if (pid != -1)
+ child = waitpid(pid, &status, WNOHANG);
+ if (child || stop || done)
+ break;
+ nanosleep(&time_to_sleep, NULL);
+ if (pid == -1)
+ stop = !is_target_alive(&target, evsel_list->core.threads);
+ if (config->timeout) {
+ stop = !stop ? true : stop;
+ } else {
+ res = print_interval_and_stop(config, ×);
+ stop = !stop ? res : stop;
+ }
+ } while (1);
+
+ return status;
+}
+
enum counter_recovery {
COUNTER_SKIP,
COUNTER_RETRY,
@@ -494,12 +540,10 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
static int __run_perf_stat(int argc, const char **argv, int run_idx)
{
int interval = stat_config.interval;
- int times = stat_config.times;
int timeout = stat_config.timeout;
char msg[BUFSIZ];
unsigned long long t0, t1;
struct evsel *counter;
- struct timespec ts;
size_t l;
int status = 0;
const bool forks = (argc > 0);
@@ -508,17 +552,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
int i, cpu;
bool second_pass = false;
- if (interval) {
- ts.tv_sec = interval / USEC_PER_MSEC;
- ts.tv_nsec = (interval % USEC_PER_MSEC) * NSEC_PER_MSEC;
- } else if (timeout) {
- ts.tv_sec = timeout / USEC_PER_MSEC;
- ts.tv_nsec = (timeout % USEC_PER_MSEC) * NSEC_PER_MSEC;
- } else {
- ts.tv_sec = 1;
- ts.tv_nsec = 0;
- }
-
if (forks) {
if (perf_evlist__prepare_workload(evsel_list, &target, argv, is_pipe,
workload_exec_failed_signal) < 0) {
@@ -675,16 +708,9 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
perf_evlist__start_workload(evsel_list);
enable_counters();
- if (interval || timeout) {
- while (!waitpid(child_pid, &status, WNOHANG)) {
- nanosleep(&ts, NULL);
- if (timeout)
- break;
- process_interval();
- if (interval_count && !(--times))
- break;
- }
- }
+ if (interval || timeout)
+ handle_events(child_pid, &stat_config);
+
if (child_pid != -1) {
if (timeout)
kill(child_pid, SIGTERM);
@@ -701,18 +727,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
psignal(WTERMSIG(status), argv[0]);
} else {
enable_counters();
- while (!done) {
- nanosleep(&ts, NULL);
- if (!is_target_alive(&target, evsel_list->core.threads))
- break;
- if (timeout)
- break;
- if (interval) {
- process_interval();
- if (interval_count && !(--times))
- break;
- }
- }
+ handle_events(-1, &stat_config);
}
disable_counters();
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 04/11] perf stat: extend -D,--delay option with -1 value
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (2 preceding siblings ...)
2020-05-06 18:20 ` [PATCH v2 03/11] perf stat: factor out event handling loop into a function Alexey Budankov
@ 2020-05-06 18:20 ` Alexey Budankov
2020-05-06 20:12 ` Arnaldo Carvalho de Melo
2020-05-06 18:21 ` [PATCH v2 05/11] perf stat: implement control commands handling Alexey Budankov
` (6 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:20 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Extend -D,--delay option with -1 value to start monitoring with
events disabled to be enabled later by enable command provided
via control file descriptor.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-stat.c | 18 ++++++++++++++----
tools/perf/util/evlist.h | 3 +++
tools/perf/util/stat.h | 2 +-
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 9775b0905146..bda777ca0420 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -383,16 +383,26 @@ static bool print_interval_and_stop(struct perf_stat_config *config, int *times)
static void enable_counters(void)
{
- if (stat_config.initial_delay)
+ if (stat_config.initial_delay < 0) {
+ pr_info(PERF_EVLIST__DISABLED_MSG);
+ return;
+ }
+
+ if (stat_config.initial_delay > 0) {
+ pr_info(PERF_EVLIST__DISABLED_MSG);
usleep(stat_config.initial_delay * USEC_PER_MSEC);
+ }
/*
* We need to enable counters only if:
* - we don't have tracee (attaching to task or cpu)
* - we have initial delay configured
*/
- if (!target__none(&target) || stat_config.initial_delay)
+ if (!target__none(&target) || stat_config.initial_delay) {
evlist__enable(evsel_list);
+ if (stat_config.initial_delay > 0)
+ pr_info(PERF_EVLIST__ENABLED_MSG);
+ }
}
static void disable_counters(void)
@@ -929,8 +939,8 @@ static struct option stat_options[] = {
"aggregate counts per thread", AGGR_THREAD),
OPT_SET_UINT(0, "per-node", &stat_config.aggr_mode,
"aggregate counts per numa node", AGGR_NODE),
- OPT_UINTEGER('D', "delay", &stat_config.initial_delay,
- "ms to wait before starting measurement after program start"),
+ OPT_INTEGER('D', "delay", &stat_config.initial_delay,
+ "ms to wait before starting measurement after program start (-1: start with events disabled"),
OPT_CALLBACK_NOOPT(0, "metric-only", &stat_config.metric_only, NULL,
"Only print computed metrics. No raw values", enable_metric_only),
OPT_BOOLEAN(0, "topdown", &topdown_run,
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 84386850c290..874ecf068ac9 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -366,6 +366,9 @@ enum evlist_ctl_cmd {
CTL_CMD_ACK = 'a'
};
+#define PERF_EVLIST__ENABLED_MSG "Events enabled\n"
+#define PERF_EVLIST__DISABLED_MSG "Events disabled\n"
+
int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
int perf_evlist__finalize_ctlfd(struct evlist *evlist);
int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index b4fdfaa7f2c0..027b9dcd902f 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -113,7 +113,7 @@ struct perf_stat_config {
FILE *output;
unsigned int interval;
unsigned int timeout;
- unsigned int initial_delay;
+ int initial_delay;
unsigned int unit_width;
unsigned int metric_only_len;
int times;
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 04/11] perf stat: extend -D,--delay option with -1 value
2020-05-06 18:20 ` [PATCH v2 04/11] perf stat: extend -D,--delay option with -1 value Alexey Budankov
@ 2020-05-06 20:12 ` Arnaldo Carvalho de Melo
2020-05-07 8:34 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-06 20:12 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Wed, May 06, 2020 at 09:20:54PM +0300, Alexey Budankov escreveu:
>
> Extend -D,--delay option with -1 value to start monitoring with
> events disabled to be enabled later by enable command provided
> via control file descriptor.
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> tools/perf/builtin-stat.c | 18 ++++++++++++++----
> tools/perf/util/evlist.h | 3 +++
> tools/perf/util/stat.h | 2 +-
You forgot to update tools/perf/Documentation/perf-stat.h
> 3 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 9775b0905146..bda777ca0420 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -383,16 +383,26 @@ static bool print_interval_and_stop(struct perf_stat_config *config, int *times)
>
> static void enable_counters(void)
> {
> - if (stat_config.initial_delay)
> + if (stat_config.initial_delay < 0) {
> + pr_info(PERF_EVLIST__DISABLED_MSG);
> + return;
> + }
> +
> + if (stat_config.initial_delay > 0) {
> + pr_info(PERF_EVLIST__DISABLED_MSG);
> usleep(stat_config.initial_delay * USEC_PER_MSEC);
> + }
>
> /*
> * We need to enable counters only if:
> * - we don't have tracee (attaching to task or cpu)
> * - we have initial delay configured
> */
> - if (!target__none(&target) || stat_config.initial_delay)
> + if (!target__none(&target) || stat_config.initial_delay) {
> evlist__enable(evsel_list);
> + if (stat_config.initial_delay > 0)
> + pr_info(PERF_EVLIST__ENABLED_MSG);
> + }
> }
>
> static void disable_counters(void)
> @@ -929,8 +939,8 @@ static struct option stat_options[] = {
> "aggregate counts per thread", AGGR_THREAD),
> OPT_SET_UINT(0, "per-node", &stat_config.aggr_mode,
> "aggregate counts per numa node", AGGR_NODE),
> - OPT_UINTEGER('D', "delay", &stat_config.initial_delay,
> - "ms to wait before starting measurement after program start"),
> + OPT_INTEGER('D', "delay", &stat_config.initial_delay,
> + "ms to wait before starting measurement after program start (-1: start with events disabled"),
> OPT_CALLBACK_NOOPT(0, "metric-only", &stat_config.metric_only, NULL,
> "Only print computed metrics. No raw values", enable_metric_only),
> OPT_BOOLEAN(0, "topdown", &topdown_run,
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 84386850c290..874ecf068ac9 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -366,6 +366,9 @@ enum evlist_ctl_cmd {
> CTL_CMD_ACK = 'a'
> };
>
> +#define PERF_EVLIST__ENABLED_MSG "Events enabled\n"
> +#define PERF_EVLIST__DISABLED_MSG "Events disabled\n"
> +
> int perf_evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
> int perf_evlist__finalize_ctlfd(struct evlist *evlist);
> int perf_evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index b4fdfaa7f2c0..027b9dcd902f 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -113,7 +113,7 @@ struct perf_stat_config {
> FILE *output;
> unsigned int interval;
> unsigned int timeout;
> - unsigned int initial_delay;
> + int initial_delay;
> unsigned int unit_width;
> unsigned int metric_only_len;
> int times;
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 04/11] perf stat: extend -D,--delay option with -1 value
2020-05-06 20:12 ` Arnaldo Carvalho de Melo
@ 2020-05-07 8:34 ` Alexey Budankov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-07 8:34 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 06.05.2020 23:12, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2020 at 09:20:54PM +0300, Alexey Budankov escreveu:
>>
>> Extend -D,--delay option with -1 value to start monitoring with
>> events disabled to be enabled later by enable command provided
>> via control file descriptor.
>
>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> ---
>> tools/perf/builtin-stat.c | 18 ++++++++++++++----
>> tools/perf/util/evlist.h | 3 +++
>> tools/perf/util/stat.h | 2 +-
>
> You forgot to update tools/perf/Documentation/perf-stat.h
Good catch. Addressed in v3. Thanks!
~Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 05/11] perf stat: implement control commands handling
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (3 preceding siblings ...)
2020-05-06 18:20 ` [PATCH v2 04/11] perf stat: extend -D,--delay option with -1 value Alexey Budankov
@ 2020-05-06 18:21 ` Alexey Budankov
2020-05-06 18:22 ` [PATCH v2 06/11] perf stat: introduce --ctl-fd[-ack] options Alexey Budankov
` (5 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:21 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Implement handling of 'enable' and 'disable' control commands
coming from control file descriptor.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-stat.c | 48 +++++++++++++++++++++++++++++----------
1 file changed, 36 insertions(+), 12 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index bda777ca0420..5aab3ff1bbea 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -460,8 +460,9 @@ static int handle_events(pid_t pid, struct perf_stat_config *config)
{
pid_t child = 0;
bool res, stop = false;
- struct timespec time_to_sleep;
- int sleep_time, status = 0, times = config->times;
+ int time_to_sleep, sleep_time, status = 0, times = config->times;
+ enum evlist_ctl_cmd cmd = CTL_CMD_UNSUPPORTED;
+ struct timespec time_start, time_stop, time_diff;
if (config->interval)
sleep_time = config->interval;
@@ -470,22 +471,45 @@ static int handle_events(pid_t pid, struct perf_stat_config *config)
else
sleep_time = 1000;
- time_to_sleep.tv_sec = sleep_time / MSEC_PER_SEC;
- time_to_sleep.tv_nsec = (sleep_time % MSEC_PER_SEC) * NSEC_PER_MSEC;
+ time_to_sleep = sleep_time;
do {
if (pid != -1)
child = waitpid(pid, &status, WNOHANG);
if (child || stop || done)
break;
- nanosleep(&time_to_sleep, NULL);
- if (pid == -1)
- stop = !is_target_alive(&target, evsel_list->core.threads);
- if (config->timeout) {
- stop = !stop ? true : stop;
- } else {
- res = print_interval_and_stop(config, ×);
- stop = !stop ? res : stop;
+ clock_gettime(CLOCK_MONOTONIC, &time_start);
+ if (!(evlist__poll(evsel_list, time_to_sleep) > 0)) { /* poll timeout or EINTR */
+ if (pid == -1)
+ stop = !is_target_alive(&target, evsel_list->core.threads);
+ if (config->timeout) {
+ stop = !stop ? true : stop;
+ } else {
+ res = print_interval_and_stop(config, ×);
+ stop = !stop ? res : stop;
+ }
+ time_to_sleep = sleep_time;
+ } else { /* fd revent */
+ if (perf_evlist__ctlfd_process(evsel_list, &cmd) > 0) {
+ switch (cmd) {
+ case CTL_CMD_ENABLE:
+ pr_info(PERF_EVLIST__ENABLED_MSG);
+ stop = print_interval_and_stop(config, ×);
+ break;
+ case CTL_CMD_DISABLE:
+ stop = print_interval_and_stop(config, ×);
+ pr_info(PERF_EVLIST__DISABLED_MSG);
+ break;
+ case CTL_CMD_ACK:
+ case CTL_CMD_UNSUPPORTED:
+ default:
+ break;
+ }
+ }
+ clock_gettime(CLOCK_MONOTONIC, &time_stop);
+ diff_timespec(&time_diff, &time_stop, &time_start);
+ time_to_sleep -= time_diff.tv_sec * MSEC_PER_SEC +
+ time_diff.tv_nsec / NSEC_PER_MSEC;
}
} while (1);
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 06/11] perf stat: introduce --ctl-fd[-ack] options
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (4 preceding siblings ...)
2020-05-06 18:21 ` [PATCH v2 05/11] perf stat: implement control commands handling Alexey Budankov
@ 2020-05-06 18:22 ` Alexey Budankov
2020-05-06 18:22 ` [PATCH v2 07/11] perf docs: extend stat mode docs with info on " Alexey Budankov
` (4 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:22 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Introduce --ctl-fd[-ack] options to pass open file descriptors
numbers from command line.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-stat.c | 10 ++++++++++
tools/perf/util/stat.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 5aab3ff1bbea..60c0d9521f74 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -187,6 +187,8 @@ static struct perf_stat_config stat_config = {
.metric_only_len = METRIC_ONLY_LEN,
.walltime_nsecs_stats = &walltime_nsecs_stats,
.big_num = true,
+ .ctl_fd = -1,
+ .ctl_fd_ack = -1
};
static inline void diff_timespec(struct timespec *r, struct timespec *a,
@@ -984,6 +986,10 @@ static struct option stat_options[] = {
"Use with 'percore' event qualifier to show the event "
"counts of one hardware thread by sum up total hardware "
"threads of same physical core"),
+ OPT_INTEGER(0, "ctl-fd", &stat_config.ctl_fd,
+ "Listen on fd descriptor for command to control measurement ('e': enable events, 'd': disable events)"),
+ OPT_INTEGER(0, "ctl-fd-ack", &stat_config.ctl_fd_ack,
+ "Send control command completion ('a') to fd ack descriptor"),
OPT_END()
};
@@ -2180,6 +2186,8 @@ int cmd_stat(int argc, const char **argv)
signal(SIGALRM, skip_signal);
signal(SIGABRT, skip_signal);
+ perf_evlist__initialize_ctlfd(evsel_list, stat_config.ctl_fd, stat_config.ctl_fd_ack);
+
status = 0;
for (run_idx = 0; forever || run_idx < stat_config.run_count; run_idx++) {
if (stat_config.run_count != 1 && verbose > 0)
@@ -2199,6 +2207,8 @@ int cmd_stat(int argc, const char **argv)
if (!forever && status != -1 && !interval)
print_counters(NULL, argc, argv);
+ perf_evlist__finalize_ctlfd(evsel_list);
+
if (STAT_RECORD) {
/*
* We synthesize the kernel mmap record just so that older tools
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 027b9dcd902f..0b0fa3a2cde2 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -130,6 +130,8 @@ struct perf_stat_config {
struct perf_cpu_map *cpus_aggr_map;
u64 *walltime_run;
struct rblist metric_events;
+ int ctl_fd;
+ int ctl_fd_ack;
};
void update_stats(struct stats *stats, u64 val);
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 07/11] perf docs: extend stat mode docs with info on --ctl-fd[-ack] options
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (5 preceding siblings ...)
2020-05-06 18:22 ` [PATCH v2 06/11] perf stat: introduce --ctl-fd[-ack] options Alexey Budankov
@ 2020-05-06 18:22 ` Alexey Budankov
2020-05-06 20:22 ` Arnaldo Carvalho de Melo
2020-05-06 18:28 ` [PATCH v2 08/11] perf record: extend -D,--delay option with -1 value Alexey Budankov
` (3 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:22 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Extend perf-stat.txt file with --ctl-fd[-ack] options description.
Document possible usage model introduced by --ctl-fd[-ack] options
by providing example bash shell script.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/Documentation/perf-stat.txt | 40 ++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 3fb5028aef08..19ed53e08294 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -164,6 +164,46 @@ 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
+--ctl-fd::
+--ctl-fd-ack::
+
+Listen on ctl-fd descriptor for command to control measurement ('e': enable events,
+'d': disable events). Optionally send control command completion ('a') to fd-ack
+descriptor to synchronize with the controlling process. Example of bash shell script
+to enable and disable events during measurements:
+
+#!/bin/bash
+
+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_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 \
+ --ctl-fd ${ctl_fd} --ctl-fd-ack ${ctl_fd_ack} \
+ -- sleep 30 &
+perf_pid=$!
+
+sleep 5 && echo 'e' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
+sleep 10 && echo 'd' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
+
+exec {ctl_fd_ack}>&-
+unlink ${ctl_ack_fifo}
+
+exec {ctl_fd}>&-
+unlink ${ctl_fifo}
+
+wait -n ${perf_pid}
+exit $?
+
+
--pre::
--post::
Pre and post measurement hooks, e.g.:
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 07/11] perf docs: extend stat mode docs with info on --ctl-fd[-ack] options
2020-05-06 18:22 ` [PATCH v2 07/11] perf docs: extend stat mode docs with info on " Alexey Budankov
@ 2020-05-06 20:22 ` Arnaldo Carvalho de Melo
2020-05-07 8:35 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-06 20:22 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Wed, May 06, 2020 at 09:22:55PM +0300, Alexey Budankov escreveu:
>
> Extend perf-stat.txt file with --ctl-fd[-ack] options description.
> Document possible usage model introduced by --ctl-fd[-ack] options
> by providing example bash shell script.
Please update the docs in the same patch that introduces the options
being documented.
- Arnaldo
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> tools/perf/Documentation/perf-stat.txt | 40 ++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index 3fb5028aef08..19ed53e08294 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -164,6 +164,46 @@ 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
>
> +--ctl-fd::
> +--ctl-fd-ack::
> +
> +Listen on ctl-fd descriptor for command to control measurement ('e': enable events,
> +'d': disable events). Optionally send control command completion ('a') to fd-ack
> +descriptor to synchronize with the controlling process. Example of bash shell script
> +to enable and disable events during measurements:
> +
> +#!/bin/bash
> +
> +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_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 \
> + --ctl-fd ${ctl_fd} --ctl-fd-ack ${ctl_fd_ack} \
> + -- sleep 30 &
> +perf_pid=$!
> +
> +sleep 5 && echo 'e' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
> +sleep 10 && echo 'd' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
> +
> +exec {ctl_fd_ack}>&-
> +unlink ${ctl_ack_fifo}
> +
> +exec {ctl_fd}>&-
> +unlink ${ctl_fifo}
> +
> +wait -n ${perf_pid}
> +exit $?
> +
> +
> --pre::
> --post::
> Pre and post measurement hooks, e.g.:
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 07/11] perf docs: extend stat mode docs with info on --ctl-fd[-ack] options
2020-05-06 20:22 ` Arnaldo Carvalho de Melo
@ 2020-05-07 8:35 ` Alexey Budankov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-07 8:35 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 06.05.2020 23:22, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2020 at 09:22:55PM +0300, Alexey Budankov escreveu:
>>
>> Extend perf-stat.txt file with --ctl-fd[-ack] options description.
>> Document possible usage model introduced by --ctl-fd[-ack] options
>> by providing example bash shell script.
>
> Please update the docs in the same patch that introduces the options
> being documented.
Addressed in v3.
~Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 08/11] perf record: extend -D,--delay option with -1 value
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (6 preceding siblings ...)
2020-05-06 18:22 ` [PATCH v2 07/11] perf docs: extend stat mode docs with info on " Alexey Budankov
@ 2020-05-06 18:28 ` Alexey Budankov
2020-05-06 18:29 ` [PATCH v2 09/11] perf record: implement control commands handling Alexey Budankov
` (2 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:28 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Extend -D,--delay option with -1 to start collection
with events disabled to be enbled later by enable
command provided via control file descriptor.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-record.c | 12 ++++++++----
tools/perf/builtin-trace.c | 2 +-
tools/perf/util/record.h | 2 +-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index e4efdbf1a81e..8a14e68b86ad 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1704,8 +1704,12 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
}
if (opts->initial_delay) {
- usleep(opts->initial_delay * USEC_PER_MSEC);
- evlist__enable(rec->evlist);
+ pr_info(PERF_EVLIST__DISABLED_MSG);
+ if (opts->initial_delay > 0) {
+ usleep(opts->initial_delay * USEC_PER_MSEC);
+ evlist__enable(rec->evlist);
+ pr_info(PERF_EVLIST__ENABLED_MSG);
+ }
}
trigger_ready(&auxtrace_snapshot_trigger);
@@ -2413,8 +2417,8 @@ static struct option __record_options[] = {
OPT_CALLBACK('G', "cgroup", &record.evlist, "name",
"monitor event in cgroup name only",
parse_cgroups),
- OPT_UINTEGER('D', "delay", &record.opts.initial_delay,
- "ms to wait before starting measurement after program start"),
+ OPT_INTEGER('D', "delay", &record.opts.initial_delay,
+ "ms to wait before starting measurement after program start (-1: start with events disabled )"),
OPT_BOOLEAN(0, "kcore", &record.opts.kcore, "copy /proc/kcore"),
OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
"user to profile"),
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index a46efb907bd4..63f69a927f3c 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4773,7 +4773,7 @@ int cmd_trace(int argc, const char **argv)
"per thread proc mmap processing timeout in ms"),
OPT_CALLBACK('G', "cgroup", &trace, "name", "monitor event in cgroup name only",
trace__parse_cgroups),
- OPT_UINTEGER('D', "delay", &trace.opts.initial_delay,
+ OPT_INTEGER('D', "delay", &trace.opts.initial_delay,
"ms to wait before starting measurement after program "
"start"),
OPTS_EVSWITCH(&trace.evswitch),
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 923565c3b155..96a73bbd8cd4 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -60,7 +60,7 @@ struct record_opts {
const char *auxtrace_snapshot_opts;
const char *auxtrace_sample_opts;
bool sample_transaction;
- unsigned initial_delay;
+ int initial_delay;
bool use_clockid;
clockid_t clockid;
u64 clockid_res_ns;
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 09/11] perf record: implement control commands handling
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (7 preceding siblings ...)
2020-05-06 18:28 ` [PATCH v2 08/11] perf record: extend -D,--delay option with -1 value Alexey Budankov
@ 2020-05-06 18:29 ` Alexey Budankov
2020-05-06 20:23 ` Arnaldo Carvalho de Melo
2020-05-06 18:29 ` [PATCH v2 10/11] perf record: introduce --ctl-fd[-ack] options Alexey Budankov
2020-05-06 18:30 ` [PATCH v2 11/11] perf docs: extend record mode docs with info on " Alexey Budankov
10 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Implement handling of 'enable' and 'disable' control commands
coming from control file descriptor.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-record.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 8a14e68b86ad..2278a3efc747 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1496,6 +1496,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
bool disabled = false, draining = false;
int fd;
float ratio = 0;
+ enum evlist_ctl_cmd cmd = CTL_CMD_UNSUPPORTED;
atexit(record__sig_exit);
signal(SIGCHLD, sig_handler);
@@ -1793,8 +1794,23 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
* Propagate error, only if there's any. Ignore positive
* number of returned events and interrupt error.
*/
- if (err > 0 || (err < 0 && errno == EINTR))
+ if (err > 0 || (err < 0 && errno == EINTR)) {
err = 0;
+ if (perf_evlist__ctlfd_process(rec->evlist, &cmd) > 0) {
+ switch (cmd) {
+ case CTL_CMD_ENABLE:
+ pr_info(PERF_EVLIST__ENABLED_MSG);
+ break;
+ case CTL_CMD_DISABLE:
+ pr_info(PERF_EVLIST__DISABLED_MSG);
+ break;
+ case CTL_CMD_ACK:
+ case CTL_CMD_UNSUPPORTED:
+ default:
+ break;
+ }
+ }
+ }
waking++;
if (evlist__filter_pollfd(rec->evlist, POLLERR | POLLHUP) == 0)
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 09/11] perf record: implement control commands handling
2020-05-06 18:29 ` [PATCH v2 09/11] perf record: implement control commands handling Alexey Budankov
@ 2020-05-06 20:23 ` Arnaldo Carvalho de Melo
2020-05-07 8:58 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-06 20:23 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Wed, May 06, 2020 at 09:29:05PM +0300, Alexey Budankov escreveu:
>
> Implement handling of 'enable' and 'disable' control commands
> coming from control file descriptor.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> tools/perf/builtin-record.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 8a14e68b86ad..2278a3efc747 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1496,6 +1496,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> bool disabled = false, draining = false;
> int fd;
> float ratio = 0;
> + enum evlist_ctl_cmd cmd = CTL_CMD_UNSUPPORTED;
>
> atexit(record__sig_exit);
> signal(SIGCHLD, sig_handler);
> @@ -1793,8 +1794,23 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> * Propagate error, only if there's any. Ignore positive
> * number of returned events and interrupt error.
> */
> - if (err > 0 || (err < 0 && errno == EINTR))
> + if (err > 0 || (err < 0 && errno == EINTR)) {
> err = 0;
> + if (perf_evlist__ctlfd_process(rec->evlist, &cmd) > 0) {
> + switch (cmd) {
> + case CTL_CMD_ENABLE:
> + pr_info(PERF_EVLIST__ENABLED_MSG);
> + break;
> + case CTL_CMD_DISABLE:
> + pr_info(PERF_EVLIST__DISABLED_MSG);
> + break;
> + case CTL_CMD_ACK:
> + case CTL_CMD_UNSUPPORTED:
Shouldn't we have a pr_debug() or even pr_err() for the unsupported one?
> + default:
> + break;
> + }
> + }
> + }
> waking++;
>
> if (evlist__filter_pollfd(rec->evlist, POLLERR | POLLHUP) == 0)
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 09/11] perf record: implement control commands handling
2020-05-06 20:23 ` Arnaldo Carvalho de Melo
@ 2020-05-07 8:58 ` Alexey Budankov
2020-05-07 17:02 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-07 8:58 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 06.05.2020 23:23, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2020 at 09:29:05PM +0300, Alexey Budankov escreveu:
>>
>> Implement handling of 'enable' and 'disable' control commands
>> coming from control file descriptor.
>>
>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>> ---
>> tools/perf/builtin-record.c | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>> index 8a14e68b86ad..2278a3efc747 100644
>> --- a/tools/perf/builtin-record.c
>> +++ b/tools/perf/builtin-record.c
>> @@ -1496,6 +1496,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>> bool disabled = false, draining = false;
>> int fd;
>> float ratio = 0;
>> + enum evlist_ctl_cmd cmd = CTL_CMD_UNSUPPORTED;
>>
>> atexit(record__sig_exit);
>> signal(SIGCHLD, sig_handler);
>> @@ -1793,8 +1794,23 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>> * Propagate error, only if there's any. Ignore positive
>> * number of returned events and interrupt error.
>> */
>> - if (err > 0 || (err < 0 && errno == EINTR))
>> + if (err > 0 || (err < 0 && errno == EINTR)) {
>> err = 0;
>> + if (perf_evlist__ctlfd_process(rec->evlist, &cmd) > 0) {
>> + switch (cmd) {
>> + case CTL_CMD_ENABLE:
>> + pr_info(PERF_EVLIST__ENABLED_MSG);
>> + break;
>> + case CTL_CMD_DISABLE:
>> + pr_info(PERF_EVLIST__DISABLED_MSG);
>> + break;
>> + case CTL_CMD_ACK:
>> + case CTL_CMD_UNSUPPORTED:
>
> Shouldn't we have a pr_debug() or even pr_err() for the unsupported one?
It already exists on lower level, at perf_evlist__ctlfd_process() (see patch 02/11).
~Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 09/11] perf record: implement control commands handling
2020-05-07 8:58 ` Alexey Budankov
@ 2020-05-07 17:02 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-07 17:02 UTC (permalink / raw)
To: Alexey Budankov
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Ingo Molnar, Andi Kleen,
linux-kernel
Em Thu, May 07, 2020 at 11:58:45AM +0300, Alexey Budankov escreveu:
>
> On 06.05.2020 23:23, Arnaldo Carvalho de Melo wrote:
> > Em Wed, May 06, 2020 at 09:29:05PM +0300, Alexey Budankov escreveu:
> >>
> >> Implement handling of 'enable' and 'disable' control commands
> >> coming from control file descriptor.
> >>
> >> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> >> ---
> >> tools/perf/builtin-record.c | 18 +++++++++++++++++-
> >> 1 file changed, 17 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> >> index 8a14e68b86ad..2278a3efc747 100644
> >> --- a/tools/perf/builtin-record.c
> >> +++ b/tools/perf/builtin-record.c
> >> @@ -1496,6 +1496,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >> bool disabled = false, draining = false;
> >> int fd;
> >> float ratio = 0;
> >> + enum evlist_ctl_cmd cmd = CTL_CMD_UNSUPPORTED;
> >>
> >> atexit(record__sig_exit);
> >> signal(SIGCHLD, sig_handler);
> >> @@ -1793,8 +1794,23 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> >> * Propagate error, only if there's any. Ignore positive
> >> * number of returned events and interrupt error.
> >> */
> >> - if (err > 0 || (err < 0 && errno == EINTR))
> >> + if (err > 0 || (err < 0 && errno == EINTR)) {
> >> err = 0;
> >> + if (perf_evlist__ctlfd_process(rec->evlist, &cmd) > 0) {
> >> + switch (cmd) {
> >> + case CTL_CMD_ENABLE:
> >> + pr_info(PERF_EVLIST__ENABLED_MSG);
> >> + break;
> >> + case CTL_CMD_DISABLE:
> >> + pr_info(PERF_EVLIST__DISABLED_MSG);
> >> + break;
> >> + case CTL_CMD_ACK:
> >> + case CTL_CMD_UNSUPPORTED:
> >
> > Shouldn't we have a pr_debug() or even pr_err() for the unsupported one?
>
> It already exists on lower level, at perf_evlist__ctlfd_process() (see patch 02/11).
oic
> ~Alexey
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 10/11] perf record: introduce --ctl-fd[-ack] options
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (8 preceding siblings ...)
2020-05-06 18:29 ` [PATCH v2 09/11] perf record: implement control commands handling Alexey Budankov
@ 2020-05-06 18:29 ` Alexey Budankov
2020-05-06 18:30 ` [PATCH v2 11/11] perf docs: extend record mode docs with info on " Alexey Budankov
10 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Introduce --ctl-fd[-ack] options to pass open file descriptors
numbers from command line.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/builtin-record.c | 9 +++++++++
tools/perf/util/record.h | 2 ++
2 files changed, 11 insertions(+)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 2278a3efc747..9cf738ff1cae 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1704,6 +1704,8 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
perf_evlist__start_workload(rec->evlist);
}
+ perf_evlist__initialize_ctlfd(rec->evlist, opts->ctl_fd, opts->ctl_fd_ack);
+
if (opts->initial_delay) {
pr_info(PERF_EVLIST__DISABLED_MSG);
if (opts->initial_delay > 0) {
@@ -1850,6 +1852,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
record__synthesize_workload(rec, true);
out_child:
+ perf_evlist__finalize_ctlfd(rec->evlist);
record__mmap_read_all(rec, true);
record__aio_mmap_read_sync(rec);
@@ -2331,6 +2334,8 @@ static struct record record = {
},
.mmap_flush = MMAP_FLUSH_DEFAULT,
.nr_threads_synthesize = 1,
+ .ctl_fd = -1,
+ .ctl_fd_ack = -1,
},
.tool = {
.sample = process_sample_event,
@@ -2526,6 +2531,10 @@ static struct option __record_options[] = {
OPT_UINTEGER(0, "num-thread-synthesize",
&record.opts.nr_threads_synthesize,
"number of threads to run for event synthesis"),
+ OPT_INTEGER(0, "ctl-fd", &record.opts.ctl_fd,
+ "Listen on fd descriptor for command to control measurement ('e': enable events, 'd': disable events)"),
+ OPT_INTEGER(0, "ctl-fd-ack", &record.opts.ctl_fd_ack,
+ "Send control command completion ('a') to fd ack descriptor"),
OPT_END()
};
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index 96a73bbd8cd4..da18aeca3623 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -69,6 +69,8 @@ struct record_opts {
int mmap_flush;
unsigned int comp_level;
unsigned int nr_threads_synthesize;
+ int ctl_fd;
+ int ctl_fd_ack;
};
extern const char * const *record_usage;
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 11/11] perf docs: extend record mode docs with info on --ctl-fd[-ack] options
2020-05-06 17:16 [PATCH v2 00/11] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (9 preceding siblings ...)
2020-05-06 18:29 ` [PATCH v2 10/11] perf record: introduce --ctl-fd[-ack] options Alexey Budankov
@ 2020-05-06 18:30 ` Alexey Budankov
2020-05-06 20:27 ` Arnaldo Carvalho de Melo
10 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-05-06 18:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Ingo Molnar,
Andi Kleen, linux-kernel
Extend perf-record.txt file with --ctl-fd[-ack] options description.
Document possible usage model introduced by --ctl-fd[-ack] options
by providing example bash shell script.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
tools/perf/Documentation/perf-record.txt | 39 ++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 561ef55743e2..eabb00ed2f5d 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -613,6 +613,45 @@ appended unit character - B/K/M/G
The number of threads to run when synthesizing events for existing processes.
By default, the number of threads equals 1.
+--ctl-fd::
+--ctl-fd-ack::
+Listen on ctl-fd descriptor for command to control measurement ('e': enable events,
+'d': disable events. Optionally send control command completion ('a') to fd-ack
+descriptor to synchronize with the controlling process. Example of bash shell script
+to enable and disable events during measurements:
+
+#!/bin/bash
+
+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_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 \
+ --ctl-fd ${ctl_fd} --ctl-fd-ack ${ctl_fd_ack} \
+ -- sleep 30 &
+perf_pid=$!
+
+sleep 5 && echo 'e' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "resumed(${e1})"
+sleep 10 && echo 'd' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "paused(${d1})"
+
+exec {ctl_fd_ack}>&-
+unlink ${ctl_ack_fifo}
+
+exec {ctl_fd}>&-
+unlink ${ctl_fifo}
+
+wait -n ${perf_pid}
+exit $?
+
+
SEE ALSO
--------
linkperf:perf-stat[1], linkperf:perf-list[1], linkperf:perf-intel-pt[1]
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 11/11] perf docs: extend record mode docs with info on --ctl-fd[-ack] options
2020-05-06 18:30 ` [PATCH v2 11/11] perf docs: extend record mode docs with info on " Alexey Budankov
@ 2020-05-06 20:27 ` Arnaldo Carvalho de Melo
2020-05-07 8:36 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-05-06 20:27 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Wed, May 06, 2020 at 09:30:50PM +0300, Alexey Budankov escreveu:
>
> Extend perf-record.txt file with --ctl-fd[-ack] options description.
> Document possible usage model introduced by --ctl-fd[-ack] options
> by providing example bash shell script.
Ditto, combine this patch with the one that introduces these options.
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> tools/perf/Documentation/perf-record.txt | 39 ++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index 561ef55743e2..eabb00ed2f5d 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -613,6 +613,45 @@ appended unit character - B/K/M/G
> The number of threads to run when synthesizing events for existing processes.
> By default, the number of threads equals 1.
>
> +--ctl-fd::
> +--ctl-fd-ack::
> +Listen on ctl-fd descriptor for command to control measurement ('e': enable events,
> +'d': disable events. Optionally send control command completion ('a') to fd-ack
> +descriptor to synchronize with the controlling process. Example of bash shell script
> +to enable and disable events during measurements:
> +
> +#!/bin/bash
> +
> +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_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 \
> + --ctl-fd ${ctl_fd} --ctl-fd-ack ${ctl_fd_ack} \
> + -- sleep 30 &
> +perf_pid=$!
> +
> +sleep 5 && echo 'e' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "resumed(${e1})"
> +sleep 10 && echo 'd' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "paused(${d1})"
> +
> +exec {ctl_fd_ack}>&-
> +unlink ${ctl_ack_fifo}
> +
> +exec {ctl_fd}>&-
> +unlink ${ctl_fifo}
> +
> +wait -n ${perf_pid}
> +exit $?
> +
> +
> SEE ALSO
> --------
> linkperf:perf-stat[1], linkperf:perf-list[1], linkperf:perf-intel-pt[1]
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 11/11] perf docs: extend record mode docs with info on --ctl-fd[-ack] options
2020-05-06 20:27 ` Arnaldo Carvalho de Melo
@ 2020-05-07 8:36 ` Alexey Budankov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-05-07 8:36 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 06.05.2020 23:27, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 06, 2020 at 09:30:50PM +0300, Alexey Budankov escreveu:
>>
>> Extend perf-record.txt file with --ctl-fd[-ack] options description.
>> Document possible usage model introduced by --ctl-fd[-ack] options
>> by providing example bash shell script.
>
> Ditto, combine this patch with the one that introduces these options.
Addressed in v3.
~Alexey
^ permalink raw reply [flat|nested] 25+ messages in thread