Re: [PATCH v2 26/48] perf evsel: Derive CPUs and threads in alloc_counts

[Namhyung Kim]

On Wed, Dec 22, 2021 at 11:47 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> Passing the number of CPUs and threads allows for an evsel's counts to
> be mismatched to its cpu map. To avoid this always derive the counts
> size from the cpu map. Change openaat-syscall-all-cpus to set the cpus

s/openaat/openat/

> to allow for this to work.
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
>  tools/perf/tests/openat-syscall-all-cpus.c | 10 +---------
>  tools/perf/util/counts.c                   |  8 ++++++--
>  tools/perf/util/counts.h                   |  2 +-
>  tools/perf/util/evsel.c                    |  2 +-
>  tools/perf/util/stat.c                     | 13 ++++++-------
>  5 files changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/tests/openat-syscall-all-cpus.c b/tools/perf/tests/openat-syscall-all-cpus.c
> index cd3dd463783f..544db0839b3b 100644
> --- a/tools/perf/tests/openat-syscall-all-cpus.c
> +++ b/tools/perf/tests/openat-syscall-all-cpus.c
> @@ -85,15 +85,7 @@ static int test__openat_syscall_event_on_all_cpus(struct test_suite *test __mayb
>                 CPU_CLR(cpus->map[cpu], &cpu_set);
>         }
>
> -       /*
> -        * Here we need to explicitly preallocate the counts, as if
> -        * we use the auto allocation it will allocate just for 1 cpu,
> -        * as we start by cpu 0.
> -        */
> -       if (evsel__alloc_counts(evsel, cpus->nr, 1) < 0) {
> -               pr_debug("evsel__alloc_counts(ncpus=%d)\n", cpus->nr);
> -               goto out_close_fd;
> -       }
> +       evsel->core.cpus = perf_cpu_map__get(cpus);

I was about to write that you need to call
evsel__alloc_counts() after this.  But it seems
evsel__read_on_cpu() can do the job later.
So I'm fine with this. :)

Thanks,
Namhyung

>
>         err = 0;
>
> diff --git a/tools/perf/util/counts.c b/tools/perf/util/counts.c
> index 582f3aeaf5e4..2b81707b9dba 100644
> --- a/tools/perf/util/counts.c
> +++ b/tools/perf/util/counts.c
> @@ -4,6 +4,7 @@
>  #include <string.h>
>  #include "evsel.h"
>  #include "counts.h"
> +#include <perf/threadmap.h>
>  #include <linux/zalloc.h>
>
>  struct perf_counts *perf_counts__new(int ncpus, int nthreads)
> @@ -55,9 +56,12 @@ void evsel__reset_counts(struct evsel *evsel)
>         perf_counts__reset(evsel->counts);
>  }
>
> -int evsel__alloc_counts(struct evsel *evsel, int ncpus, int nthreads)
> +int evsel__alloc_counts(struct evsel *evsel)
>  {
> -       evsel->counts = perf_counts__new(ncpus, nthreads);
> +       struct perf_cpu_map *cpus = evsel__cpus(evsel);
> +       int nthreads = perf_thread_map__nr(evsel->core.threads);
> +
> +       evsel->counts = perf_counts__new(cpus ? cpus->nr : 1, nthreads);
>         return evsel->counts != NULL ? 0 : -ENOMEM;
>  }
>
> diff --git a/tools/perf/util/counts.h b/tools/perf/util/counts.h
> index 7ff36bf6d644..3e275e9c60d1 100644
> --- a/tools/perf/util/counts.h
> +++ b/tools/perf/util/counts.h
> @@ -40,7 +40,7 @@ void perf_counts__delete(struct perf_counts *counts);
>  void perf_counts__reset(struct perf_counts *counts);
>
>  void evsel__reset_counts(struct evsel *evsel);
> -int evsel__alloc_counts(struct evsel *evsel, int ncpus, int nthreads);
> +int evsel__alloc_counts(struct evsel *evsel);
>  void evsel__free_counts(struct evsel *evsel);
>
>  #endif /* __PERF_COUNTS_H */
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 656c30b988ce..6c9af21776e6 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1578,7 +1578,7 @@ int __evsel__read_on_cpu(struct evsel *evsel, int cpu, int thread, bool scale)
>         if (FD(evsel, cpu, thread) < 0)
>                 return -EINVAL;
>
> -       if (evsel->counts == NULL && evsel__alloc_counts(evsel, cpu + 1, thread + 1) < 0)
> +       if (evsel->counts == NULL && evsel__alloc_counts(evsel) < 0)
>                 return -ENOMEM;
>
>         if (readn(FD(evsel, cpu, thread), &count, nv * sizeof(u64)) <= 0)
> diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
> index c69b221f5e3e..995cb5003133 100644
> --- a/tools/perf/util/stat.c
> +++ b/tools/perf/util/stat.c
> @@ -152,11 +152,13 @@ static void evsel__free_stat_priv(struct evsel *evsel)
>         zfree(&evsel->stats);
>  }
>
> -static int evsel__alloc_prev_raw_counts(struct evsel *evsel, int ncpus, int nthreads)
> +static int evsel__alloc_prev_raw_counts(struct evsel *evsel)
>  {
> +       int cpu_map_nr = evsel__nr_cpus(evsel);
> +       int nthreads = perf_thread_map__nr(evsel->core.threads);
>         struct perf_counts *counts;
>
> -       counts = perf_counts__new(ncpus, nthreads);
> +       counts = perf_counts__new(cpu_map_nr, nthreads);
>         if (counts)
>                 evsel->prev_raw_counts = counts;
>
> @@ -177,12 +179,9 @@ static void evsel__reset_prev_raw_counts(struct evsel *evsel)
>
>  static int evsel__alloc_stats(struct evsel *evsel, bool alloc_raw)
>  {
> -       int ncpus = evsel__nr_cpus(evsel);
> -       int nthreads = perf_thread_map__nr(evsel->core.threads);
> -
>         if (evsel__alloc_stat_priv(evsel) < 0 ||
> -           evsel__alloc_counts(evsel, ncpus, nthreads) < 0 ||
> -           (alloc_raw && evsel__alloc_prev_raw_counts(evsel, ncpus, nthreads) < 0))
> +           evsel__alloc_counts(evsel) < 0 ||
> +           (alloc_raw && evsel__alloc_prev_raw_counts(evsel) < 0))
>                 return -ENOMEM;
>
>         return 0;
> --
> 2.34.1.307.g9b7440fafd-goog
>