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

[sashiko-bot@kernel.org]

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?

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

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