@@ -267,12 +267,6 @@ void __i915_gem_object_pages_fini(struct drm_i915_gem_object *obj)
if (!list_empty(&obj->vma.list)) {
struct i915_vma *vma;
- /*
- * Note that the vma keeps an object reference while
- * it is active, so it *should* not sleep while we
- * destroy it. Our debug code errs insits it *might*.
- * For the moment, play along.
- */
spin_lock(&obj->vma.lock);
while ((vma = list_first_entry_or_null(&obj->vma.list,
struct i915_vma,
@@ -280,13 +274,7 @@ void __i915_gem_object_pages_fini(struct drm_i915_gem_object *obj)
GEM_BUG_ON(vma->obj != obj);
spin_unlock(&obj->vma.lock);
- /* Verify that the vma is unbound under the vm mutex. */
- mutex_lock(&vma->vm->mutex);
- atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
- __i915_vma_unbind(vma);
- mutex_unlock(&vma->vm->mutex);
-
- __i915_vma_put(vma);
+ i915_vma_destroy(vma);
spin_lock(&obj->vma.lock);
}
@@ -167,7 +167,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
out:
i915_gem_object_lock(obj, NULL);
- __i915_vma_put(vma);
+ i915_vma_destroy(vma);
i915_gem_object_unlock(obj);
return err;
}
@@ -264,7 +264,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
return err;
i915_gem_object_lock(obj, NULL);
- __i915_vma_put(vma);
+ i915_vma_destroy(vma);
i915_gem_object_unlock(obj);
if (igt_timeout(end_time,
@@ -105,14 +105,19 @@ void __i915_vm_close(struct i915_address_space *vm)
list_for_each_entry_safe(vma, vn, &vm->bound_list, vm_link) {
struct drm_i915_gem_object *obj = vma->obj;
- /* Keep the obj (and hence the vma) alive as _we_ destroy it */
- if (!kref_get_unless_zero(&obj->base.refcount))
+ if (!kref_get_unless_zero(&obj->base.refcount)) {
+ /*
+ * Unbind the dying vma to ensure the bound_list
+ * is completely drained. We leave the destruction to
+ * the object destructor.
+ */
+ atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
+ WARN_ON(__i915_vma_unbind(vma));
continue;
+ }
- atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
- WARN_ON(__i915_vma_unbind(vma));
- __i915_vma_put(vma);
-
+ /* Keep the obj (and hence the vma) alive as _we_ destroy it */
+ i915_vma_destroy_locked(vma);
i915_gem_object_put(obj);
}
GEM_BUG_ON(!list_empty(&vm->bound_list));
@@ -1562,15 +1562,27 @@ void i915_vma_reopen(struct i915_vma *vma)
void i915_vma_release(struct kref *ref)
{
struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
+
+ i915_vm_put(vma->vm);
+ i915_active_fini(&vma->active);
+ GEM_WARN_ON(vma->resource);
+ i915_vma_free(vma);
+}
+
+static void force_unbind(struct i915_vma *vma)
+{
+ if (!drm_mm_node_allocated(&vma->node))
+ return;
+
+ atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
+ WARN_ON(__i915_vma_unbind(vma));
+ GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
+}
+
+static void release_references(struct i915_vma *vma)
+{
struct drm_i915_gem_object *obj = vma->obj;
- if (drm_mm_node_allocated(&vma->node)) {
- mutex_lock(&vma->vm->mutex);
- atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
- WARN_ON(__i915_vma_unbind(vma));
- mutex_unlock(&vma->vm->mutex);
- GEM_BUG_ON(drm_mm_node_allocated(&vma->node));
- }
GEM_BUG_ON(i915_vma_is_active(vma));
spin_lock(&obj->vma.lock);
@@ -1580,11 +1592,49 @@ void i915_vma_release(struct kref *ref)
spin_unlock(&obj->vma.lock);
__i915_vma_remove_closed(vma);
- i915_vm_put(vma->vm);
- i915_active_fini(&vma->active);
- GEM_WARN_ON(vma->resource);
- i915_vma_free(vma);
+ __i915_vma_put(vma);
+}
+
+/**
+ * i915_vma_destroy_locked - Remove all weak reference to the vma and put
+ * the initial reference.
+ *
+ * This function should be called when it's decided the vma isn't needed
+ * anymore. The caller must assure that it doesn't race with another lookup
+ * plus destroy, typically by taking an appropriate reference.
+ *
+ * Current callsites are
+ * - __i915_gem_object_pages_fini()
+ * - __i915_vm_close() - Blocks the above function by taking a reference on
+ * the object.
+ * - __i915_vma_parked() - Blocks the above functions by taking an open-count on
+ * the vm and a reference on the object.
+ *
+ * Because of locks taken during destruction, a vma is also guaranteed to
+ * stay alive while the following locks are held if it was looked up while
+ * holding one of the locks:
+ * - vm->mutex
+ * - obj->vma.lock
+ * - gt->closed_lock
+ *
+ * A vma user can also temporarily keep the vma alive while holding a vma
+ * reference.
+ */
+void i915_vma_destroy_locked(struct i915_vma *vma)
+{
+ lockdep_assert_held(&vma->vm->mutex);
+
+ force_unbind(vma);
+ release_references(vma);
+}
+
+void i915_vma_destroy(struct i915_vma *vma)
+{
+ mutex_lock(&vma->vm->mutex);
+ force_unbind(vma);
+ mutex_unlock(&vma->vm->mutex);
+ release_references(vma);
}
void i915_vma_parked(struct intel_gt *gt)
@@ -1618,7 +1668,7 @@ void i915_vma_parked(struct intel_gt *gt)
if (i915_gem_object_trylock(obj, NULL)) {
INIT_LIST_HEAD(&vma->closed_link);
- __i915_vma_put(vma);
+ i915_vma_destroy(vma);
i915_gem_object_unlock(obj);
} else {
/* back you go.. */
@@ -236,6 +236,9 @@ static inline void __i915_vma_put(struct i915_vma *vma)
kref_put(&vma->ref, i915_vma_release);
}
+void i915_vma_destroy_locked(struct i915_vma *vma);
+void i915_vma_destroy(struct i915_vma *vma);
+
#define assert_vma_held(vma) dma_resv_assert_held((vma)->obj->base.resv)
static inline void i915_vma_lock(struct i915_vma *vma)