[Intel-gfx] [PATCH] drm/i915: Nuke struct_mutex from context_setparam
Chris Wilson
chris at chris-wilson.co.uk
Tue Sep 11 13:22:06 UTC 2018
Userspace should be free to race against itself and shoot itself in
the foot if it so desires to adjust a parameter at the same time as
submitting a batch to that context. As such, the struct_mutex in context
setparam is only being used to serialise userspace against itself and
not for any protection of internal structs and so is superfluous.
v2: Separate user_flags from internal flags to reduce chance of
interference; and use locked bit ops for user updates.
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
---
drivers/gpu/drm/i915/i915_gem_context.c | 20 +++++--------
drivers/gpu/drm/i915/i915_gem_context.h | 33 +++++++++++++---------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +-
3 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 747b8170a15a..f772593b99ab 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -862,7 +862,7 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
ret = -EINVAL;
break;
case I915_CONTEXT_PARAM_NO_ZEROMAP:
- args->value = ctx->flags & CONTEXT_NO_ZEROMAP;
+ args->value = test_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags);
break;
case I915_CONTEXT_PARAM_GTT_SIZE:
if (ctx->ppgtt)
@@ -896,27 +896,23 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
struct drm_i915_file_private *file_priv = file->driver_priv;
struct drm_i915_gem_context_param *args = data;
struct i915_gem_context *ctx;
- int ret;
+ int ret = 0;
ctx = i915_gem_context_lookup(file_priv, args->ctx_id);
if (!ctx)
return -ENOENT;
- ret = i915_mutex_lock_interruptible(dev);
- if (ret)
- goto out;
-
switch (args->param) {
case I915_CONTEXT_PARAM_BAN_PERIOD:
ret = -EINVAL;
break;
case I915_CONTEXT_PARAM_NO_ZEROMAP:
- if (args->size) {
+ if (args->size)
ret = -EINVAL;
- } else {
- ctx->flags &= ~CONTEXT_NO_ZEROMAP;
- ctx->flags |= args->value ? CONTEXT_NO_ZEROMAP : 0;
- }
+ else if (args->value)
+ set_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags);
+ else
+ clear_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags);
break;
case I915_CONTEXT_PARAM_NO_ERROR_CAPTURE:
if (args->size)
@@ -960,9 +956,7 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
ret = -EINVAL;
break;
}
- mutex_unlock(&dev->struct_mutex);
-out:
i915_gem_context_put(ctx);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index e09673ca731d..08165f6a0a84 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -116,16 +116,21 @@ struct i915_gem_context {
*/
struct rcu_head rcu;
+ /**
+ * @user_flags: small set of booleans controlled by the user
+ */
+ unsigned long user_flags;
+#define UCONTEXT_NO_ZEROMAP 0
+#define UCONTEXT_NO_ERROR_CAPTURE 1
+#define UCONTEXT_BANNABLE 2
+
/**
* @flags: small set of booleans
*/
unsigned long flags;
-#define CONTEXT_NO_ZEROMAP BIT(0)
-#define CONTEXT_NO_ERROR_CAPTURE 1
-#define CONTEXT_CLOSED 2
-#define CONTEXT_BANNABLE 3
-#define CONTEXT_BANNED 4
-#define CONTEXT_FORCE_SINGLE_SUBMISSION 5
+#define CONTEXT_BANNED 0
+#define CONTEXT_CLOSED 1
+#define CONTEXT_FORCE_SINGLE_SUBMISSION 2
/**
* @hw_id: - unique identifier for the context
@@ -209,37 +214,37 @@ static inline bool i915_gem_context_is_closed(const struct i915_gem_context *ctx
static inline void i915_gem_context_set_closed(struct i915_gem_context *ctx)
{
GEM_BUG_ON(i915_gem_context_is_closed(ctx));
- __set_bit(CONTEXT_CLOSED, &ctx->flags);
+ set_bit(CONTEXT_CLOSED, &ctx->flags);
}
static inline bool i915_gem_context_no_error_capture(const struct i915_gem_context *ctx)
{
- return test_bit(CONTEXT_NO_ERROR_CAPTURE, &ctx->flags);
+ return test_bit(UCONTEXT_NO_ERROR_CAPTURE, &ctx->user_flags);
}
static inline void i915_gem_context_set_no_error_capture(struct i915_gem_context *ctx)
{
- __set_bit(CONTEXT_NO_ERROR_CAPTURE, &ctx->flags);
+ set_bit(UCONTEXT_NO_ERROR_CAPTURE, &ctx->user_flags);
}
static inline void i915_gem_context_clear_no_error_capture(struct i915_gem_context *ctx)
{
- __clear_bit(CONTEXT_NO_ERROR_CAPTURE, &ctx->flags);
+ clear_bit(UCONTEXT_NO_ERROR_CAPTURE, &ctx->user_flags);
}
static inline bool i915_gem_context_is_bannable(const struct i915_gem_context *ctx)
{
- return test_bit(CONTEXT_BANNABLE, &ctx->flags);
+ return test_bit(UCONTEXT_BANNABLE, &ctx->user_flags);
}
static inline void i915_gem_context_set_bannable(struct i915_gem_context *ctx)
{
- __set_bit(CONTEXT_BANNABLE, &ctx->flags);
+ set_bit(UCONTEXT_BANNABLE, &ctx->user_flags);
}
static inline void i915_gem_context_clear_bannable(struct i915_gem_context *ctx)
{
- __clear_bit(CONTEXT_BANNABLE, &ctx->flags);
+ clear_bit(UCONTEXT_BANNABLE, &ctx->user_flags);
}
static inline bool i915_gem_context_is_banned(const struct i915_gem_context *ctx)
@@ -249,7 +254,7 @@ static inline bool i915_gem_context_is_banned(const struct i915_gem_context *ctx
static inline void i915_gem_context_set_banned(struct i915_gem_context *ctx)
{
- __set_bit(CONTEXT_BANNED, &ctx->flags);
+ set_bit(CONTEXT_BANNED, &ctx->flags);
}
static inline bool i915_gem_context_force_single_submission(const struct i915_gem_context *ctx)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 22b4cb775576..aaa8a26aae86 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -743,7 +743,7 @@ static int eb_select_context(struct i915_execbuffer *eb)
}
eb->context_flags = 0;
- if (ctx->flags & CONTEXT_NO_ZEROMAP)
+ if (test_bit(UCONTEXT_NO_ZEROMAP, &ctx->user_flags))
eb->context_flags |= __EXEC_OBJECT_NEEDS_BIAS;
return 0;
--
2.19.0.rc2
More information about the Intel-gfx
mailing list