RE: [PATCH v2 7/7] vfio: Remove calls to vfio_group_add_container_user()

Tian, Kevin
2022-04-22T02:11:39Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Friday, April 22, 2022 12:29 AM
>
> When the open_device() op is called the container_users is incremented and
> held incremented until close_device(). Thus, so long as drivers call
> functions within their open_device()/close_device() region they do not
> need to worry about the container_users.
>
> These functions can all only be called between open_device() and
> close_device():
>
> vfio_pin_pages()
> vfio_unpin_pages()
> vfio_dma_rw()
> vfio_register_notifier()
> vfio_unregister_notifier()
>
> Eliminate the calls to vfio_group_add_container_user() and add
> vfio_assert_device_open() to detect driver mis-use.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>, with one nit

> @@ -1544,8 +1550,10 @@ 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);
> - if (!--device->open_count && device->ops->close_device)
> + vfio_assert_device_open(device);
> + if (device->open_count == 1 && device->ops->close_device)
> device->ops->close_device(device);
> + device->open_count--;
> mutex_unlock(&device->dev_set->lock);

Is it necessary to add assertion here? This is the only place to
decrement the counter and no similar assertion in other release()/
put() functions.

Thanks
Kevin