Messages in this thread | | | From | Ian Rogers <> | Date | Fri, 29 Apr 2022 18:10:05 -0700 | Subject | Re: [PATCH RFC 19/21] perf stat: Add requires_cpu flag for uncore |
| |
On Fri, Apr 29, 2022 at 3:58 PM Namhyung Kim <namhyung@kernel.org> wrote: > > 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;
Nice cleanup! Could we add some comments here as to what the booleans mean/imply?
Thanks, Ian
> > + 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 > >
| |