Re: [PATCH v3 3/3] perf stat: Fix metric-only aggregation index 1;115;0c Aggregation index was being computed using the evsel's cpumap which may have a different (typically the same or fewer) entries.

From: Ian Rogers
Date: Wed Feb 21 2024 - 02:06:22 EST


Sorry, subject got messed up. Will resend.

Thanks,
Ian

On Tue, Feb 20, 2024 at 10:45 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> Before:
> ```
> $ perf stat --metric-only -A -M memory_bandwidth_total -a sleep 1
>
> Performance counter stats for 'system wide':
>
> MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total
> CPU0 12.8 0.0 12.9 12.7 0.0 12.6
> CPU1
>
> 1.007806367 seconds time elapsed
> ```
>
> After:
> ```
> $ perf stat --metric-only -A -M memory_bandwidth_total -a sleep 1
>
> Performance counter stats for 'system wide':
>
> MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total MB/s memory_bandwidth_total
> CPU0 15.4 0.0 15.3 15.0 0.0 14.9
> CPU18 0.0 0.0 13.5 5.2 0.0 11.9
>
> 1.007858736 seconds time elapsed
> ```
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx> |
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> ---
> tools/perf/util/stat-display.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> index ae37395f90c0..bfc1d705f437 100644
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -1137,11 +1137,16 @@ static void print_no_aggr_metric(struct perf_stat_config *config,
> u64 ena, run, val;
> double uval;
> struct perf_stat_evsel *ps = counter->stats;
> - int aggr_idx = perf_cpu_map__idx(evsel__cpus(counter), cpu);
> + int aggr_idx = 0;
>
> - if (aggr_idx < 0)
> + if (!perf_cpu_map__has(evsel__cpus(counter), cpu))
> continue;
>
> + cpu_aggr_map__for_each_idx(aggr_idx, config->aggr_map) {
> + if (config->aggr_map->map[aggr_idx].cpu.cpu == cpu.cpu)
> + break;
> + }
> +
> os->evsel = counter;
> os->id = aggr_cpu_id__cpu(cpu, /*data=*/NULL);
> if (first) {
> --
> 2.44.0.rc1.240.g4c46232300-goog
>