[Intel-gfx] [PATCH 2/2] drm/i915: Don't check for idleness before retiring after a GPU hang

[Daniel Vetter]

On Mon, Aug 08, 2016 at 08:28:48AM +0100, Chris Wilson wrote:
> When we force the cleanup after a GPU hang, we want to retire all
> requests, or else we may leak them if truly wedged (and the GPU never
> advances again). Converting to the active request helpers had the issue
> of doing the check against busyness before reporting the request, so if
> we claim the GPU had hung but this engine hadn't we could potential skip
> the request cleanup - triggering the self-check BUG.
> 
> Fixes: dcff85c8443e ("drm/i915: Enable i915_gem_wait_for_idle() ...")

Was confused for a while why this is a problem du to lockless
wait_for_idle until I realized that that patch also converted the engine
reset cleanup over to the new active helpers. 

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index f4f8eaa90f2a..cd5229301aae 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2423,15 +2423,11 @@ static void i915_gem_reset_engine_cleanup(struct intel_engine_cs *engine)
>  	struct drm_i915_gem_request *request;
>  	struct intel_ring *ring;
>  
> -	request = i915_gem_active_peek(&engine->last_request,
> -				       &engine->i915->drm.struct_mutex);
> -
>  	/* Mark all pending requests as complete so that any concurrent
>  	 * (lockless) lookup doesn't try and wait upon the request as we
>  	 * reset it.
>  	 */
> -	if (request)
> -		intel_engine_init_seqno(engine, request->fence.seqno);
> +	intel_engine_init_seqno(engine, engine->last_submitted_seqno);
>  
>  	/*
>  	 * Clear the execlists queue up before freeing the requests, as those
> @@ -2453,6 +2449,8 @@ static void i915_gem_reset_engine_cleanup(struct intel_engine_cs *engine)
>  	 * implicit references on things like e.g. ppgtt address spaces through
>  	 * the request.
>  	 */
> +	request = i915_gem_active_raw(&engine->last_request,
> +				      &engine->i915->drm.struct_mutex);
>  	if (request)
>  		i915_gem_request_retire_upto(request);
>  	GEM_BUG_ON(intel_engine_is_active(engine));
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch