Re: [PATCH V1 18/23] libperf evlist: Allow mixing per-thread and per-cpu mmaps

[Ian Rogers <irogers@google.com>]

On Thu, May 5, 2022 at 9:58 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> mmap_per_evsel() will skip events that do not match the CPU, so all CPUs
> can be iterated in any case.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

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

Thanks,
Ian

> ---
>  tools/lib/perf/evlist.c | 36 +++++++-----------------------------
>  1 file changed, 7 insertions(+), 29 deletions(-)
>
> diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> index 048b546f9444..0acf43946479 100644
> --- a/tools/lib/perf/evlist.c
> +++ b/tools/lib/perf/evlist.c
> @@ -508,29 +508,6 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
>         return 0;
>  }
>
> -static int
> -mmap_per_thread(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
> -               struct perf_mmap_param *mp)
> -{
> -       int thread;
> -       int nr_threads = perf_thread_map__nr(evlist->threads);
> -
> -       for (thread = 0; thread < nr_threads; thread++) {
> -               int output = -1;
> -               int output_overwrite = -1;
> -
> -               if (mmap_per_evsel(evlist, ops, thread, mp, 0, thread,
> -                                  &output, &output_overwrite))
> -                       goto out_unmap;
> -       }
> -
> -       return 0;
> -
> -out_unmap:
> -       perf_evlist__munmap(evlist);
> -       return -1;
> -}
> -
>  static int
>  mmap_per_cpu(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
>              struct perf_mmap_param *mp)
> @@ -561,9 +538,14 @@ static int perf_evlist__nr_mmaps(struct perf_evlist *evlist)
>  {
>         int nr_mmaps;
>
> +       /* One for each CPU */
>         nr_mmaps = perf_cpu_map__nr(evlist->all_cpus);
> -       if (perf_cpu_map__empty(evlist->all_cpus))
> -               nr_mmaps = perf_thread_map__nr(evlist->threads);
> +       if (perf_cpu_map__empty(evlist->all_cpus)) {

Here with the current definition of perf_cpu_map__empty this is a NULL
or first element is any CPU (aka dummy) test. Given our conversation
this may not work as intended for that definition.

> +               /* Plus one for each thread */
> +               nr_mmaps += perf_thread_map__nr(evlist->threads);
> +               /* Minus the per-thread CPU (-1) */
> +               nr_mmaps -= 1;
> +       }
>
>         return nr_mmaps;
>  }
> @@ -573,7 +555,6 @@ int perf_evlist__mmap_ops(struct perf_evlist *evlist,
>                           struct perf_mmap_param *mp)
>  {
>         struct perf_evsel *evsel;
> -       const struct perf_cpu_map *cpus = evlist->all_cpus;
>
>         if (!ops || !ops->get || !ops->mmap)
>                 return -EINVAL;
> @@ -592,9 +573,6 @@ int perf_evlist__mmap_ops(struct perf_evlist *evlist,
>         if (evlist->pollfd.entries == NULL && perf_evlist__alloc_pollfd(evlist) < 0)
>                 return -ENOMEM;
>
> -       if (perf_cpu_map__empty(cpus))
> -               return mmap_per_thread(evlist, ops, mp);
> -
>         return mmap_per_cpu(evlist, ops, mp);
>  }
>
> --
> 2.25.1
>