[PATCH 1/2] pci: export pci_dev_unlock() and the respective unlock

[PATCH 1/2] pci: export pci_dev_unlock() and the respective unlock

[Luis Chamberlain]

Other places in the kernel use this form, and so just
provide a common path for it.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/pci/pci.c   | 6 ++++--
 include/linux/pci.h | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f09821af1d2e..b1d9bb3f5ae2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5027,7 +5027,7 @@ static void pci_dev_lock(struct pci_dev *dev)
 }
 
 /* Return 1 on successful lock, 0 on contention */
-static int pci_dev_trylock(struct pci_dev *dev)
+int pci_dev_trylock(struct pci_dev *dev)
 {
 	if (pci_cfg_access_trylock(dev)) {
 		if (device_trylock(&dev->dev))
@@ -5037,12 +5037,14 @@ static int pci_dev_trylock(struct pci_dev *dev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(pci_dev_trylock);
 
-static void pci_dev_unlock(struct pci_dev *dev)
+void pci_dev_unlock(struct pci_dev *dev)
 {
 	device_unlock(&dev->dev);
 	pci_cfg_access_unlock(dev);
 }
+EXPORT_SYMBOL_GPL(pci_dev_unlock);
 
 static void pci_dev_save_and_disable(struct pci_dev *dev)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6248e044dd29..c55368f58965 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1353,6 +1353,9 @@ int devm_request_pci_bus_resources(struct device *dev,
 /* Temporary until new and working PCI SBR API in place */
 int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
 
+int pci_dev_trylock(struct pci_dev *dev);
+void pci_dev_unlock(struct pci_dev *dev);
+
 #define pci_bus_for_each_resource(bus, res, i)				\
 	for (i = 0;							\
 	    (res = pci_bus_resource_n(bus, i)) || i < PCI_BRIDGE_RESOURCE_NUM; \
-- 
2.30.2

[PATCH 2/2] vfio: use the new pci_dev_trylock() helper to simplify try lock

[Luis Chamberlain]

Use the new pci_dev_trylock() helper to simplify our locking.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/vfio/pci/vfio_pci.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index bd7c482c948a..02b05f7b9a91 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -477,13 +477,10 @@ static void vfio_pci_disable(struct vfio_pci_device *vdev)
 	 * We can not use the "try" reset interface here, which will
 	 * overwrite the previously restored configuration information.
 	 */
-	if (vdev->reset_works && pci_cfg_access_trylock(pdev)) {
-		if (device_trylock(&pdev->dev)) {
-			if (!__pci_reset_function_locked(pdev))
-				vdev->needs_reset = false;
-			device_unlock(&pdev->dev);
-		}
-		pci_cfg_access_unlock(pdev);
+	if (vdev->reset_works && pci_dev_trylock(pdev)) {
+		if (!__pci_reset_function_locked(pdev))
+			vdev->needs_reset = false;
+		pci_dev_unlock(pdev);
 	}
 
 	pci_restore_state(pdev);
-- 
2.30.2

Re: [PATCH 1/2] pci: export pci_dev_unlock() and the respective unlock

[Niklas Schnelle]

Hello Luis, Hello Bjorn,

Interesting timing, I currently have a very similar patch lying around though
with also exporting pci_dev_lock(). I'm planning to use that for upcoming
support of automatic PCI devices recovery on s390x following the
Documentation/PCI/pci-error-recovery.rst recovery flow. There too exprting
these functions would make the code simpler to grok in my opinion. So if Bjorn
accepts this there could soon be another user, not sure if one would want to
then already export pci_dev_lock() too or wait until my patches so it's not
exported without users.

Best regards,
Niklas Schnelle

Re: [PATCH 1/2] pci: export pci_dev_unlock() and the respective unlock

[Bjorn Helgaas]

Please update the subject line to match the convention:

  PCI: Export pci_dev_trylock() and pci_dev_unlock()

On Mon, Jun 21, 2021 at 05:03:09PM -0700, Luis Chamberlain wrote:
> Other places in the kernel use this form, and so just
> provide a common path for it.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

With tweaks mentioned here:

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/pci.c   | 6 ++++--
>  include/linux/pci.h | 3 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index f09821af1d2e..b1d9bb3f5ae2 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5027,7 +5027,7 @@ static void pci_dev_lock(struct pci_dev *dev)
>  }
>  
>  /* Return 1 on successful lock, 0 on contention */
> -static int pci_dev_trylock(struct pci_dev *dev)
> +int pci_dev_trylock(struct pci_dev *dev)
>  {
>  	if (pci_cfg_access_trylock(dev)) {
>  		if (device_trylock(&dev->dev))
> @@ -5037,12 +5037,14 @@ static int pci_dev_trylock(struct pci_dev *dev)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(pci_dev_trylock);
>  
> -static void pci_dev_unlock(struct pci_dev *dev)
> +void pci_dev_unlock(struct pci_dev *dev)
>  {
>  	device_unlock(&dev->dev);
>  	pci_cfg_access_unlock(dev);
>  }
> +EXPORT_SYMBOL_GPL(pci_dev_unlock);
>  
>  static void pci_dev_save_and_disable(struct pci_dev *dev)
>  {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 6248e044dd29..c55368f58965 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1353,6 +1353,9 @@ int devm_request_pci_bus_resources(struct device *dev,
>  /* Temporary until new and working PCI SBR API in place */
>  int pci_bridge_secondary_bus_reset(struct pci_dev *dev);
>  
> +int pci_dev_trylock(struct pci_dev *dev);
> +void pci_dev_unlock(struct pci_dev *dev);

Move next to pci_cfg_access_lock(), which seems a little more related.

>  #define pci_bus_for_each_resource(bus, res, i)				\
>  	for (i = 0;							\
>  	    (res = pci_bus_resource_n(bus, i)) || i < PCI_BRIDGE_RESOURCE_NUM; \
> -- 
> 2.30.2
>