[RFC 10/10] drm/i915/vm_bind: Fix vm->vm_bind_mutex and vm->mutex nesting

From: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
To: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Cc: matthew.brost@intel.com, paulo.r.zanoni@intel.com,
	tvrtko.ursulin@intel.com, lionel.g.landwerlin@intel.com,
	thomas.hellstrom@intel.com, matthew.auld@intel.com,
	jason@jlekstrand.net, daniel.vetter@intel.com,
	christian.koenig@amd.com
Subject: [RFC 10/10] drm/i915/vm_bind: Fix vm->vm_bind_mutex and vm->mutex nesting
Date: Fri,  1 Jul 2022 15:50:55 -0700	[thread overview]
Message-ID: <20220701225055.8204-11-niranjana.vishwanathapura@intel.com> (raw)
In-Reply-To: <20220701225055.8204-1-niranjana.vishwanathapura@intel.com>

VM_BIND functionality maintain that vm->vm_bind_mutex will never be taken
while holding vm->mutex.
However, while closing 'vm', vma is destroyed while holding vm->mutex.
But vma releasing needs to take vm->vm_bind_mutex in order to delete vma
from the vm_bind_list. To avoid this, destroy the vma outside vm->mutex
while closing the 'vm'.

Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gtt.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 4ab3bda644ff..4f707d0eb3ef 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -109,7 +109,8 @@ int map_pt_dma_locked(struct i915_address_space *vm, struct drm_i915_gem_object
 	return 0;
 }
 
-static void clear_vm_list(struct list_head *list)
+static void clear_vm_list(struct list_head *list,
+			  struct list_head *destroy_list)
 {
 	struct i915_vma *vma, *vn;
 
@@ -138,8 +139,7 @@ static void clear_vm_list(struct list_head *list)
 			i915_vm_resv_get(vma->vm);
 			vma->vm_ddestroy = true;
 		} else {
-			i915_vma_destroy_locked(vma);
-			i915_gem_object_put(obj);
+			list_move_tail(&vma->vm_link, destroy_list);
 		}
 
 	}
@@ -147,16 +147,29 @@ static void clear_vm_list(struct list_head *list)
 
 static void __i915_vm_close(struct i915_address_space *vm)
 {
+	struct i915_vma *vma, *vn;
+	struct list_head list;
+
+	INIT_LIST_HEAD(&list);
+
 	mutex_lock(&vm->mutex);
 
-	clear_vm_list(&vm->bound_list);
-	clear_vm_list(&vm->unbound_list);
+	clear_vm_list(&vm->bound_list, &list);
+	clear_vm_list(&vm->unbound_list, &list);
 
 	/* Check for must-fix unanticipated side-effects */
 	GEM_BUG_ON(!list_empty(&vm->bound_list));
 	GEM_BUG_ON(!list_empty(&vm->unbound_list));
 
 	mutex_unlock(&vm->mutex);
+
+	/* Destroy vmas outside vm->mutex */
+	list_for_each_entry_safe(vma, vn, &list, vm_link) {
+		struct drm_i915_gem_object *obj = vma->obj;
+
+		i915_vma_destroy(vma);
+		i915_gem_object_put(obj);
+	}
 }
 
 /* lock the vm into the current ww, if we lock one, we lock all */
-- 
2.21.0.rc0.32.g243a4c7e27

