On 9/8/22 2:44 PM, Jason Gunthorpe wrote:
> The vfio.group_lock is now only used to serialize vfio_group creation
> and destruction, we don't need a micro-optimization of searching,
> unlocking, then allocating and searching again. Just hold the lock
> the whole time.
>
> Rename the function to 'vfio_get_group()' to reflect that it doesn't
> always create something.
> move
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
> ---
> drivers/vfio/vfio_main.c | 48 ++++++++++++++++------------------------
> 1 file changed, 19 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index 77264d836d5200..4ab13808b536e1 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -319,17 +319,6 @@ __vfio_group_get_from_iommu(struct iommu_group *iommu_group)
> return NULL;
> }
>
> -static struct vfio_group *
> -vfio_group_get_from_iommu(struct iommu_group *iommu_group)
> -{
> - struct vfio_group *group;
> -
> - mutex_lock(&vfio.group_lock);
> - group = __vfio_group_get_from_iommu(iommu_group);
> - mutex_unlock(&vfio.group_lock);
> - return group;
> -}
> -
> static void vfio_group_release(struct device *dev)
> {
> struct vfio_group *group = container_of(dev, struct vfio_group, dev);
> @@ -376,16 +365,26 @@ static struct vfio_group *vfio_group_alloc(struct iommu_group *iommu_group,
> return group;
> }
>
> -static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group,
> - enum vfio_group_type type)
> +/*
> + * Return a struct vfio_group * for the given iommu_group. If no vfio_group
> + * already exists then create a new one.
> + */
> +static struct vfio_group *vfio_get_group(struct iommu_group *iommu_group,
> + enum vfio_group_type type)
> {
> struct vfio_group *group;
> struct vfio_group *ret;
> int err;
>
> - group = vfio_group_alloc(iommu_group, type);
> - if (IS_ERR(group))
> - return group;
> + mutex_lock(&vfio.group_lock);
> +
> + ret = __vfio_group_get_from_iommu(iommu_group);
> + if (ret)
> + goto err_unlock;
> +
> + group = ret = vfio_group_alloc(iommu_group, type);
> + if (IS_ERR(ret))
> + goto err_unlock;
>
> err = dev_set_name(&group->dev, "%s%d",
> group->type == VFIO_NO_IOMMU ? "noiommu-" : "",
> @@ -395,13 +394,6 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group,
> goto err_put;
> }
>
> - mutex_lock(&vfio.group_lock);
> -
> - /* Did we race creating this group? */
> - ret = __vfio_group_get_from_iommu(iommu_group);
> - if (ret)
> - goto err_unlock;
> -
> err = cdev_device_add(&group->cdev, &group->dev);
> if (err) {
> ret = ERR_PTR(err);
> @@ -413,10 +405,10 @@ static struct vfio_group *vfio_create_group(struct iommu_group *iommu_group,
> mutex_unlock(&vfio.group_lock);
> return group;
>
> -err_unlock:
> - mutex_unlock(&vfio.group_lock);
> err_put:
> put_device(&group->dev);
> +err_unlock:
> + mutex_unlock(&vfio.group_lock);
> return ret;
> }
>
> @@ -514,7 +506,7 @@ static struct vfio_group *vfio_noiommu_group_alloc(struct device *dev,
> if (ret)
> goto out_put_group;
>
> - group = vfio_create_group(iommu_group, type);
> + group = vfio_get_group(iommu_group, type);
> if (IS_ERR(group)) {
> ret = PTR_ERR(group);
> goto out_remove_device;
> @@ -564,9 +556,7 @@ static struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
> return ERR_PTR(-EINVAL);
> }
>
> - group = vfio_group_get_from_iommu(iommu_group);
> - if (!group)
> - group = vfio_create_group(iommu_group, VFIO_IOMMU);
> + group = vfio_get_group(iommu_group, VFIO_IOMMU);
>
> /* The vfio_group holds a reference to the iommu_group */
> iommu_group_put(iommu_group);
