> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Saturday, October 2, 2021 7:22 AM
>
> The next patch adds a struct device to the struct vfio_group, and it is
> confusing/bad practice to have two krefs in the same struct. This kref is
> controlling the period when the vfio_group is registered in sysfs, and
> visible in the internal lookup. Switch it to a refcount_t instead.
>
> The refcount_dec_and_mutex_lock() is still required because we need
> atomicity of the list searches and sysfs presence.
>
> Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
> drivers/vfio/vfio.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index bf233943dc992f..dbe7edd88ce35c 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -69,7 +69,7 @@ struct vfio_unbound_dev {
> };
>
> struct vfio_group {
> - struct kref kref;
> + refcount_t users;
> int minor;
> atomic_t container_users;
> struct iommu_group *iommu_group;
> @@ -381,7 +381,7 @@ static struct vfio_group *vfio_create_group(struct
> iommu_group *iommu_group,
> if (!group)
> return ERR_PTR(-ENOMEM);
>
> - kref_init(&group->kref);
> + refcount_set(&group->users, 1);
> INIT_LIST_HEAD(&group->device_list);
> mutex_init(&group->device_lock);
> INIT_LIST_HEAD(&group->unbound_list);
> @@ -441,10 +441,10 @@ static struct vfio_group *vfio_create_group(struct
> iommu_group *iommu_group,
> return group;
> }
>
> -/* called with vfio.group_lock held */
> -static void vfio_group_release(struct kref *kref)
> +static void vfio_group_put(struct vfio_group *group)
> {
> - struct vfio_group *group = container_of(kref, struct vfio_group, kref);
> + if (!refcount_dec_and_mutex_lock(&group->users, &vfio.group_lock))
> + return;
>
> WARN_ON(!list_empty(&group->device_list));
> WARN_ON(atomic_read(&group->container_users));
> @@ -456,15 +456,9 @@ static void vfio_group_release(struct kref *kref)
> vfio_group_unlock_and_free(group);
> }
>
> -static void vfio_group_put(struct vfio_group *group)
> -{
> - kref_put_mutex(&group->kref, vfio_group_release,
> &vfio.group_lock);
> -}
> -
> -/* Assume group_lock or group reference is held */
> static void vfio_group_get(struct vfio_group *group)
> {
> - kref_get(&group->kref);
> + refcount_inc(&group->users);
> }
>
> static struct vfio_group *vfio_group_get_from_minor(int minor)
> @@ -1662,6 +1656,7 @@ struct vfio_group
> *vfio_group_get_external_user(struct file *filep)
> if (ret)
> return ERR_PTR(ret);
>
> + /* Since the caller holds the fget on the file users must be >= 1 */
> vfio_group_get(group);
>
> return group;
> --
> 2.33.0
