| Message ID | 20211223074541.3318938-26-irogers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Refactor perf cpumap | expand |
On Wed, Dec 22, 2021 at 11:47 PM Ian Rogers <irogers@google.com> wrote: > > Correct use of cpumap index in print_no_aggr_metric. > > Signed-off-by: Ian Rogers <irogers@google.com> > --- [SNIP] > @@ -924,29 +921,32 @@ static void print_no_aggr_metric(struct perf_stat_config *config, > struct evlist *evlist, > char *prefix) > { > - int cpu; > - int nrcpus = 0; > - struct evsel *counter; > - u64 ena, run, val; > - double uval; > - struct aggr_cpu_id id; > + int cpu, nrcpus; > > nrcpus = evlist->core.cpus->nr; > for (cpu = 0; cpu < nrcpus; cpu++) { > + struct evsel *counter; > bool first = true; > > if (prefix) > fputs(prefix, config->output); > evlist__for_each_entry(evlist, counter) { > - id = aggr_cpu_id__empty(); > - id.core = cpu; > + u64 ena, run, val; > + double uval; > + struct aggr_cpu_id id; > + int idx = perf_cpu_map__idx(evsel__cpus(counter), cpu); Not sure about this. Here the 'cpu' is an index for the evlist->core.cpus, not a CPU number. But the perf_cpu_map__idx() requires a CPU number, right? Thanks, Namhyung > + > + if (idx < 0) > + continue; > + > + id = aggr_cpu_id__cpu(cpu, /*data=*/NULL); > if (first) { > aggr_printout(config, counter, id, 0); > first = false; > } > - val = perf_counts(counter->counts, cpu, 0)->val; > - ena = perf_counts(counter->counts, cpu, 0)->ena; > - run = perf_counts(counter->counts, cpu, 0)->run; > + val = perf_counts(counter->counts, idx, 0)->val; > + ena = perf_counts(counter->counts, idx, 0)->ena; > + run = perf_counts(counter->counts, idx, 0)->run; > > uval = val * counter->scale; > printout(config, id, 0, counter, uval, prefix, > -- > 2.34.1.307.g9b7440fafd-goog >
On Tue, Dec 28, 2021 at 4:09 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Wed, Dec 22, 2021 at 11:47 PM Ian Rogers <irogers@google.com> wrote: > > > > Correct use of cpumap index in print_no_aggr_metric. > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > [SNIP] > > @@ -924,29 +921,32 @@ static void print_no_aggr_metric(struct perf_stat_config *config, > > struct evlist *evlist, > > char *prefix) > > { > > - int cpu; > > - int nrcpus = 0; > > - struct evsel *counter; > > - u64 ena, run, val; > > - double uval; > > - struct aggr_cpu_id id; > > + int cpu, nrcpus; > > > > nrcpus = evlist->core.cpus->nr; > > for (cpu = 0; cpu < nrcpus; cpu++) { > > + struct evsel *counter; > > bool first = true; > > > > if (prefix) > > fputs(prefix, config->output); > > evlist__for_each_entry(evlist, counter) { > > - id = aggr_cpu_id__empty(); > > - id.core = cpu; > > + u64 ena, run, val; > > + double uval; > > + struct aggr_cpu_id id; > > + int idx = perf_cpu_map__idx(evsel__cpus(counter), cpu); > > Not sure about this. Here the 'cpu' is an index for the > evlist->core.cpus, not a CPU number. But the > perf_cpu_map__idx() requires a CPU number, right? Thanks for the reviews! You are right, I think it makes sense two have two indices here, evlist and counter. I will change the outer loop to use perf_cpu_map__for_each_cpu. v3 will have the other fixes you've pointed out too. Thanks, Ian > Thanks, > Namhyung > > > > + > > + if (idx < 0) > > + continue; > > + > > + id = aggr_cpu_id__cpu(cpu, /*data=*/NULL); > > if (first) { > > aggr_printout(config, counter, id, 0); > > first = false; > > } > > - val = perf_counts(counter->counts, cpu, 0)->val; > > - ena = perf_counts(counter->counts, cpu, 0)->ena; > > - run = perf_counts(counter->counts, cpu, 0)->run; > > + val = perf_counts(counter->counts, idx, 0)->val; > > + ena = perf_counts(counter->counts, idx, 0)->ena; > > + run = perf_counts(counter->counts, idx, 0)->run; > > > > uval = val * counter->scale; > > printout(config, id, 0, counter, uval, prefix, > > -- > > 2.34.1.307.g9b7440fafd-goog > >
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c index 870b1db71fbc..821511ba22cc 100644 --- a/tools/perf/util/stat-display.c +++ b/tools/perf/util/stat-display.c @@ -120,11 +120,10 @@ static void aggr_printout(struct perf_stat_config *config, id.die, config->csv_output ? 0 : -3, id.core, config->csv_sep); - } else if (id.core > -1) { + } else if (id.cpu > -1) { fprintf(config->output, "CPU%*d%s", config->csv_output ? 0 : -7, - evsel__cpus(evsel)->map[id.core], - config->csv_sep); + id.cpu, config->csv_sep); } break; case AGGR_THREAD: @@ -334,7 +333,7 @@ static int first_shadow_cpu(struct perf_stat_config *config, int cpu, idx; if (config->aggr_mode == AGGR_NONE) - return id->core; + return id->cpu; if (!config->aggr_get_id) return 0; @@ -697,10 +696,9 @@ static void print_counter_aggrdata(struct perf_stat_config *config, fprintf(output, "%s", prefix); uval = val * counter->scale; - if (cpu != -1) { - id = aggr_cpu_id__empty(); - id.core = cpu; - } + if (cpu != -1) + id = aggr_cpu_id__cpu(cpu, /*data=*/NULL); + printout(config, id, nr, counter, uval, prefix, run, ena, 1.0, &rt_stat); if (!metric_only) @@ -911,8 +909,7 @@ static void print_counter(struct perf_stat_config *config, fprintf(output, "%s", prefix); uval = val * counter->scale; - id = aggr_cpu_id__empty(); - id.core = cpu; + id = aggr_cpu_id__cpu(cpu, /*data=*/NULL); printout(config, id, 0, counter, uval, prefix, run, ena, 1.0, &rt_stat); @@ -924,29 +921,32 @@ static void print_no_aggr_metric(struct perf_stat_config *config, struct evlist *evlist, char *prefix) { - int cpu; - int nrcpus = 0; - struct evsel *counter; - u64 ena, run, val; - double uval; - struct aggr_cpu_id id; + int cpu, nrcpus; nrcpus = evlist->core.cpus->nr; for (cpu = 0; cpu < nrcpus; cpu++) { + struct evsel *counter; bool first = true; if (prefix) fputs(prefix, config->output); evlist__for_each_entry(evlist, counter) { - id = aggr_cpu_id__empty(); - id.core = cpu; + u64 ena, run, val; + double uval; + struct aggr_cpu_id id; + int idx = perf_cpu_map__idx(evsel__cpus(counter), cpu); + + if (idx < 0) + continue; + + id = aggr_cpu_id__cpu(cpu, /*data=*/NULL); if (first) { aggr_printout(config, counter, id, 0); first = false; } - val = perf_counts(counter->counts, cpu, 0)->val; - ena = perf_counts(counter->counts, cpu, 0)->ena; - run = perf_counts(counter->counts, cpu, 0)->run; + val = perf_counts(counter->counts, idx, 0)->val; + ena = perf_counts(counter->counts, idx, 0)->ena; + run = perf_counts(counter->counts, idx, 0)->run; uval = val * counter->scale; printout(config, id, 0, counter, uval, prefix,
Correct use of cpumap index in print_no_aggr_metric. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/stat-display.c | 42 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-)