vfio: Change struct vfio_group::opened from an atomic to bool

This is not a performance path, just use the group_rwsem to protect the
value.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Tested-by: Matthew Rosato <mjrosato@linux.ibm.com>
Link: https://lore.kernel.org/r/2-v2-d035a1842d81+1bf-vfio_group_locking_jgg@nvidia.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index 4261eeec9e73c66f5f70e76786af75b01e87bbc2..12d4b3efd4639ec6f1dba511ef563f8ee62ba7ad 100644 (file)
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -73,7 +73,7 @@ struct vfio_group {
        struct mutex                    device_lock;
        struct list_head                vfio_next;
        struct list_head                container_next;
-       atomic_t                        opened;
+       bool                            opened;
        enum vfio_group_type            type;
        unsigned int                    dev_counter;
        struct rw_semaphore             group_rwsem;
@@ -1213,30 +1213,30 @@ static int vfio_group_fops_open(struct inode *inode, struct file *filep)
 {
        struct vfio_group *group =
                container_of(inode->i_cdev, struct vfio_group, cdev);
-       int opened;
+       int ret;
 
-       /* users can be zero if this races with vfio_group_put() */
-       if (!refcount_inc_not_zero(&group->users))
-               return -ENODEV;
+       down_write(&group->group_rwsem);
 
-       if (group->type == VFIO_NO_IOMMU && !capable(CAP_SYS_RAWIO)) {
-               vfio_group_put(group);
-               return -EPERM;
+       /* users can be zero if this races with vfio_group_put() */
+       if (!refcount_inc_not_zero(&group->users)) {
+               ret = -ENODEV;
+               goto err_unlock;
        }
 
-       /* Do we need multiple instances of the group open?  Seems not. */
-       opened = atomic_cmpxchg(&group->opened, 0, 1);
-       if (opened) {
-               vfio_group_put(group);
-               return -EBUSY;
+       if (group->type == VFIO_NO_IOMMU && !capable(CAP_SYS_RAWIO)) {
+               ret = -EPERM;
+               goto err_put;
        }
 
-       /* Is something still in use from a previous open? */
-       if (group->container) {
-               atomic_dec(&group->opened);
-               vfio_group_put(group);
-               return -EBUSY;
+       /*
+        * Do we need multiple instances of the group open?  Seems not.
+        * Is something still in use from a previous open?
+        */
+       if (group->opened || group->container) {
+               ret = -EBUSY;
+               goto err_put;
        }
+       group->opened = true;
 
        /* Warn if previous user didn't cleanup and re-init to drop them */
        if (WARN_ON(group->notifier.head))
@@ -1244,7 +1244,13 @@ static int vfio_group_fops_open(struct inode *inode, struct file *filep)
 
        filep->private_data = group;
 
+       up_write(&group->group_rwsem);
        return 0;
+err_put:
+       vfio_group_put(group);
+err_unlock:
+       up_write(&group->group_rwsem);
+       return ret;
 }
 
 static int vfio_group_fops_release(struct inode *inode, struct file *filep)
@@ -1255,7 +1261,9 @@ static int vfio_group_fops_release(struct inode *inode, struct file *filep)
 
        vfio_group_try_dissolve_container(group);
 
-       atomic_dec(&group->opened);
+       down_write(&group->group_rwsem);
+       group->opened = false;
+       up_write(&group->group_rwsem);
 
        vfio_group_put(group);