Re: [PATCH v2 3/5] vfio: Don't leak a group reference if the group already exists

[Jason Gunthorpe <jgg@xxxxxxxxxx>]

On Wed, Oct 13, 2021 at 06:09:10PM +0200, Christoph Hellwig wrote:
> > @@ -775,12 +776,7 @@ static struct vfio_group *vfio_group_find_or_alloc(struct device *dev)
> >  	if (group)
> >  		goto out_put;
> >  
> > -	/* a newly created vfio_group keeps the reference. */
> >  	group = vfio_create_group(iommu_group, VFIO_IOMMU);
> > -	if (IS_ERR(group))
> > -		goto out_put;
> > -	return group;
> > -
> >  out_put:
> >  	iommu_group_put(iommu_group);
> >  	return group;
> 
> I'd simplify this down to:
> 
> 	group = vfio_group_get_from_iommu(iommu_group);
> 	if (!group)
> 		group = vfio_create_group(iommu_group, VFIO_IOMMU);

Yes, OK,  I changed it into this:

	group = vfio_group_get_from_iommu(iommu_group);
	if (!group)
		group = vfio_create_group(iommu_group, VFIO_IOMMU);

	/* The vfio_group holds a reference to the iommu_group */
	iommu_group_put(iommu_group);
	return group;
}

Which I think is clearer on the comment too

Thanks,
Jason