Mailing List Archive

[PATCH 6/6] PCI: hv: Use async probing to reduce boot time
Commit 414428c5da1c ("PCI: hv: Lock PCI bus on device eject") added the
pci_lock_rescan_remove() and pci_unlock_rescan_remove() to address the
race between create_root_hv_pci_bus() and hv_eject_device_work(), but it
doesn't really work well.

Now with hbus->state_lock and other fixes, the race is resolved, so
remove pci_{lock,unlock}_rescan_remove().

Also enable async probing to reduce boot time.

Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/pci/controller/pci-hyperv.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
index 2c0b86b20408..08ab389e27cc 100644
--- a/drivers/pci/controller/pci-hyperv.c
+++ b/drivers/pci/controller/pci-hyperv.c
@@ -2312,12 +2312,16 @@ static int create_root_hv_pci_bus(struct hv_pcibus_device *hbus)
if (error)
return error;

- pci_lock_rescan_remove();
+ /*
+ * pci_lock_rescan_remove() and pci_unlock_rescan_remove() are
+ * unnecessary here, because we hold the hbus->state_lock, meaning
+ * hv_eject_device_work() and pci_devices_present_work() can't race
+ * with create_root_hv_pci_bus().
+ */
hv_pci_assign_numa_node(hbus);
pci_bus_assign_resources(bridge->bus);
hv_pci_assign_slots(hbus);
pci_bus_add_devices(bridge->bus);
- pci_unlock_rescan_remove();
hbus->state = hv_pcibus_installed;
return 0;
}
@@ -4003,6 +4007,9 @@ static struct hv_driver hv_pci_drv = {
.remove = hv_pci_remove,
.suspend = hv_pci_suspend,
.resume = hv_pci_resume,
+ .driver = {
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ },
};

static void __exit exit_hv_pci_drv(void)
--
2.25.1
RE: [PATCH 6/6] PCI: hv: Use async probing to reduce boot time [ In reply to ]
From: Dexuan Cui <decui@microsoft.com>
>
> Commit 414428c5da1c ("PCI: hv: Lock PCI bus on device eject") added the
> pci_lock_rescan_remove() and pci_unlock_rescan_remove() to address the
> race between create_root_hv_pci_bus() and hv_eject_device_work(), but it
> doesn't really work well.
>
> Now with hbus->state_lock and other fixes, the race is resolved, so
> remove pci_{lock,unlock}_rescan_remove().

Commit 414428c5da1c added the calls to pci_lock/unlock_rescan_remove()
in both create_root_hv_pci_bus() and in hv_eject_device_work(). This patch
removes the calls only in create_reboot_hv_pci_bus(), but leaves them in
hv_eject_device_work(), in hv_pci_remove(), and in pci_devices_present_work().
So evidently it is still needed to provide exclusion for other cases. Perhaps your
commit message could clarify that only the exclusion of create_root_hv_pci_bus()
is now superfluous because of the hbus->state_lock. And commit 414428c5da1c
isn't fully reverted because evidently the lock is still needed in
hv_eject_device_work().

Michael

>
> Also enable async probing to reduce boot time.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/pci/controller/pci-hyperv.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c
> index 2c0b86b20408..08ab389e27cc 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -2312,12 +2312,16 @@ static int create_root_hv_pci_bus(struct
> hv_pcibus_device *hbus)
> if (error)
> return error;
>
> - pci_lock_rescan_remove();
> + /*
> + * pci_lock_rescan_remove() and pci_unlock_rescan_remove() are
> + * unnecessary here, because we hold the hbus->state_lock, meaning
> + * hv_eject_device_work() and pci_devices_present_work() can't race
> + * with create_root_hv_pci_bus().
> + */
> hv_pci_assign_numa_node(hbus);
> pci_bus_assign_resources(bridge->bus);
> hv_pci_assign_slots(hbus);
> pci_bus_add_devices(bridge->bus);
> - pci_unlock_rescan_remove();
> hbus->state = hv_pcibus_installed;
> return 0;
> }
> @@ -4003,6 +4007,9 @@ static struct hv_driver hv_pci_drv = {
> .remove = hv_pci_remove,
> .suspend = hv_pci_suspend,
> .resume = hv_pci_resume,
> + .driver = {
> + .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> + },
> };
>
> static void __exit exit_hv_pci_drv(void)
> --
> 2.25.1
RE: [PATCH 6/6] PCI: hv: Use async probing to reduce boot time [ In reply to ]
> From: Michael Kelley (LINUX) <mikelley@microsoft.com>
> Sent: Wednesday, March 29, 2023 9:55 AM
>
> From: Dexuan Cui <decui@microsoft.com>
> >
> > Commit 414428c5da1c ("PCI: hv: Lock PCI bus on device eject") added the
> > pci_lock_rescan_remove() and pci_unlock_rescan_remove() to address the
> > race between create_root_hv_pci_bus() and hv_eject_device_work(), but it
> > doesn't really work well.
> >
> > Now with hbus->state_lock and other fixes, the race is resolved, so
> > remove pci_{lock,unlock}_rescan_remove().
>
> Commit 414428c5da1c added the calls to pci_lock/unlock_rescan_remove()
> in both create_root_hv_pci_bus() and in hv_eject_device_work(). This
> patch removes the calls only in create_reboot_hv_pci_bus(), but leaves
> them in hv_eject_device_work(), in hv_pci_remove(), and in
> pci_devices_present_work().
> So evidently it is still needed to provide exclusion for other cases. Perhaps
> your commit message could clarify that only the exclusion of
> create_root_hv_pci_bus() > is now superfluous because of the
> hbus->state_lock. And commit 414428c5da1c > isn't fully reverted
> because evidently the lock is still needed in hv_eject_device_work().
>
> Michael

Thanks! I'll update the commit message in v2.