Re: [PATCH v2] PCI: Avoid unnecessary cpu_hotplug_disable() for non-NUMA devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch removes CPU hotplug protection during PCI device driver probing, exposing drivers to concurrent hotplug races, breaking NUMA locality, and violating CPU isolation.
--

commit d0d4eccba5faab11b523664a86d8feec90710056
Author: weipengliang <weipengliang@xiaomi.com>

PCI: Avoid unnecessary cpu_hotplug_disable() for non-NUMA devices

This commit avoids taking the CPU hotplug lock when probing non-NUMA PCI
devices or VFs being probed from a PF context. It also adjusts the scope of
the hotplug lock in the NUMA path to cover a smaller region around the target
CPU selection and work queueing.

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index d10ece0889f0f..8a7da7bf24580 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
[ ... ]
> @@ -377,7 +377,6 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
>  	node = dev_to_node(&dev->dev);
>  	dev->is_probed = 1;
>  
> -	cpu_hotplug_disable();
>  	/*
>  	 * Prevent nesting work_on_cpu() for the case where a Virtual Function
>  	 * device is probed from work_on_cpu() of the Physical device.
> @@ -385,10 +384,13 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
>  	if (node < 0 || node >= MAX_NUMNODES || !node_online(node) ||
>  	    pci_physfn_is_probed(dev)) {
>  		error = local_pci_probe(&ddi);

[Severity: High]
Does bypassing cpu_hotplug_disable() here expose the driver probe path to
concurrent CPU hotplug events?

Historically, cpu_hotplug_disable() wrapped the entirety of PCI probing to
prevent hotplug events from causing race conditions or deadlocks inside
driver probe functions.

> +		dev->is_probed = 0;
> +		return error;
>  	} else {
>  		struct pci_probe_arg arg = { .ddi = &ddi };
>  
>  		INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
> +		cpu_hotplug_disable();
[ ... ]
> @@ -407,10 +409,12 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
>  				wq = system_percpu_wq;
>  			queue_work_on(cpu, wq, &arg.work);
>  			rcu_read_unlock();
> +			cpu_hotplug_enable();
>  			flush_work(&arg.work);

[Severity: High]
Does enabling CPU hotplug before flush_work() defeat the protection for the
actual probe execution?

If the targeted CPU goes offline while flush_work() is waiting, the workqueue
core might unbind the worker pool and migrate the pending probe work. Because
unbound workers fall back to cpu_possible_mask, the probe might execute on an
incorrect NUMA node or an isolated CPU, violating the housekeeping_cpumask
constraints evaluated earlier.

>  			error = arg.ret;
>  		} else {
>  			rcu_read_unlock();
> +			cpu_hotplug_enable();
>  			error = local_pci_probe(&ddi);

[Severity: High]
Similar to the workqueue path, does calling local_pci_probe() after
cpu_hotplug_enable() leave the driver initialization unprotected against
concurrent CPU hotplug events?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522075453.473-1-weipengliang@xiaomi.com?part=1