Re: [PATCH 3/5] perf record: Change evlist->ctl_fd to use fdarray_flag__non_perf_event

[Ian Rogers]

On Wed, Aug 24, 2022 at 12:28 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> Patch "perf record: Fix way of handling non-perf-event pollfds" added a
> generic way to handle non-perf-event file descriptors like evlist->ctl_fd.
> Use it instead of handling evlist->ctl_fd separately.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>

Acked-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
>  tools/perf/builtin-record.c | 15 +--------------
>  tools/perf/util/evlist.c    | 19 ++-----------------
>  tools/perf/util/evlist.h    |  1 -
>  3 files changed, 3 insertions(+), 32 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index e0be48c47f65..cefb3028f565 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1198,18 +1198,7 @@ static int record__alloc_thread_data(struct record *rec, struct evlist *evlist)
>                         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,
> -                                                     &evlist->core.pollfd);
> -                       if (ret < 0) {
> -                               pr_err("Failed to duplicate descriptor in main thread pollfd\n");
> -                               goto out_free;
> -                       }
> -                       thread_data[t].ctlfd_pos = ret;
> -                       pr_debug2("thread_data[%p]: pollfd[%d] <- ctl_fd=%d\n",
> -                                thread_data, thread_data[t].ctlfd_pos,
> -                                evlist->core.pollfd.entries[evlist->ctl_fd.pos].fd);
> +                       thread_data[t].ctlfd_pos = -1; /* Not used */
>                 }
>         }
>
> @@ -2610,8 +2599,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>                         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]);
>                 }
>
>                 if (evlist__ctlfd_process(rec->evlist, &cmd) > 0) {
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 0b2222d05577..4c5e6e9f8d11 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1898,7 +1898,8 @@ int evlist__initialize_ctlfd(struct evlist *evlist, int fd, int ack)
>         }
>
>         evlist->ctl_fd.pos = perf_evlist__add_pollfd(&evlist->core, fd, NULL, POLLIN,
> -                                                    fdarray_flag__nonfilterable);
> +                                                    fdarray_flag__nonfilterable |
> +                                                    fdarray_flag__non_perf_event);
>         if (evlist->ctl_fd.pos < 0) {
>                 evlist->ctl_fd.pos = -1;
>                 pr_err("Failed to add ctl fd entry: %m\n");
> @@ -2148,22 +2149,6 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
>         return err;
>  }
>
> -int evlist__ctlfd_update(struct evlist *evlist, struct pollfd *update)
> -{
> -       int ctlfd_pos = evlist->ctl_fd.pos;
> -       struct pollfd *entries = evlist->core.pollfd.entries;
> -
> -       if (!evlist__ctlfd_initialized(evlist))
> -               return 0;
> -
> -       if (entries[ctlfd_pos].fd != update->fd ||
> -           entries[ctlfd_pos].events != update->events)
> -               return -1;
> -
> -       entries[ctlfd_pos].revents = update->revents;
> -       return 0;
> -}
> -
>  struct evsel *evlist__find_evsel(struct evlist *evlist, int idx)
>  {
>         struct evsel *evsel;
> diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
> index 351ba2887a79..3a464585d397 100644
> --- a/tools/perf/util/evlist.h
> +++ b/tools/perf/util/evlist.h
> @@ -418,7 +418,6 @@ void evlist__close_control(int ctl_fd, int ctl_fd_ack, bool *ctl_fd_close);
>  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_update(struct evlist *evlist, struct pollfd *update);
>  int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd);
>  int evlist__ctlfd_ack(struct evlist *evlist);
>
> --
> 2.25.1
>