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

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

[weipengliang]

When a PCI device is on an invalid/offline NUMA node or is a VF being
probed from PF context, the code path calls local_pci_probe() directly
without needing work_on_cpu(). In these cases, the cpu_hotplug_disable()
/ cpu_hotplug_enable() round-trip is unnecessary overhead.

Move cpu_hotplug_disable() into the NUMA-valid else branch so non-NUMA
devices skip it entirely, returning early after local_pci_probe().
---
 drivers/pci/pci-driver.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3f59001785a..d3df90f58cf4 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -384,7 +384,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.
@@ -392,9 +391,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);
+		dev->is_probed = 0;
+		return error;
 	} else {
 		struct pci_probe_arg arg = { .ddi = &ddi };
 
+		cpu_hotplug_disable();
+
 		INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
 		/*
 		 * The target election and the enqueue of the work must be within
-- 
2.47.1.windows.1

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

[Lukas Wunner]

On Fri, May 22, 2026 at 02:11:24PM +0800, weipengliang wrote:
> When a PCI device is on an invalid/offline NUMA node or is a VF being
> probed from PF context, the code path calls local_pci_probe() directly
> without needing work_on_cpu(). In these cases, the cpu_hotplug_disable()
> / cpu_hotplug_enable() round-trip is unnecessary overhead.
> 
> Move cpu_hotplug_disable() into the NUMA-valid else branch so non-NUMA
> devices skip it entirely, returning early after local_pci_probe().

Hm, this will retain the cpu_hotplug_enable() outside the else-branch,
which isn't very readable IMHO.  How about moving cpu_hotplug_enable()
directly below each rcu_read_unlock() call?

> @@ -392,9 +391,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);
> +		dev->is_probed = 0;
> +		return error;
>  	} else {
>  		struct pci_probe_arg arg = { .ddi = &ddi };
>  
> +		cpu_hotplug_disable();
> +
>  		INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
>  		/*
>  		 * The target election and the enqueue of the work must be within

I'd also move the cpu_hotplug_disable() below INIT_WORK_ONSTACK().
For one it minimizes the critical section, and also INIT_WORK_ONSTACK()
initializes a portion of "arg" declared immediately above, so it's
more readable to keep declaration + initialization together.

Thanks,

Lukas

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

[weipengliang]

When a PCI device is on an invalid/offline NUMA node or is a VF being
probed from PF context, the code path calls local_pci_probe() directly
without needing work_on_cpu(). In these cases, the cpu_hotplug_disable()
/ cpu_hotplug_enable() round-trip is unnecessary overhead.

Move cpu_hotplug_disable() into the NUMA-valid else branch so non-NUMA
devices skip it entirely, returning early after local_pci_probe(). Also
move cpu_hotplug_enable() directly below each rcu_read_unlock() call to
keep the disable/enable symmetrical and improve readability.

Changes since v1:
- Moved cpu_hotplug_enable() directly below each rcu_read_unlock()
  (Lukas Wunner)
- Moved cpu_hotplug_disable() below INIT_WORK_ONSTACK() to minimize
  critical section and keep declaration + initialization together
  (Lukas Wunner)

Signed-off-by: weipengliang <weipengliang@xiaomi.com>
---
 drivers/pci/pci-driver.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3f59001785a..ea213555fb6c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -384,7 +384,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.
@@ -392,10 +391,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);
+		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();
 		/*
 		 * The target election and the enqueue of the work must be within
 		 * the same RCU read side section so that when the workqueue pool
@@ -414,10 +416,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);
 			error = arg.ret;
 		} else {
 			rcu_read_unlock();
+			cpu_hotplug_enable();
 			error = local_pci_probe(&ddi);
 		}
 
@@ -425,7 +429,6 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 	}
 
 	dev->is_probed = 0;
-	cpu_hotplug_enable();
 	return error;
 }
 
-- 
2.47.1.windows.1

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

[Lukas Wunner]

On Fri, May 22, 2026 at 03:54:52PM +0800, weipengliang wrote:
> @@ -392,10 +391,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);
> +		dev->is_probed = 0;
> +		return error;
>  	} else {

Hm, looks like these two newly added lines are unnecessary.
Am I missing something?

Thanks,

Lukas

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

[weipengliang]

On Fri, May 22, 2026 at 10:02:41AM +0200, Lukas Wunner wrote:
> Hm, looks like these two newly added lines are unnecessary.
> Am I missing something?

You're right. Since cpu_hotplug_enable() has been moved into the
branches, the early return is no longer needed -- both paths now
fall through to the common cleanup at the end.

I'll drop these two lines in v3.

Thanks,
weipengliang

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

[weipengliang]

Hi Lukas,

Regarding cpu_hotplug_enable() placement in the else block: would you
prefer it right after each rcu_read_unlock(), or a single call before
destroy_work_on_stack() at the end? The latter keeps the code simpler
while the critical section stays minimal -- flush_work() and
destroy_work_on_stack() are both short and don't depend on CPU affinity.

Thanks,
weipengliang

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

[weipengliang]

Hi Lukas,

I just received AI review feedback from sashiko.dev on this patch:
https://sashiko.dev/#/patchset/20260522075453.473-1-weipengliang@xiaomi.com?part=1

The reviewer raised a concern about enabling CPU hotplug before
flush_work():

  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.

Given this, I think it's better to place cpu_hotplug_enable() after
destroy_work_on_stack() at the end of the else block. This keeps
hotplug disabled during both flush_work() and local_pci_probe(),
while the extra time during destroy_work_on_stack() is negligible.
I'll update v3 accordingly.

Thanks,
weipengliang

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

[weipengliang]

When a PCI device is on an invalid/offline NUMA node or is a VF being
probed from PF context, the code path calls local_pci_probe() directly
without needing work_on_cpu(). In these cases, the cpu_hotplug_disable()
/ cpu_hotplug_enable() round-trip is unnecessary overhead.

Move cpu_hotplug_disable() into the NUMA-valid else branch so non-NUMA
devices skip it entirely. Place a single cpu_hotplug_enable() after
destroy_work_on_stack() to keep hotplug disabled during flush_work()
and local_pci_probe(), preventing CPU offline from migrating the probe
work to an incorrect NUMA node.

Signed-off-by: weipengliang <weipengliang@xiaomi.com>
---
 drivers/pci/pci-driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3f59001785a..27c1cc7c247c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -384,7 +384,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.
@@ -396,6 +395,7 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 		struct pci_probe_arg arg = { .ddi = &ddi };
 
 		INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
+		cpu_hotplug_disable();
 		/*
 		 * The target election and the enqueue of the work must be within
 		 * the same RCU read side section so that when the workqueue pool
@@ -421,11 +421,11 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 			error = local_pci_probe(&ddi);
 		}
 
+		cpu_hotplug_enable();
 		destroy_work_on_stack(&arg.work);
 	}
 
 	dev->is_probed = 0;
-	cpu_hotplug_enable();
 	return error;
 }
 
-- 
2.47.1.windows.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Commit message discrepancy regarding the placement of cpu_hotplug_enable().
--

commit 9acd77cab9fa6f681c21a1bfe522a33f79ec6605
Author: weipengliang <weipengliang@xiaomi.com>

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

This commit limits cpu_hotplug_disable() and cpu_hotplug_enable() calls
to the NUMA-valid code path. This removes overhead for non-NUMA devices
where local_pci_probe() can be called directly.

> Move cpu_hotplug_disable() into the NUMA-valid else branch so non-NUMA
> devices skip it entirely. Place a single cpu_hotplug_enable() after
> destroy_work_on_stack() to keep hotplug disabled during flush_work()
> and local_pci_probe(), preventing CPU offline from migrating the probe
> work to an incorrect NUMA node.

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
[ ... ]
> @@ -414,11 +414,11 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
>  			error = local_pci_probe(&ddi);
>  		}
>  
> +		cpu_hotplug_enable();
>  		destroy_work_on_stack(&arg.work);

[Severity: Low]
This isn't a bug, but does the commit message accurately describe the
code sequence here?

The commit message mentions placing cpu_hotplug_enable() after
destroy_work_on_stack(), but the code actually places it immediately
before.

Could the commit message be updated to match the code, or should the
code be adjusted to match the intent described in the commit message?

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

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

[weipengliang]

When a PCI device is on an invalid/offline NUMA node or is a VF being
probed from PF context, the code path calls local_pci_probe() directly
without needing work_on_cpu(). In these cases, the cpu_hotplug_disable()
/ cpu_hotplug_enable() round-trip is unnecessary overhead.

Move cpu_hotplug_disable() into the NUMA-valid else branch so non-NUMA
devices skip it entirely. Place a single cpu_hotplug_enable() before
destroy_work_on_stack() to keep hotplug disabled during flush_work()
and local_pci_probe(), preventing CPU offline from migrating the probe
work to an incorrect NUMA node.

Signed-off-by: weipengliang <weipengliang@xiaomi.com>
---
 drivers/pci/pci-driver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3f59001785a..27c1cc7c247c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -384,7 +384,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.
@@ -396,6 +395,7 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 		struct pci_probe_arg arg = { .ddi = &ddi };
 
 		INIT_WORK_ONSTACK(&arg.work, local_pci_probe_callback);
+		cpu_hotplug_disable();
 		/*
 		 * The target election and the enqueue of the work must be within
 		 * the same RCU read side section so that when the workqueue pool
@@ -421,11 +421,11 @@ static int pci_call_probe(struct pci_driver *drv, struct pci_dev *dev,
 			error = local_pci_probe(&ddi);
 		}
 
+		cpu_hotplug_enable();
 		destroy_work_on_stack(&arg.work);
 	}
 
 	dev->is_probed = 0;
-	cpu_hotplug_enable();
 	return error;
 }
 
-- 
2.47.1.windows.1

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

[sashiko-bot]

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