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
