[Intel-xe] [PATCH 3/5] drm/xe: Fix extobj dropping issue.

Maarten Lankhorst maarten.lankhorst at linux.intel.com
Fri May 26 12:10:59 UTC 2023


I believe originally we did the right thing, but now we hit an issue.

Presumably, this was changed when converting from an array to a list
in a squashed commit:
"drm/xe: Use a list for the vm's external objects"

When deleting a vma with an extobj in the list, ensure if it's freed
but still listed on the vm, we take a different vma and add that to
the extobj list instead.

I'll add an igt, and make sure it doesn't happen again.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/194
---
 drivers/gpu/drm/xe/xe_vm.c | 95 +++++++++++++++++++++++---------------
 1 file changed, 58 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index f73e08b60670..dd1335e12d4c 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -909,12 +909,14 @@ static struct xe_vma *xe_vma_create(struct xe_vm *vm,
 	return vma;
 }
 
-static void vm_remove_extobj(struct xe_vma *vma)
+static bool vm_remove_extobj(struct xe_vma *vma)
 {
 	if (!list_empty(&vma->extobj.link)) {
 		vma->vm->extobj.entries--;
 		list_del_init(&vma->extobj.link);
+		return true;
 	}
+	return false;
 }
 
 static void xe_vma_destroy_late(struct xe_vma *vma)
@@ -955,6 +957,51 @@ static void vma_destroy_work_func(struct work_struct *w)
 	xe_vma_destroy_late(vma);
 }
 
+static struct xe_vma *
+bo_has_vm_references_locked(struct xe_bo *bo, struct xe_vm *vm,
+			    struct xe_vma *ignore)
+{
+	struct xe_vma *vma;
+
+	list_for_each_entry(vma, &bo->vmas, bo_link) {
+		if (vma != ignore && vma->vm == vm && !vma->destroyed)
+			return vma;
+	}
+
+	return NULL;
+}
+
+static bool bo_has_vm_references(struct xe_bo *bo, struct xe_vm *vm,
+				 struct xe_vma *ignore)
+{
+	struct ww_acquire_ctx ww;
+	bool ret;
+
+	xe_bo_lock(bo, &ww, 0, false);
+	ret = !!bo_has_vm_references_locked(bo, vm, ignore);
+	xe_bo_unlock(bo, &ww);
+
+	return ret;
+}
+
+static void __vm_insert_extobj(struct xe_vm *vm, struct xe_vma *vma)
+{
+	list_add(&vma->extobj.link, &vm->extobj.list);
+	vm->extobj.entries++;
+}
+
+static void vm_insert_extobj(struct xe_vm *vm, struct xe_vma *vma)
+{
+	struct xe_bo *bo = vma->bo;
+
+	lockdep_assert_held_write(&vm->lock);
+
+	if (bo_has_vm_references(bo, vm, vma))
+		return;
+
+	__vm_insert_extobj(vm, vma);
+}
+
 static void vma_destroy_cb(struct dma_fence *fence,
 			   struct dma_fence_cb *cb)
 {
@@ -980,11 +1027,19 @@ static void xe_vma_destroy(struct xe_vma *vma, struct dma_fence *fence)
 	} else {
 		xe_bo_assert_held(vma->bo);
 		list_del(&vma->bo_link);
+
 		spin_lock(&vm->notifier.list_lock);
 		list_del(&vma->notifier.rebind_link);
 		spin_unlock(&vm->notifier.list_lock);
-		if (!vma->bo->vm)
-			vm_remove_extobj(vma);
+
+		if (!vma->bo->vm && vm_remove_extobj(vma)) {
+			struct xe_vma *other;
+
+			other = bo_has_vm_references_locked(vma->bo, vm, NULL);
+
+			if (other)
+				__vm_insert_extobj(vm, other);
+		}
 	}
 
 	xe_vm_assert_held(vm);
@@ -2481,40 +2536,6 @@ static int vm_bind_ioctl_async(struct xe_vm *vm, struct xe_vma *vma,
 	return err;
 }
 
-static bool bo_has_vm_references(struct xe_bo *bo, struct xe_vm *vm,
-				 struct xe_vma *ignore)
-{
-	struct ww_acquire_ctx ww;
-	struct xe_vma *vma;
-	bool ret = false;
-
-	xe_bo_lock(bo, &ww, 0, false);
-	list_for_each_entry(vma, &bo->vmas, bo_link) {
-		if (vma != ignore && vma->vm == vm && !vma->destroyed) {
-			ret = true;
-			break;
-		}
-	}
-	xe_bo_unlock(bo, &ww);
-
-	return ret;
-}
-
-static int vm_insert_extobj(struct xe_vm *vm, struct xe_vma *vma)
-{
-	struct xe_bo *bo = vma->bo;
-
-	lockdep_assert_held_write(&vm->lock);
-
-	if (bo_has_vm_references(bo, vm, vma))
-		return 0;
-
-	list_add(&vma->extobj.link, &vm->extobj.list);
-	vm->extobj.entries++;
-
-	return 0;
-}
-
 static int __vm_bind_ioctl_lookup_vma(struct xe_vm *vm, struct xe_bo *bo,
 				      u64 addr, u64 range, u32 op)
 {
-- 
2.34.1



More information about the Intel-xe mailing list