All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: Michel@gaia.fi.intel.com
Subject: Re: [PATCH 2/7] drm/i915/selftests: Use a sacrificial context for hang testing
Date: Mon, 05 Feb 2018 16:02:55 +0200	[thread overview]
Message-ID: <87607bv8j4.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20180205092201.19476-2-chris@chris-wilson.co.uk>

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

> Avoid injecting hangs in to the i915->kernel_context in case the GPU
> reset leaves corruption in the context image in its wake (leading to
> continual failures and system hangs after the selftests are ostensibly
> complete). Use a sacrificial kernel_context instead.
>
> v2: Closing a context is tricky; export a function (for selftests) from
> i915_gem_context.c to get it right.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com
> ---
>  drivers/gpu/drm/i915/selftests/intel_hangcheck.c | 39 +++++++++++++-----------
>  drivers/gpu/drm/i915/selftests/mock_context.c    | 11 +++++++
>  drivers/gpu/drm/i915/selftests/mock_context.h    |  3 ++
>  3 files changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> index a4f4ff22389b..e0b662a2b8ae 100644
> --- a/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> +++ b/drivers/gpu/drm/i915/selftests/intel_hangcheck.c
> @@ -33,6 +33,7 @@ struct hang {
>  	struct drm_i915_private *i915;
>  	struct drm_i915_gem_object *hws;
>  	struct drm_i915_gem_object *obj;
> +	struct i915_gem_context *ctx;
>  	u32 *seqno;
>  	u32 *batch;
>  };
> @@ -45,9 +46,15 @@ static int hang_init(struct hang *h, struct drm_i915_private *i915)
>  	memset(h, 0, sizeof(*h));
>  	h->i915 = i915;
>  
> +	h->ctx = kernel_context(i915);
> +	if (IS_ERR(h->ctx))
> +		return PTR_ERR(h->ctx);
> +
>  	h->hws = i915_gem_object_create_internal(i915, PAGE_SIZE);
> -	if (IS_ERR(h->hws))
> -		return PTR_ERR(h->hws);
> +	if (IS_ERR(h->hws)) {
> +		err = PTR_ERR(h->hws);
> +		goto err_ctx;
> +	}
>  
>  	h->obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
>  	if (IS_ERR(h->obj)) {
> @@ -79,6 +86,8 @@ static int hang_init(struct hang *h, struct drm_i915_private *i915)
>  	i915_gem_object_put(h->obj);
>  err_hws:
>  	i915_gem_object_put(h->hws);
> +err_ctx:
> +	kernel_context_close(h->ctx);
>  	return err;
>  }
>  
> @@ -196,9 +205,7 @@ static int emit_recurse_batch(struct hang *h,
>  }
>  
>  static struct drm_i915_gem_request *
> -hang_create_request(struct hang *h,
> -		    struct intel_engine_cs *engine,
> -		    struct i915_gem_context *ctx)
> +hang_create_request(struct hang *h, struct intel_engine_cs *engine)
>  {
>  	struct drm_i915_gem_request *rq;
>  	int err;
> @@ -225,7 +232,7 @@ hang_create_request(struct hang *h,
>  		h->batch = vaddr;
>  	}
>  
> -	rq = i915_gem_request_alloc(engine, ctx);
> +	rq = i915_gem_request_alloc(engine, h->ctx);
>  	if (IS_ERR(rq))
>  		return rq;
>  
> @@ -306,6 +313,8 @@ static void hang_fini(struct hang *h)
>  	i915_gem_object_unpin_map(h->hws);
>  	i915_gem_object_put(h->hws);
>  
> +	kernel_context_close(h->ctx);
> +
>  	flush_test(h->i915, I915_WAIT_LOCKED);
>  }
>  
> @@ -341,7 +350,7 @@ static int igt_hang_sanitycheck(void *arg)
>  		if (!intel_engine_can_store_dword(engine))
>  			continue;
>  
> -		rq = hang_create_request(&h, engine, i915->kernel_context);
> +		rq = hang_create_request(&h, engine);
>  		if (IS_ERR(rq)) {
>  			err = PTR_ERR(rq);
>  			pr_err("Failed to create request for %s, err=%d\n",
> @@ -478,8 +487,7 @@ static int __igt_reset_engine(struct drm_i915_private *i915, bool active)
>  				struct drm_i915_gem_request *rq;
>  
>  				mutex_lock(&i915->drm.struct_mutex);
> -				rq = hang_create_request(&h, engine,
> -							 i915->kernel_context);
> +				rq = hang_create_request(&h, engine);
>  				if (IS_ERR(rq)) {
>  					err = PTR_ERR(rq);
>  					mutex_unlock(&i915->drm.struct_mutex);
> @@ -686,8 +694,7 @@ static int __igt_reset_engine_others(struct drm_i915_private *i915,
>  				struct drm_i915_gem_request *rq;
>  
>  				mutex_lock(&i915->drm.struct_mutex);
> -				rq = hang_create_request(&h, engine,
> -							 i915->kernel_context);
> +				rq = hang_create_request(&h, engine);
>  				if (IS_ERR(rq)) {
>  					err = PTR_ERR(rq);
>  					mutex_unlock(&i915->drm.struct_mutex);
> @@ -842,7 +849,7 @@ static int igt_wait_reset(void *arg)
>  	if (err)
>  		goto unlock;
>  
> -	rq = hang_create_request(&h, i915->engine[RCS], i915->kernel_context);
> +	rq = hang_create_request(&h, i915->engine[RCS]);
>  	if (IS_ERR(rq)) {
>  		err = PTR_ERR(rq);
>  		goto fini;
> @@ -921,7 +928,7 @@ static int igt_reset_queue(void *arg)
>  		if (!intel_engine_can_store_dword(engine))
>  			continue;
>  
> -		prev = hang_create_request(&h, engine, i915->kernel_context);
> +		prev = hang_create_request(&h, engine);
>  		if (IS_ERR(prev)) {
>  			err = PTR_ERR(prev);
>  			goto fini;
> @@ -935,9 +942,7 @@ static int igt_reset_queue(void *arg)
>  			struct drm_i915_gem_request *rq;
>  			unsigned int reset_count;
>  
> -			rq = hang_create_request(&h,
> -						 engine,
> -						 i915->kernel_context);
> +			rq = hang_create_request(&h, engine);
>  			if (IS_ERR(rq)) {
>  				err = PTR_ERR(rq);
>  				goto fini;
> @@ -1048,7 +1053,7 @@ static int igt_handle_error(void *arg)
>  	if (err)
>  		goto err_unlock;
>  
> -	rq = hang_create_request(&h, engine, i915->kernel_context);
> +	rq = hang_create_request(&h, engine);
>  	if (IS_ERR(rq)) {
>  		err = PTR_ERR(rq);
>  		goto err_fini;
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index bbf80d42e793..501becc47c0c 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -92,3 +92,14 @@ live_context(struct drm_i915_private *i915, struct drm_file *file)
>  
>  	return i915_gem_create_context(i915, file->driver_priv);
>  }
> +
> +struct i915_gem_context *
> +kernel_context(struct drm_i915_private *i915)

kernel_context_create would be more symmetric.

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

> +{
> +	return i915_gem_context_create_kernel(i915, I915_PRIORITY_NORMAL);
> +}
> +
> +void kernel_context_close(struct i915_gem_context *ctx)
> +{
> +	context_close(ctx);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.h b/drivers/gpu/drm/i915/selftests/mock_context.h
> index 2f432c03d413..29b9d60a158b 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.h
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.h
> @@ -36,4 +36,7 @@ void mock_context_close(struct i915_gem_context *ctx);
>  struct i915_gem_context *
>  live_context(struct drm_i915_private *i915, struct drm_file *file);
>  
> +struct i915_gem_context *kernel_context(struct drm_i915_private *i915);
> +void kernel_context_close(struct i915_gem_context *ctx);
> +
>  #endif /* !__MOCK_CONTEXT_H */
> -- 
> 2.15.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-02-05 14:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-05  9:21 [PATCH 1/7] drm/i915/selftests: Flush old resets between engines Chris Wilson
2018-02-05  9:21 ` [PATCH 2/7] drm/i915/selftests: Use a sacrificial context for hang testing Chris Wilson
2018-02-05 14:02   ` Mika Kuoppala [this message]
2018-02-05 14:06     ` Chris Wilson
2018-02-05  9:21 ` [PATCH 3/7] drm/i915/execlists: Move the reset bits to a more natural home Chris Wilson
2018-02-05 15:19   ` Mika Kuoppala
2018-02-05  9:21 ` [PATCH 4/7] drm/i915: Skip post-reset request emission if the engine is not idle Chris Wilson
2018-02-05  9:21 ` [PATCH 5/7] drm/i915: Show the GPU state when declaring wedged Chris Wilson
2018-02-05  9:51   ` Mika Kuoppala
2018-02-05 10:02     ` Chris Wilson
2018-02-05  9:22 ` [PATCH 6/7] drm/i915/execlists: Remove the startup spam Chris Wilson
2018-02-05  9:22 ` [PATCH 7/7] drm/i915: Remove unbannable context spam from reset Chris Wilson
2018-02-05  9:30   ` Chris Wilson
2018-02-05 13:27     ` Chris Wilson
2018-02-05  9:58 ` ✗ Fi.CI.BAT: warning for series starting with [1/7] drm/i915/selftests: Flush old resets between engines Patchwork
2018-02-05 14:27 ` [PATCH 1/7] " Mika Kuoppala

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=87607bv8j4.fsf@gaia.fi.intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=Michel@gaia.fi.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.