| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fUsWcxivcG0M6bju2YaiNmO1d9SPFDviJWm7kdqSxx2Qg@mail.gmail.com>
Date: Wed, 24 Aug 2022 08:40:53 -0700
From: Ian Rogers <irogers@...gle.com>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Andi Kleen <ak@...ux.intel.com>,
Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>,
linux-kernel@...r.kernel.org
Subject: Re: [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@...el.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@...el.com>
Acked-by: Ian Rogers <irogers@...gle.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
>
Powered by blists - more mailing lists