Re: [PATCH 2/6] perf/x86/intel/uncore: Fix PCI device refcount leak in UPI discovery

[Mi, Dapeng]

On 5/12/2026 7:05 AM, Zide Chen wrote:
> pci_get_domain_bus_and_slot() increments the reference count of the
> returned PCI device and therefore requires a matching pci_dev_put().
>
> In skx_upi_topology_cb() and discover_upi_topology(), the lookup is
> performed inside a loop, but pci_dev_put() is only called once after
> the loop. As a result, references from all previous iterations are
> leaked.
>
> Move pci_dev_put(dev) into the if (dev) block immediately after
> upi_fill_topology() returns.
>
> Opportunistically, fix uninitialized variable in skx_upi_topology_cb().
>
> Fixes: 4cfce57fa42d ("perf/x86/intel/uncore: Enable UPI topology discovery for Skylake Server")
> Fixes: f680b6e6062e ("perf/x86/intel/uncore: Enable UPI topology discovery for Icelake Server")
> Signed-off-by: Zide Chen <zide.chen@xxxxxxxxx>
> ---
>  arch/x86/events/intel/uncore_snbep.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
> index 215d33e260ed..c9ce206fcbb6 100644
> --- a/arch/x86/events/intel/uncore_snbep.c
> +++ b/arch/x86/events/intel/uncore_snbep.c
> @@ -4261,7 +4261,7 @@ static int upi_fill_topology(struct pci_dev *dev, struct intel_uncore_topology *
>  static int skx_upi_topology_cb(struct intel_uncore_type *type, int segment,
>  				int die, u64 cpu_bus_msr)
>  {
> -	int idx, ret;
> +	int idx, ret = 0;
>  	struct intel_uncore_topology *upi;
>  	unsigned int devfn;
>  	struct pci_dev *dev = NULL;
> @@ -4274,12 +4274,12 @@ static int skx_upi_topology_cb(struct intel_uncore_type *type, int segment,
>  		dev = pci_get_domain_bus_and_slot(segment, bus, devfn);
>  		if (dev) {
>  			ret = upi_fill_topology(dev, upi, idx);
> +			pci_dev_put(dev);
>  			if (ret)
>  				break;
>  		}
>  	}
>  
> -	pci_dev_put(dev);
>  	return ret;
>  }
>  
> @@ -5499,6 +5499,7 @@ static int discover_upi_topology(struct intel_uncore_type *type, int ubox_did, i
>  							  devfn);
>  			if (dev) {
>  				ret = upi_fill_topology(dev, upi, idx);
> +				pci_dev_put(dev);
>  				if (ret)
>  					goto err;
>  			}
> @@ -5506,7 +5507,6 @@ static int discover_upi_topology(struct intel_uncore_type *type, int ubox_did, i
>  	}
>  err:
>  	pci_dev_put(ubox);
> -	pci_dev_put(dev);

Should we move the "pci_dev_put(ubox)" into the while loop as well? In
theory, the ubox device could be found multiple times.

Besides, could you please search "pci_get_device()" in uncore code, it
seems some functions don't call pci_dev_put() or only calls it once, like
the funciton spr_update_device_location() ...

Thanks.



>  	return ret;
>  }
>