Re: [PATCH v3] PCI: Call local_pci_probe() directly if current CPU is in the right node

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for workqueue recursive locking deadlock hazard due to conflating NUMA affinity optimization with deadlock avoidance.
--

commit 00ce8bc63ea5f4dc858bb9687e393f23de7912a4
Author: Waiman Long <longman@redhat.com>

PCI: Call local_pci_probe() directly if current CPU is in the right node

This commit attempts to fix a lockdep recursive warning in pci_call_probe()
by directly calling local_pci_probe() when the current task is a kworker
executing on the desired NUMA node.

> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c

[ ... ]

> @@ -376,14 +378,27 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
>  	 */
>  	node = dev_to_node(&dev->dev);
>  	dev->is_probed = 1;
> +	node_invalid = node < 0 || node >= MAX_NUMNODES || !node_online(node);
> +	node_cpus = node_invalid ? cpu_online_mask : cpumask_of_node(node);
> +
> +	/*
> +	 * If the current task is a wq kworker activated by queue_work_on()
> +	 * below, the kworker is affined to a designated CPU and won't be
> +	 * switched to another one. So the current CPU can be checked to see
> +	 * if it is in the right node.
> +	 */
> +	if (current->flags & PF_WQ_WORKER) {
> +		cpu_in_node = cpumask_test_cpu(get_cpu(), node_cpus);
> +		put_cpu();
> +	}
>  
>  	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.
> +	 * device is probed from work_on_cpu() of the Physical device or when
> +	 * the current CPU is in the desired node.
>  	 */
> -	if (node < 0 || node >= MAX_NUMNODES || !node_online(node) ||
> -	    pci_physfn_is_probed(dev)) {
> +	if (node_invalid || cpu_in_node || pci_physfn_is_probed(dev)) {

[Severity: High]
This is a pre-existing issue, but does using NUMA locality as a proxy for
deadlock avoidance provide an incomplete fix for the workqueue recursive
locking hazard?

While this suppresses the lockdep warning for typical setups where a parent
and child share the same NUMA node, what happens if a nested probe occurs
for a device assigned to a different NUMA node?

If cpu_in_node evaluates to false during a cross-node nested probe, it
appears the code will fall through to the else block, queue a new work item
to pci_probe_wq, and call flush_work() on it.

If this nested probe is initiated from a work item already executing on
pci_probe_wq, wouldn't waiting on a new work item queued to the same
workqueue trigger the exact same recursive lockdep warning, and potentially
deadlock if the workqueue's max_active workers are exhausted?

>  		error = local_pci_probe(&ddi);
>  	} else {
>  		struct pci_probe_arg arg = { .ddi = &ddi };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607221103.703133-1-longman@redhat.com?part=1