> From: Jason Gunthorpe <jgg@xxxxxxxxxx> > Sent: Thursday, October 14, 2021 12:19 AM > > 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 > with above: Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
