drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 21 Jul 2017 12:32:30 +0000 (13:32 +0100)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Thu, 27 Jul 2017 07:38:47 +0000 (09:38 +0200)
Since we make call i915_gem_context_mark_guilty() concurrently when
resetting different engines in parallel, we need to make sure that our
updates are safe for the unlocked access.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170721123238.16428-12-chris@chris-wilson.co.uk
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/i915_gem_context.c
drivers/gpu/drm/i915/i915_gem_context.h
drivers/gpu/drm/i915/i915_gem_request.c
drivers/gpu/drm/i915/i915_gpu_error.c

index ef089fe0353ef75e83d5608b7940c1ac591569d8..b051122c960bf9ff882f611df8b70bdca95a8adc 100644 (file)
@@ -602,7 +602,7 @@ struct drm_i915_file_private {
  * to limit the badly behaving clients access to gpu.
  */
 #define I915_MAX_CLIENT_CONTEXT_BANS 3
-       int context_bans;
+       atomic_t context_bans;
 };
 
 /* Used by dp and fdi links */
index fcf7fee7ca7192c6b257ede6a49064c3af113dfc..a9ea83ea321b30c13a9a6f3b3b8e0c0ca046d229 100644 (file)
@@ -2740,34 +2740,38 @@ i915_gem_object_pwrite_gtt(struct drm_i915_gem_object *obj,
        return 0;
 }
 
-static bool ban_context(const struct i915_gem_context *ctx)
+static bool ban_context(const struct i915_gem_context *ctx,
+                       unsigned int score)
 {
        return (i915_gem_context_is_bannable(ctx) &&
-               ctx->ban_score >= CONTEXT_SCORE_BAN_THRESHOLD);
+               score >= CONTEXT_SCORE_BAN_THRESHOLD);
 }
 
 static void i915_gem_context_mark_guilty(struct i915_gem_context *ctx)
 {
-       ctx->guilty_count++;
-       ctx->ban_score += CONTEXT_SCORE_GUILTY;
-       if (ban_context(ctx))
-               i915_gem_context_set_banned(ctx);
+       unsigned int score;
+       bool banned;
 
-       DRM_DEBUG_DRIVER("context %s marked guilty (score %d) banned? %s\n",
-                        ctx->name, ctx->ban_score,
-                        yesno(i915_gem_context_is_banned(ctx)));
+       atomic_inc(&ctx->guilty_count);
 
-       if (!i915_gem_context_is_banned(ctx) || IS_ERR_OR_NULL(ctx->file_priv))
+       score = atomic_add_return(CONTEXT_SCORE_GUILTY, &ctx->ban_score);
+       banned = ban_context(ctx, score);
+       DRM_DEBUG_DRIVER("context %s marked guilty (score %d) banned? %s\n",
+                        ctx->name, score, yesno(banned));
+       if (!banned)
                return;
 
-       ctx->file_priv->context_bans++;
-       DRM_DEBUG_DRIVER("client %s has had %d context banned\n",
-                        ctx->name, ctx->file_priv->context_bans);
+       i915_gem_context_set_banned(ctx);
+       if (!IS_ERR_OR_NULL(ctx->file_priv)) {
+               atomic_inc(&ctx->file_priv->context_bans);
+               DRM_DEBUG_DRIVER("client %s has had %d context banned\n",
+                                ctx->name, atomic_read(&ctx->file_priv->context_bans));
+       }
 }
 
 static void i915_gem_context_mark_innocent(struct i915_gem_context *ctx)
 {
-       ctx->active_count++;
+       atomic_inc(&ctx->active_count);
 }
 
 struct drm_i915_gem_request *
index 1a87d04e7937c126c5f1883b948941521fe1194a..ed91ac8ca832c3f030b4280755f10efdf62e0650 100644 (file)
@@ -977,7 +977,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv)
 
 static bool client_is_banned(struct drm_i915_file_private *file_priv)
 {
-       return file_priv->context_bans > I915_MAX_CLIENT_CONTEXT_BANS;
+       return atomic_read(&file_priv->context_bans) > I915_MAX_CLIENT_CONTEXT_BANS;
 }
 
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
@@ -1179,8 +1179,8 @@ int i915_gem_context_reset_stats_ioctl(struct drm_device *dev,
        else
                args->reset_count = 0;
 
-       args->batch_active = READ_ONCE(ctx->guilty_count);
-       args->batch_pending = READ_ONCE(ctx->active_count);
+       args->batch_active = atomic_read(&ctx->guilty_count);
+       args->batch_pending = atomic_read(&ctx->active_count);
 
        ret = 0;
 out:
index 04320f80f9f4d487fde343b0659c1000be13a9a1..2d02918a449ec11c215393e8c4ce3a179060c583 100644 (file)
@@ -191,17 +191,17 @@ struct i915_gem_context {
        u32 desc_template;
 
        /** guilty_count: How many times this context has caused a GPU hang. */
-       unsigned int guilty_count;
+       atomic_t guilty_count;
        /**
         * @active_count: How many times this context was active during a GPU
         * hang, but did not cause it.
         */
-       unsigned int active_count;
+       atomic_t active_count;
 
 #define CONTEXT_SCORE_GUILTY           10
 #define CONTEXT_SCORE_BAN_THRESHOLD    40
        /** ban_score: Accumulated score of all hangs caused by this context. */
-       int ban_score;
+       atomic_t ban_score;
 
        /** remap_slice: Bitmask of cache lines that need remapping */
        u8 remap_slice;
index d93a185c0f0a855fd9b2c1b4e3338e1ceb16398f..68e406a53c0497238860c720d1bb4651994c9c7e 100644 (file)
@@ -374,8 +374,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
        i915_gem_request_remove_from_client(request);
 
        /* Retirement decays the ban score as it is a sign of ctx progress */
-       if (request->ctx->ban_score > 0)
-               request->ctx->ban_score--;
+       atomic_dec_if_positive(&request->ctx->ban_score);
 
        /* The backing object for the context is done after switching to the
         * *next* context. Therefore we cannot retire the previous context until
index ae70283470a6c7b9ee60c22f7e652d9abfe697fb..ed5a1eb839ad57951bf56b3ae0e886cbfbf88428 100644 (file)
@@ -1266,7 +1266,7 @@ static void record_request(struct drm_i915_gem_request *request,
                           struct drm_i915_error_request *erq)
 {
        erq->context = request->ctx->hw_id;
-       erq->ban_score = request->ctx->ban_score;
+       erq->ban_score = atomic_read(&request->ctx->ban_score);
        erq->seqno = request->global_seqno;
        erq->jiffies = request->emitted_jiffies;
        erq->head = request->head;
@@ -1357,9 +1357,9 @@ static void record_context(struct drm_i915_error_context *e,
 
        e->handle = ctx->user_handle;
        e->hw_id = ctx->hw_id;
-       e->ban_score = ctx->ban_score;
-       e->guilty = ctx->guilty_count;
-       e->active = ctx->active_count;
+       e->ban_score = atomic_read(&ctx->ban_score);
+       e->guilty = atomic_read(&ctx->guilty_count);
+       e->active = atomic_read(&ctx->active_count);
 }
 
 static void request_record_user_bo(struct drm_i915_gem_request *request,