[Intel-gfx] [PATCH 1/5] drm/i915: Remove stale asserts from i915_gem_find_active_request()

[Tvrtko Ursulin]

On 29/05/2018 14:29, Chris Wilson wrote:
> Since we use i915_gem_find_active_request() from inside
> intel_engine_dump() and may call that at any time, we do not guarantee
> that the engine is paused nor that the signal kthreads and irq handler
> are suspended, so we cannot assert that the breadcrumb doesn't advance
> and that the irq hasn't happened on another CPU signaling the request we
> believe to be idle.
> 
> Fixes: f636edb214a5 ("drm/i915: Make i915_engine_info pretty printer to standalone")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++---------
>   1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 05f44ca35a06..530d6d0109b4 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2972,23 +2972,22 @@ i915_gem_find_active_request(struct intel_engine_cs *engine)
>   	struct i915_request *request, *active = NULL;
>   	unsigned long flags;
>   
> -	/* We are called by the error capture and reset at a random
> -	 * point in time. In particular, note that neither is crucially
> -	 * ordered with an interrupt. After a hang, the GPU is dead and we
> -	 * assume that no more writes can happen (we waited long enough for
> -	 * all writes that were in transaction to be flushed) - adding an
> +	/*
> +	 * We are called by the error capture, reset and to dump engine
> +	 * state at random points in time. In particular, note that neither is
> +	 * crucially ordered with an interrupt. After a hang, the GPU is dead
> +	 * and we assume that no more writes can happen (we waited long enough
> +	 * for all writes that were in transaction to be flushed) - adding an
>   	 * extra delay for a recent interrupt is pointless. Hence, we do
>   	 * not need an engine->irq_seqno_barrier() before the seqno reads.
> +	 * At all other times, we must assume the GPU is still running, but
> +	 * we only care about the snapshot of this moment.
>   	 */
>   	spin_lock_irqsave(&engine->timeline.lock, flags);
>   	list_for_each_entry(request, &engine->timeline.requests, link) {
>   		if (__i915_request_completed(request, request->global_seqno))
>   			continue;
>   
> -		GEM_BUG_ON(request->engine != engine);

Removal of this one belongs to virtual engine perhaps?

Regards,

Tvrtko

> -		GEM_BUG_ON(test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
> -				    &request->fence.flags));
> -
>   		active = request;
>   		break;
>   	}
>