[Intel-gfx] [PATCH] drm/i915: Double check vma placement upon gaining the vm lock

[Chris Wilson]

The current unbind + retry of i915_gem_object_ggtt_pin() allows for
someone else to sneak and claim the vma under a different placement
before the first GGTT bind is complete. Leading to confusion and
complaints all over.

Ideally we would pull the evict + rebind under the same lock, but for
now, simply try again.

Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 39 +++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_vma.c |  6 +++++
 2 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 61395b03443e..b0878d35ed1d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -938,33 +938,38 @@ i915_gem_object_ggtt_pin(struct drm_i915_gem_object *obj,
 	if (IS_ERR(vma))
 		return vma;
 
-	if (i915_vma_misplaced(vma, size, alignment, flags)) {
-		if (flags & PIN_NONBLOCK) {
-			if (i915_vma_is_pinned(vma) || i915_vma_is_active(vma))
-				return ERR_PTR(-ENOSPC);
-
-			if (flags & PIN_MAPPABLE &&
-			    vma->fence_size > ggtt->mappable_end / 2)
-				return ERR_PTR(-ENOSPC);
+	do {
+		if (i915_vma_misplaced(vma, size, alignment, flags)) {
+			if (flags & PIN_NONBLOCK) {
+				if (i915_vma_is_pinned(vma) ||
+				    i915_vma_is_active(vma))
+					return ERR_PTR(-ENOSPC);
+
+				if (flags & PIN_MAPPABLE &&
+				    vma->fence_size > ggtt->mappable_end / 2)
+					return ERR_PTR(-ENOSPC);
+			}
+
+			ret = i915_vma_unbind(vma);
+			if (ret)
+				return ERR_PTR(ret);
 		}
 
-		ret = i915_vma_unbind(vma);
-		if (ret)
-			return ERR_PTR(ret);
-	}
+		ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
+	} while (ret == -EAGAIN); /* retry if we lost our placement */
+	if (ret)
+		return ERR_PTR(ret);
 
 	if (vma->fence && !i915_gem_object_is_tiled(obj)) {
 		mutex_lock(&ggtt->vm.mutex);
 		ret = i915_vma_revoke_fence(vma);
 		mutex_unlock(&ggtt->vm.mutex);
-		if (ret)
+		if (ret) {
+			i915_vma_unpin(vma);
 			return ERR_PTR(ret);
+		}
 	}
 
-	ret = i915_vma_pin(vma, size, alignment, flags | PIN_GLOBAL);
-	if (ret)
-		return ERR_PTR(ret);
-
 	return vma;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index e5512f26e20a..f6e661428b02 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -905,6 +905,12 @@ int i915_vma_pin(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
 			__i915_vma_set_map_and_fenceable(vma);
 	}
 
+	/* Somebody else managed to gazump our placement? */
+	if (i915_vma_misplaced(vma, size, alignment, flags)) {
+		err = -EAGAIN;
+		goto err_active;
+	}
+
 	GEM_BUG_ON(!vma->pages);
 	err = i915_vma_bind(vma,
 			    vma->obj ? vma->obj->cache_level : 0,
-- 
2.24.0