Re: [PATCH 4/6] perf/x86/intel/uncore: Move die_to_cpu() to uncore.c

["Chen, Zide" <zide.chen@intel.com>]

On 5/12/2026 7:58 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Recursive `cpus_read_lock()` acquisition leads to deadlock when used by the planned initialization callback.
> --
> 
> commit d8b9d983f61a73e17f268ae570b5186a67f1a6ae
> Author: Zide Chen <zide.chen@intel.com>
> 
> perf/x86/intel/uncore: Move die_to_cpu() to uncore.c
> 
> This commit moves die_to_cpu() into uncore.c and renames it to
> uncore_die_to_cpu(). This change prepares the function to be reused by a
> planned MSR global initialization callback.
> 
>> diff --git a/arch/x86/events/intel/uncore.c b/arch/x86/events/intel/uncore.c
>> index e9cc1ba921c5e..2bbe4cc1df3e2 100644
>> --- a/arch/x86/events/intel/uncore.c
>> +++ b/arch/x86/events/intel/uncore.c
> [ ... ]
>> @@ -83,6 +83,25 @@ int uncore_device_to_die(struct pci_dev *dev)
>>  	return -1;
>>  }
>>  
>> +int uncore_die_to_cpu(int die)
>> +{
>> +	int res = 0, cpu;
>> +
>> +	/*
>> +	 * Using cpus_read_lock() to ensure cpu is not going down between
>> +	 * looking at cpu_online_mask.
>> +	 */
>> +	cpus_read_lock();
> 
> Will the planned MSR global initialization callback already hold
> cpus_read_lock() when it calls this function?
> 
> If the initialization path (such as uncore_discovery_msr() calling into
> __parse_discovery_table()) already holds the lock, does acquiring it a
> second time here risk a lockdep warning or a deadlock?
> 
> Since cpus_read_lock() is backed by a percpu rwsem, a second read lock by
> the same thread can block if a writer (like CPU hotplug) is pending. Does
> this API need an unlocked variant for initialization paths, or should it
> rely on the caller to manage the lock entirely?

Yes, you are right. Seems the best option would be to remove the lock
from uncore_die_to_cpu(), and rely on callers to manage the lock.


> 
>> +	for_each_online_cpu(cpu) {
>> +		if (topology_logical_die_id(cpu) == die) {
>> +			res = cpu;
>> +			break;
>> +		}
>> +	}
>> +	cpus_read_unlock();
>> +	return res;
>> +}
>