[Intel-gfx] [PATCH 1/2] drm/i915: Clean up pinned bo capture

[Daniel Vetter]

Somewhere between removing the pinned bo list and adding full ppgtt
support we've started to no longer filter the active buffers out of
the captured pinned buffers. I've tried to dig out exactly where but
didn't spot it.

At first I've thought (checked, but without warning about it) array
overrun. But we have the same issue when counting the pinned bo.

Still not pretty, so switch over to walking the inactive per-vm list,
which will naturally filter out all active buffers.

v2: While at it only capture pinned bo for the global gtt, we don't
care about them for real ppgtt (since only execbuf pins those, and
only temporarily).

v3: Don't WARN_ON about overruns, this code is still racy. So just
debug output instead.

Cc: Chris Wilson <chris at chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 43 ++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index c4536e185b75..b76443778e8e 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -696,35 +696,33 @@ static u32 capture_active_bo(struct drm_i915_error_buffer *err,
 
 	list_for_each_entry(vma, head, mm_list) {
 		capture_bo(err++, vma);
-		if (++i == count)
+		if (++i == count) {
+			DRM_DEBUG("Capture array overrun\n");
 			break;
+		}
 	}
 
 	return i;
 }
 
 static u32 capture_pinned_bo(struct drm_i915_error_buffer *err,
-			     int count, struct list_head *head,
-			     struct i915_address_space *vm)
+			     int count, struct list_head *head)
 {
-	struct drm_i915_gem_object *obj;
-	struct drm_i915_error_buffer * const first = err;
-	struct drm_i915_error_buffer * const last = err + count;
-
-	list_for_each_entry(obj, head, global_list) {
-		struct i915_vma *vma;
+	struct i915_vma *vma;
+	int i = 0;
 
-		if (err == last)
+	list_for_each_entry(vma, head, mm_list) {
+		if (vma->pin_count == 0)
 			break;
 
-		list_for_each_entry(vma, &obj->vma_list, vma_link)
-			if (vma->vm == vm && vma->pin_count > 0) {
-				capture_bo(err++, vma);
-				break;
-			}
+		capture_bo(err++, vma);
+		if (++i == count) {
+			DRM_DEBUG("Capture array overrun\n");
+			break;
+		}
 	}
 
-	return err - first;
+	return i;
 }
 
 /* Generate a semi-unique error code. The code is not meant to have meaning, The
@@ -1085,7 +1083,6 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
 				const int ndx)
 {
 	struct drm_i915_error_buffer *active_bo = NULL, *pinned_bo = NULL;
-	struct drm_i915_gem_object *obj;
 	struct i915_vma *vma;
 	int i;
 
@@ -1094,12 +1091,12 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
 		i++;
 	error->active_bo_count[ndx] = i;
 
-	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		list_for_each_entry(vma, &obj->vma_list, vma_link)
-			if (vma->vm == vm && vma->pin_count > 0) {
-				i++;
+	if (i915_is_ggtt(vm)) {
+		list_for_each_entry(vma, &vm->inactive_list, mm_list) {
+			if (vma->pin_count == 0)
 				break;
-			}
+			i++;
+		}
 	}
 	error->pinned_bo_count[ndx] = i - error->active_bo_count[ndx];
 
@@ -1119,7 +1116,7 @@ static void i915_gem_capture_vm(struct drm_i915_private *dev_priv,
 		error->pinned_bo_count[ndx] =
 			capture_pinned_bo(pinned_bo,
 					  error->pinned_bo_count[ndx],
-					  &dev_priv->mm.bound_list, vm);
+					  &vm->inactive_list);
 	error->active_bo[ndx] = active_bo;
 	error->pinned_bo[ndx] = pinned_bo;
 }
-- 
1.9.3