Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
@ 2017-07-09  9:35 Chris Wilson
  2017-07-09  9:53 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-07-10 14:14 ` [PATCH] " Mika Kuoppala
  0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2017-07-09  9:35 UTC (permalink / raw)
  To: intel-gfx

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>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 +-
 drivers/gpu/drm/i915/i915_gem.c         | 32 ++++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_gem_context.c |  6 +++---
 drivers/gpu/drm/i915/i915_gem_context.h |  6 +++---
 drivers/gpu/drm/i915/i915_gem_request.c |  3 +--
 drivers/gpu/drm/i915/i915_gpu_error.c   |  8 ++++----
 6 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81cd21ecfa7d..62208b3bf417 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -596,7 +596,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 */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1564cadda94d..cbe70a94b663 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2739,34 +2739,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 *
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 1a87d04e7937..ed91ac8ca832 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -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:
diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index 04320f80f9f4..2d02918a449e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -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;
diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
index 483af8921060..8ba89abe5441 100644
--- a/drivers/gpu/drm/i915/i915_gem_request.c
+++ b/drivers/gpu/drm/i915/i915_gem_request.c
@@ -370,8 +370,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
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index ae70283470a6..ed5a1eb839ad 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -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,
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
  2017-07-09  9:35 [PATCH] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates Chris Wilson
@ 2017-07-09  9:53 ` Patchwork
  2017-07-10 14:14 ` [PATCH] " Mika Kuoppala
  1 sibling, 0 replies; 4+ messages in thread
From: Patchwork @ 2017-07-09  9:53 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
URL   : https://patchwork.freedesktop.org/series/27040/
State : success

== Summary ==

Series 27040v1 drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
https://patchwork.freedesktop.org/api/1.0/series/27040/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                skip       -> PASS       (fi-skl-x1585l)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-b:
                dmesg-warn -> PASS       (fi-pnv-d510) fdo#101597
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#101597 https://bugs.freedesktop.org/show_bug.cgi?id=101597
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:279  pass:268  dwarn:0   dfail:0   fail:0   skip:11  time:449s
fi-bdw-gvtdvm    total:279  pass:265  dwarn:0   dfail:0   fail:0   skip:14  time:428s
fi-blb-e6850     total:279  pass:224  dwarn:1   dfail:0   fail:0   skip:54  time:360s
fi-bsw-n3050     total:279  pass:243  dwarn:0   dfail:0   fail:0   skip:36  time:528s
fi-bxt-j4205     total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:514s
fi-byt-j1900     total:279  pass:254  dwarn:1   dfail:0   fail:0   skip:24  time:486s
fi-byt-n2820     total:279  pass:250  dwarn:1   dfail:0   fail:0   skip:28  time:484s
fi-glk-2a        total:279  pass:260  dwarn:0   dfail:0   fail:0   skip:19  time:591s
fi-hsw-4770      total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:440s
fi-hsw-4770r     total:279  pass:263  dwarn:0   dfail:0   fail:0   skip:16  time:417s
fi-ilk-650       total:279  pass:229  dwarn:0   dfail:0   fail:0   skip:50  time:421s
fi-ivb-3520m     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:495s
fi-ivb-3770      total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:474s
fi-kbl-7500u     total:279  pass:261  dwarn:0   dfail:0   fail:0   skip:18  time:458s
fi-kbl-7560u     total:279  pass:268  dwarn:1   dfail:0   fail:0   skip:10  time:568s
fi-kbl-r         total:279  pass:260  dwarn:1   dfail:0   fail:0   skip:18  time:580s
fi-pnv-d510      total:279  pass:222  dwarn:2   dfail:0   fail:0   skip:55  time:559s
fi-skl-6260u     total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:458s
fi-skl-6700hq    total:279  pass:262  dwarn:0   dfail:0   fail:0   skip:17  time:589s
fi-skl-6700k     total:279  pass:257  dwarn:4   dfail:0   fail:0   skip:18  time:469s
fi-skl-6770hq    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:479s
fi-skl-gvtdvm    total:279  pass:266  dwarn:0   dfail:0   fail:0   skip:13  time:439s
fi-skl-x1585l    total:279  pass:269  dwarn:0   dfail:0   fail:0   skip:10  time:487s
fi-snb-2520m     total:279  pass:251  dwarn:0   dfail:0   fail:0   skip:28  time:546s
fi-snb-2600      total:279  pass:250  dwarn:0   dfail:0   fail:0   skip:29  time:406s

edc3bde190ad0559ad1c7c63e0efd8d5b4b24b2c drm-tip: 2017y-07m-07d-17h-12m-03s UTC integration manifest
eb585eb drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_5147/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
  2017-07-09  9:35 [PATCH] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates Chris Wilson
  2017-07-09  9:53 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-07-10 14:14 ` Mika Kuoppala
  2017-07-11 11:27   ` Chris Wilson
  1 sibling, 1 reply; 4+ messages in thread
From: Mika Kuoppala @ 2017-07-10 14:14 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> 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>

And we dont ever clear the context bannable flag so
this should be fine.

One improvement would be that only the triggering
ban would spew the message. But now thinking about it,
the dual message would reveal the concurrent ban and
is more informative.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  2 +-
>  drivers/gpu/drm/i915/i915_gem.c         | 32 ++++++++++++++++++--------------
>  drivers/gpu/drm/i915/i915_gem_context.c |  6 +++---
>  drivers/gpu/drm/i915/i915_gem_context.h |  6 +++---
>  drivers/gpu/drm/i915/i915_gem_request.c |  3 +--
>  drivers/gpu/drm/i915/i915_gpu_error.c   |  8 ++++----
>  6 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 81cd21ecfa7d..62208b3bf417 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -596,7 +596,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 */
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1564cadda94d..cbe70a94b663 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2739,34 +2739,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 *
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 1a87d04e7937..ed91ac8ca832 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -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:
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index 04320f80f9f4..2d02918a449e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -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;
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 483af8921060..8ba89abe5441 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -370,8 +370,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
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index ae70283470a6..ed5a1eb839ad 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -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,
> -- 
> 2.13.2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates
  2017-07-10 14:14 ` [PATCH] " Mika Kuoppala
@ 2017-07-11 11:27   ` Chris Wilson
  0 siblings, 0 replies; 4+ messages in thread
From: Chris Wilson @ 2017-07-11 11:27 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2017-07-10 15:14:19)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > 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>
> 
> And we dont ever clear the context bannable flag so
> this should be fine.
> 
> One improvement would be that only the triggering
> ban would spew the message. But now thinking about it,
> the dual message would reveal the concurrent ban and
> is more informative.

It only tells you that we encountered a race condition in handling a
reset on two engines. A double ban message is going to be more confusing
than anything.

Did I send the patch to add the user message for the per-engine reset? I
was having withdrawal symptoms from not seeing it before mesa died.

Otoh, we clearly do propagate the error to userspace, but on the other
hand I'm used to seeing it. It's akin to the kernel reporting when it
sends a terminal signal to a process. I've been trying to find that to
see if they discuss the same issue and in particular why its hidden
behind kconfig and sysctl.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-07-11 11:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-09  9:35 [PATCH] drm/i915: Make i915_gem_context_mark_guilty() safe for unlocked updates Chris Wilson
2017-07-09  9:53 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-07-10 14:14 ` [PATCH] " Mika Kuoppala
2017-07-11 11:27   ` Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox