Re: [PATCH V3 6/8] perf/x86/intel/uncore: Introduce PMU flags and broken state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removal of `box->cpu = -1` introduces a race condition where concurrent `perf_event_open` can install events on an offlining CPU during migration.
--

commit 90f307cb2cce79d442ba12ad2d9b5ba2c1d5018e
Author: Zide Chen <zide.chen@intel.com>

perf/x86/intel/uncore: Introduce PMU flags and broken state
    
This commit replaces the boolean 'registered' field in intel_uncore_pmu with
an unsigned long 'flags' field. It introduces a PMU_BROKEN flag to track box
setup failures, which is sticky and clears only on module reload or reboot.
Broken PMUs are skipped in CPU hotplug and box allocation paths.

> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
> index 06ef89f6ccc28..feb8c3b0076b2 100644
> --- a/arch/x86/events/intel/uncore.c
> +++ b/arch/x86/events/intel/uncore.c

[ ... ]

> @@ -1507,7 +1526,8 @@ static void uncore_change_type_ctx(struct intel_uncore_type *type, int old_cpu,
[ ... ]
>  		WARN_ON_ONCE(box->cpu != -1 && box->cpu != old_cpu);
> -		box->cpu = -1;
>  		cpumask_clear_cpu(old_cpu, &pmu->cpu_mask);
> -		if (new_cpu < 0)
> +		if (new_cpu < 0) {
> +			box->cpu = -1;
>  			continue;
> +		}
>  
> -		if (!uncore_die_has_box(type, die, pmu->pmu_idx))
> +		/* An inactive box doesn't need migration. */
> +		if (box->cpu == -1)
>  			continue;
>  		uncore_pmu_cancel_hrtimer(box);
>  		perf_pmu_migrate_context(&pmu->pmu, old_cpu, new_cpu);

[Severity: Medium]
Does removing the temporary assignment of box->cpu = -1 before the
perf_pmu_migrate_context() call introduce a race condition with concurrent
sys_perf_event_open() calls?

During the CPU offline process in uncore_change_type_ctx(), old_cpu is still
marked as online. If a user concurrently calls sys_perf_event_open(),
uncore_pmu_event_init() locklessly reads box->cpu. Since it is no longer
temporarily set to -1, it sees old_cpu and sets event->cpu = old_cpu.

If sys_perf_event_open() acquires the CPU's context mutex just after
perf_pmu_migrate_context() has finished and unlocked it, the core perf code
will successfully install the new event into old_cpu's context after the
migration has completed. When old_cpu fully offlines, this stranded event is
forcefully killed, leaving the user with a dead event even though the syscall
succeeded.

Previously, setting box->cpu = -1 prevented this by safely rejecting such
concurrent open requests with -EINVAL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611160033.66760-1-zide.chen@intel.com?part=6