Skip to contentSkip to conversation

drm/i915: Cope with request list state change during error state capture

PATCH Superseded Tomas Elf 2 participants 2 messages Oct 19, 2015

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Tomas Elf

author
#0 lore raw

PATCH v2 3/8 drm/i915: Cope with request list state change during error state capture

Since we're not synchronizing the ring request list during error state capture the request list state might change between the time the corresponding error request list was allocated and dimensioned to the time when the ring request list is actually captured into the error state. If this happens then do an early exit and be aware that the captured error state might not be fully reliable.

* v2:
- Chris Wilson: Removed WARN_ON from size check since having the error state request list and the live driver request list diverge like this is a legitimate behaviour.

- Tomas Elf: Removed update of num_request field since this made no sense. Just exit and move on.

---
 drivers/gpu/drm/i915/i915_gpu_error.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/gpu/drm/i915/i915_gpu_error.c +12 -0
index 2f04e4f..b08a76b 100644
@@ -1071,6 +1071,18 @@ static void i915_gem_record_rings(struct drm_device *dev,
 		list_for_each_entry(request, &ring->request_list, list) {
 			struct drm_i915_error_request *erq;
 
+			if (count >= error->ring[i].num_requests) {
+				/*
+				 * If the ring request list was changed in
+				 * between the point where the error request
+				 * list was created and dimensioned and this
+				 * point then just exit early to avoid crashes.
+				 */
+				DRM_ERROR("Request list changed size since allocation (%u->%u)\n",
+					error->ring[i].num_requests, count);
+				break;
+			}
+
 			erq = &error->ring[i].requests[count++];
 			erq->seqno = request->seqno;
 			erq->jiffies = request->emitted_jiffies;
Signature · 6 lines
1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#1 lore raw
Quoted 27 lines · Tomas Elf

Since we're not synchronizing the ring request list during error state capture the request list state might change between the time the corresponding error request list was allocated and dimensioned to the time when the ring request list is actually captured into the error state. If this happens then do an early exit and be aware that the captured error state might not be fully reliable.

* v2:
- Chris Wilson: Removed WARN_ON from size check since having the error state request list and the live driver request list diverge like this is a legitimate behaviour.

- Tomas Elf: Removed update of num_request field since this made no sense. Just exit and move on.

---
 drivers/gpu/drm/i915/i915_gpu_error.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/gpu/drm/i915/i915_gpu_error.c +0 -0
index 2f04e4f..b08a76b 100644
@@ -1071,6 +1071,18 @@ static void i915_gem_record_rings(struct drm_device *dev,
 		list_for_each_entry(request, &ring->request_list, list) {
 			struct drm_i915_error_request *erq;

drivers/gpu/drm/i915/i915_gpu_error.c static void i915_gem_record_rings(struct drm_device *dev, · line 1082

Quoted 10 lines
drivers/gpu/drm/i915/i915_gpu_error.c +9 -0
 
+			if (count >= error->ring[i].num_requests) {
+				/*
+				 * If the ring request list was changed in
+				 * between the point where the error request
+				 * list was created and dimensioned and this
+				 * point then just exit early to avoid crashes.
+				 */
+				DRM_ERROR("Request list changed size since allocation (%u->%u)\n",
+					error->ring[i].num_requests, count);

DRM_ERROR still results in reports about dmesg noise, I think DRM_DEBUG_DRV is the suitable one here.
-Daniel

Quoted 13 lines
drivers/gpu/drm/i915/i915_gpu_error.c +3 -0
+				break;
+			}
+
 			erq = &error->ring[i].requests[count++];
 			erq->seqno = request->seqno;
 			erq->jiffies = request->emitted_jiffies;
Signature · 6 lines
1.9.1

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx