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

[Sandipan Das]

On 02-11-2026 18:25, Peter Zijlstra wrote:
> 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@xxxxxxx>
>> ---
>>  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?

Yes, my understanding is that __perf_pmu_remove() will schedule out all
the associated events and when the final one is stop()-ed, the hrtimer
also stops.