| Message ID | 20221201145535.589687-7-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Move group specific code into group.c | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Thursday, December 1, 2022 10:56 PM subject: 'helpfers' -> 'helpers' > > This makes vfio_device_open/close() to be the top level helpers for device > open and close. It handles the open_count, and vfio_device_first_open() > vfio_device_last_close() do the container/iommufd use/unuse and the > device > open. > > Current vfio_device_open() handles the device open and the device file > open which is group specific. After this change, the group specific code > is in the vfio_device_open_file() which would be moved to separate group > source code in future. It's clearer as below: -- vfio: Make vfio_device_open() truly device specific Then move group related logic into vfio_device_open_file(). Accordingly introduce a vfio_device_close() to pair up. -- With above: Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On 2022/12/2 13:44, Tian, Kevin wrote: >> From: Liu, Yi L <yi.l.liu@intel.com> >> Sent: Thursday, December 1, 2022 10:56 PM > > subject: 'helpfers' -> 'helpers' > >> >> This makes vfio_device_open/close() to be the top level helpers for device >> open and close. It handles the open_count, and vfio_device_first_open() >> vfio_device_last_close() do the container/iommufd use/unuse and the >> device >> open. >> >> Current vfio_device_open() handles the device open and the device file >> open which is group specific. After this change, the group specific code >> is in the vfio_device_open_file() which would be moved to separate group >> source code in future. > > It's clearer as below: > -- > vfio: Make vfio_device_open() truly device specific > > Then move group related logic into vfio_device_open_file(). Accordingly > introduce a vfio_device_close() to pair up. > -- > > With above: > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> thanks, I'll update your comments into the github branch.
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 4f2d32d4a3d0..964d44a35624 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -828,20 +828,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 @@ -870,12 +891,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); } @@ -903,7 +920,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; @@ -1086,12 +1103,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);
This makes vfio_device_open/close() to be the top level helpers for device open and close. It handles the open_count, and vfio_device_first_open() vfio_device_last_close() do the container/iommufd use/unuse and the device open. Current vfio_device_open() handles the device open and the device file open which is group specific. After this change, the group specific code is in the vfio_device_open_file() which would be moved to separate group source code in future. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/vfio/vfio_main.c | 46 +++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 17 deletions(-)