From: Ben Widawsky <ben@bwidawsk.net>
To: intel-gfx@lists.freedesktop.org
Cc: Ben Widawsky <ben@bwidawsk.net>
Subject: [PATCH 03/15] drm/i915: relative_constants_mode race fix
Date: Fri, 18 Nov 2011 18:24:20 -0800 [thread overview]
Message-ID: <1321669472-8045-4-git-send-email-ben@bwidawsk.net> (raw)
In-Reply-To: <1321669472-8045-1-git-send-email-ben@bwidawsk.net>
After my refactoring, Chris noticed that we had a bug.
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.
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 67 ++++++++++++++--------------
1 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 3693e83..1d66c24 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1003,39 +1003,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
return -EINVAL;
}
- mode = args->flags & I915_EXEC_CONSTANTS_MASK;
- switch (mode) {
- case I915_EXEC_CONSTANTS_REL_GENERAL:
- case I915_EXEC_CONSTANTS_ABSOLUTE:
- case I915_EXEC_CONSTANTS_REL_SURFACE:
- if (ring == &dev_priv->ring[RCS] &&
- mode != dev_priv->relative_constants_mode) {
- if (INTEL_INFO(dev)->gen < 4)
- return -EINVAL;
-
- 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:
- DRM_ERROR("execbuf with unknown constants: %d\n", mode);
- return -EINVAL;
- }
-
if (args->buffer_count < 1) {
DRM_ERROR("execbuf with %d buffers\n", args->buffer_count);
return -EINVAL;
@@ -1159,6 +1126,40 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
}
}
+ mode = args->flags & I915_EXEC_CONSTANTS_MASK;
+ switch (mode) {
+ case I915_EXEC_CONSTANTS_REL_GENERAL:
+ case I915_EXEC_CONSTANTS_ABSOLUTE:
+ case I915_EXEC_CONSTANTS_REL_SURFACE:
+ if (ring == &dev_priv->ring[RCS] &&
+ mode != dev_priv->relative_constants_mode) {
+ if (INTEL_INFO(dev)->gen < 4)
+ return -EINVAL;
+
+ if (INTEL_INFO(dev)->gen > 5 &&
+ mode == I915_EXEC_CONSTANTS_REL_SURFACE)
+ return -EINVAL;
+
+ 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;
+ }
+ break;
+ default:
+ DRM_ERROR("execbuf with unknown constants: %d\n", mode);
+ ret = -EINVAL;
+ goto err;
+ }
+
trace_i915_gem_ring_dispatch(ring, seqno);
exec_start = batch_obj->gtt_offset + args->batch_start_offset;
--
1.7.7.3
next prev parent reply other threads:[~2011-11-19 2:24 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-19 2:24 [PATCH 00/15] [RFC] Forced throttling/scheduling Ben Widawsky
2011-11-19 2:24 ` [PATCH 01/15] drm/i915: refactor debugfs open function Ben Widawsky
2011-11-20 18:50 ` Kenneth Graunke
2011-11-19 2:24 ` [PATCH 02/15] drm/i915: refactor debugfs create functions Ben Widawsky
2011-11-19 2:24 ` Ben Widawsky [this message]
2011-11-23 22:30 ` [PATCH 03/15] drm/i915: relative_constants_mode race fix Ben Widawsky
2011-11-19 2:24 ` [PATCH 04/15] drm/i915: drop lock support for i915_wait_request Ben Widawsky
2011-11-19 2:24 ` [PATCH 05/15] drm/i915: remove mm structure from file_priv Ben Widawsky
2011-11-19 2:24 ` [PATCH 06/15] drm/i915: Keep track of drm_file in file_priv Ben Widawsky
2011-11-19 2:24 ` [PATCH 07/15] drm/i915: Keep track of request counts Ben Widawsky
2011-11-19 2:24 ` [PATCH 08/15] drm/i915: fairness Ben Widawsky
2011-11-19 2:24 ` [PATCH 09/15] drm/i915: Keep track of open i915 clients Ben Widawsky
2011-11-19 2:24 ` [PATCH 10/15] drm/i915: debugfs entry for " Ben Widawsky
2011-11-19 2:24 ` [PATCH 11/15] drm/i915: debugfs entries for scheduler params Ben Widawsky
2011-11-19 2:24 ` [PATCH 12/15] drm/i915: infrastructure to support scheduler types Ben Widawsky
2011-11-19 2:24 ` [PATCH 13/15] drm/i915: get/set scheduler type from debugfs Ben Widawsky
2011-11-19 2:24 ` [PATCH 14/15] drm/i915: Implement batch scheduler Ben Widawsky
2011-11-19 2:24 ` [PATCH 15/15] drm/i915: Add handling for batch parameters in debugfs Ben Widawsky
2011-11-29 14:30 ` [PATCH 00/15] [RFC] Forced throttling/scheduling Eugeni Dodonov
2012-04-02 18:28 ` Ben Widawsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1321669472-8045-4-git-send-email-ben@bwidawsk.net \
--to=ben@bwidawsk.net \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).