diff mbox series

[2/3] vfio: centralize module refcount in subsystem layer

Message ID 20210518192133.59195-2-mgurtovoy@nvidia.com (mailing list archive)
State New, archived
Headers show
Series [1/3] vfio/platform: fix module_put call in error flow | expand

Commit Message

Max Gurtovoy May 18, 2021, 7:21 p.m. UTC 
Remove code duplication and move module refcounting to the subsystem
module.

Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 drivers/vfio/fsl-mc/vfio_fsl_mc.c            | 16 +++-------------
 drivers/vfio/mdev/vfio_mdev.c                | 13 +------------
 drivers/vfio/pci/vfio_pci.c                  |  7 -------
 drivers/vfio/platform/vfio_platform_common.c |  6 ------
 drivers/vfio/vfio.c                          | 10 ++++++++++
 5 files changed, 14 insertions(+), 38 deletions(-)

Comments

Eric Auger May 26, 2021, 5:42 p.m. UTC | #1 
Hi Max,

On 5/18/21 9:21 PM, Max Gurtovoy wrote:
> Remove code duplication and move module refcounting to the subsystem
> module.
> 
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

> ---
>  drivers/vfio/fsl-mc/vfio_fsl_mc.c            | 16 +++-------------
>  drivers/vfio/mdev/vfio_mdev.c                | 13 +------------
>  drivers/vfio/pci/vfio_pci.c                  |  7 -------
>  drivers/vfio/platform/vfio_platform_common.c |  6 ------
>  drivers/vfio/vfio.c                          | 10 ++++++++++
>  5 files changed, 14 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> index 980e59551301..90cad109583b 100644
> --- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> +++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
> @@ -140,26 +140,18 @@ static int vfio_fsl_mc_open(struct vfio_device *core_vdev)
>  {
>  	struct vfio_fsl_mc_device *vdev =
>  		container_of(core_vdev, struct vfio_fsl_mc_device, vdev);
> -	int ret;
> -
> -	if (!try_module_get(THIS_MODULE))
> -		return -ENODEV;
> +	int ret = 0;
>  
>  	mutex_lock(&vdev->reflck->lock);
>  	if (!vdev->refcnt) {
>  		ret = vfio_fsl_mc_regions_init(vdev);
>  		if (ret)
> -			goto err_reg_init;
> +			goto out;
>  	}
>  	vdev->refcnt++;
> -
> +out:
>  	mutex_unlock(&vdev->reflck->lock);
>  
> -	return 0;
> -
> -err_reg_init:
> -	mutex_unlock(&vdev->reflck->lock);
> -	module_put(THIS_MODULE);
>  	return ret;
>  }
>  
> @@ -196,8 +188,6 @@ static void vfio_fsl_mc_release(struct vfio_device *core_vdev)
>  	}
>  
>  	mutex_unlock(&vdev->reflck->lock);
> -
> -	module_put(THIS_MODULE);
>  }
>  
>  static long vfio_fsl_mc_ioctl(struct vfio_device *core_vdev,
> diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
> index 922729071c5a..5ef4815609ed 100644
> --- a/drivers/vfio/mdev/vfio_mdev.c
> +++ b/drivers/vfio/mdev/vfio_mdev.c
> @@ -26,19 +26,10 @@ static int vfio_mdev_open(struct vfio_device *core_vdev)
>  	struct mdev_device *mdev = to_mdev_device(core_vdev->dev);
>  	struct mdev_parent *parent = mdev->type->parent;
>  
> -	int ret;
> -
>  	if (unlikely(!parent->ops->open))
>  		return -EINVAL;
>  
> -	if (!try_module_get(THIS_MODULE))
> -		return -ENODEV;
> -
> -	ret = parent->ops->open(mdev);
> -	if (ret)
> -		module_put(THIS_MODULE);
> -
> -	return ret;
> +	return parent->ops->open(mdev);
>  }
>  
>  static void vfio_mdev_release(struct vfio_device *core_vdev)
> @@ -48,8 +39,6 @@ static void vfio_mdev_release(struct vfio_device *core_vdev)
>  
>  	if (likely(parent->ops->release))
>  		parent->ops->release(mdev);
> -
> -	module_put(THIS_MODULE);
>  }
>  
>  static long vfio_mdev_unlocked_ioctl(struct vfio_device *core_vdev,
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index bd7c482c948a..f6729baa1bf4 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -558,8 +558,6 @@ static void vfio_pci_release(struct vfio_device *core_vdev)
>  	}
>  
>  	mutex_unlock(&vdev->reflck->lock);
> -
> -	module_put(THIS_MODULE);
>  }
>  
>  static int vfio_pci_open(struct vfio_device *core_vdev)
> @@ -568,9 +566,6 @@ static int vfio_pci_open(struct vfio_device *core_vdev)
>  		container_of(core_vdev, struct vfio_pci_device, vdev);
>  	int ret = 0;
>  
> -	if (!try_module_get(THIS_MODULE))
> -		return -ENODEV;
> -
>  	mutex_lock(&vdev->reflck->lock);
>  
>  	if (!vdev->refcnt) {
> @@ -584,8 +579,6 @@ static int vfio_pci_open(struct vfio_device *core_vdev)
>  	vdev->refcnt++;
>  error:
>  	mutex_unlock(&vdev->reflck->lock);
> -	if (ret)
> -		module_put(THIS_MODULE);
>  	return ret;
>  }
>  
> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
> index 470fcf7dac56..703164df7637 100644
> --- a/drivers/vfio/platform/vfio_platform_common.c
> +++ b/drivers/vfio/platform/vfio_platform_common.c
> @@ -241,8 +241,6 @@ static void vfio_platform_release(struct vfio_device *core_vdev)
>  	}
>  
>  	mutex_unlock(&driver_lock);
> -
> -	module_put(vdev->parent_module);
>  }
>  
>  static int vfio_platform_open(struct vfio_device *core_vdev)
> @@ -251,9 +249,6 @@ static int vfio_platform_open(struct vfio_device *core_vdev)
>  		container_of(core_vdev, struct vfio_platform_device, vdev);
>  	int ret;
>  
> -	if (!try_module_get(vdev->parent_module))
> -		return -ENODEV;
> -
>  	mutex_lock(&driver_lock);
>  
>  	if (!vdev->refcnt) {
> @@ -291,7 +286,6 @@ static int vfio_platform_open(struct vfio_device *core_vdev)
>  	vfio_platform_regions_cleanup(vdev);
>  err_reg:
>  	mutex_unlock(&driver_lock);
> -	module_put(vdev->parent_module);
>  	return ret;
>  }
>  
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 5e631c359ef2..02cc51ce6891 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -1369,8 +1369,14 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
>  	if (IS_ERR(device))
>  		return PTR_ERR(device);
>  
> +	if (!try_module_get(device->dev->driver->owner)) {
> +		vfio_device_put(device);
> +		return -ENODEV;
> +	}
> +
>  	ret = device->ops->open(device);
>  	if (ret) {
> +		module_put(device->dev->driver->owner);
>  		vfio_device_put(device);
>  		return ret;
>  	}
> @@ -1382,6 +1388,7 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
>  	ret = get_unused_fd_flags(O_CLOEXEC);
>  	if (ret < 0) {
>  		device->ops->release(device);
> +		module_put(device->dev->driver->owner);
>  		vfio_device_put(device);
>  		return ret;
>  	}
> @@ -1392,6 +1399,7 @@ static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
>  		put_unused_fd(ret);
>  		ret = PTR_ERR(filep);
>  		device->ops->release(device);
> +		module_put(device->dev->driver->owner);
>  		vfio_device_put(device);
>  		return ret;
>  	}
> @@ -1550,6 +1558,8 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
>  
>  	device->ops->release(device);
>  
> +	module_put(device->dev->driver->owner);
> +
>  	vfio_group_try_dissolve_container(device->group);
>  
>  	vfio_device_put(device);
>
diff mbox series

Patch

diff --git a/drivers/vfio/fsl-mc/vfio_fsl_mc.c b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
index 980e59551301..90cad109583b 100644
--- a/drivers/vfio/fsl-mc/vfio_fsl_mc.c
+++ b/drivers/vfio/fsl-mc/vfio_fsl_mc.c
@@ -140,26 +140,18 @@  static int vfio_fsl_mc_open(struct vfio_device *core_vdev)
 {
 	struct vfio_fsl_mc_device *vdev =
 		container_of(core_vdev, struct vfio_fsl_mc_device, vdev);
-	int ret;
-
-	if (!try_module_get(THIS_MODULE))
-		return -ENODEV;
+	int ret = 0;
 
 	mutex_lock(&vdev->reflck->lock);
 	if (!vdev->refcnt) {
 		ret = vfio_fsl_mc_regions_init(vdev);
 		if (ret)
-			goto err_reg_init;
+			goto out;
 	}
 	vdev->refcnt++;
-
+out:
 	mutex_unlock(&vdev->reflck->lock);
 
-	return 0;
-
-err_reg_init:
-	mutex_unlock(&vdev->reflck->lock);
-	module_put(THIS_MODULE);
 	return ret;
 }
 
@@ -196,8 +188,6 @@  static void vfio_fsl_mc_release(struct vfio_device *core_vdev)
 	}
 
 	mutex_unlock(&vdev->reflck->lock);
-
-	module_put(THIS_MODULE);
 }
 
 static long vfio_fsl_mc_ioctl(struct vfio_device *core_vdev,
diff --git a/drivers/vfio/mdev/vfio_mdev.c b/drivers/vfio/mdev/vfio_mdev.c
index 922729071c5a..5ef4815609ed 100644
--- a/drivers/vfio/mdev/vfio_mdev.c
+++ b/drivers/vfio/mdev/vfio_mdev.c
@@ -26,19 +26,10 @@  static int vfio_mdev_open(struct vfio_device *core_vdev)
 	struct mdev_device *mdev = to_mdev_device(core_vdev->dev);
 	struct mdev_parent *parent = mdev->type->parent;
 
-	int ret;
-
 	if (unlikely(!parent->ops->open))
 		return -EINVAL;
 
-	if (!try_module_get(THIS_MODULE))
-		return -ENODEV;
-
-	ret = parent->ops->open(mdev);
-	if (ret)
-		module_put(THIS_MODULE);
-
-	return ret;
+	return parent->ops->open(mdev);
 }
 
 static void vfio_mdev_release(struct vfio_device *core_vdev)
@@ -48,8 +39,6 @@  static void vfio_mdev_release(struct vfio_device *core_vdev)
 
 	if (likely(parent->ops->release))
 		parent->ops->release(mdev);
-
-	module_put(THIS_MODULE);
 }
 
 static long vfio_mdev_unlocked_ioctl(struct vfio_device *core_vdev,
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index bd7c482c948a..f6729baa1bf4 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -558,8 +558,6 @@  static void vfio_pci_release(struct vfio_device *core_vdev)
 	}
 
 	mutex_unlock(&vdev->reflck->lock);
-
-	module_put(THIS_MODULE);
 }
 
 static int vfio_pci_open(struct vfio_device *core_vdev)
@@ -568,9 +566,6 @@  static int vfio_pci_open(struct vfio_device *core_vdev)
 		container_of(core_vdev, struct vfio_pci_device, vdev);
 	int ret = 0;
 
-	if (!try_module_get(THIS_MODULE))
-		return -ENODEV;
-
 	mutex_lock(&vdev->reflck->lock);
 
 	if (!vdev->refcnt) {
@@ -584,8 +579,6 @@  static int vfio_pci_open(struct vfio_device *core_vdev)
 	vdev->refcnt++;
 error:
 	mutex_unlock(&vdev->reflck->lock);
-	if (ret)
-		module_put(THIS_MODULE);
 	return ret;
 }
 
diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c
index 470fcf7dac56..703164df7637 100644
--- a/drivers/vfio/platform/vfio_platform_common.c
+++ b/drivers/vfio/platform/vfio_platform_common.c
@@ -241,8 +241,6 @@  static void vfio_platform_release(struct vfio_device *core_vdev)
 	}
 
 	mutex_unlock(&driver_lock);
-
-	module_put(vdev->parent_module);
 }
 
 static int vfio_platform_open(struct vfio_device *core_vdev)
@@ -251,9 +249,6 @@  static int vfio_platform_open(struct vfio_device *core_vdev)
 		container_of(core_vdev, struct vfio_platform_device, vdev);
 	int ret;
 
-	if (!try_module_get(vdev->parent_module))
-		return -ENODEV;
-
 	mutex_lock(&driver_lock);
 
 	if (!vdev->refcnt) {
@@ -291,7 +286,6 @@  static int vfio_platform_open(struct vfio_device *core_vdev)
 	vfio_platform_regions_cleanup(vdev);
 err_reg:
 	mutex_unlock(&driver_lock);
-	module_put(vdev->parent_module);
 	return ret;
 }
 
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 5e631c359ef2..02cc51ce6891 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1369,8 +1369,14 @@  static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
 	if (IS_ERR(device))
 		return PTR_ERR(device);
 
+	if (!try_module_get(device->dev->driver->owner)) {
+		vfio_device_put(device);
+		return -ENODEV;
+	}
+
 	ret = device->ops->open(device);
 	if (ret) {
+		module_put(device->dev->driver->owner);
 		vfio_device_put(device);
 		return ret;
 	}
@@ -1382,6 +1388,7 @@  static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
 	ret = get_unused_fd_flags(O_CLOEXEC);
 	if (ret < 0) {
 		device->ops->release(device);
+		module_put(device->dev->driver->owner);
 		vfio_device_put(device);
 		return ret;
 	}
@@ -1392,6 +1399,7 @@  static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
 		put_unused_fd(ret);
 		ret = PTR_ERR(filep);
 		device->ops->release(device);
+		module_put(device->dev->driver->owner);
 		vfio_device_put(device);
 		return ret;
 	}
@@ -1550,6 +1558,8 @@  static int vfio_device_fops_release(struct inode *inode, struct file *filep)
 
 	device->ops->release(device);
 
+	module_put(device->dev->driver->owner);
+
 	vfio_group_try_dissolve_container(device->group);
 
 	vfio_device_put(device);