Messages in this thread | | | From | Namhyung Kim <> | Date | Fri, 29 Apr 2022 15:57:50 -0700 | Subject | Re: [PATCH RFC 19/21] perf stat: Add requires_cpu flag for uncore |
| |
On Fri, Apr 22, 2022 at 9:25 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > > Uncore events require a CPU i.e. it cannot be -1. > > The evsel system_wide flag is intended for events that should be on every > CPU, which does not make sense for uncore events because uncore events do > not map one-to-one with CPUs. > > These 2 requirements are not exactly the same, so introduce a new flag > 'requires_cpu' the uncore case.
Yeah, I like this change! I was often confused by the two different things.
Thanks, Namhyung
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/lib/perf/evlist.c | 4 +++- > tools/lib/perf/include/internal/evsel.h | 1 + > tools/perf/builtin-stat.c | 5 +---- > tools/perf/util/evsel.c | 1 + > tools/perf/util/parse-events.c | 2 +- > 5 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c > index 37dfa9d936a7..9fbcca3fc836 100644 > --- a/tools/lib/perf/evlist.c > +++ b/tools/lib/perf/evlist.c > @@ -42,7 +42,9 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist, > if (!evsel->own_cpus || evlist->has_user_cpus) { > perf_cpu_map__put(evsel->cpus); > evsel->cpus = perf_cpu_map__get(evlist->user_requested_cpus); > - } else if (!evsel->system_wide && perf_cpu_map__empty(evlist->user_requested_cpus)) { > + } else if (!evsel->system_wide && > + !evsel->requires_cpu && > + perf_cpu_map__empty(evlist->user_requested_cpus)) { > perf_cpu_map__put(evsel->cpus); > evsel->cpus = perf_cpu_map__get(evlist->user_requested_cpus); > } else if (evsel->cpus != evsel->own_cpus) { > diff --git a/tools/lib/perf/include/internal/evsel.h b/tools/lib/perf/include/internal/evsel.h > index cfc9ebd7968e..77fbb8b97e5c 100644 > --- a/tools/lib/perf/include/internal/evsel.h > +++ b/tools/lib/perf/include/internal/evsel.h > @@ -50,6 +50,7 @@ struct perf_evsel { > /* parse modifier helper */ > int nr_members; > bool system_wide; > + bool requires_cpu; > int idx; > }; > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c > index a96f106dc93a..8972ae546cfe 100644 > --- a/tools/perf/builtin-stat.c > +++ b/tools/perf/builtin-stat.c > @@ -364,9 +364,6 @@ static int read_counter_cpu(struct evsel *counter, struct timespec *rs, int cpu_ > if (!counter->supported) > return -ENOENT; > > - if (counter->core.system_wide) > - nthreads = 1; > - > for (thread = 0; thread < nthreads; thread++) { > struct perf_counts_values *count; > > @@ -2224,7 +2221,7 @@ static void setup_system_wide(int forks) > struct evsel *counter; > > evlist__for_each_entry(evsel_list, counter) { > - if (!counter->core.system_wide && > + if (!counter->core.requires_cpu && > strcmp(counter->name, "duration_time")) { > return; > } > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 2a1729e7aee4..81bbddb6fbc0 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -382,6 +382,7 @@ struct evsel *evsel__clone(struct evsel *orig) > evsel->core.threads = perf_thread_map__get(orig->core.threads); > evsel->core.nr_members = orig->core.nr_members; > evsel->core.system_wide = orig->core.system_wide; > + evsel->core.requires_cpu = orig->core.requires_cpu; > > if (orig->name) { > evsel->name = strdup(orig->name); > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index dd84fed698a3..783359017548 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -350,7 +350,7 @@ __add_event(struct list_head *list, int *idx, > (*idx)++; > evsel->core.cpus = cpus; > evsel->core.own_cpus = perf_cpu_map__get(cpus); > - evsel->core.system_wide = pmu ? pmu->is_uncore : false; > + evsel->core.requires_cpu = pmu ? pmu->is_uncore : false; > evsel->auto_merge_stats = auto_merge_stats; > > if (name) > -- > 2.25.1 >
| |