diff mbox

drm/i915: clarify concurrent hang detect/gpu reset consistency

Message ID 1354807417-16318-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter Dec. 6, 2012, 3:23 p.m. UTC
Damien Lespiau wondered how race the gpu reset/hang detection code is
against concurrent gpu resets/hang detections or combinations thereof.
Luckily the single work item is guranteed to never run concurrently,
so reset handling is already single-threaded.

Hence we only have to worry about concurrent hang detections, or a
hang detection firing off while we're still processing an older gpu
reset request. Due to the new mechanism of setting the reset in
progress flag and the ordering guaranteed by the schedule_work
function there's nothing to do but add a comment explaining why we're
safe.

The only thing I've noticed is that we still try to reset the gpu now,
even when it is declared terminally wedged. Add a check for that to
avoid continous warnings about failed resets, in case the hangcheck
timer ever gets stuck.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_irq.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index f7f9c21..09c33ac 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -851,9 +851,20 @@  static void i915_error_work_func(struct work_struct *work)
 
 	kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, error_event);
 
-	if (i915_reset_in_progress(error)) {
+	/*
+	 * Note that there's only one work item which does gpu resets, so we
+	 * need not worry about concurrent gpu resets potentially incrementing
+	 * error->reset_counter twice. We only need to take care of another
+	 * racing irq/hangcheck declaring the gpu dead for a second time. A
+	 * quick check for that is good enough: schedule_work ensures the
+	 * correct ordering between hang detection and this work item, and since
+	 * the reset in-progress bit is only ever set by code outside of this
+	 * work we don't need to worry about any other races.
+	 */
+	if (i915_reset_in_progress(error) && !i915_terminally_wedged(error)) {
 		DRM_DEBUG_DRIVER("resetting chip\n");
-		kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE, reset_event);
+		kobject_uevent_env(&dev->primary->kdev.kobj, KOBJ_CHANGE,
+				   reset_event);
 
 		ret = i915_reset(dev);