Thread (193 messages) 193 messages, 8 authors, 2012-05-12

[ 108/167] [PATCH] drm/i915: relative_constants_mode race fix

From: Ben Hutchings <hidden>
Date: 2012-05-09 06:10:14
Also in: lkml
Subsystem: drm drivers, intel drm i915 driver (meteor lake, dg2 and older excluding poulsbo, moorestown and derivative), the rest · Maintainers: David Airlie, Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Linus Torvalds

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Ben Widawsky <redacted>

commit e2971bdab2b761683353da383c0fd5ac704d1cca upstream.

dev_priv keeps track of the current addressing mode that gets set at
execbuffer time. Unfortunately the existing code was doing this before
acquiring struct_mutex which leaves a race with another thread also
doing an execbuffer. If that wasn't bad enough, relocate_slow drops
struct_mutex which opens a much more likely error where another thread
comes in and modifies the state while relocate_slow is being slow.

The solution here is to just defer setting this state until we
absolutely need it, and we know we'll have struct_mutex for the
remainder of our code path.

v2: Keith noticed a bug in the original patch.

Signed-off-by: Ben Widawsky <redacted>
Reviewed-by: Daniel Vetter <redacted>
Signed-off-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Ben Hutchings <redacted>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   29 +++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index c681dc1..68e5b41 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1033,19 +1033,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 			if (INTEL_INFO(dev)->gen > 5 &&
 			    mode == I915_EXEC_CONSTANTS_REL_SURFACE)
 				return -EINVAL;
-
-			ret = intel_ring_begin(ring, 4);
-			if (ret)
-				return ret;
-
-			intel_ring_emit(ring, MI_NOOP);
-			intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
-			intel_ring_emit(ring, INSTPM);
-			intel_ring_emit(ring,
-					I915_EXEC_CONSTANTS_MASK << 16 | mode);
-			intel_ring_advance(ring);
-
-			dev_priv->relative_constants_mode = mode;
 		}
 		break;
 	default:
@@ -1176,6 +1163,22 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		}
 	}
 
+	if (ring == &dev_priv->ring[RCS] &&
+	    mode != dev_priv->relative_constants_mode) {
+		ret = intel_ring_begin(ring, 4);
+		if (ret)
+				goto err;
+
+		intel_ring_emit(ring, MI_NOOP);
+		intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
+		intel_ring_emit(ring, INSTPM);
+		intel_ring_emit(ring,
+				I915_EXEC_CONSTANTS_MASK << 16 | mode);
+		intel_ring_advance(ring);
+
+		dev_priv->relative_constants_mode = mode;
+	}
+
 	trace_i915_gem_ring_dispatch(ring, seqno);
 
 	exec_start = batch_obj->gtt_offset + args->batch_start_offset;
-- 
1.7.10


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help