Re: [PATCH 1/5] vfio: Delete vfio_get/put_group from vfio_iommu_group_notifier()

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Oct 04, 2021 at 10:01:52PM -0600, Alex Williamson wrote:

> I think the commit log argument is that notifies racing the group
> release are harmless so long as the container is unused, and releasing
> a group with active container users would be unbalanced, which
> justifies the WARN_ON added here.  

Yes

I changed it like this:

@@ -327,6 +327,10 @@ static void vfio_group_unlock_and_free(struct vfio_group *group)
        struct vfio_unbound_dev *unbound, *tmp;
 
        mutex_unlock(&vfio.group_lock);
+       /*
+        * Unregister outside of lock.  A spurious callback is harmless now
+        * that the group is no longer in vfio.group_list.
+        */
        iommu_group_unregister_notifier(group->iommu_group, &group->nb);
 
        list_for_each_entry_safe(unbound, tmp,
@@ -413,12 +418,6 @@ static void vfio_group_release(struct kref *kref)
        struct vfio_group *group = container_of(kref, struct vfio_group, kref);
        struct iommu_group *iommu_group = group->iommu_group;
 
-       /*
-        * These data structures all have paired operations that can only be
-        * undone when the caller holds a live reference on the group. Since all
-        * pairs must be undone these WARN_ON's indicate some caller did not
-        * properly hold the group reference.
-        */
        WARN_ON(!list_empty(&group->device_list));
        WARN_ON(atomic_read(&group->container_users));
        WARN_ON(group->notifier.head);

Thanks,
Jason