vfio: Move vfio_device_assign_container() into vfio_device_first_open()

The only thing this function does is assert the group has an assigned
container and incrs refcounts.

The overall model we have is that once a container_users refcount is
incremented it cannot be de-assigned from the group -
vfio_group_ioctl_unset_container() will fail and the group FD cannot be
closed.

Thus we do not need to check this on every device FD open, just the
first. Reorganize the code so that only the first open and last close
manages the container.

Link: https://lore.kernel.org/r/[email protected]
Reviewed-by: Kevin Tian <[email protected]>
Reviewed-by: Yi Liu <[email protected]>
Reviewed-by: Alex Williamson <[email protected]>
Tested-by: Alex Williamson <[email protected]>
Tested-by: Nicolin Chen <[email protected]>
Tested-by: Yi Liu <[email protected]>
Tested-by: Lixiao Yang <[email protected]>
Tested-by: Matthew Rosato <[email protected]>
Tested-by: Yu He <[email protected]>
Signed-off-by: Jason Gunthorpe <[email protected]>

diff --git a/drivers/vfio/container.c b/drivers/vfio/container.c
index d74164abbf401d42cbcf42b6f5f7282dd7769de7..dd79a66ec62cad579d255c7f5a821f69c82a83e6 100644 (file)
--- a/drivers/vfio/container.c
+++ b/drivers/vfio/container.c
@@ -531,11 +531,11 @@ int vfio_device_assign_container(struct vfio_device *device)
 
 void vfio_device_unassign_container(struct vfio_device *device)
 {
-       mutex_lock(&device->group->group_lock);
+       lockdep_assert_held_write(&device->group->group_lock);
+
        WARN_ON(device->group->container_users <= 1);
        device->group->container_users--;
        fput(device->group->opened_file);
-       mutex_unlock(&device->group->group_lock);
 }
 
 /*

diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 2e8346d13c16cac9ee7eb0f19400ab1496e9fb70..717c7f404feeea62c9e8ec65a4f1fbf850a9ff88 100644 (file)
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -749,18 +749,24 @@ static int vfio_device_first_open(struct vfio_device *device)
         * during close_device.
         */
        mutex_lock(&device->group->group_lock);
+       ret = vfio_device_assign_container(device);
+       if (ret)
+               goto err_module_put;
+
        device->kvm = device->group->kvm;
        if (device->ops->open_device) {
                ret = device->ops->open_device(device);
                if (ret)
-                       goto err_module_put;
+                       goto err_container;
        }
        vfio_device_container_register(device);
        mutex_unlock(&device->group->group_lock);
        return 0;
 
-err_module_put:
+err_container:
        device->kvm = NULL;
+       vfio_device_unassign_container(device);
+err_module_put:
        mutex_unlock(&device->group->group_lock);
        module_put(device->dev->driver->owner);
        return ret;
@@ -775,6 +781,7 @@ static void vfio_device_last_close(struct vfio_device *device)
        if (device->ops->close_device)
                device->ops->close_device(device);
        device->kvm = NULL;
+       vfio_device_unassign_container(device);
        mutex_unlock(&device->group->group_lock);
        module_put(device->dev->driver->owner);
 }
@@ -784,18 +791,12 @@ static struct file *vfio_device_open(struct vfio_device *device)
        struct file *filep;
        int ret;
 
-       mutex_lock(&device->group->group_lock);
-       ret = vfio_device_assign_container(device);
-       mutex_unlock(&device->group->group_lock);
-       if (ret)
-               return ERR_PTR(ret);
-
        mutex_lock(&device->dev_set->lock);
        device->open_count++;
        if (device->open_count == 1) {
                ret = vfio_device_first_open(device);
                if (ret)
-                       goto err_unassign_container;
+                       goto err_unlock;
        }
        mutex_unlock(&device->dev_set->lock);
 
@@ -830,10 +831,9 @@ err_close_device:
        mutex_lock(&device->dev_set->lock);
        if (device->open_count == 1)
                vfio_device_last_close(device);
-err_unassign_container:
+err_unlock:
        device->open_count--;
        mutex_unlock(&device->dev_set->lock);
-       vfio_device_unassign_container(device);
        return ERR_PTR(ret);
 }
 
@@ -1040,8 +1040,6 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
        device->open_count--;
        mutex_unlock(&device->dev_set->lock);
 
-       vfio_device_unassign_container(device);
-
        vfio_device_put_registration(device);
 
        return 0;