[SRU,bionic/master,1/1] UBUNTU: SAUCE: base/dd: limit release function changes to vfio driver only
diff mbox series

Message ID 20181206111648.5616-1-apw@canonical.com
State New
Headers show
Series
  • [SRU,bionic/master,1/1] UBUNTU: SAUCE: base/dd: limit release function changes to vfio driver only
Related show

Commit Message

Andy Whitcroft Dec. 6, 2018, 11:16 a.m. UTC
It seems that we can trigger the new race detection after remove()
with some drivers which clear the driver as they unreference
the module.  This leads us to fail to clear down those devices which
triggers suspend/resume issues.

Limit the core changes to only apply to the vfio driver while we
work with upstream on a more generic fix.

Fixes: 876dcb5f4576 ("UBUNTU: SAUCE: vfio -- release device lock before userspace requests")
BugLink: http://bugs.launchpad.net/bugs/1803942
Signed-off-by: Andy Whitcroft <apw@canonical.com>
---
 drivers/base/dd.c           | 9 ++++++++-
 drivers/vfio/pci/vfio_pci.c | 2 ++
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

Andy Whitcroft Dec. 6, 2018, 11:25 a.m. UTC | #1
Nack.  Seems I have picked up an old copy of the patch.

-apw
Colin Ian King Dec. 6, 2018, 11:27 a.m. UTC | #2
On 06/12/2018 11:16, Andy Whitcroft wrote:
> It seems that we can trigger the new race detection after remove()
> with some drivers which clear the driver as they unreference
> the module.  This leads us to fail to clear down those devices which
> triggers suspend/resume issues.
> 
> Limit the core changes to only apply to the vfio driver while we
> work with upstream on a more generic fix.
> 
> Fixes: 876dcb5f4576 ("UBUNTU: SAUCE: vfio -- release device lock before userspace requests")
> BugLink: http://bugs.launchpad.net/bugs/1803942
> Signed-off-by: Andy Whitcroft <apw@canonical.com>
> ---
>  drivers/base/dd.c           | 9 ++++++++-
>  drivers/vfio/pci/vfio_pci.c | 2 ++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 37c01054521b..6f7c38648ea0 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -820,6 +820,8 @@ int driver_attach(struct device_driver *drv)
>  }
>  EXPORT_SYMBOL_GPL(driver_attach);
>  
> +void *vfio_pci_driver_ptr;
> +
>  /*
>   * __device_release_driver() must be called with @dev lock held.
>   * When called for a USB interface, @dev->parent lock must be held as well.
> @@ -872,8 +874,13 @@ static void __device_release_driver(struct device *dev, struct device *parent)
>  		 * A concurrent invocation of the same function might
>  		 * have released the driver successfully while this one
>  		 * was waiting, so check for that.
> +		 * LP: #1792099
> +		 *
> +		 * Limit this to the vfio_pci_driver as some drivers NULL
> +		 * out this pointer in their remove() function.
> +		 * LP: #1803942
>  		 */
> -		if (dev->driver != drv)
> +		if (drv == vfio_pci_driver_ptr && dev->driver != drv)
>  			return;
>  
>  		device_links_driver_cleanup(dev);
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index f041b1a6cf66..85d4fd395efb 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -1310,6 +1310,8 @@ static struct pci_driver vfio_pci_driver = {
>  	.remove		= vfio_pci_remove,
>  	.err_handler	= &vfio_err_handlers,
>  };
> +void *vfio_pci_driver_ptr = &vfio_pci_driver;
> +EXPORT_SYMBOL(vfio_pci_driver_ptr);
>  
>  struct vfio_devices {
>  	struct vfio_device **devices;
> 

As a temporary workaround this seems OK to me, but only as a workaround.
 I'm assuming that this will be replaced by the official upstream fix at
a later date.

Acked-by: Colin Ian King <colin.king@canonical.com>

Patch
diff mbox series

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 37c01054521b..6f7c38648ea0 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -820,6 +820,8 @@  int driver_attach(struct device_driver *drv)
 }
 EXPORT_SYMBOL_GPL(driver_attach);
 
+void *vfio_pci_driver_ptr;
+
 /*
  * __device_release_driver() must be called with @dev lock held.
  * When called for a USB interface, @dev->parent lock must be held as well.
@@ -872,8 +874,13 @@  static void __device_release_driver(struct device *dev, struct device *parent)
 		 * A concurrent invocation of the same function might
 		 * have released the driver successfully while this one
 		 * was waiting, so check for that.
+		 * LP: #1792099
+		 *
+		 * Limit this to the vfio_pci_driver as some drivers NULL
+		 * out this pointer in their remove() function.
+		 * LP: #1803942
 		 */
-		if (dev->driver != drv)
+		if (drv == vfio_pci_driver_ptr && dev->driver != drv)
 			return;
 
 		device_links_driver_cleanup(dev);
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index f041b1a6cf66..85d4fd395efb 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1310,6 +1310,8 @@  static struct pci_driver vfio_pci_driver = {
 	.remove		= vfio_pci_remove,
 	.err_handler	= &vfio_err_handlers,
 };
+void *vfio_pci_driver_ptr = &vfio_pci_driver;
+EXPORT_SYMBOL(vfio_pci_driver_ptr);
 
 struct vfio_devices {
 	struct vfio_device **devices;