Re: [RFC PATCH v2 5/9] perf pmu: Add perf_pmu__probe() for live capability probing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf pmu: select_probe_cpu() ignores task CPU affinity mask
--

commit a706bded05059ea05d5c1e851935d2f599c67616
Author: Ian Rogers <irogers@google.com>

perf pmu: Add perf_pmu__probe() for live capability probing

This commit adds struct perf_pmu::probed and perf_pmu__probe() to actively
probe live PMU capabilities via perf_event_open(), covering counting and
sampling modes, precise_ip levels, sample formats, privilege exclusion
modifiers, execution features, and event group size limits.

> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index cd8211f872911..83c8d6f392ba9 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c

[ ... ]

> @@ -2832,9 +2847,1219 @@ void perf_pmu__delete(struct perf_pmu *pmu)

[ ... ]

> +static int select_probe_cpu(const struct perf_pmu *pmu)
> +{
> +	struct perf_cpu_map *online = perf_cpu_map__new_online_cpus();
> +	int cpu = 0;
> +
> +	if (pmu->cpus && !perf_cpu_map__is_empty(pmu->cpus)) {
> +		int nr = perf_cpu_map__nr(pmu->cpus);
> +
> +		cpu = perf_cpu_map__cpu(pmu->cpus, 0).cpu;
> +		if (online) {
> +			for (int i = 0; i < nr; i++) {
> +				struct perf_cpu c = perf_cpu_map__cpu(pmu->cpus, i);
> +
> +				if (perf_cpu_map__has(online, c)) {
> +					cpu = c.cpu;
> +					break;
> +				}
> +			}
> +		}
> +	} else if (online && !perf_cpu_map__is_empty(online)) {
> +		cpu = perf_cpu_map__cpu(online, 0).cpu;
> +	}
> +
> +	perf_cpu_map__put(online);
> +	return cpu >= 0 ? cpu : 0;
> +}

[ ... ]

> +static void probe_group_limits(const struct perf_event_attr *attrs, size_t nr_attrs,
> +			       pid_t pid, int cpu, unsigned long flags,
> +			       int *max_open, bool *open_capped,
> +			       int *max_active, bool *active_capped)
> +{
> +	struct perf_event_attr leader_attr;
> +	int leader_fd;
> +	int fds[MAX_PROBE_GROUP_SIZE];
> +	int nr_followers = 0;
> +	bool has_time_read = true;
> +	bool active_ok = true;
> +	enum rlimit_action set_rlimit = NO_CHANGE;
> +	cpu_set_t saved_affinity, probe_affinity;
> +	bool affinity_changed = false;
> +
> +	*max_open = 0;
> +	*open_capped = false;
> +	*max_active = 0;
> +	*active_capped = false;
> +
> +	if (!attrs || nr_attrs == 0)
> +		return;
> +
> +	if (pid == 0 && cpu >= 0 && cpu < CPU_SETSIZE) {
> +		CPU_ZERO(&saved_affinity);
> +		if (sched_getaffinity(0, sizeof(saved_affinity), &saved_affinity) == 0) {
> +			CPU_ZERO(&probe_affinity);
> +			CPU_SET(cpu, &probe_affinity);
> +			if (sched_setaffinity(0, sizeof(probe_affinity), &probe_affinity) == 0)
> +				affinity_changed = true;
> +		}
> +	}

[Severity: Medium]
What happens in probe_group_limits() when perf runs inside a cgroup cpuset
(such as a container or a systemd unit with AllowedCPUs=) that excludes the
first online CPU of a PMU?

In select_probe_cpu(), the probe CPU is chosen as the first online CPU in
pmu->cpus (or online) without checking whether that CPU is in the calling
thread's allowed affinity mask via sched_getaffinity().

For non-uncore PMUs, try_open_attr_variants() opens events with pid = 0 and
cpu = pmu_cpu, which sys_perf_event_open() allows even when pmu_cpu is
outside the task's cpuset. However, when probe_group_limits() then tries to
pin the calling thread to cpu with sched_setaffinity(), it fails with -EINVAL.

When check_group_active() subsequently enables the (pid = 0, cpu = pmu_cpu)
group via PERF_EVENT_IOC_ENABLE and spins in userspace on a different CPU,
the kernel sets the event to PERF_EVENT_STATE_INACTIVE (incrementing
TOTAL_TIME_ENABLED), while event_filter_match() prevents scheduling it onto
the PMU because event->cpu != smp_processor_id(). As a result,
TOTAL_TIME_RUNNING stays 0 (delta_run != delta_ena), check_group_active()
returns false even for a single event, and max_active_group_size and
max_active_gp_group_size are reported as 0.

Should select_probe_cpu() prefer an online CPU that is also present in the
calling thread's sched_getaffinity() mask when available?

[ ... ]

> +	leader_attr = attrs[0];
> +	leader_attr.disabled = 1;
> +	leader_attr.read_format = PERF_FORMAT_TOTAL_TIME_ENABLED | PERF_FORMAT_TOTAL_TIME_RUNNING;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261017185442.2137623-1-irogers@google.com?part=5