Re: [PATCH] perf/x86/amd/uncore: Avoid context migration from non-owners

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Nov 02, 2026 at 03:52:07PM +0530, Sandipan Das wrote:
> Each uncore context is owned by a single CPU that hosts all events for
> the corresponding uncore domain and is advertised through the sysfs
> cpumask attribute. When a CPU goes offline, the context is migrated to
> an online sibling if the departing CPU is the owner.
> 
> Before commit d6389d3ccc13 ("perf/x86/amd/uncore: Refactor uncore
> management"), uncore_down_prepare() checked whether the CPU going
> offline owned the context. The check was lost when the function was
> replaced by amd_uncore_ctx_move(). Hence, when a CPU that is not the
> owner goes offline, ownership is always handed over to the first online
> sibling in the uncore domain irrespective of whether another sibling
> already owns it. This goes unnoticed as long as the owner is the
> lowest-numbered online CPU in the domain, since the first online
> sibling is then the owner itself. Once ownership has moved, that no
> longer holds.
> 
> For example, consider an L3 domain spanning CPUs 8-15,136-143 and owned
> by CPU 8.
> 
>   $ cat /sys/bus/event_source/devices/amd_l3/cpumask
>   0,8,16,24,32,40,48,56,64,72,80,88,96,104,112,120
> 
>   $ echo 0 | sudo tee /sys/devices/system/cpu/cpu8/online
>   $ cat /sys/bus/event_source/devices/amd_l3/cpumask
>   0,9,16,24,32,40,48,56,64,72,80,88,96,104,112,120
> 
>   $ echo 1 | sudo tee /sys/devices/system/cpu/cpu8/online
>   $ cat /sys/bus/event_source/devices/amd_l3/cpumask
>   0,9,16,24,32,40,48,56,64,72,80,88,96,104,112,120
> 
>   $ echo 0 | sudo tee /sys/devices/system/cpu/cpu10/online
>   $ cat /sys/bus/event_source/devices/amd_l3/cpumask
>   0,8-9,16,24,32,40,48,56,64,72,80,88,96,104,112,120
> 
> Offlining CPU 8 correctly moves ownership to CPU 9 and onlining it
> again leaves CPU 9 as the owner. However, offlining CPU 10 then makes
> CPU 8 the owner while existing events remain on CPU 9. Both CPUs are
> now advertised for the same domain, so tools that open an event on
> every CPU in the cpumask count the domain twice, and the context gets
> modified from two CPUs without any synchronization.
> 
> Restore the ownership check that was lost during the refactor.
> 
> Fixes: d6389d3ccc13 ("perf/x86/amd/uncore: Refactor uncore management")
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> ---
>  arch/x86/events/amd/uncore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
> index 7aa5a5b652a2..c307323f6390 100644
> --- a/arch/x86/events/amd/uncore.c
> +++ b/arch/x86/events/amd/uncore.c
> @@ -595,7 +595,7 @@ static void amd_uncore_ctx_move(struct amd_uncore *uncore, unsigned int cpu)
>  	for (i = 0; i < uncore->num_pmus; i++) {
>  		pmu = &uncore->pmus[i];
>  		curr = *per_cpu_ptr(pmu->ctx, cpu);
> -		if (!curr)
> +		if (!curr || curr->cpu != cpu)
>  			continue;
>  
>  		/* Migrate to a shared sibling if possible */

Now, I see the Intel stuff cancels the hrtimer here; but AFAICT the AMD
bits tie the hrtimer to the ctx, and when the ctx goes empty -- due to
__perf_pmu_remove() it will get stopped there, right?