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

[Lukas Wunner <lukas@wunner.de>]

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