Re: [PATCHv3 1/5] pci: make pci_stop_dev concurrent safe

Lukas Wunner
2024-11-07T14:17:49Z

On Tue, Oct 22, 2024 at 03:48:47PM -0700, Keith Busch wrote:
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -31,18 +31,16 @@ static int pci_pwrctl_unregister(struct device *dev, void *data)
>
> static void pci_stop_dev(struct pci_dev *dev)
> {
> - pci_pme_active(dev, false);
> -
> - if (pci_dev_is_added(dev)) {
> - device_for_each_child(dev->dev.parent, dev_of_node(&dev->dev),
> - pci_pwrctl_unregister);
> - device_release_driver(&dev->dev);
> - pci_proc_detach_device(dev);
> - pci_remove_sysfs_dev_files(dev);
> - of_pci_remove_node(dev);
> + if (!pci_dev_test_and_clear_added(dev))
> + return;
>
> - pci_dev_assign_added(dev, false);
> - }
> + pci_pme_active(dev, false);
> + device_for_each_child(dev->dev.parent, dev_of_node(&dev->dev),
> + pci_pwrctl_unregister);
> + device_release_driver(&dev->dev);
> + pci_proc_detach_device(dev);
> + pci_remove_sysfs_dev_files(dev);
> + of_pci_remove_node(dev);
> }

The above is now queued for v6.13 as commit 6d6d962a8dc2 on pci/locking.

I note there's a behavioral change here:

Previously "pci_pme_active(dev, false)" was called unconditionally,
now only if the "added" flag has been set. The commit message
doesn't explain why this change is fine, so perhaps it's inadvertent?

Thanks,

Lukas