[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aOdO8beKmaY8M5Ir@google.com>
Date: Thu, 9 Oct 2025 14:58:09 +0900
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
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 v2] perf bpf_counter: Fix opening of "any"(-1) CPU events
On Wed, Oct 08, 2025 at 09:23:47AM -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 so let's not change that. 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 | 13 +++++++++++++
> tools/perf/util/bpf_counter.c | 1 +
> 2 files changed, 14 insertions(+)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 7006f848f87a..0fc6884c1bf1 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2540,6 +2540,7 @@ int cmd_stat(int argc, const char **argv)
> unsigned int interval, timeout;
> const char * const stat_subcommands[] = { "record", "report" };
> char errbuf[BUFSIZ];
> + struct evsel *counter;
>
> setlocale(LC_ALL, "");
>
> @@ -2797,6 +2798,18 @@ int cmd_stat(int argc, const char **argv)
>
> evlist__warn_user_requested_cpus(evsel_list, target.cpu_list);
>
> + evlist__for_each_entry(evsel_list, counter) {
> + /*
> + * Setup BPF counters to require CPUs as any(-1) isn't
> + * supported. evlist__create_maps below will propagate this
> + * information to the evsels. Note, evsel__is_bperf isn't yet
> + * set up, and this change must happen early, so directly use
> + * the bpf_counter variable.
> + */
> + if (counter->bpf_counter)
Is it set for --bpf-counters as well?
What about this?
if ((counter->bpf_counter || target.use_bpf) &&
!target__has_cpu())
Thanks,
Namhyung
> + 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