Messages in this thread | | | From | Ian Rogers <> | Date | Fri, 6 May 2022 06:47:14 -0700 | Subject | Re: [PATCH V2 13/23] perf evlist: Add evlist__add_dummy_on_all_cpus() |
| |
On Fri, May 6, 2022 at 5:26 AM Adrian Hunter <adrian.hunter@intel.com> wrote: > > Add evlist__add_dummy_on_all_cpus() to enable creating a system-wide dummy > event that sets up the system-wide maps before map propagation. > > For convenience, add evlist__add_aux_dummy() so that the logic can be used > whether or not the event needs to be system-wide. > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/perf/util/evlist.c | 40 ++++++++++++++++++++++++++++++++++++++++ > tools/perf/util/evlist.h | 5 +++++ > 2 files changed, 45 insertions(+) > > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c > index 78c47cbafbc2..c16bd4836314 100644 > --- a/tools/perf/util/evlist.c > +++ b/tools/perf/util/evlist.c > @@ -264,6 +264,46 @@ int evlist__add_dummy(struct evlist *evlist) > return 0; > } > > +static void evlist__add_on_all_cpus(struct evlist *evlist, struct evsel *evsel) > +{ > + evsel->core.system_wide = true; > + > + /* All CPUs */ > + perf_cpu_map__put(evsel->core.own_cpus); > + evsel->core.own_cpus = perf_cpu_map__new(NULL); > + perf_cpu_map__put(evsel->core.cpus); > + evsel->core.cpus = perf_cpu_map__get(evsel->core.own_cpus); > + > + /* No threads */ > + perf_thread_map__put(evsel->core.threads); > + evsel->core.threads = perf_thread_map__new_dummy(); > + > + evlist__add(evlist, evsel); > +} > + > +struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide) > +{ > + struct evsel *evsel = evlist__dummy_event(evlist); > + > + if (!evsel) > + return NULL; > + > + evsel->core.attr.exclude_kernel = 1; > + evsel->core.attr.exclude_guest = 1; > + evsel->core.attr.exclude_hv = 1; > + evsel->core.attr.freq = 0; > + evsel->core.attr.sample_period = 1; > + evsel->no_aux_samples = true; > + evsel->name = strdup("dummy:u"); > + > + if (system_wide) > + evlist__add_on_all_cpus(evlist, evsel); > + else > + evlist__add(evlist, evsel); > + > + return evsel; > +} > + > static int evlist__add_attrs(struct evlist *evlist, struct perf_event_attr *attrs, size_t nr_attrs) > { > struct evsel *evsel, *n; > diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h > index 4062f5aebfc1..1bde9ccf4e7d 100644 > --- a/tools/perf/util/evlist.h > +++ b/tools/perf/util/evlist.h > @@ -114,6 +114,11 @@ int arch_evlist__add_default_attrs(struct evlist *evlist); > struct evsel *arch_evlist__leader(struct list_head *list); > > int evlist__add_dummy(struct evlist *evlist); > +struct evsel *evlist__add_aux_dummy(struct evlist *evlist, bool system_wide); > +static inline struct evsel *evlist__add_dummy_on_all_cpus(struct evlist *evlist)
Sorry to be a language lawyer. What I hope to clean up with CPU maps is that:
empty == dummy == any CPU https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/lib/perf/cpumap.c?h=perf/core#n279
Given every CPU map should be empty or contain any CPU then it seems they all meet the definition of empty - so something is wrong.
The cpu map here is explicitly opened so that it gets all online CPUs: https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/lib/perf/cpumap.c?h=perf/core#n174
From: https://github.com/torvalds/linux/blob/master/Documentation/admin-guide/cputopology.rst there are example topologies like: kernel_max: 31 offline: 2,4-31,32-63 online: 0-1,3 possible: 0-31 present: 0-31
all_cpus could mean the union of offline and online CPUs, possible CPUs or present CPUs. You are saying that in the perf code all_cpus should be the same as all online cpus as only those CPUs are valid with perf_event_open. That's true but offline CPUs can be made online. If that happens here then the dummy events will have a CPU map that rather than being for all CPUs will be for all online CPUs at the point it was opened. Having online in the function name I think captures the time dependent nature of this - but if you think that's too much could we add a comment? I'm trying to avoid a situation, like with the CPU map code, where all and online are interchangeable leading to the code being unnecessarily confusing unless you read every line.
Thanks, Ian
> +{ > + return evlist__add_aux_dummy(evlist, true); > +} > > int evlist__add_sb_event(struct evlist *evlist, struct perf_event_attr *attr, > evsel__sb_cb_t cb, void *data); > -- > 2.25.1 >
| |