lkml.org 
[lkml]   [2022]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 1/5] perf record: Fix way of handling non-perf-event pollfds
On Wed, Aug 24, 2022 at 12:28 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> perf record __cmd_record() does not poll evlist pollfds. Instead it polls
> thread_data[0].pollfd. That happens whether or not threads are being used.
>
> perf record duplicates evlist mmap pollfds as needed for separate threads.
> The non-perf-event represented by evlist->ctl_fd has to handled separately,
> which is done explicitly, duplicating it into the thread_data[0] pollfds.
> That approach neglects any other non-perf-event file descriptors. Currently
> there is also done_fd which needs the same handling.
>
> Add a new generalized approach.
>
> Add fdarray_flag__non_perf_event to identify the file descriptors that
> need the special handling. For those cases, also keep a mapping of the
> evlist pollfd index and thread pollfd index, so that the evlist revents
> can be updated.
>
> Although this patch adds the new handling, it does not take it into use.
> There is no functional change, but it is the precursor to a fix, so is
> marked as a fix.
>
> Fixes: 415ccb58f68a ("perf record: Introduce thread specific data array")
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Acked-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
> tools/lib/api/fd/array.h | 5 ++-
> tools/perf/builtin-record.c | 80 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 83 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/api/fd/array.h b/tools/lib/api/fd/array.h
> index 60ad197c8ee9..5c01f7b05dfb 100644
> --- a/tools/lib/api/fd/array.h
> +++ b/tools/lib/api/fd/array.h
> @@ -31,8 +31,9 @@ struct fdarray {
> };
>
> enum fdarray_flags {
> - fdarray_flag__default = 0x00000000,
> - fdarray_flag__nonfilterable = 0x00000001
> + fdarray_flag__default = 0x00000000,
> + fdarray_flag__nonfilterable = 0x00000001,
> + fdarray_flag__non_perf_event = 0x00000002,
> };
>
> void fdarray__init(struct fdarray *fda, int nr_autogrow);
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 4713f0f3a6cf..e0be48c47f65 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -143,6 +143,11 @@ static const char *thread_spec_tags[THREAD_SPEC__MAX] = {
> "undefined", "cpu", "core", "package", "numa", "user"
> };
>
> +struct pollfd_index_map {
> + int evlist_pollfd_index;
> + int thread_pollfd_index;
> +};
> +
> struct record {
> struct perf_tool tool;
> struct record_opts opts;
> @@ -171,6 +176,9 @@ struct record {
> int nr_threads;
> struct thread_mask *thread_masks;
> struct record_thread *thread_data;
> + struct pollfd_index_map *index_map;
> + size_t index_map_sz;
> + size_t index_map_cnt;
> };
>
> static volatile int done;
> @@ -1074,6 +1082,70 @@ static void record__free_thread_data(struct record *rec)
> zfree(&rec->thread_data);
> }
>
> +static int record__map_thread_evlist_pollfd_indexes(struct record *rec,
> + int evlist_pollfd_index,
> + int thread_pollfd_index)
> +{
> + size_t x = rec->index_map_cnt;
> +
> + if (realloc_array_as_needed(rec->index_map, rec->index_map_sz, x, NULL))
> + return -ENOMEM;
> + rec->index_map[x].evlist_pollfd_index = evlist_pollfd_index;
> + rec->index_map[x].thread_pollfd_index = thread_pollfd_index;
> + rec->index_map_cnt += 1;
> + return 0;
> +}
> +
> +static int record__update_evlist_pollfd_from_thread(struct record *rec,
> + struct evlist *evlist,
> + struct record_thread *thread_data)
> +{
> + struct pollfd *e_entries = evlist->core.pollfd.entries;
> + struct pollfd *t_entries = thread_data->pollfd.entries;
> + int err = 0;
> + size_t i;
> +
> + for (i = 0; i < rec->index_map_cnt; i++) {
> + int e_pos = rec->index_map[i].evlist_pollfd_index;
> + int t_pos = rec->index_map[i].thread_pollfd_index;
> +
> + if (e_entries[e_pos].fd != t_entries[t_pos].fd ||
> + e_entries[e_pos].events != t_entries[t_pos].events) {
> + pr_err("Thread and evlist pollfd index mismatch\n");
> + err = -EINVAL;
> + continue;
> + }
> + e_entries[e_pos].revents = t_entries[t_pos].revents;
> + }
> + return err;
> +}
> +
> +static int record__dup_non_perf_events(struct record *rec,
> + struct evlist *evlist,
> + struct record_thread *thread_data)
> +{
> + struct fdarray *fda = &evlist->core.pollfd;
> + int i, ret;
> +
> + for (i = 0; i < fda->nr; i++) {
> + if (!(fda->priv[i].flags & fdarray_flag__non_perf_event))
> + continue;
> + ret = fdarray__dup_entry_from(&thread_data->pollfd, i, fda);
> + if (ret < 0) {
> + pr_err("Failed to duplicate descriptor in main thread pollfd\n");
> + return ret;
> + }
> + pr_debug2("thread_data[%p]: pollfd[%d] <- non_perf_event fd=%d\n",
> + thread_data, ret, fda->entries[i].fd);
> + ret = record__map_thread_evlist_pollfd_indexes(rec, i, ret);
> + if (ret < 0) {
> + pr_err("Failed to map thread and evlist pollfd indexes\n");
> + return ret;
> + }
> + }
> + return 0;
> +}
> +
> static int record__alloc_thread_data(struct record *rec, struct evlist *evlist)
> {
> int t, ret;
> @@ -1121,6 +1193,11 @@ static int record__alloc_thread_data(struct record *rec, struct evlist *evlist)
> thread_data[t].pipes.msg[0]);
> } else {
> thread_data[t].tid = gettid();
> +
> + ret = record__dup_non_perf_events(rec, evlist, &thread_data[t]);
> + if (ret < 0)
> + goto out_free;
> +
> if (evlist->ctl_fd.pos == -1)
> continue;
> ret = fdarray__dup_entry_from(&thread_data[t].pollfd, evlist->ctl_fd.pos,
> @@ -2530,6 +2607,9 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
> record__thread_munmap_filtered, NULL) == 0)
> draining = true;
>
> + err = record__update_evlist_pollfd_from_thread(rec, rec->evlist, thread);
> + if (err)
> + goto out_child;
> evlist__ctlfd_update(rec->evlist,
> &thread->pollfd.entries[thread->ctlfd_pos]);
> }
> --
> 2.25.1
>

\
 
 \ /
  Last update: 2022-08-24 17:42    [W:0.093 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site