Re: [PATCH] perf evlist: don't restrict uncore events to PMU CPUs

[Ian Rogers]

On Tue, Sep 8, 2026 at 8:57 PM Nia Su <nia.su@xxxxxxxxxx> wrote:
>
> On Wed, Sep 9, 2026 at 1:11 AM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> >
> > On Mon, Sep 7, 2026 at 8:23 PM Nia Su <nia.su@xxxxxxxxxx> wrote:
> > >
> > > On Fri, Sep 4, 2026 at 11:34 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> > > >
> > > > On Fri, Sep 4, 2026 at 6:45 AM Nia Su <nia.su@xxxxxxxxxx> wrote:
> > > > >
> > > > > The PMU CPU map has different semantics for core and uncore PMUs.
> > > > >
> > > > > For core PMUs, pmu_cpus contains the CPUs on which the event can be
> > > > > opened. Intersect the user-requested CPU map with the PMU CPU map to
> > > > > reject CPUs unsupported by the PMU.
> > > > >
> > > > > For uncore PMUs, pmu_cpus is just the default CPU to open the event on,
> > > > > not the set of CPUs it can only be opened on.
> > > >
> > > > This isn't quite true. On a dual socket Intel machine, `pmu_cpus` will
> > > > contain a CPU to open the event on per-socket.
> > >
> > > Yes, I agree. My description was not precise enough. What I wanted to say
> > > was that pmu_cpus provides the default CPU(s) used to open an event. It
> > > is not necessarily an allowlist. A user may specify another valid CPU,
> > > which the PMU driver can resolve to the appropriate control CPU.
> > >
> > > > > The user-requested CPU may
> > > > > override the default and be used directly, and the driver can redirect
> > > > > the event to the appropriate control CPU.
> > > >
> > > > Agreed.
> > > >
> > > > > Currently, __perf_evlist__propagate_maps() intersects the user-requested
> > > > > CPU map with pmu_cpus for all PMUs. If the user-requested CPU is not in
> > > > > pmu_cpus, the intersection becomes empty and the evsel is removed before
> > > > > perf_event_open() is called.
> > > >
> > > > Could you give a command line example of the perf tool doing this? I
> > > > ask because I'm surprised the uncore specifying a CPU behavior has
> > > > been broken. As well as `perf record -C` there is also a somewhat
> > > > newer cpu event qualifier.
> > >
> > > I tested both approaches, but they still fail whenever a perf event is
> > > assigned to a CPU outside the uncore PMU's sysfs cpumask. for example,
> > > assigning the event to CPU 1 fails when the cpumask is 0:
> > >
> > > perf stat -C 1 -e <uncore event> -- <program>
> > >
> > > Here, for an uncore PMU, pmu_cpumask() reads the sysfs cpumask into
> > > pmu->cpus. When the event is created, perf copies pmu->cpus to the
> > > local pmu_cpus variable and then stores it in evsel->core.pmu_cpus.
> > > In this case, evsel->core.pmu_cpus is {0}. The CPU specified with
> > > "-C 1" is stored in evsel->core.cpus as {1}. Then, in this function,
> > > __perf_evlist__propagate_maps() then intersects the two maps, resulting
> > > in an empty map, so the event is removed before reaching the PMU driver.
> > >
> > > > > Skip the intersection when evsel->requires_cpu is true, which is set for
> > > > > uncore PMUs. is_pmu_core is also false for other non-core PMUs, so
> > > > > gating on it would have loosened this check for them too. Core PMUs
> > > > > continue to be restricted to their PMU CPU map.
> > > > >
> > > > > requires_cpu can also be set for BPF counters regardless of PMU type,
> > > > > but only when has_user_cpus is false, so this guard has no effect there
> > > > > today.
> > > >
> > > > So the separation between libperf and perf is frustrating, and libperf
> > > > lacks the necessary PMU abstraction. Using requires_cpu as a proxy for
> > > > an uncore PMU test is okay, but it would be better if we had a boolean
> > > > flag especially for this. Given this, a comment should be added saying
> > > > that requires_cpu is used as a proxy for an uncore PMU. The real
> > > > meaning of requires_cpu is that the event doesn't support
> > > > perf_event_open with a thread.
> > > >
> > > > Thanks,
> > > > Ian
> > >
> > > That makes sense. I’ll add a comment clarifying that requires_cpu is used
> > > as a proxy for uncore PMUs and keep the current approach for now.
> >
> > So I think the flag we can use is `evsel->is_pmu_core` which captures
> > the !uncore behavior. I validated the fixes tag is correct.
>
> I did consider using evsel->is_pmu_core, but it is false not only for
> uncore PMUs,
> but also for software, breakpoint, tracepoint, and other non-core
> events. For uncore
> PMUs, cpumask is stored in pmu->cpus and copied to pmu_cpus; for other PMUs
> without a CPU map, pmu_cpus falls back to the online CPU map.
>
> Thus, with perf stat -C <offline_cpu> -e task-clock -- sleep 1,
> pmu_cpus falls back to
> the online CPU map while evsel->cpus contains the requested offline
> CPU. Intersecting
> the two maps would therefore filter out the offline CPU, which seems
> reasonable. For
> this reason, I don’t think this flag is appropriate here. What do you think?

I guess we could either add a new bool to perf_evsel or do something like:
```
bool is_pmu_uncore = !evsel->is_pmu_core && evsel->requires_cpu;
```
it seems this will still be wrong for BPF events that set requires_cpu
and could do on a software event (from builtin-stat.c):
```
 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 and target information.
*/
if ((counter->bpf_counter || target.use_bpf) && !target__has_cpu(&target))
counter->core.requires_cpu = true;
}
```
So I guess the only way to go is a new bool.

Thanks,
Ian

> Thanks,
> Nia
>
> > Thanks,
> > Ian
> >
> > > Thanks,
> > > Nia
> > >
> > > > > Fixes: 811082e4b668 ("perf parse-events: Support user CPUs mixed with threads/processes")
> > > > >
> > > > > Assisted-by: Claude Sonnet 5
> > > > > Signed-off-by: Nia Su <nia.su@xxxxxxxxxx>
> > > > > ---
> > > > >  tools/lib/perf/evlist.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
> > > > > index 1f210dadd666ff818a23712ab95acaa659ccf8dc..a1064175f12ba655a9f7fe42e7c7e4c49ae60423 100644
> > > > > --- a/tools/lib/perf/evlist.c
> > > > > +++ b/tools/lib/perf/evlist.c
> > > > > @@ -80,7 +80,8 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
> > > > >         }
> > > > >
> > > > >         /* Ensure cpus only references valid PMU CPUs. */
> > > > > -       if (!perf_cpu_map__has_any_cpu(evsel->cpus) &&
> > > > > +       if (!evsel->requires_cpu &&
> > > > > +           !perf_cpu_map__has_any_cpu(evsel->cpus) &&
> > > > >             !perf_cpu_map__is_subset(evsel->pmu_cpus, evsel->cpus)) {
> > > > >                 struct perf_cpu_map *tmp = perf_cpu_map__intersect(evsel->pmu_cpus, evsel->cpus);
> > > > >
> > > > >
> > > > > ---
> > > > > base-commit: 73e3f0710014fe6d4ed98cfc02292f6121db7558
> > > > > change-id: 20260904-perf-evlist-uncore-cpu-b4-418af0f17c80
> > > > >
> > > > > --
> > > > >
>