linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI: Allow drivers to opt in to async probing
@ 2025-07-04  7:38 Lukas Wunner
  2025-07-08 23:11 ` Bjorn Helgaas
  2025-07-14 13:45 ` Christoph Hellwig
  0 siblings, 2 replies; 8+ messages in thread
From: Lukas Wunner @ 2025-07-04  7:38 UTC (permalink / raw)
  To: Bjorn Helgaas, Dmitry Torokhov, linux-pci
  Cc: Yaron Avizrat, Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso,
	Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma,
	Ira Weiny, Dan Williams, Even Xu, Xinpeng Sun, Jean Delvare,
	Alexander Usyskin, Adrian Hunter, Keith Busch, Jens Axboe,
	Christoph Hellwig, Sagi Grimberg, Alan Stern, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Stuart Hayes, David Jeffery,
	Jeremy Allison

The PCI core has historically not allowed drivers to opt in to async
probing:  Even though drivers may set "PROBE_PREFER_ASYNCHRONOUS", initial
probing always happens synchronously.  That's because the PCI core uses
device_attach() instead of device_initial_probe().

Should a driver return -EPROBE_DEFER on initial probe, reprobing later on
does honor the PROBE_PREFER_ASYNCHRONOUS setting, which is inconsistent.

The choice of device_attach() is likely not deliberate:  It was introduced
in 2013 with commit 58d9a38f6fac ("PCI: Skip attaching driver in
device_add()"), but asynchronous probing was added two years later with
commit 765230b5f084 ("driver-core: add asynchronous probing support for
drivers").

According to the kernel-doc of "enum probe_type", "the end goal is to
switch the kernel to use asynchronous probing by default".  To this end,
use device_initial_probe() to allow asynchronous probing.  The function
returns void, making the return value check unnecessary.

Initial PCI probing often takes on the order of seconds even on laptops,
so this may speed up booting significantly.

Curiously, a small number of PCI drivers already opt in to asynchronous
probing.  Their maintainters (who are all cc'ed) should watch out for
issues, now that asynchronous probing is not just allowed for deferred
probing, but also initial probing:

hl_pci_driver        drivers/accel/habanalabs/common/habanalabs_drv.c
cxl_pci_driver       drivers/cxl/pci.c
quicki2c_driver      drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
quickspi_driver      drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
i801_driver          drivers/i2c/busses/i2c-i801.c
mei_me_driver        drivers/misc/mei/pci-me.c
mei_vsc_drv          drivers/misc/mei/platform-vsc.c
sdhci_driver         drivers/mmc/host/sdhci-pci-core.c
nvme_driver          drivers/nvme/host/pci.c
ehci_pci_driver      drivers/usb/host/ehci-pci.c
hvfb_pci_stub_driver drivers/video/fbdev/hyperv_fb.c

All other driver maintainers may test asynchronous probing by specifying
the command line parameter "driver_async_probe=drv_name1,drv_name2,...",
and on success setting "probe_type = PROBE_PREFER_ASYNCHRONOUS" in the
pci_driver struct.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/bus.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 69048869ef1c..b77fd30bbfd9 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -341,7 +341,6 @@ void pci_bus_add_device(struct pci_dev *dev)
 {
 	struct device_node *dn = dev->dev.of_node;
 	struct platform_device *pdev;
-	int retval;
 
 	/*
 	 * Can not put in pci_device_add yet because resources
@@ -372,9 +371,7 @@ void pci_bus_add_device(struct pci_dev *dev)
 	if (!dn || of_device_is_available(dn))
 		pci_dev_allow_binding(dev);
 
-	retval = device_attach(&dev->dev);
-	if (retval < 0 && retval != -EPROBE_DEFER)
-		pci_warn(dev, "device attach failed (%d)\n", retval);
+	device_initial_probe(&dev->dev);
 
 	pci_dev_assign_added(dev);
 }
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-04  7:38 [PATCH] PCI: Allow drivers to opt in to async probing Lukas Wunner
@ 2025-07-08 23:11 ` Bjorn Helgaas
  2025-07-14 13:45 ` Christoph Hellwig
  1 sibling, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2025-07-08 23:11 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Dmitry Torokhov, linux-pci, Yaron Avizrat, Koby Elbaz,
	Konstantin Sinyuk, Davidlohr Bueso, Jonathan Cameron, Dave Jiang,
	Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams, Even Xu,
	Xinpeng Sun, Jean Delvare, Alexander Usyskin, Adrian Hunter,
	Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
	Alan Stern, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Stuart Hayes, David Jeffery, Jeremy Allison

On Fri, Jul 04, 2025 at 09:38:33AM +0200, Lukas Wunner wrote:
> The PCI core has historically not allowed drivers to opt in to async
> probing:  Even though drivers may set "PROBE_PREFER_ASYNCHRONOUS", initial
> probing always happens synchronously.  That's because the PCI core uses
> device_attach() instead of device_initial_probe().
> 
> Should a driver return -EPROBE_DEFER on initial probe, reprobing later on
> does honor the PROBE_PREFER_ASYNCHRONOUS setting, which is inconsistent.
> 
> The choice of device_attach() is likely not deliberate:  It was introduced
> in 2013 with commit 58d9a38f6fac ("PCI: Skip attaching driver in
> device_add()"), but asynchronous probing was added two years later with
> commit 765230b5f084 ("driver-core: add asynchronous probing support for
> drivers").
> 
> According to the kernel-doc of "enum probe_type", "the end goal is to
> switch the kernel to use asynchronous probing by default".  To this end,
> use device_initial_probe() to allow asynchronous probing.  The function
> returns void, making the return value check unnecessary.
> 
> Initial PCI probing often takes on the order of seconds even on laptops,
> so this may speed up booting significantly.
> 
> Curiously, a small number of PCI drivers already opt in to asynchronous
> probing.  Their maintainters (who are all cc'ed) should watch out for
> issues, now that asynchronous probing is not just allowed for deferred
> probing, but also initial probing:
> 
> hl_pci_driver        drivers/accel/habanalabs/common/habanalabs_drv.c
> cxl_pci_driver       drivers/cxl/pci.c
> quicki2c_driver      drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> quickspi_driver      drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> i801_driver          drivers/i2c/busses/i2c-i801.c
> mei_me_driver        drivers/misc/mei/pci-me.c
> mei_vsc_drv          drivers/misc/mei/platform-vsc.c
> sdhci_driver         drivers/mmc/host/sdhci-pci-core.c
> nvme_driver          drivers/nvme/host/pci.c
> ehci_pci_driver      drivers/usb/host/ehci-pci.c
> hvfb_pci_stub_driver drivers/video/fbdev/hyperv_fb.c
> 
> All other driver maintainers may test asynchronous probing by specifying
> the command line parameter "driver_async_probe=drv_name1,drv_name2,...",
> and on success setting "probe_type = PROBE_PREFER_ASYNCHRONOUS" in the
> pci_driver struct.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>

Applied to pci/enumeration for v6.17, thanks!

> ---
>  drivers/pci/bus.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 69048869ef1c..b77fd30bbfd9 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -341,7 +341,6 @@ void pci_bus_add_device(struct pci_dev *dev)
>  {
>  	struct device_node *dn = dev->dev.of_node;
>  	struct platform_device *pdev;
> -	int retval;
>  
>  	/*
>  	 * Can not put in pci_device_add yet because resources
> @@ -372,9 +371,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>  	if (!dn || of_device_is_available(dn))
>  		pci_dev_allow_binding(dev);
>  
> -	retval = device_attach(&dev->dev);
> -	if (retval < 0 && retval != -EPROBE_DEFER)
> -		pci_warn(dev, "device attach failed (%d)\n", retval);
> +	device_initial_probe(&dev->dev);
>  
>  	pci_dev_assign_added(dev);
>  }
> -- 
> 2.47.2
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-04  7:38 [PATCH] PCI: Allow drivers to opt in to async probing Lukas Wunner
  2025-07-08 23:11 ` Bjorn Helgaas
@ 2025-07-14 13:45 ` Christoph Hellwig
  2025-07-14 14:20   ` Lukas Wunner
  1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2025-07-14 13:45 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Bjorn Helgaas, Dmitry Torokhov, linux-pci, Yaron Avizrat,
	Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso, Jonathan Cameron,
	Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny,
	Dan Williams, Even Xu, Xinpeng Sun, Jean Delvare,
	Alexander Usyskin, Adrian Hunter, Keith Busch, Jens Axboe,
	Christoph Hellwig, Sagi Grimberg, Alan Stern, K. Y. Srinivasan,
	Haiyang Zhang, Wei Liu, Dexuan Cui, Stuart Hayes, David Jeffery,
	Jeremy Allison

On Fri, Jul 04, 2025 at 09:38:33AM +0200, Lukas Wunner wrote:
> The PCI core has historically not allowed drivers to opt in to async
> probing:  Even though drivers may set "PROBE_PREFER_ASYNCHRONOUS", initial
> probing always happens synchronously.  That's because the PCI core uses
> device_attach() instead of device_initial_probe().

Is it?  I see frequent reordering of Qemu NVMe with the current kernel,
and have to disable it for some of my test setups that require different
device characteristics.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-14 13:45 ` Christoph Hellwig
@ 2025-07-14 14:20   ` Lukas Wunner
  2025-07-15  6:13     ` Christoph Hellwig
  0 siblings, 1 reply; 8+ messages in thread
From: Lukas Wunner @ 2025-07-14 14:20 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Bjorn Helgaas, Dmitry Torokhov, linux-pci, Yaron Avizrat,
	Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso, Jonathan Cameron,
	Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny,
	Dan Williams, Even Xu, Xinpeng Sun, Jean Delvare,
	Alexander Usyskin, Adrian Hunter, Keith Busch, Jens Axboe,
	Sagi Grimberg, Alan Stern, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Stuart Hayes, David Jeffery, Jeremy Allison

On Mon, Jul 14, 2025 at 03:45:02PM +0200, Christoph Hellwig wrote:
> On Fri, Jul 04, 2025 at 09:38:33AM +0200, Lukas Wunner wrote:
> > The PCI core has historically not allowed drivers to opt in to async
> > probing:  Even though drivers may set "PROBE_PREFER_ASYNCHRONOUS", initial
> > probing always happens synchronously.  That's because the PCI core uses
> > device_attach() instead of device_initial_probe().
> 
> Is it?  I see frequent reordering of Qemu NVMe with the current kernel,
> and have to disable it for some of my test setups that require different
> device characteristics.

There's a PCI_DEV_ALLOW_BINDING flag in struct pci_dev which is honored
by pci_bus_match().  Initially the flag is false, so when the device is
enumerated, it's not allowed to probe.  Later on in pci_bus_add_device(),
the flag is set to true and initial probing happens.  It always happens
synchronously because:

  pci_bus_add_device()
    pci_dev_allow_binding()
    device_attach()
      __device_attach(dev, allow_async = false)

I guess what happens in your case is, *after* initial probing has
concluded and user space is up and running, a driver is unbound
from the device and another driver is subsequently re-bound.
E.g. "nvme" is unbound and "virtio-pci" is bound instead.

That happens synchronously, but does not prevent parallel binding
of the same driver to another device because the PCI_DEV_ALLOW_BINDING
flag is true for all devices after initial probing has happened:

  bind_store()
    device_driver_attach()
      __driver_probe_device()
        really_probe()

So that would be a second way (aside from -EPROBE_DEFER) to achieve
asynchronous probing.  The commit aims at asynchronous *initial* probing
to speed up booting.  I may not have made this sufficiently clear in the
commit message. :(

Thanks,

Lukas

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-14 14:20   ` Lukas Wunner
@ 2025-07-15  6:13     ` Christoph Hellwig
  2025-07-15  6:35       ` dan.j.williams
  0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2025-07-15  6:13 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Christoph Hellwig, Bjorn Helgaas, Dmitry Torokhov, linux-pci,
	Yaron Avizrat, Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso,
	Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma,
	Ira Weiny, Dan Williams, Even Xu, Xinpeng Sun, Jean Delvare,
	Alexander Usyskin, Adrian Hunter, Keith Busch, Jens Axboe,
	Sagi Grimberg, Alan Stern, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Stuart Hayes, David Jeffery, Jeremy Allison

On Mon, Jul 14, 2025 at 04:20:03PM +0200, Lukas Wunner wrote:
> I guess what happens in your case is, *after* initial probing has
> concluded and user space is up and running, a driver is unbound
> from the device and another driver is subsequently re-bound.
> E.g. "nvme" is unbound and "virtio-pci" is bound instead.

How?  This is a non-modular simply kernel running on kvm.  There
should be no re-binding, and binding nvme devices to virtio of course
also doesn't make sense.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-15  6:13     ` Christoph Hellwig
@ 2025-07-15  6:35       ` dan.j.williams
  2025-07-15  8:42         ` Lukas Wunner
  0 siblings, 1 reply; 8+ messages in thread
From: dan.j.williams @ 2025-07-15  6:35 UTC (permalink / raw)
  To: Christoph Hellwig, Lukas Wunner
  Cc: Christoph Hellwig, Bjorn Helgaas, Dmitry Torokhov, linux-pci,
	Yaron Avizrat, Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso,
	Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma,
	Ira Weiny, Dan Williams, Even Xu, Xinpeng Sun, Jean Delvare,
	Alexander Usyskin, Adrian Hunter, Keith Busch, Jens Axboe,
	Sagi Grimberg, Alan Stern, K. Y. Srinivasan, Haiyang Zhang,
	Wei Liu, Dexuan Cui, Stuart Hayes, David Jeffery, Jeremy Allison

Christoph Hellwig wrote:
> On Mon, Jul 14, 2025 at 04:20:03PM +0200, Lukas Wunner wrote:
> > I guess what happens in your case is, *after* initial probing has
> > concluded and user space is up and running, a driver is unbound
> > from the device and another driver is subsequently re-bound.
> > E.g. "nvme" is unbound and "virtio-pci" is bound instead.
> 
> How?  This is a non-modular simply kernel running on kvm.  There
> should be no re-binding, and binding nvme devices to virtio of course
> also doesn't make sense.

I too could have swore I see async behavior with cxl_pci. I believe this
patch is only affecting async behavior when the driver is loaded before
initial arrival of the PCI device.

For the typical modular driver case the late arriving driver also
arranges async probing. Lo and behold on current upstream:

[   13.002750] __driver_attach: pci 0000:35:00.0: probing driver cxl_pci asynchronously

...so this patch is only a change in behavior for built-in drivers
loaded before PCI initial scan afaics.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-15  6:35       ` dan.j.williams
@ 2025-07-15  8:42         ` Lukas Wunner
  2025-07-15 16:26           ` Bjorn Helgaas
  0 siblings, 1 reply; 8+ messages in thread
From: Lukas Wunner @ 2025-07-15  8:42 UTC (permalink / raw)
  To: dan.j.williams, Bjorn Helgaas
  Cc: Christoph Hellwig, Dmitry Torokhov, linux-pci, Yaron Avizrat,
	Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso, Jonathan Cameron,
	Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, Even Xu,
	Xinpeng Sun, Jean Delvare, Alexander Usyskin, Adrian Hunter,
	Keith Busch, Jens Axboe, Sagi Grimberg, Alan Stern,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Stuart Hayes, David Jeffery, Jeremy Allison

On Mon, Jul 14, 2025 at 11:35:35PM -0700, dan.j.williams@intel.com wrote:
> I too could have swore I see async behavior with cxl_pci. I believe this
> patch is only affecting async behavior when the driver is loaded before
> initial arrival of the PCI device.
> 
> For the typical modular driver case the late arriving driver also
> arranges async probing. Lo and behold on current upstream:
> 
> [   13.002750] __driver_attach: pci 0000:35:00.0: probing driver cxl_pci asynchronously
> 
> ...so this patch is only a change in behavior for built-in drivers
> loaded before PCI initial scan afaics.

Right, thank you!

FWIW, below is a rephrased commit message which seeks to make more
clear that only initial probing of built-in drivers is affected.

In case Bjorn wants to replace the commit on pci/enumeration.
I can also submit this as v2 if desired.  No code changes,
just a rephrased commit message.

-- >8 --

Subject: [PATCH] PCI: Allow built-in drivers to use async initial probing

The PCI core has historically not allowed built-in drivers to opt in to
async initial probing:  Drivers may set "PROBE_PREFER_ASYNCHRONOUS", but
initial probing always happens synchronously.  That's because the PCI core
uses device_attach() instead of device_initial_probe().

Should a driver return -EPROBE_DEFER on initial probe, reprobing later on
does honor the PROBE_PREFER_ASYNCHRONOUS setting.  Modular drivers are
also allowed to probe asynchronously, which is inconsistent.

The choice of device_attach() is likely not deliberate:  It was introduced
in 2013 with commit 58d9a38f6fac ("PCI: Skip attaching driver in
device_add()"), but asynchronous probing was added two years later with
commit 765230b5f084 ("driver-core: add asynchronous probing support for
drivers").

According to the kernel-doc of "enum probe_type", "the end goal is to
switch the kernel to use asynchronous probing by default".  To this end,
use device_initial_probe() to allow asynchronous initial probing.  The
function returns void, making the return value check unnecessary.

Initial PCI probing often takes on the order of seconds even on laptops,
so this may speed up booting significantly.

A small number of PCI drivers already opt in to asynchronous probing.
Their maintainers (who are all cc'ed) should watch out for issues, now
that asynchronous probing is not just allowed for deferred and modular
probing, but also initial probing:

  hl_pci_driver        drivers/accel/habanalabs/common/habanalabs_drv.c
  cxl_pci_driver       drivers/cxl/pci.c
  quicki2c_driver      drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
  quickspi_driver      drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
  i801_driver          drivers/i2c/busses/i2c-i801.c
  mei_me_driver        drivers/misc/mei/pci-me.c
  mei_vsc_drv          drivers/misc/mei/platform-vsc.c
  sdhci_driver         drivers/mmc/host/sdhci-pci-core.c
  nvme_driver          drivers/nvme/host/pci.c
  ehci_pci_driver      drivers/usb/host/ehci-pci.c
  hvfb_pci_stub_driver drivers/video/fbdev/hyperv_fb.c

All other driver maintainers may test asynchronous probing by specifying
the command line parameter "driver_async_probe=drv_name1,drv_name2,...",
and on success setting "probe_type = PROBE_PREFER_ASYNCHRONOUS" in the
pci_driver struct.

Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Link: https://patch.msgid.link/53abe6f5ac7c631f95f5d061aa748b192eda0379.1751614426.git.lukas@wunner.de
---
 drivers/pci/bus.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 6904886..b77fd30b 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -341,7 +341,6 @@ void pci_bus_add_device(struct pci_dev *dev)
 {
 	struct device_node *dn = dev->dev.of_node;
 	struct platform_device *pdev;
-	int retval;
 
 	/*
 	 * Can not put in pci_device_add yet because resources
@@ -372,9 +371,7 @@ void pci_bus_add_device(struct pci_dev *dev)
 	if (!dn || of_device_is_available(dn))
 		pci_dev_allow_binding(dev);
 
-	retval = device_attach(&dev->dev);
-	if (retval < 0 && retval != -EPROBE_DEFER)
-		pci_warn(dev, "device attach failed (%d)\n", retval);
+	device_initial_probe(&dev->dev);
 
 	pci_dev_assign_added(dev);
 }
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] PCI: Allow drivers to opt in to async probing
  2025-07-15  8:42         ` Lukas Wunner
@ 2025-07-15 16:26           ` Bjorn Helgaas
  0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2025-07-15 16:26 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: dan.j.williams, Christoph Hellwig, Dmitry Torokhov, linux-pci,
	Yaron Avizrat, Koby Elbaz, Konstantin Sinyuk, Davidlohr Bueso,
	Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma,
	Ira Weiny, Even Xu, Xinpeng Sun, Jean Delvare, Alexander Usyskin,
	Adrian Hunter, Keith Busch, Jens Axboe, Sagi Grimberg, Alan Stern,
	K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Stuart Hayes, David Jeffery, Jeremy Allison

On Tue, Jul 15, 2025 at 10:42:47AM +0200, Lukas Wunner wrote:
> On Mon, Jul 14, 2025 at 11:35:35PM -0700, dan.j.williams@intel.com wrote:
> > I too could have swore I see async behavior with cxl_pci. I believe this
> > patch is only affecting async behavior when the driver is loaded before
> > initial arrival of the PCI device.
> > 
> > For the typical modular driver case the late arriving driver also
> > arranges async probing. Lo and behold on current upstream:
> > 
> > [   13.002750] __driver_attach: pci 0000:35:00.0: probing driver cxl_pci asynchronously
> > 
> > ...so this patch is only a change in behavior for built-in drivers
> > loaded before PCI initial scan afaics.
> 
> Right, thank you!
> 
> FWIW, below is a rephrased commit message which seeks to make more
> clear that only initial probing of built-in drivers is affected.
> 
> In case Bjorn wants to replace the commit on pci/enumeration.
> I can also submit this as v2 if desired.  No code changes,
> just a rephrased commit message.

Updated the commit log as below, thanks!

> -- >8 --
> 
> Subject: [PATCH] PCI: Allow built-in drivers to use async initial probing
> 
> The PCI core has historically not allowed built-in drivers to opt in to
> async initial probing:  Drivers may set "PROBE_PREFER_ASYNCHRONOUS", but
> initial probing always happens synchronously.  That's because the PCI core
> uses device_attach() instead of device_initial_probe().
> 
> Should a driver return -EPROBE_DEFER on initial probe, reprobing later on
> does honor the PROBE_PREFER_ASYNCHRONOUS setting.  Modular drivers are
> also allowed to probe asynchronously, which is inconsistent.
> 
> The choice of device_attach() is likely not deliberate:  It was introduced
> in 2013 with commit 58d9a38f6fac ("PCI: Skip attaching driver in
> device_add()"), but asynchronous probing was added two years later with
> commit 765230b5f084 ("driver-core: add asynchronous probing support for
> drivers").
> 
> According to the kernel-doc of "enum probe_type", "the end goal is to
> switch the kernel to use asynchronous probing by default".  To this end,
> use device_initial_probe() to allow asynchronous initial probing.  The
> function returns void, making the return value check unnecessary.
> 
> Initial PCI probing often takes on the order of seconds even on laptops,
> so this may speed up booting significantly.
> 
> A small number of PCI drivers already opt in to asynchronous probing.
> Their maintainers (who are all cc'ed) should watch out for issues, now
> that asynchronous probing is not just allowed for deferred and modular
> probing, but also initial probing:
> 
>   hl_pci_driver        drivers/accel/habanalabs/common/habanalabs_drv.c
>   cxl_pci_driver       drivers/cxl/pci.c
>   quicki2c_driver      drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
>   quickspi_driver      drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
>   i801_driver          drivers/i2c/busses/i2c-i801.c
>   mei_me_driver        drivers/misc/mei/pci-me.c
>   mei_vsc_drv          drivers/misc/mei/platform-vsc.c
>   sdhci_driver         drivers/mmc/host/sdhci-pci-core.c
>   nvme_driver          drivers/nvme/host/pci.c
>   ehci_pci_driver      drivers/usb/host/ehci-pci.c
>   hvfb_pci_stub_driver drivers/video/fbdev/hyperv_fb.c
> 
> All other driver maintainers may test asynchronous probing by specifying
> the command line parameter "driver_async_probe=drv_name1,drv_name2,...",
> and on success setting "probe_type = PROBE_PREFER_ASYNCHRONOUS" in the
> pci_driver struct.
> 
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Link: https://patch.msgid.link/53abe6f5ac7c631f95f5d061aa748b192eda0379.1751614426.git.lukas@wunner.de
> ---
>  drivers/pci/bus.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 6904886..b77fd30b 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -341,7 +341,6 @@ void pci_bus_add_device(struct pci_dev *dev)
>  {
>  	struct device_node *dn = dev->dev.of_node;
>  	struct platform_device *pdev;
> -	int retval;
>  
>  	/*
>  	 * Can not put in pci_device_add yet because resources
> @@ -372,9 +371,7 @@ void pci_bus_add_device(struct pci_dev *dev)
>  	if (!dn || of_device_is_available(dn))
>  		pci_dev_allow_binding(dev);
>  
> -	retval = device_attach(&dev->dev);
> -	if (retval < 0 && retval != -EPROBE_DEFER)
> -		pci_warn(dev, "device attach failed (%d)\n", retval);
> +	device_initial_probe(&dev->dev);
>  
>  	pci_dev_assign_added(dev);
>  }
> -- 
> 2.47.2
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-07-15 16:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-04  7:38 [PATCH] PCI: Allow drivers to opt in to async probing Lukas Wunner
2025-07-08 23:11 ` Bjorn Helgaas
2025-07-14 13:45 ` Christoph Hellwig
2025-07-14 14:20   ` Lukas Wunner
2025-07-15  6:13     ` Christoph Hellwig
2025-07-15  6:35       ` dan.j.williams
2025-07-15  8:42         ` Lukas Wunner
2025-07-15 16:26           ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).