Messages in this thread | | | From | Ian Rogers <> | Date | Mon, 10 Oct 2022 16:39:48 -0700 | Subject | Re: [PATCH 18/19] perf stat: Display percore events properly |
| |
On Sun, Oct 9, 2022 at 10:36 PM Namhyung Kim <namhyung@kernel.org> wrote: > > The recent change in the perf stat broke the percore event display. > Note that the aggr counts are already processed so that the every > sibling thread in the same core will get the per-core counter values.
Could we add a test given this has broken once?
> Check percore evsels and skip the sibling threads in the display. > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Ian Rogers <irogers@google.com>
Thanks, Ian
> --- > tools/perf/builtin-stat.c | 16 ---------------- > tools/perf/util/stat-display.c | 27 +++++++++++++++++++++++++-- > 2 files changed, 25 insertions(+), 18 deletions(-) > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index d92815f4eae0..b3a39d4c86a7 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -1403,18 +1403,6 @@ static struct aggr_cpu_id perf_stat__get_cpu_cached(struct perf_stat_config *con > return perf_stat__get_aggr(config, perf_stat__get_cpu, cpu); > } > > -static bool term_percore_set(void) > -{ > - struct evsel *counter; > - > - evlist__for_each_entry(evsel_list, counter) { > - if (counter->percore) > - return true; > - } > - > - return false; > -} > - > static aggr_cpu_id_get_t aggr_mode__get_aggr(enum aggr_mode aggr_mode) > { > switch (aggr_mode) { > @@ -1427,8 +1415,6 @@ static aggr_cpu_id_get_t aggr_mode__get_aggr(enum aggr_mode aggr_mode) > case AGGR_NODE: > return aggr_cpu_id__node; > case AGGR_NONE: > - if (term_percore_set()) > - return aggr_cpu_id__core; > return aggr_cpu_id__cpu;; > case AGGR_GLOBAL: > return aggr_cpu_id__global; > @@ -1452,8 +1438,6 @@ static aggr_get_id_t aggr_mode__get_id(enum aggr_mode aggr_mode) > case AGGR_NODE: > return perf_stat__get_node_cached; > case AGGR_NONE: > - if (term_percore_set()) > - return perf_stat__get_core_cached; > return perf_stat__get_cpu_cached; > case AGGR_GLOBAL: > return perf_stat__get_global_cached; > diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c > index 0c0e22c175a1..e0c0df99d40d 100644 > --- a/tools/perf/util/stat-display.c > +++ b/tools/perf/util/stat-display.c > @@ -1094,7 +1094,8 @@ static void print_percore(struct perf_stat_config *config, > { > bool metric_only = config->metric_only; > FILE *output = config->output; > - int s; > + struct cpu_aggr_map *core_map; > + int s, c, i; > bool first = true; > > if (!config->aggr_map || !config->aggr_get_id) > @@ -1103,13 +1104,35 @@ static void print_percore(struct perf_stat_config *config, > if (config->percore_show_thread) > return print_counter(config, counter, prefix); > > - for (s = 0; s < config->aggr_map->nr; s++) { > + core_map = cpu_aggr_map__empty_new(config->aggr_map->nr); > + if (core_map == NULL) { > + fprintf(output, "Cannot allocate per-core aggr map for display\n"); > + return; > + } > + > + for (s = 0, c = 0; s < config->aggr_map->nr; s++) { > + struct perf_cpu curr_cpu = config->aggr_map->map[s].cpu; > + struct aggr_cpu_id core_id = aggr_cpu_id__core(curr_cpu, NULL); > + bool found = false; > + > + for (i = 0; i < c; i++) { > + if (aggr_cpu_id__equal(&core_map->map[i], &core_id)) { > + found = true; > + break; > + } > + } > + if (found) > + continue; > + > if (prefix && metric_only) > fprintf(output, "%s", prefix); > > print_counter_aggrdata(config, counter, s, > prefix, metric_only, &first); > + > + core_map->map[c++] = core_id; > } > + free(core_map); > > if (metric_only) > fputc('\n', output); > -- > 2.38.0.rc1.362.ged0d419d3c-goog >
| |