[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAP-5=fW946mri4wEKBzR+XYzbfBH_foV8KW+AG6bU=H=byt2_Q@mail.gmail.com>
Date: Wed, 8 Oct 2025 08:08:27 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
Tengda Wu <wutengda@...weicloud.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] perf bpf_counter: Fix opening of "any"(-1) CPU events
On Wed, Oct 8, 2025 at 12:35 AM Namhyung Kim <namhyung@...nel.org> wrote:
>
> Hi Ian,
>
> On Tue, Oct 07, 2025 at 11:58:26AM -0700, Ian Rogers wrote:
> > The bperf BPF counter code doesn't handle "any"(-1) CPU events, always
> > wanting to aggregate a count against a CPU, which avoids the need for
> > atomics. Force evsels used for BPF counters to require a CPU when not
> > in system-wide mode so that the "any"(-1) value isn't used during map
> > propagation and evsel's CPU map matches that of the PMU.
> >
> > Fixes: b91917c0c6fa ("perf bpf_counter: Fix handling of cpumap fixing hybrid")
> > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > ---
> > tools/perf/builtin-stat.c | 12 ++++++++++++
> > tools/perf/util/bpf_counter.c | 1 +
> > 2 files changed, 13 insertions(+)
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index 7006f848f87a..7fdc7f273a48 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -2797,6 +2797,18 @@ int cmd_stat(int argc, const char **argv)
> >
> > evlist__warn_user_requested_cpus(evsel_list, target.cpu_list);
> >
> > + if (target.use_bpf && !target.system_wide) {
>
> The target.use_bpf only checks the --bpf-counters option. But IIUC it's
> possible to use BPF only for selected events with ':b' modifier. I
> think you need to check each evsel with evsel__is_bperf().
>
> Also system_wide might not be set for -C/--cpu option. Probably you
> want target__has_cpu() instead of target.system_wide.
Thanks Namhyung, I'll fix in v2.
Ian
> Thanks,
> Namhyung
>
>
> > + /*
> > + * Setup BPF counters to require CPUs as any(-1) isn't
> > + * supported. evlist__create_maps below will propagate this
> > + * information to the evsels.
> > + */
> > + struct evsel *counter;
> > +
> > + evlist__for_each_entry(evsel_list, counter)
> > + counter->core.requires_cpu = true;
> > + }
> > +
> > if (evlist__create_maps(evsel_list, &target) < 0) {
> > if (target__has_task(&target)) {
> > pr_err("Problems finding threads of monitor\n");
> > diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c
> > index ca5d01b9017d..d3e5933b171b 100644
> > --- a/tools/perf/util/bpf_counter.c
> > +++ b/tools/perf/util/bpf_counter.c
> > @@ -495,6 +495,7 @@ static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd,
> > * following evsel__open_per_cpu call
> > */
> > evsel->leader_skel = skel;
> > + assert(!perf_cpu_map__has_any_cpu_or_is_empty(evsel->core.cpus));
> > evsel__open(evsel, evsel->core.cpus, evsel->core.threads);
> >
> > out:
> > --
> > 2.51.0.710.ga91ca5db03-goog
> >
Powered by blists - more mailing lists