diff mbox series

[RFC,v2,06/11] vfio: Move device open/close code to be helpfers

Message ID 20221124122702.26507-7-yi.l.liu@intel.com (mailing list archive)
State New, archived
Headers show
Series Move group specific code into group.c | expand

Commit Message

Yi Liu Nov. 24, 2022, 12:26 p.m. UTC
This makes the device open and close be in paired helpers.

vfio_device_open(), and vfio_device_close() handles the open_count, and
calls vfio_device_first_open(), and vfio_device_last_close() when
open_count condition is met. This also helps to avoid open code for device
in the vfio_group_ioctl_get_device_fd(), and prepares for further moving
vfio group specific code into separate file.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/vfio/vfio_main.c | 46 +++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 17 deletions(-)

Comments

Tian, Kevin Nov. 28, 2022, 8:21 a.m. UTC | #1
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Thursday, November 24, 2022 8:27 PM
> 
> This makes the device open and close be in paired helpers.
> 
> vfio_device_open(), and vfio_device_close() handles the open_count, and
> calls vfio_device_first_open(), and vfio_device_last_close() when
> open_count condition is met. This also helps to avoid open code for device
> in the vfio_group_ioctl_get_device_fd(), and prepares for further moving

I didn't get which 'open code' is referred to here:

> @@ -918,7 +935,7 @@ static int vfio_group_ioctl_get_device_fd(struct
> vfio_group *group,
>  		goto err_put_device;
>  	}
> 
> -	filep = vfio_device_open(device);
> +	filep = vfio_device_open_file(device);

it's simply a replacement of function calls.
Yi Liu Nov. 28, 2022, 9:27 a.m. UTC | #2
On 2022/11/28 16:21, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Thursday, November 24, 2022 8:27 PM
>>
>> This makes the device open and close be in paired helpers.
>>
>> vfio_device_open(), and vfio_device_close() handles the open_count, and
>> calls vfio_device_first_open(), and vfio_device_last_close() when
>> open_count condition is met. This also helps to avoid open code for device
>> in the vfio_group_ioctl_get_device_fd(), and prepares for further moving
> 
> I didn't get which 'open code' is referred to here:

it's the device->open_count things. but you are right, it's not in the
vfio_group_ioctl_get_device_fd().

>> @@ -918,7 +935,7 @@ static int vfio_group_ioctl_get_device_fd(struct
>> vfio_group *group,
>>   		goto err_put_device;
>>   	}
>>
>> -	filep = vfio_device_open(device);
>> +	filep = vfio_device_open_file(device);
> 
> it's simply a replacement of function calls.

so more accurate description is splitting the vfio_device_open() into
common vfio_device_open() which is paired with vfio_device_close(), and
another wrapper to deal with device open and device file open, which is
group path specific.
diff mbox series

Patch

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index fcb9f778fc9b..c3d58cbc2aa9 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -846,20 +846,41 @@  static void vfio_device_last_close(struct vfio_device *device)
 	module_put(device->dev->driver->owner);
 }
 
-static struct file *vfio_device_open(struct vfio_device *device)
+static int vfio_device_open(struct vfio_device *device)
 {
-	struct file *filep;
-	int ret;
+	int ret = 0;
 
 	mutex_lock(&device->dev_set->lock);
 	device->open_count++;
 	if (device->open_count == 1) {
 		ret = vfio_device_first_open(device);
 		if (ret)
-			goto err_unlock;
+			device->open_count--;
 	}
 	mutex_unlock(&device->dev_set->lock);
 
+	return ret;
+}
+
+static void vfio_device_close(struct vfio_device *device)
+{
+	mutex_lock(&device->dev_set->lock);
+	vfio_assert_device_open(device);
+	if (device->open_count == 1)
+		vfio_device_last_close(device);
+	device->open_count--;
+	mutex_unlock(&device->dev_set->lock);
+}
+
+static struct file *vfio_device_open_file(struct vfio_device *device)
+{
+	struct file *filep;
+	int ret;
+
+	ret = vfio_device_open(device);
+	if (ret)
+		goto err_out;
+
 	/*
 	 * We can't use anon_inode_getfd() because we need to modify
 	 * the f_mode flags directly to allow more than just ioctls
@@ -885,12 +906,8 @@  static struct file *vfio_device_open(struct vfio_device *device)
 	return filep;
 
 err_close_device:
-	mutex_lock(&device->dev_set->lock);
-	if (device->open_count == 1)
-		vfio_device_last_close(device);
-err_unlock:
-	device->open_count--;
-	mutex_unlock(&device->dev_set->lock);
+	vfio_device_close(device);
+err_out:
 	return ERR_PTR(ret);
 }
 
@@ -918,7 +935,7 @@  static int vfio_group_ioctl_get_device_fd(struct vfio_group *group,
 		goto err_put_device;
 	}
 
-	filep = vfio_device_open(device);
+	filep = vfio_device_open_file(device);
 	if (IS_ERR(filep)) {
 		ret = PTR_ERR(filep);
 		goto err_put_fdno;
@@ -1105,12 +1122,7 @@  static int vfio_device_fops_release(struct inode *inode, struct file *filep)
 {
 	struct vfio_device *device = filep->private_data;
 
-	mutex_lock(&device->dev_set->lock);
-	vfio_assert_device_open(device);
-	if (device->open_count == 1)
-		vfio_device_last_close(device);
-	device->open_count--;
-	mutex_unlock(&device->dev_set->lock);
+	vfio_device_close(device);
 
 	vfio_device_put_registration(device);