* [PATCH v12 01/15] tools/libperf: avoid internal moving of fdarray fds
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
@ 2020-07-17 6:59 ` Alexey Budankov
2020-07-21 12:40 ` Arnaldo Carvalho de Melo
2020-07-17 6:59 ` [PATCH v12 02/15] tools/libperf: add flags to fdarray fds objects Alexey Budankov
` (13 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 6:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Avoid moving of fds by fdarray__filter() so fds indices returned
by fdarray__add() can be used for access and processing of
objects at struct pollfd *entries.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/lib/api/fd/array.c | 11 +++++------
tools/perf/tests/fdarray.c | 20 ++------------------
2 files changed, 7 insertions(+), 24 deletions(-)
diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index 58d44d5eee31..89f9a2193c2d 100644
--- a/tools/lib/api/fd/array.c
+++ b/tools/lib/api/fd/array.c
@@ -93,22 +93,21 @@ int fdarray__filter(struct fdarray *fda, short revents,
return 0;
for (fd = 0; fd < fda->nr; ++fd) {
+ if (!fda->entries[fd].events)
+ continue;
+
if (fda->entries[fd].revents & revents) {
if (entry_destructor)
entry_destructor(fda, fd, arg);
+ fda->entries[fd].revents = fda->entries[fd].events = 0;
continue;
}
- if (fd != nr) {
- fda->entries[nr] = fda->entries[fd];
- fda->priv[nr] = fda->priv[fd];
- }
-
++nr;
}
- return fda->nr = nr;
+ return nr;
}
int fdarray__poll(struct fdarray *fda, int timeout)
diff --git a/tools/perf/tests/fdarray.c b/tools/perf/tests/fdarray.c
index c7c81c4a5b2b..d0c8a05aab2f 100644
--- a/tools/perf/tests/fdarray.c
+++ b/tools/perf/tests/fdarray.c
@@ -12,6 +12,7 @@ static void fdarray__init_revents(struct fdarray *fda, short revents)
for (fd = 0; fd < fda->nr; ++fd) {
fda->entries[fd].fd = fda->nr - fd;
+ fda->entries[fd].events = revents;
fda->entries[fd].revents = revents;
}
}
@@ -29,7 +30,7 @@ static int fdarray__fprintf_prefix(struct fdarray *fda, const char *prefix, FILE
int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_unused)
{
- int nr_fds, expected_fd[2], fd, err = TEST_FAIL;
+ int nr_fds, err = TEST_FAIL;
struct fdarray *fda = fdarray__new(5, 5);
if (fda == NULL) {
@@ -55,7 +56,6 @@ int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_
fdarray__init_revents(fda, POLLHUP);
fda->entries[2].revents = POLLIN;
- expected_fd[0] = fda->entries[2].fd;
pr_debug("\nfiltering all but fda->entries[2]:");
fdarray__fprintf_prefix(fda, "before", stderr);
@@ -66,17 +66,9 @@ int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_
goto out_delete;
}
- if (fda->entries[0].fd != expected_fd[0]) {
- pr_debug("\nfda->entries[0].fd=%d != %d\n",
- fda->entries[0].fd, expected_fd[0]);
- goto out_delete;
- }
-
fdarray__init_revents(fda, POLLHUP);
fda->entries[0].revents = POLLIN;
- expected_fd[0] = fda->entries[0].fd;
fda->entries[3].revents = POLLIN;
- expected_fd[1] = fda->entries[3].fd;
pr_debug("\nfiltering all but (fda->entries[0], fda->entries[3]):");
fdarray__fprintf_prefix(fda, "before", stderr);
@@ -88,14 +80,6 @@ int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_
goto out_delete;
}
- for (fd = 0; fd < 2; ++fd) {
- if (fda->entries[fd].fd != expected_fd[fd]) {
- pr_debug("\nfda->entries[%d].fd=%d != %d\n", fd,
- fda->entries[fd].fd, expected_fd[fd]);
- goto out_delete;
- }
- }
-
pr_debug("\n");
err = 0;
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v12 01/15] tools/libperf: avoid internal moving of fdarray fds
2020-07-17 6:59 ` [PATCH v12 01/15] tools/libperf: avoid internal moving of fdarray fds Alexey Budankov
@ 2020-07-21 12:40 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-21 12:40 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Fri, Jul 17, 2020 at 09:59:12AM +0300, Alexey Budankov escreveu:
>
> Avoid moving of fds by fdarray__filter() so fds indices returned
> by fdarray__add() can be used for access and processing of
> objects at struct pollfd *entries.
Thanks, applied.
- Arnaldo
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/lib/api/fd/array.c | 11 +++++------
> tools/perf/tests/fdarray.c | 20 ++------------------
> 2 files changed, 7 insertions(+), 24 deletions(-)
>
> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index 58d44d5eee31..89f9a2193c2d 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -93,22 +93,21 @@ int fdarray__filter(struct fdarray *fda, short revents,
> return 0;
>
> for (fd = 0; fd < fda->nr; ++fd) {
> + if (!fda->entries[fd].events)
> + continue;
> +
> if (fda->entries[fd].revents & revents) {
> if (entry_destructor)
> entry_destructor(fda, fd, arg);
>
> + fda->entries[fd].revents = fda->entries[fd].events = 0;
> continue;
> }
>
> - if (fd != nr) {
> - fda->entries[nr] = fda->entries[fd];
> - fda->priv[nr] = fda->priv[fd];
> - }
> -
> ++nr;
> }
>
> - return fda->nr = nr;
> + return nr;
> }
>
> int fdarray__poll(struct fdarray *fda, int timeout)
> diff --git a/tools/perf/tests/fdarray.c b/tools/perf/tests/fdarray.c
> index c7c81c4a5b2b..d0c8a05aab2f 100644
> --- a/tools/perf/tests/fdarray.c
> +++ b/tools/perf/tests/fdarray.c
> @@ -12,6 +12,7 @@ static void fdarray__init_revents(struct fdarray *fda, short revents)
>
> for (fd = 0; fd < fda->nr; ++fd) {
> fda->entries[fd].fd = fda->nr - fd;
> + fda->entries[fd].events = revents;
> fda->entries[fd].revents = revents;
> }
> }
> @@ -29,7 +30,7 @@ static int fdarray__fprintf_prefix(struct fdarray *fda, const char *prefix, FILE
>
> int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_unused)
> {
> - int nr_fds, expected_fd[2], fd, err = TEST_FAIL;
> + int nr_fds, err = TEST_FAIL;
> struct fdarray *fda = fdarray__new(5, 5);
>
> if (fda == NULL) {
> @@ -55,7 +56,6 @@ int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_
>
> fdarray__init_revents(fda, POLLHUP);
> fda->entries[2].revents = POLLIN;
> - expected_fd[0] = fda->entries[2].fd;
>
> pr_debug("\nfiltering all but fda->entries[2]:");
> fdarray__fprintf_prefix(fda, "before", stderr);
> @@ -66,17 +66,9 @@ int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_
> goto out_delete;
> }
>
> - if (fda->entries[0].fd != expected_fd[0]) {
> - pr_debug("\nfda->entries[0].fd=%d != %d\n",
> - fda->entries[0].fd, expected_fd[0]);
> - goto out_delete;
> - }
> -
> fdarray__init_revents(fda, POLLHUP);
> fda->entries[0].revents = POLLIN;
> - expected_fd[0] = fda->entries[0].fd;
> fda->entries[3].revents = POLLIN;
> - expected_fd[1] = fda->entries[3].fd;
>
> pr_debug("\nfiltering all but (fda->entries[0], fda->entries[3]):");
> fdarray__fprintf_prefix(fda, "before", stderr);
> @@ -88,14 +80,6 @@ int test__fdarray__filter(struct test *test __maybe_unused, int subtest __maybe_
> goto out_delete;
> }
>
> - for (fd = 0; fd < 2; ++fd) {
> - if (fda->entries[fd].fd != expected_fd[fd]) {
> - pr_debug("\nfda->entries[%d].fd=%d != %d\n", fd,
> - fda->entries[fd].fd, expected_fd[fd]);
> - goto out_delete;
> - }
> - }
> -
> pr_debug("\n");
>
> err = 0;
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 02/15] tools/libperf: add flags to fdarray fds objects
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
2020-07-17 6:59 ` [PATCH v12 01/15] tools/libperf: avoid internal moving of fdarray fds Alexey Budankov
@ 2020-07-17 6:59 ` Alexey Budankov
2020-07-21 12:53 ` Arnaldo Carvalho de Melo
2020-07-17 7:00 ` [PATCH v12 03/15] tools/libperf: avoid counting of nonfilterable fdarray fds Alexey Budankov
` (12 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 6:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Store flags per struct pollfd *entries object in a bitmap of int size.
Implement fdarray_flag__nonfilterable flag to skip object from counting
by fdarray__filter(). Fixed fdarray test issue reported by kernel test
robot.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Reported-by: kernel test robot <rong.a.chen@intel.com>
---
tools/lib/api/fd/array.c | 9 +++++++--
tools/lib/api/fd/array.h | 16 ++++++++++++----
tools/lib/perf/evlist.c | 6 +++---
tools/lib/perf/include/internal/evlist.h | 2 +-
tools/perf/tests/fdarray.c | 2 +-
tools/perf/util/evlist.c | 2 +-
6 files changed, 25 insertions(+), 12 deletions(-)
diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index 89f9a2193c2d..01b3b89f9797 100644
--- a/tools/lib/api/fd/array.c
+++ b/tools/lib/api/fd/array.c
@@ -8,6 +8,7 @@
#include <poll.h>
#include <stdlib.h>
#include <unistd.h>
+#include <string.h>
void fdarray__init(struct fdarray *fda, int nr_autogrow)
{
@@ -19,7 +20,7 @@ void fdarray__init(struct fdarray *fda, int nr_autogrow)
int fdarray__grow(struct fdarray *fda, int nr)
{
- void *priv;
+ struct priv *priv;
int nr_alloc = fda->nr_alloc + nr;
size_t psize = sizeof(fda->priv[0]) * nr_alloc;
size_t size = sizeof(struct pollfd) * nr_alloc;
@@ -34,6 +35,9 @@ int fdarray__grow(struct fdarray *fda, int nr)
return -ENOMEM;
}
+ memset(&entries[fda->nr_alloc], 0, sizeof(struct pollfd) * nr);
+ memset(&priv[fda->nr_alloc], 0, sizeof(fda->priv[0]) * nr);
+
fda->nr_alloc = nr_alloc;
fda->entries = entries;
fda->priv = priv;
@@ -69,7 +73,7 @@ void fdarray__delete(struct fdarray *fda)
free(fda);
}
-int fdarray__add(struct fdarray *fda, int fd, short revents)
+int fdarray__add(struct fdarray *fda, int fd, short revents, enum fdarray_flags flags)
{
int pos = fda->nr;
@@ -79,6 +83,7 @@ int fdarray__add(struct fdarray *fda, int fd, short revents)
fda->entries[fda->nr].fd = fd;
fda->entries[fda->nr].events = revents;
+ fda->priv[fda->nr].flags = flags;
fda->nr++;
return pos;
}
diff --git a/tools/lib/api/fd/array.h b/tools/lib/api/fd/array.h
index b39557d1a88f..7fcf21a33c0c 100644
--- a/tools/lib/api/fd/array.h
+++ b/tools/lib/api/fd/array.h
@@ -21,19 +21,27 @@ struct fdarray {
int nr_alloc;
int nr_autogrow;
struct pollfd *entries;
- union {
- int idx;
- void *ptr;
+ struct priv {
+ union {
+ int idx;
+ void *ptr;
+ };
+ unsigned int flags;
} *priv;
};
+enum fdarray_flags {
+ fdarray_flag__default = 0x00000000,
+ fdarray_flag__nonfilterable = 0x00000001
+};
+
void fdarray__init(struct fdarray *fda, int nr_autogrow);
void fdarray__exit(struct fdarray *fda);
struct fdarray *fdarray__new(int nr_alloc, int nr_autogrow);
void fdarray__delete(struct fdarray *fda);
-int fdarray__add(struct fdarray *fda, int fd, short revents);
+int fdarray__add(struct fdarray *fda, int fd, short revents, enum fdarray_flags flags);
int fdarray__poll(struct fdarray *fda, int timeout);
int fdarray__filter(struct fdarray *fda, short revents,
void (*entry_destructor)(struct fdarray *fda, int fd, void *arg),
diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 6a875a0f01bb..2208444ecb44 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -305,9 +305,9 @@ int perf_evlist__alloc_pollfd(struct perf_evlist *evlist)
}
int perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd,
- void *ptr, short revent)
+ void *ptr, short revent, enum fdarray_flags flags)
{
- int pos = fdarray__add(&evlist->pollfd, fd, revent | POLLERR | POLLHUP);
+ int pos = fdarray__add(&evlist->pollfd, fd, revent | POLLERR | POLLHUP, flags);
if (pos >= 0) {
evlist->pollfd.priv[pos].ptr = ptr;
@@ -488,7 +488,7 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
revent = !overwrite ? POLLIN : 0;
if (!evsel->system_wide &&
- perf_evlist__add_pollfd(evlist, fd, map, revent) < 0) {
+ perf_evlist__add_pollfd(evlist, fd, map, revent, fdarray_flag__default) < 0) {
perf_mmap__put(map);
return -1;
}
diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
index 74dc8c3f0b66..2d0fa02b036f 100644
--- a/tools/lib/perf/include/internal/evlist.h
+++ b/tools/lib/perf/include/internal/evlist.h
@@ -45,7 +45,7 @@ struct perf_evlist_mmap_ops {
int perf_evlist__alloc_pollfd(struct perf_evlist *evlist);
int perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd,
- void *ptr, short revent);
+ void *ptr, short revent, enum fdarray_flags flags);
int perf_evlist__mmap_ops(struct perf_evlist *evlist,
struct perf_evlist_mmap_ops *ops,
diff --git a/tools/perf/tests/fdarray.c b/tools/perf/tests/fdarray.c
index d0c8a05aab2f..d9eca8e86a6b 100644
--- a/tools/perf/tests/fdarray.c
+++ b/tools/perf/tests/fdarray.c
@@ -112,7 +112,7 @@ int test__fdarray__add(struct test *test __maybe_unused, int subtest __maybe_unu
}
#define FDA_ADD(_idx, _fd, _revents, _nr) \
- if (fdarray__add(fda, _fd, _revents) < 0) { \
+ if (fdarray__add(fda, _fd, _revents, fdarray_flag__default) < 0) { \
pr_debug("\n%d: fdarray__add(fda, %d, %d) failed!", \
__LINE__,_fd, _revents); \
goto out_delete; \
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1b884695b4d3..bcbe0cb8482e 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -497,7 +497,7 @@ int perf_evlist__enable_event_idx(struct evlist *evlist,
int evlist__add_pollfd(struct evlist *evlist, int fd)
{
- return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN);
+ return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN, fdarray_flag__default);
}
int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v12 02/15] tools/libperf: add flags to fdarray fds objects
2020-07-17 6:59 ` [PATCH v12 02/15] tools/libperf: add flags to fdarray fds objects Alexey Budankov
@ 2020-07-21 12:53 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-21 12:53 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Fri, Jul 17, 2020 at 09:59:45AM +0300, Alexey Budankov escreveu:
>
> Store flags per struct pollfd *entries object in a bitmap of int size.
> Implement fdarray_flag__nonfilterable flag to skip object from counting
> by fdarray__filter(). Fixed fdarray test issue reported by kernel test
> robot.
Thanks, applied.
- Arnaldo
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> Reported-by: kernel test robot <rong.a.chen@intel.com>
> ---
> tools/lib/api/fd/array.c | 9 +++++++--
> tools/lib/api/fd/array.h | 16 ++++++++++++----
> tools/lib/perf/evlist.c | 6 +++---
> tools/lib/perf/include/internal/evlist.h | 2 +-
> tools/perf/tests/fdarray.c | 2 +-
> tools/perf/util/evlist.c | 2 +-
> 6 files changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index 89f9a2193c2d..01b3b89f9797 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -8,6 +8,7 @@
> #include <poll.h>
> #include <stdlib.h>
> #include <unistd.h>
> +#include <string.h>
>
> void fdarray__init(struct fdarray *fda, int nr_autogrow)
> {
> @@ -19,7 +20,7 @@ void fdarray__init(struct fdarray *fda, int nr_autogrow)
>
> int fdarray__grow(struct fdarray *fda, int nr)
> {
> - void *priv;
> + struct priv *priv;
> int nr_alloc = fda->nr_alloc + nr;
> size_t psize = sizeof(fda->priv[0]) * nr_alloc;
> size_t size = sizeof(struct pollfd) * nr_alloc;
> @@ -34,6 +35,9 @@ int fdarray__grow(struct fdarray *fda, int nr)
> return -ENOMEM;
> }
>
> + memset(&entries[fda->nr_alloc], 0, sizeof(struct pollfd) * nr);
> + memset(&priv[fda->nr_alloc], 0, sizeof(fda->priv[0]) * nr);
> +
> fda->nr_alloc = nr_alloc;
> fda->entries = entries;
> fda->priv = priv;
> @@ -69,7 +73,7 @@ void fdarray__delete(struct fdarray *fda)
> free(fda);
> }
>
> -int fdarray__add(struct fdarray *fda, int fd, short revents)
> +int fdarray__add(struct fdarray *fda, int fd, short revents, enum fdarray_flags flags)
> {
> int pos = fda->nr;
>
> @@ -79,6 +83,7 @@ int fdarray__add(struct fdarray *fda, int fd, short revents)
>
> fda->entries[fda->nr].fd = fd;
> fda->entries[fda->nr].events = revents;
> + fda->priv[fda->nr].flags = flags;
> fda->nr++;
> return pos;
> }
> diff --git a/tools/lib/api/fd/array.h b/tools/lib/api/fd/array.h
> index b39557d1a88f..7fcf21a33c0c 100644
> --- a/tools/lib/api/fd/array.h
> +++ b/tools/lib/api/fd/array.h
> @@ -21,19 +21,27 @@ struct fdarray {
> int nr_alloc;
> int nr_autogrow;
> struct pollfd *entries;
> - union {
> - int idx;
> - void *ptr;
> + struct priv {
> + union {
> + int idx;
> + void *ptr;
> + };
> + unsigned int flags;
> } *priv;
> };
>
> +enum fdarray_flags {
> + fdarray_flag__default = 0x00000000,
> + fdarray_flag__nonfilterable = 0x00000001
> +};
> +
> void fdarray__init(struct fdarray *fda, int nr_autogrow);
> void fdarray__exit(struct fdarray *fda);
>
> struct fdarray *fdarray__new(int nr_alloc, int nr_autogrow);
> void fdarray__delete(struct fdarray *fda);
>
> -int fdarray__add(struct fdarray *fda, int fd, short revents);
> +int fdarray__add(struct fdarray *fda, int fd, short revents, enum fdarray_flags flags);
> int fdarray__poll(struct fdarray *fda, int timeout);
> int fdarray__filter(struct fdarray *fda, short revents,
> void (*entry_destructor)(struct fdarray *fda, int fd, void *arg),
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index 6a875a0f01bb..2208444ecb44 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -305,9 +305,9 @@ int perf_evlist__alloc_pollfd(struct perf_evlist *evlist)
> }
>
> int perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd,
> - void *ptr, short revent)
> + void *ptr, short revent, enum fdarray_flags flags)
> {
> - int pos = fdarray__add(&evlist->pollfd, fd, revent | POLLERR | POLLHUP);
> + int pos = fdarray__add(&evlist->pollfd, fd, revent | POLLERR | POLLHUP, flags);
>
> if (pos >= 0) {
> evlist->pollfd.priv[pos].ptr = ptr;
> @@ -488,7 +488,7 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
> revent = !overwrite ? POLLIN : 0;
>
> if (!evsel->system_wide &&
> - perf_evlist__add_pollfd(evlist, fd, map, revent) < 0) {
> + perf_evlist__add_pollfd(evlist, fd, map, revent, fdarray_flag__default) < 0) {
> perf_mmap__put(map);
> return -1;
> }
> diff --git a/tools/lib/perf/include/internal/evlist.h b/tools/lib/perf/include/internal/evlist.h
> index 74dc8c3f0b66..2d0fa02b036f 100644
> --- a/tools/lib/perf/include/internal/evlist.h
> +++ b/tools/lib/perf/include/internal/evlist.h
> @@ -45,7 +45,7 @@ struct perf_evlist_mmap_ops {
>
> int perf_evlist__alloc_pollfd(struct perf_evlist *evlist);
> int perf_evlist__add_pollfd(struct perf_evlist *evlist, int fd,
> - void *ptr, short revent);
> + void *ptr, short revent, enum fdarray_flags flags);
>
> int perf_evlist__mmap_ops(struct perf_evlist *evlist,
> struct perf_evlist_mmap_ops *ops,
> diff --git a/tools/perf/tests/fdarray.c b/tools/perf/tests/fdarray.c
> index d0c8a05aab2f..d9eca8e86a6b 100644
> --- a/tools/perf/tests/fdarray.c
> +++ b/tools/perf/tests/fdarray.c
> @@ -112,7 +112,7 @@ int test__fdarray__add(struct test *test __maybe_unused, int subtest __maybe_unu
> }
>
> #define FDA_ADD(_idx, _fd, _revents, _nr) \
> - if (fdarray__add(fda, _fd, _revents) < 0) { \
> + if (fdarray__add(fda, _fd, _revents, fdarray_flag__default) < 0) { \
> pr_debug("\n%d: fdarray__add(fda, %d, %d) failed!", \
> __LINE__,_fd, _revents); \
> goto out_delete; \
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 1b884695b4d3..bcbe0cb8482e 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -497,7 +497,7 @@ int perf_evlist__enable_event_idx(struct evlist *evlist,
>
> int evlist__add_pollfd(struct evlist *evlist, int fd)
> {
> - return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN);
> + return perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN, fdarray_flag__default);
> }
>
> int evlist__filter_pollfd(struct evlist *evlist, short revents_and_mask)
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 03/15] tools/libperf: avoid counting of nonfilterable fdarray fds
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
2020-07-17 6:59 ` [PATCH v12 01/15] tools/libperf: avoid internal moving of fdarray fds Alexey Budankov
2020-07-17 6:59 ` [PATCH v12 02/15] tools/libperf: add flags to fdarray fds objects Alexey Budankov
@ 2020-07-17 7:00 ` Alexey Budankov
2020-07-21 19:59 ` Arnaldo Carvalho de Melo
2020-07-17 7:00 ` [PATCH v12 04/15] perf evlist: introduce control file descriptors Alexey Budankov
` (11 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Avoid counting of struct pollfd *entries objects with
fdarray_flag__nonfilterable flag by fdarray__filter().
Nonfilterable objects are still processed if requested
revents have been signaled for them.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/lib/api/fd/array.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
index 01b3b89f9797..5e6cb9debe37 100644
--- a/tools/lib/api/fd/array.c
+++ b/tools/lib/api/fd/array.c
@@ -109,7 +109,8 @@ int fdarray__filter(struct fdarray *fda, short revents,
continue;
}
- ++nr;
+ if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
+ ++nr;
}
return nr;
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v12 03/15] tools/libperf: avoid counting of nonfilterable fdarray fds
2020-07-17 7:00 ` [PATCH v12 03/15] tools/libperf: avoid counting of nonfilterable fdarray fds Alexey Budankov
@ 2020-07-21 19:59 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-21 19:59 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Fri, Jul 17, 2020 at 10:00:15AM +0300, Alexey Budankov escreveu:
>
> Avoid counting of struct pollfd *entries objects with
> fdarray_flag__nonfilterable flag by fdarray__filter().
> Nonfilterable objects are still processed if requested
> revents have been signaled for them.
Thanks, applied.
- Arnaldo
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/lib/api/fd/array.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/api/fd/array.c b/tools/lib/api/fd/array.c
> index 01b3b89f9797..5e6cb9debe37 100644
> --- a/tools/lib/api/fd/array.c
> +++ b/tools/lib/api/fd/array.c
> @@ -109,7 +109,8 @@ int fdarray__filter(struct fdarray *fda, short revents,
> continue;
> }
>
> - ++nr;
> + if (!(fda->priv[fd].flags & fdarray_flag__nonfilterable))
> + ++nr;
> }
>
> return nr;
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 04/15] perf evlist: introduce control file descriptors
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (2 preceding siblings ...)
2020-07-17 7:00 ` [PATCH v12 03/15] tools/libperf: avoid counting of nonfilterable fdarray fds Alexey Budankov
@ 2020-07-17 7:00 ` Alexey Budankov
2020-07-17 7:01 ` [PATCH v12 05/15] perf evlist: implement control command handling functions Alexey Budankov
` (10 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:00 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, 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>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evlist.c | 3 +++
tools/perf/util/evlist.h | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index bcbe0cb8482e..36eb50aba1f5 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.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 38901c0d1599..fe1fe40e044b 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -74,6 +74,11 @@ struct evlist {
pthread_t th;
volatile int done;
} thread;
+ struct {
+ int fd; /* control file descriptor */
+ int ack; /* ack file descriptor for control commands */
+ int pos; /* index at evlist core object to check signals */
+ } ctl_fd;
};
struct evsel_str_handler {
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 05/15] perf evlist: implement control command handling functions
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (3 preceding siblings ...)
2020-07-17 7:00 ` [PATCH v12 04/15] perf evlist: introduce control file descriptors Alexey Budankov
@ 2020-07-17 7:01 ` Alexey Budankov
2020-07-22 12:27 ` Arnaldo Carvalho de Melo
2020-07-22 12:41 ` Arnaldo Carvalho de Melo
2020-07-17 7:02 ` [PATCH v12 06/15] perf stat: factor out body of event handling loop for system wide Alexey Budankov
` (9 subsequent siblings)
14 siblings, 2 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:01 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Implement functions of initialization, finalization and processing
of control command messages coming from control file descriptors.
Allocate control file descriptor as descriptor at struct pollfd
object of evsel_list for atomic poll() operation.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evlist.c | 140 +++++++++++++++++++++++++++++++++++++++
tools/perf/util/evlist.h | 18 +++++
2 files changed, 158 insertions(+)
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 36eb50aba1f5..fe900d82815c 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1714,3 +1714,143 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
}
return leader;
}
+
+int evlist__initialize_ctlfd(struct evlist *evlist, int fd, int ack)
+{
+ if (fd == -1) {
+ pr_debug("Control descriptor is not initialized\n");
+ return 0;
+ }
+
+ evlist->ctl_fd.pos = perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
+ fdarray_flag__nonfilterable);
+ 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.fd = fd;
+ evlist->ctl_fd.ack = ack;
+
+ return 0;
+}
+
+bool evlist__ctlfd_initialized(struct evlist *evlist)
+{
+ return evlist->ctl_fd.pos >= 0;
+}
+
+int evlist__finalize_ctlfd(struct evlist *evlist)
+{
+ struct pollfd *entries = evlist->core.pollfd.entries;
+
+ if (!evlist__ctlfd_initialized(evlist))
+ return 0;
+
+ entries[evlist->ctl_fd.pos].fd = -1;
+ entries[evlist->ctl_fd.pos].events = 0;
+ entries[evlist->ctl_fd.pos].revents = 0;
+
+ evlist->ctl_fd.pos = -1;
+ evlist->ctl_fd.ack = -1;
+ evlist->ctl_fd.fd = -1;
+
+ return 0;
+}
+
+static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
+ char *cmd_data, size_t data_size)
+{
+ int err;
+ char c;
+ size_t bytes_read = 0;
+
+ memset(cmd_data, 0, data_size);
+ data_size--;
+
+ do {
+ err = read(evlist->ctl_fd.fd, &c, 1);
+ if (err > 0) {
+ if (c == '\n' || c == '\0')
+ break;
+ cmd_data[bytes_read++] = c;
+ if (bytes_read == data_size)
+ break;
+ } else {
+ if (err == -1)
+ pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd.fd);
+ break;
+ }
+ } while (1);
+
+ pr_debug("Message from ctl_fd: \"%s%s\"\n", cmd_data,
+ bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0");
+
+ if (err > 0) {
+ if (!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,
+ (sizeof(EVLIST_CTL_CMD_DISABLE_TAG)-1))) {
+ *cmd = EVLIST_CTL_CMD_DISABLE;
+ }
+ }
+
+ return err;
+}
+
+static int evlist__ctlfd_ack(struct evlist *evlist)
+{
+ int err;
+
+ if (evlist->ctl_fd.ack == -1)
+ return 0;
+
+ err = write(evlist->ctl_fd.ack, EVLIST_CTL_CMD_ACK_TAG,
+ sizeof(EVLIST_CTL_CMD_ACK_TAG));
+ if (err == -1)
+ pr_err("failed to write to ctl_ack_fd %d: %m\n", evlist->ctl_fd.ack);
+
+ return err;
+}
+
+int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
+{
+ int err = 0;
+ char cmd_data[EVLIST_CTL_CMD_MAX_LEN];
+ int ctlfd_pos = evlist->ctl_fd.pos;
+ struct pollfd *entries = evlist->core.pollfd.entries;
+
+ if (!evlist__ctlfd_initialized(evlist) || !entries[ctlfd_pos].revents)
+ return 0;
+
+ if (entries[ctlfd_pos].revents & POLLIN) {
+ err = evlist__ctlfd_recv(evlist, cmd, cmd_data,
+ EVLIST_CTL_CMD_MAX_LEN);
+ if (err > 0) {
+ switch (*cmd) {
+ case EVLIST_CTL_CMD_ENABLE:
+ evlist__enable(evlist);
+ break;
+ case EVLIST_CTL_CMD_DISABLE:
+ evlist__disable(evlist);
+ break;
+ case EVLIST_CTL_CMD_ACK:
+ case EVLIST_CTL_CMD_UNSUPPORTED:
+ default:
+ pr_debug("ctlfd: unsupported %d\n", *cmd);
+ break;
+ }
+ if (!(*cmd == EVLIST_CTL_CMD_ACK || *cmd == EVLIST_CTL_CMD_UNSUPPORTED))
+ evlist__ctlfd_ack(evlist);
+ }
+ }
+
+ if (entries[ctlfd_pos].revents & (POLLHUP | POLLERR))
+ 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 fe1fe40e044b..cc628798734d 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -359,4 +359,22 @@ void perf_evlist__force_leader(struct evlist *evlist);
struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
struct evsel *evsel,
bool close);
+#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
+#define EVLIST_CTL_CMD_DISABLE_TAG "disable"
+#define EVLIST_CTL_CMD_ACK_TAG "ack\n"
+
+#define EVLIST_CTL_CMD_MAX_LEN 64
+
+enum evlist_ctl_cmd {
+ EVLIST_CTL_CMD_UNSUPPORTED = 0,
+ EVLIST_CTL_CMD_ENABLE,
+ EVLIST_CTL_CMD_DISABLE,
+ EVLIST_CTL_CMD_ACK
+};
+
+int evlist__initialize_ctlfd(struct evlist *evlist, int ctl_fd, int ctl_fd_ack);
+int evlist__finalize_ctlfd(struct evlist *evlist);
+bool evlist__ctlfd_initialized(struct evlist *evlist);
+int 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 v12 05/15] perf evlist: implement control command handling functions
2020-07-17 7:01 ` [PATCH v12 05/15] perf evlist: implement control command handling functions Alexey Budankov
@ 2020-07-22 12:27 ` Arnaldo Carvalho de Melo
2020-07-22 12:34 ` Arnaldo Carvalho de Melo
2020-07-22 12:41 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-22 12:27 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Fri, Jul 17, 2020 at 10:01:33AM +0300, Alexey Budankov escreveu:
>
> Implement functions of initialization, finalization and processing
> of control command messages coming from control file descriptors.
> Allocate control file descriptor as descriptor at struct pollfd
> object of evsel_list for atomic poll() operation.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
I'm applying locally for testing, but can you answer what is the reason
for that newline in the CMD_ACK_TAG? I tried looking at previous
discussion but couldn't quickly find any reasoning for that
- Arnaldo
> +++ b/tools/perf/util/evlist.h
> @@ -359,4 +359,22 @@ void perf_evlist__force_leader(struct evlist *evlist);
> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
> struct evsel *evsel,
> bool close);
> +#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
> +#define EVLIST_CTL_CMD_DISABLE_TAG "disable"
> +#define EVLIST_CTL_CMD_ACK_TAG "ack\n"
What for? That '\n'?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 05/15] perf evlist: implement control command handling functions
2020-07-22 12:27 ` Arnaldo Carvalho de Melo
@ 2020-07-22 12:34 ` Arnaldo Carvalho de Melo
2020-07-22 12:45 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-22 12:34 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Wed, Jul 22, 2020 at 09:27:34AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Jul 17, 2020 at 10:01:33AM +0300, Alexey Budankov escreveu:
> >
> > Implement functions of initialization, finalization and processing
> > of control command messages coming from control file descriptors.
> > Allocate control file descriptor as descriptor at struct pollfd
> > object of evsel_list for atomic poll() operation.
> >
> > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
>
> I'm applying locally for testing, but can you answer what is the reason
> for that newline in the CMD_ACK_TAG? I tried looking at previous
> discussion but couldn't quickly find any reasoning for that
>
> - Arnaldo
>
> > +++ b/tools/perf/util/evlist.h
> > @@ -359,4 +359,22 @@ void perf_evlist__force_leader(struct evlist *evlist);
> > struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
> > struct evsel *evsel,
> > bool close);
> > +#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
> > +#define EVLIST_CTL_CMD_DISABLE_TAG "disable"
> > +#define EVLIST_CTL_CMD_ACK_TAG "ack\n"
>
> What for? That '\n'?
Oh, I see, this is not a _command_, it is a _response_, and the sender
expects a newline, right?
So I think this needs to be marked clearly as:
// Commands accepted by 'perf record'
#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
#define EVLIST_CTL_CMD_DISABLE_TAG "disable"
// Responses sent by 'perf record'
#define EVLIST_CTL_RSP_ACK "ack"
And then make the response add the newline, as conceivably we could have
something like "nack", "invalid", "enoperm", etc, no?
Anyway, this can be done on top of this series, so I'm continuing
processing it locally.
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 05/15] perf evlist: implement control command handling functions
2020-07-22 12:34 ` Arnaldo Carvalho de Melo
@ 2020-07-22 12:45 ` Alexey Budankov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-22 12:45 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 22.07.2020 15:34, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 22, 2020 at 09:27:34AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Jul 17, 2020 at 10:01:33AM +0300, Alexey Budankov escreveu:
>>>
>>> Implement functions of initialization, finalization and processing
>>> of control command messages coming from control file descriptors.
>>> Allocate control file descriptor as descriptor at struct pollfd
>>> object of evsel_list for atomic poll() operation.
>>>
>>> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
>>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>>> Acked-by: Namhyung Kim <namhyung@kernel.org>
>>
>> I'm applying locally for testing, but can you answer what is the reason
>> for that newline in the CMD_ACK_TAG? I tried looking at previous
>> discussion but couldn't quickly find any reasoning for that
>>
>> - Arnaldo
>>
>>> +++ b/tools/perf/util/evlist.h
>>> @@ -359,4 +359,22 @@ void perf_evlist__force_leader(struct evlist *evlist);
>>> struct evsel *perf_evlist__reset_weak_group(struct evlist *evlist,
>>> struct evsel *evsel,
>>> bool close);
>>> +#define EVLIST_CTL_CMD_ENABLE_TAG "enable"
>>> +#define EVLIST_CTL_CMD_DISABLE_TAG "disable"
>>> +#define EVLIST_CTL_CMD_ACK_TAG "ack\n"
>>
>> What for? That '\n'?
>
> Oh, I see, this is not a _command_, it is a _response_, and the sender
> expects a newline, right?
Right. Shell commands can expect strings terminated by \n, e.g. 'read -u'.
See sample bash script in 00/15 for example.
>
> So I think this needs to be marked clearly as:
>
> // Commands accepted by 'perf record'
>
> #define EVLIST_CTL_CMD_ENABLE_TAG "enable"
> #define EVLIST_CTL_CMD_DISABLE_TAG "disable"
>
>
> // Responses sent by 'perf record'
>
> #define EVLIST_CTL_RSP_ACK "ack"
>
> And then make the response add the newline, as conceivably we could have
> something like "nack", "invalid", "enoperm", etc, no?
That above makes sense.
>
> Anyway, this can be done on top of this series, so I'm continuing
> processing it locally.
>
> - Arnaldo
>
Alexei
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v12 05/15] perf evlist: implement control command handling functions
2020-07-17 7:01 ` [PATCH v12 05/15] perf evlist: implement control command handling functions Alexey Budankov
2020-07-22 12:27 ` Arnaldo Carvalho de Melo
@ 2020-07-22 12:41 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-22 12:41 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Fri, Jul 17, 2020 at 10:01:33AM +0300, Alexey Budankov escreveu:
>
> Implement functions of initialization, finalization and processing
> of control command messages coming from control file descriptors.
> Allocate control file descriptor as descriptor at struct pollfd
> object of evsel_list for atomic poll() operation.
>
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/evlist.c | 140 +++++++++++++++++++++++++++++++++++++++
> tools/perf/util/evlist.h | 18 +++++
> 2 files changed, 158 insertions(+)
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 36eb50aba1f5..fe900d82815c 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1714,3 +1714,143 @@ struct evsel *perf_evlist__reset_weak_group(struct evlist *evsel_list,
> }
> return leader;
> }
> +
> +int evlist__initialize_ctlfd(struct evlist *evlist, int fd, int ack)
> +{
> + if (fd == -1) {
> + pr_debug("Control descriptor is not initialized\n");
> + return 0;
> + }
> +
> + evlist->ctl_fd.pos = perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> + fdarray_flag__nonfilterable);
> + 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.fd = fd;
> + evlist->ctl_fd.ack = ack;
> +
> + return 0;
> +}
> +
> +bool evlist__ctlfd_initialized(struct evlist *evlist)
> +{
> + return evlist->ctl_fd.pos >= 0;
> +}
> +
> +int evlist__finalize_ctlfd(struct evlist *evlist)
> +{
> + struct pollfd *entries = evlist->core.pollfd.entries;
> +
> + if (!evlist__ctlfd_initialized(evlist))
> + return 0;
> +
> + entries[evlist->ctl_fd.pos].fd = -1;
> + entries[evlist->ctl_fd.pos].events = 0;
> + entries[evlist->ctl_fd.pos].revents = 0;
> +
> + evlist->ctl_fd.pos = -1;
> + evlist->ctl_fd.ack = -1;
> + evlist->ctl_fd.fd = -1;
> +
> + return 0;
> +}
> +
> +static int evlist__ctlfd_recv(struct evlist *evlist, enum evlist_ctl_cmd *cmd,
> + char *cmd_data, size_t data_size)
> +{
> + int err;
> + char c;
> + size_t bytes_read = 0;
> +
> + memset(cmd_data, 0, data_size);
> + data_size--;
> +
> + do {
> + err = read(evlist->ctl_fd.fd, &c, 1);
> + if (err > 0) {
> + if (c == '\n' || c == '\0')
> + break;
> + cmd_data[bytes_read++] = c;
> + if (bytes_read == data_size)
> + break;
> + } else {
> + if (err == -1)
> + pr_err("Failed to read from ctlfd %d: %m\n", evlist->ctl_fd.fd);
> + break;
> + }
> + } while (1);
Ok, here you don't know how many bytes you have to read, the protocol is
a string ending in \n or \0, so you can't use 'readn()', but see below
for the write() case.
Also is this ctf_fd in non-blocking mode? Can't this get stuck waiting
forever?
> + pr_debug("Message from ctl_fd: \"%s%s\"\n", cmd_data,
> + bytes_read == data_size ? "" : c == '\n' ? "\\n" : "\\0");
> +
> + if (err > 0) {
> + 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,
> + (sizeof(EVLIST_CTL_CMD_DISABLE_TAG)-1))) {
> + *cmd = EVLIST_CTL_CMD_DISABLE;
> + }
> + }
> +
> + return err;
> +}
> +
> +static int evlist__ctlfd_ack(struct evlist *evlist)
> +{
> + int err;
> +
> + if (evlist->ctl_fd.ack == -1)
> + return 0;
> +
> + err = write(evlist->ctl_fd.ack, EVLIST_CTL_CMD_ACK_TAG,
> + sizeof(EVLIST_CTL_CMD_ACK_TAG));
> + if (err == -1)
> + pr_err("failed to write to ctl_ack_fd %d: %m\n", evlist->ctl_fd.ack);
Also the function to use here is 'writen()':
/*
* Write exactly 'n' bytes or return an error.
*/
ssize_t writen(int fd, const void *buf, size_t n)
{
/* ion does not modify buf. */
return ion(false, fd, (void *)buf, n);
}
"tools/lib/perf/lib.c"
-----------------------------
I.e. libperf's fd based fwrite().
Again, this can be done on top of this patch set, so I'll continue
as-is.
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 06/15] perf stat: factor out body of event handling loop for system wide
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (4 preceding siblings ...)
2020-07-17 7:01 ` [PATCH v12 05/15] perf evlist: implement control command handling functions Alexey Budankov
@ 2020-07-17 7:02 ` Alexey Budankov
2020-07-17 7:02 ` [PATCH v12 07/15] perf stat: move target check to loop control statement Alexey Budankov
` (8 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Introduce handle_interval() function that factors out body of event
handling loop for attach and system wide monitoring use cases.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 922d9961ba98..80aea49e4dac 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -475,6 +475,16 @@ static void process_interval(void)
print_counters(&rs, 0, NULL);
}
+static bool handle_interval(unsigned int interval, int *times)
+{
+ if (interval) {
+ process_interval();
+ if (interval_count && !(--(*times)))
+ return true;
+ }
+ return false;
+}
+
static void enable_counters(void)
{
if (stat_config.initial_delay)
@@ -809,13 +819,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
nanosleep(&ts, NULL);
if (!is_target_alive(&target, evsel_list->core.threads))
break;
- if (timeout)
+ if (timeout || handle_interval(interval, ×))
break;
- if (interval) {
- process_interval();
- if (interval_count && !(--times))
- break;
- }
}
}
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 07/15] perf stat: move target check to loop control statement
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (5 preceding siblings ...)
2020-07-17 7:02 ` [PATCH v12 06/15] perf stat: factor out body of event handling loop for system wide Alexey Budankov
@ 2020-07-17 7:02 ` Alexey Budankov
2020-07-17 7:03 ` [PATCH v12 08/15] perf stat: factor out body of event handling loop for fork case Alexey Budankov
` (7 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:02 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Check for target existence in loop control statement jointly external
asynchronous 'done' signal.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 80aea49e4dac..87eebf3f55ae 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -815,10 +815,8 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
psignal(WTERMSIG(status), argv[0]);
} else {
enable_counters();
- while (!done) {
+ while (!done && is_target_alive(&target, evsel_list->core.threads)) {
nanosleep(&ts, NULL);
- if (!is_target_alive(&target, evsel_list->core.threads))
- break;
if (timeout || handle_interval(interval, ×))
break;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 08/15] perf stat: factor out body of event handling loop for fork case
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (6 preceding siblings ...)
2020-07-17 7:02 ` [PATCH v12 07/15] perf stat: move target check to loop control statement Alexey Budankov
@ 2020-07-17 7:03 ` Alexey Budankov
2020-07-17 7:04 ` [PATCH v12 09/15] perf stat: factor out event handling loop into dispatch_events() Alexey Budankov
` (6 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Factor out body of event handling loop for fork case reusing
handle_interval() function.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 87eebf3f55ae..91f31518948e 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -792,10 +792,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
if (interval || timeout) {
while (!waitpid(child_pid, &status, WNOHANG)) {
nanosleep(&ts, NULL);
- if (timeout)
- break;
- process_interval();
- if (interval_count && !(--times))
+ if (timeout || handle_interval(interval, ×))
break;
}
}
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 09/15] perf stat: factor out event handling loop into dispatch_events()
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (7 preceding siblings ...)
2020-07-17 7:03 ` [PATCH v12 08/15] perf stat: factor out body of event handling loop for fork case Alexey Budankov
@ 2020-07-17 7:04 ` Alexey Budankov
2020-07-17 7:04 ` [PATCH v12 10/15] perf stat: extend -D,--delay option with -1 value Alexey Budankov
` (5 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:04 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Consolidate event dispatching loops for fork, attach and system
wide monitoring use cases into common dispatch_events() function.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 91f31518948e..a5a0f4841003 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -550,6 +550,27 @@ static bool is_target_alive(struct target *_target,
return false;
}
+static int dispatch_events(bool forks, int timeout, int interval, int *times, struct timespec *ts)
+{
+ int child_exited = 0, status = 0;
+
+ while (!done) {
+ if (forks)
+ child_exited = waitpid(child_pid, &status, WNOHANG);
+ else
+ child_exited = !is_target_alive(&target, evsel_list->core.threads) ? 1 : 0;
+
+ if (child_exited)
+ break;
+
+ nanosleep(ts, NULL);
+ if (timeout || handle_interval(interval, times))
+ break;
+ }
+
+ return status;
+}
+
enum counter_recovery {
COUNTER_SKIP,
COUNTER_RETRY,
@@ -789,13 +810,8 @@ 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 || handle_interval(interval, ×))
- break;
- }
- }
+ if (interval || timeout)
+ status = dispatch_events(forks, timeout, interval, ×, &ts);
if (child_pid != -1) {
if (timeout)
kill(child_pid, SIGTERM);
@@ -812,11 +828,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
psignal(WTERMSIG(status), argv[0]);
} else {
enable_counters();
- while (!done && is_target_alive(&target, evsel_list->core.threads)) {
- nanosleep(&ts, NULL);
- if (timeout || handle_interval(interval, ×))
- break;
- }
+ status = dispatch_events(forks, timeout, interval, ×, &ts);
}
disable_counters();
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 10/15] perf stat: extend -D,--delay option with -1 value
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (8 preceding siblings ...)
2020-07-17 7:04 ` [PATCH v12 09/15] perf stat: factor out event handling loop into dispatch_events() Alexey Budankov
@ 2020-07-17 7:04 ` Alexey Budankov
2020-07-17 7:05 ` [PATCH v12 11/15] perf stat: implement control commands handling Alexey Budankov
` (4 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:04 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, 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>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-stat.txt | 5 +++--
tools/perf/builtin-stat.c | 18 ++++++++++++++----
tools/perf/util/evlist.h | 3 +++
tools/perf/util/stat.h | 2 +-
4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index b029ee728a0b..9f32f6cd558d 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -238,8 +238,9 @@ mode, use --per-node in addition to -a. (system-wide).
-D msecs::
--delay msecs::
-After starting the program, wait msecs before measuring. This is useful to
-filter out the startup phase of the program, which is often very different.
+After starting the program, wait msecs before measuring (-1: start with events
+disabled). This is useful to filter out the startup phase of the program,
+which is often very different.
-T::
--transaction::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index a5a0f4841003..9d5c503e698f 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -487,16 +487,26 @@ static bool handle_interval(unsigned int interval, int *times)
static void enable_counters(void)
{
- if (stat_config.initial_delay)
+ if (stat_config.initial_delay < 0) {
+ pr_info(EVLIST_DISABLED_MSG);
+ return;
+ }
+
+ if (stat_config.initial_delay > 0) {
+ pr_info(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(EVLIST_ENABLED_MSG);
+ }
}
static void disable_counters(void)
@@ -1053,8 +1063,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, "metric-no-group", &stat_config.metric_no_group,
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index cc628798734d..8ee12c1f0526 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -377,4 +377,7 @@ int evlist__finalize_ctlfd(struct evlist *evlist);
bool evlist__ctlfd_initialized(struct evlist *evlist);
int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
+#define EVLIST_ENABLED_MSG "Events enabled\n"
+#define EVLIST_DISABLED_MSG "Events disabled\n"
+
#endif /* __PERF_EVLIST_H */
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 6911c7249199..41d59f192931 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -116,7 +116,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* [PATCH v12 11/15] perf stat: implement control commands handling
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (9 preceding siblings ...)
2020-07-17 7:04 ` [PATCH v12 10/15] perf stat: extend -D,--delay option with -1 value Alexey Budankov
@ 2020-07-17 7:05 ` Alexey Budankov
2020-07-22 12:56 ` Arnaldo Carvalho de Melo
2020-07-17 7:05 ` [PATCH v12 12/15] perf stat: introduce --control fd:ctl-fd[,ack-fd] options Alexey Budankov
` (3 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:05 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, 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. If poll event splits initiated timeout
interval then the reminder is calculated and still waited in the
following evlist__poll() call.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 83 ++++++++++++++++++++++++++++++---------
1 file changed, 65 insertions(+), 18 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 9d5c503e698f..5280a45af5dc 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -560,9 +560,61 @@ static bool is_target_alive(struct target *_target,
return false;
}
-static int dispatch_events(bool forks, int timeout, int interval, int *times, struct timespec *ts)
+static void process_evlist(struct evlist *evlist, unsigned int interval)
+{
+ enum evlist_ctl_cmd cmd = EVLIST_CTL_CMD_UNSUPPORTED;
+
+ if (evlist__ctlfd_process(evlist, &cmd) > 0) {
+ switch (cmd) {
+ case EVLIST_CTL_CMD_ENABLE:
+ pr_info(EVLIST_ENABLED_MSG);
+ if (interval)
+ process_interval();
+ break;
+ case EVLIST_CTL_CMD_DISABLE:
+ if (interval)
+ process_interval();
+ pr_info(EVLIST_DISABLED_MSG);
+ break;
+ case EVLIST_CTL_CMD_ACK:
+ case EVLIST_CTL_CMD_UNSUPPORTED:
+ default:
+ break;
+ }
+ }
+}
+
+static void compute_tts(struct timespec *time_start, struct timespec *time_stop,
+ int *time_to_sleep)
+{
+ int tts = *time_to_sleep;
+ struct timespec time_diff;
+
+ diff_timespec(&time_diff, time_stop, time_start);
+
+ tts -= time_diff.tv_sec * MSEC_PER_SEC +
+ time_diff.tv_nsec / NSEC_PER_MSEC;
+
+ if (tts < 0)
+ tts = 0;
+
+ *time_to_sleep = tts;
+}
+
+static int dispatch_events(bool forks, int timeout, int interval, int *times)
{
int child_exited = 0, status = 0;
+ int time_to_sleep, sleep_time;
+ struct timespec time_start, time_stop;
+
+ if (interval)
+ sleep_time = interval;
+ else if (timeout)
+ sleep_time = timeout;
+ else
+ sleep_time = 1000;
+
+ time_to_sleep = sleep_time;
while (!done) {
if (forks)
@@ -573,9 +625,16 @@ static int dispatch_events(bool forks, int timeout, int interval, int *times, st
if (child_exited)
break;
- nanosleep(ts, NULL);
- if (timeout || handle_interval(interval, times))
- break;
+ clock_gettime(CLOCK_MONOTONIC, &time_start);
+ if (!(evlist__poll(evsel_list, time_to_sleep) > 0)) { /* poll timeout or EINTR */
+ if (timeout || handle_interval(interval, times))
+ break;
+ time_to_sleep = sleep_time;
+ } else { /* fd revent */
+ process_evlist(evsel_list, interval);
+ clock_gettime(CLOCK_MONOTONIC, &time_stop);
+ compute_tts(&time_start, &time_stop, &time_to_sleep);
+ }
}
return status;
@@ -644,7 +703,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
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);
@@ -653,17 +711,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) {
@@ -821,7 +868,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
enable_counters();
if (interval || timeout)
- status = dispatch_events(forks, timeout, interval, ×, &ts);
+ status = dispatch_events(forks, timeout, interval, ×);
if (child_pid != -1) {
if (timeout)
kill(child_pid, SIGTERM);
@@ -838,7 +885,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
psignal(WTERMSIG(status), argv[0]);
} else {
enable_counters();
- status = dispatch_events(forks, timeout, interval, ×, &ts);
+ status = dispatch_events(forks, timeout, interval, ×);
}
disable_counters();
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v12 11/15] perf stat: implement control commands handling
2020-07-17 7:05 ` [PATCH v12 11/15] perf stat: implement control commands handling Alexey Budankov
@ 2020-07-22 12:56 ` Arnaldo Carvalho de Melo
2020-07-22 13:18 ` Alexey Budankov
0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2020-07-22 12:56 UTC (permalink / raw)
To: Alexey Budankov
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Em Fri, Jul 17, 2020 at 10:05:06AM +0300, Alexey Budankov escreveu:
>
> Implement handling of 'enable' and 'disable' control commands coming
> from control file descriptor. If poll event splits initiated timeout
> interval then the reminder is calculated and still waited in the
> following evlist__poll() call.
So the cover letter is not preserved in the git history, only going from
the Link tag to the patch series to then lookup the cover letter and
then find examples of each feature, I'll now have to do that and collect
the parts that are needed to test this specific feature implemented in
this specific patch, add the command here so that we can test patch by
patch, please next time try to have the testing steps in each patch.
This way we avoid introducing bisection regressions, as having the set
of instructions only on the cover letter encourages testing only the
whole patchkit, not patch by patch.
Thanks,
- Arnaldo
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-stat.c | 83 ++++++++++++++++++++++++++++++---------
> 1 file changed, 65 insertions(+), 18 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 9d5c503e698f..5280a45af5dc 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -560,9 +560,61 @@ static bool is_target_alive(struct target *_target,
> return false;
> }
>
> -static int dispatch_events(bool forks, int timeout, int interval, int *times, struct timespec *ts)
> +static void process_evlist(struct evlist *evlist, unsigned int interval)
> +{
> + enum evlist_ctl_cmd cmd = EVLIST_CTL_CMD_UNSUPPORTED;
> +
> + if (evlist__ctlfd_process(evlist, &cmd) > 0) {
> + switch (cmd) {
> + case EVLIST_CTL_CMD_ENABLE:
> + pr_info(EVLIST_ENABLED_MSG);
> + if (interval)
> + process_interval();
> + break;
> + case EVLIST_CTL_CMD_DISABLE:
> + if (interval)
> + process_interval();
> + pr_info(EVLIST_DISABLED_MSG);
> + break;
> + case EVLIST_CTL_CMD_ACK:
> + case EVLIST_CTL_CMD_UNSUPPORTED:
> + default:
> + break;
> + }
> + }
> +}
> +
> +static void compute_tts(struct timespec *time_start, struct timespec *time_stop,
> + int *time_to_sleep)
> +{
> + int tts = *time_to_sleep;
> + struct timespec time_diff;
> +
> + diff_timespec(&time_diff, time_stop, time_start);
> +
> + tts -= time_diff.tv_sec * MSEC_PER_SEC +
> + time_diff.tv_nsec / NSEC_PER_MSEC;
> +
> + if (tts < 0)
> + tts = 0;
> +
> + *time_to_sleep = tts;
> +}
> +
> +static int dispatch_events(bool forks, int timeout, int interval, int *times)
> {
> int child_exited = 0, status = 0;
> + int time_to_sleep, sleep_time;
> + struct timespec time_start, time_stop;
> +
> + if (interval)
> + sleep_time = interval;
> + else if (timeout)
> + sleep_time = timeout;
> + else
> + sleep_time = 1000;
> +
> + time_to_sleep = sleep_time;
>
> while (!done) {
> if (forks)
> @@ -573,9 +625,16 @@ static int dispatch_events(bool forks, int timeout, int interval, int *times, st
> if (child_exited)
> break;
>
> - nanosleep(ts, NULL);
> - if (timeout || handle_interval(interval, times))
> - break;
> + clock_gettime(CLOCK_MONOTONIC, &time_start);
> + if (!(evlist__poll(evsel_list, time_to_sleep) > 0)) { /* poll timeout or EINTR */
> + if (timeout || handle_interval(interval, times))
> + break;
> + time_to_sleep = sleep_time;
> + } else { /* fd revent */
> + process_evlist(evsel_list, interval);
> + clock_gettime(CLOCK_MONOTONIC, &time_stop);
> + compute_tts(&time_start, &time_stop, &time_to_sleep);
> + }
> }
>
> return status;
> @@ -644,7 +703,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> 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);
> @@ -653,17 +711,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) {
> @@ -821,7 +868,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> enable_counters();
>
> if (interval || timeout)
> - status = dispatch_events(forks, timeout, interval, ×, &ts);
> + status = dispatch_events(forks, timeout, interval, ×);
> if (child_pid != -1) {
> if (timeout)
> kill(child_pid, SIGTERM);
> @@ -838,7 +885,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> psignal(WTERMSIG(status), argv[0]);
> } else {
> enable_counters();
> - status = dispatch_events(forks, timeout, interval, ×, &ts);
> + status = dispatch_events(forks, timeout, interval, ×);
> }
>
> disable_counters();
> --
> 2.24.1
>
>
--
- Arnaldo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v12 11/15] perf stat: implement control commands handling
2020-07-22 12:56 ` Arnaldo Carvalho de Melo
@ 2020-07-22 13:18 ` Alexey Budankov
0 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-22 13:18 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
On 22.07.2020 15:56, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jul 17, 2020 at 10:05:06AM +0300, Alexey Budankov escreveu:
>>
>> Implement handling of 'enable' and 'disable' control commands coming
>> from control file descriptor. If poll event splits initiated timeout
>> interval then the reminder is calculated and still waited in the
>> following evlist__poll() call.
>
> So the cover letter is not preserved in the git history, only going from
> the Link tag to the patch series to then lookup the cover letter and
> then find examples of each feature, I'll now have to do that and collect
> the parts that are needed to test this specific feature implemented in
> this specific patch, add the command here so that we can test patch by
> patch, please next time try to have the testing steps in each patch.
Accepted. Will be providing possible test steps as a part of description
for relevant patches.
Thanks,
Alexei
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v12 12/15] perf stat: introduce --control fd:ctl-fd[,ack-fd] options
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (10 preceding siblings ...)
2020-07-17 7:05 ` [PATCH v12 11/15] perf stat: implement control commands handling Alexey Budankov
@ 2020-07-17 7:05 ` Alexey Budankov
2020-07-17 7:07 ` [PATCH v12 13/15] perf record: extend -D,--delay option with -1 value Alexey Budankov
` (2 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:05 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Introduce --control fd:ctl-fd[,ack-fd] options to pass open file
descriptors numbers from command line. Extend perf-stat.txt file
with --control fd:ctl-fd[,ack-fd] options description. Document
possible usage model introduced by --control fd:ctl-fd[,ack-fd]
options by providing example bash shell script.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-stat.txt | 39 +++++++++++++++++++++++++
tools/perf/builtin-stat.c | 40 +++++++++++++++++++++++++-
tools/perf/util/stat.h | 2 ++
3 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 9f32f6cd558d..c9bfefc051fb 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -176,6 +176,45 @@ with it. --append may be used here. Examples:
3>results perf stat --log-fd 3 -- $cmd
3>>results perf stat --log-fd 3 --append -- $cmd
+--control fd:ctl-fd[,ack-fd]
+Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
+'disable': disable events). Measurements can be started with events disabled using
+--delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
+to synchronize with the controlling process. Example of bash shell script to enable and
+disable events during measurements:
+
+#!/bin/bash
+
+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 \
+ --control fd:${ctl_fd},${ctl_fd_ack} \
+ -- sleep 30 &
+perf_pid=$!
+
+sleep 5 && echo 'enable' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
+sleep 10 && echo 'disable' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
+
+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.:
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 5280a45af5dc..483a28ef4ec4 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -188,6 +188,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 bool cpus_map_matched(struct evsel *a, struct evsel *b)
@@ -867,7 +869,7 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
perf_evlist__start_workload(evsel_list);
enable_counters();
- if (interval || timeout)
+ if (interval || timeout || evlist__ctlfd_initialized(evsel_list))
status = dispatch_events(forks, timeout, interval, ×);
if (child_pid != -1) {
if (timeout)
@@ -1039,6 +1041,33 @@ static int parse_metric_groups(const struct option *opt,
&stat_config.metric_events);
}
+static int parse_control_option(const struct option *opt,
+ const char *str,
+ int unset __maybe_unused)
+{
+ char *comma = NULL, *endptr = NULL;
+ struct perf_stat_config *config = (struct perf_stat_config *)opt->value;
+
+ if (strncmp(str, "fd:", 3))
+ return -EINVAL;
+
+ config->ctl_fd = strtoul(&str[3], &endptr, 0);
+ if (endptr == &str[3])
+ return -EINVAL;
+
+ comma = strchr(str, ',');
+ if (comma) {
+ if (endptr != comma)
+ return -EINVAL;
+
+ config->ctl_fd_ack = strtoul(comma + 1, &endptr, 0);
+ if (endptr == comma + 1 || *endptr != '\0')
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
@@ -1140,6 +1169,10 @@ static struct option stat_options[] = {
"libpfm4 event selector. use 'perf list' to list available events",
parse_libpfm_events_option),
#endif
+ OPT_CALLBACK(0, "control", &stat_config, "fd:ctl-fd[,ack-fd]",
+ "Listen on ctl-fd descriptor for command to control measurement ('enable': enable events, 'disable': disable events).\n"
+ "\t\t\t Optionally send control command completion ('ack\\n') to ack-fd descriptor.",
+ parse_control_option),
OPT_END()
};
@@ -2309,6 +2342,9 @@ int cmd_stat(int argc, const char **argv)
signal(SIGALRM, skip_signal);
signal(SIGABRT, skip_signal);
+ if (evlist__initialize_ctlfd(evsel_list, stat_config.ctl_fd, stat_config.ctl_fd_ack))
+ goto out;
+
status = 0;
for (run_idx = 0; forever || run_idx < stat_config.run_count; run_idx++) {
if (stat_config.run_count != 1 && verbose > 0)
@@ -2328,6 +2364,8 @@ int cmd_stat(int argc, const char **argv)
if (!forever && status != -1 && (!interval || stat_config.summary))
print_counters(NULL, argc, argv);
+ 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 41d59f192931..f8778cffd941 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -133,6 +133,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 perf_stat__set_big_num(int set);
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 13/15] perf record: extend -D,--delay option with -1 value
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (11 preceding siblings ...)
2020-07-17 7:05 ` [PATCH v12 12/15] perf stat: introduce --control fd:ctl-fd[,ack-fd] options Alexey Budankov
@ 2020-07-17 7:07 ` Alexey Budankov
2020-07-17 7:07 ` [PATCH v12 14/15] perf record: implement control commands handling Alexey Budankov
2020-07-17 7:08 ` [PATCH v12 15/15] perf record: introduce --control fd:ctl-fd[,ack-fd] options Alexey Budankov
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:07 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, 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 enabled later by 'enable' command provided via
control file descriptor.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-record.txt | 5 +++--
tools/perf/builtin-record.c | 12 ++++++++----
tools/perf/builtin-trace.c | 2 +-
tools/perf/util/record.h | 2 +-
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index fa8a5fcd27ab..a84376605805 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -407,8 +407,9 @@ if combined with -a or -C options.
-D::
--delay=::
-After starting the program, wait msecs before measuring. This is useful to
-filter out the startup phase of the program, which is often very different.
+After starting the program, wait msecs before measuring (-1: start with events
+disabled). This is useful to filter out the startup phase of the program, which
+is often very different.
-I::
--intr-regs::
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 19b1d5effb7a..cd1892c4844b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1749,8 +1749,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(EVLIST_DISABLED_MSG);
+ if (opts->initial_delay > 0) {
+ usleep(opts->initial_delay * USEC_PER_MSEC);
+ evlist__enable(rec->evlist);
+ pr_info(EVLIST_ENABLED_MSG);
+ }
}
trigger_ready(&auxtrace_snapshot_trigger);
@@ -2462,8 +2466,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 a333a9a64f27..bea461b6f937 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4812,7 +4812,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 39d1de4b2a36..da138dcb4d34 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -61,7 +61,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 v12 14/15] perf record: implement control commands handling
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (12 preceding siblings ...)
2020-07-17 7:07 ` [PATCH v12 13/15] perf record: extend -D,--delay option with -1 value Alexey Budankov
@ 2020-07-17 7:07 ` Alexey Budankov
2020-07-17 7:08 ` [PATCH v12 15/15] perf record: introduce --control fd:ctl-fd[,ack-fd] options Alexey Budankov
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:07 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, 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>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-record.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index cd1892c4844b..632e61fe70bd 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1527,6 +1527,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 = EVLIST_CTL_CMD_UNSUPPORTED;
atexit(record__sig_exit);
signal(SIGCHLD, sig_handler);
@@ -1846,6 +1847,21 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
draining = true;
}
+ if (evlist__ctlfd_process(rec->evlist, &cmd) > 0) {
+ switch (cmd) {
+ case EVLIST_CTL_CMD_ENABLE:
+ pr_info(EVLIST_ENABLED_MSG);
+ break;
+ case EVLIST_CTL_CMD_DISABLE:
+ pr_info(EVLIST_DISABLED_MSG);
+ break;
+ case EVLIST_CTL_CMD_ACK:
+ case EVLIST_CTL_CMD_UNSUPPORTED:
+ default:
+ break;
+ }
+ }
+
/*
* When perf is starting the traced process, at the end events
* die with the process and we wait for that. Thus no need to
--
2.24.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v12 15/15] perf record: introduce --control fd:ctl-fd[,ack-fd] options
2020-07-17 6:45 [PATCH v12 00/15] perf: support enable and disable commands in stat and record modes Alexey Budankov
` (13 preceding siblings ...)
2020-07-17 7:07 ` [PATCH v12 14/15] perf record: implement control commands handling Alexey Budankov
@ 2020-07-17 7:08 ` Alexey Budankov
14 siblings, 0 replies; 25+ messages in thread
From: Alexey Budankov @ 2020-07-17 7:08 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Jiri Olsa, Namhyung Kim, Alexander Shishkin, Peter Zijlstra,
Ingo Molnar, Andi Kleen, linux-kernel
Introduce --control fd:ctl-fd[,ack-fd] options to pass open file
descriptors numbers from command line. Extend perf-record.txt file
with --control fd:ctl-fd[,ack-fd] options description. Document
possible usage model introduced by --control fd:ctl-fd[,ack-fd]
options by providing example bash shell script.
Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-record.txt | 39 ++++++++++++++++++++++++
tools/perf/builtin-record.c | 37 ++++++++++++++++++++++
tools/perf/util/record.h | 2 ++
3 files changed, 78 insertions(+)
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index a84376605805..3f72d8e261f3 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -627,6 +627,45 @@ option. The -e option and this one can be mixed and matched. Events
can be grouped using the {} notation.
endif::HAVE_LIBPFM[]
+--control fd:ctl-fd[,ack-fd]
+Listen on ctl-fd descriptor for command to control measurement ('enable': enable events,
+'disable': disable events). Measurements can be started with events disabled using
+--delay=-1 option. Optionally send control command completion ('ack\n') to ack-fd descriptor
+to synchronize with the controlling process. Example of bash shell script to enable and
+disable events during measurements:
+
+#!/bin/bash
+
+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 \
+ --control fd:${ctl_fd},${ctl_fd_ack} \
+ -- sleep 30 &
+perf_pid=$!
+
+sleep 5 && echo 'enable' >&${ctl_fd} && read -u ${ctl_fd_ack} e1 && echo "enabled(${e1})"
+sleep 10 && echo 'disable' >&${ctl_fd} && read -u ${ctl_fd_ack} d1 && echo "disabled(${d1})"
+
+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]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 632e61fe70bd..0718aa71b4ba 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1749,6 +1749,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
perf_evlist__start_workload(rec->evlist);
}
+ if (evlist__initialize_ctlfd(rec->evlist, opts->ctl_fd, opts->ctl_fd_ack))
+ goto out_child;
+
if (opts->initial_delay) {
pr_info(EVLIST_DISABLED_MSG);
if (opts->initial_delay > 0) {
@@ -1895,6 +1898,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
record__synthesize_workload(rec, true);
out_child:
+ evlist__finalize_ctlfd(rec->evlist);
record__mmap_read_all(rec, true);
record__aio_mmap_read_sync(rec);
@@ -2244,6 +2248,33 @@ static int record__parse_mmap_pages(const struct option *opt,
return ret;
}
+static int parse_control_option(const struct option *opt,
+ const char *str,
+ int unset __maybe_unused)
+{
+ char *comma = NULL, *endptr = NULL;
+ struct record_opts *config = (struct record_opts *)opt->value;
+
+ if (strncmp(str, "fd:", 3))
+ return -EINVAL;
+
+ config->ctl_fd = strtoul(&str[3], &endptr, 0);
+ if (endptr == &str[3])
+ return -EINVAL;
+
+ comma = strchr(str, ',');
+ if (comma) {
+ if (endptr != comma)
+ return -EINVAL;
+
+ config->ctl_fd_ack = strtoul(comma + 1, &endptr, 0);
+ if (endptr == comma + 1 || *endptr != '\0')
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static void switch_output_size_warn(struct record *rec)
{
u64 wakeup_size = evlist__mmap_size(rec->opts.mmap_pages);
@@ -2380,6 +2411,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,
@@ -2581,6 +2614,10 @@ static struct option __record_options[] = {
"libpfm4 event selector. use 'perf list' to list available events",
parse_libpfm_events_option),
#endif
+ OPT_CALLBACK(0, "control", &record.opts, "fd:ctl-fd[,ack-fd]",
+ "Listen on ctl-fd descriptor for command to control measurement ('enable': enable events, 'disable': disable events).\n"
+ "\t\t\t Optionally send control command completion ('ack\\n') to ack-fd descriptor.",
+ parse_control_option),
OPT_END()
};
diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
index da138dcb4d34..4cb72a478af1 100644
--- a/tools/perf/util/record.h
+++ b/tools/perf/util/record.h
@@ -70,6 +70,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