Re: [PATCH V1 13/23] perf evlist: Add evlist__add_system_wide_dummy()

[Ian Rogers]

On Fri, May 6, 2022 at 1:14 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
>
> On 6/05/22 02:56, Ian Rogers wrote:
> > On Thu, May 5, 2022 at 9:57 AM Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> >>
> >> Add evlist__add_system_wide_dummy() to enable creating a system-wide dummy
> >> event that sets up the system-wide maps before map propagation.
> >
> > Perhaps this should be:
> > evlist__add_dummy_on_all_online_cpus()
>
> Ok, but offline isn't an option, so I'll drop 'online' from the name.

I'm not sure I follow. There can be CPUs that are offline (echo 0 >
/sys/devices/system/cpu/cpu2/online) and then brought online while
perf is running. The output obviously won't be right as we're missing
dummy events. Losing online from the name here seems to remove a
useful clue.

Thanks,
Ian

> > my thoughts being that a system wide dummy sounds like the dummy CPU
> > map, ie {-1} whilst what happens here is opening on all online CPUs
> > {0-35} on a 36 hyperthread system. Note also that the libperf
> > cpu_map__read_all_cpu_map code doesn't discover sysfs' mount properly,
> > as done in:
> > tools/lib/api/fs/fs.h
> > Some tech debt.
> >
> >> 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@xxxxxxxxx>
> >> ---
> >>  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..58ea562ddbd2 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_system_wide(struct evlist *evlist, struct evsel *evsel)
> >
> > This would be:
> > evlist__add_on_all_online_cpus(..)
> >
> >> +{
> >> +       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_system_wide(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..dd1af114e033 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_system_wide_dummy(struct evlist *evlist)
> >> +{
> >> +       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
> >>
>