I think Chris was pointing out the following, hopefully I caught his
meaning right.
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 more importantly we know we'll have struct_mutex
for the remainder of the code path.
Cc: Chris Wilson <ch...@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 20587a3..d419c30 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1068,10 +1068,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
if (eb == NULL)
return -ENOMEM;
- eb->mode = args->flags & I915_EXEC_CONSTANTS_MASK;
- ret = i915_gem_set_constant_offset(ring, eb->mode);
- if (ret)
- return ret;
if (args->buffer_count < 1) {
DRM_ERROR("execbuf with %d buffers\n", args->buffer_count);
@@ -1187,6 +1183,12 @@ i915_gem_do_execbuffer(struct drm_device *dev, void
*data,
exec_start = eb->batch_obj->gtt_offset + args->batch_start_offset;
exec_len = args->batch_len;
+
+ eb->mode = args->flags & I915_EXEC_CONSTANTS_MASK;
+ ret = i915_gem_set_constant_offset(ring, eb->mode);
+ if (ret)
+ goto err;
+
if (cliprects) {
for (i = 0; i < args->num_cliprects; i++) {
ret = i915_emit_box(dev, &cliprects[i],
--
1.7.7
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
