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

> Since commit 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy"), we
> prune the engine->active.requests list prior to preemption, thus
> removing the trace of the currently executing request. If that request
> hangs rather than be preempted, we conclude that no active request was
> on the GPU. Fortunately, this only impacts our debugging, and not our
> means of hang detection or recovery.
>
> References: 22b7a426bbe1 ("drm/i915/execlists: Preempt-to-busy")
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuopp...@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c | 29 ++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c 
> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 06ff7695fa29..93878fd42a7a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1643,7 +1643,7 @@ static bool match_ring(struct i915_request *rq)
>  struct i915_request *
>  intel_engine_find_active_request(struct intel_engine_cs *engine)
>  {
> -     struct i915_request *request, *active = NULL;
> +     struct i915_request *rq, *active = NULL;
>  
>       /*
>        * We are called by the error capture, reset and to dump engine
> @@ -1657,18 +1657,35 @@ intel_engine_find_active_request(struct 
> intel_engine_cs *engine)
>        * we only care about the snapshot of this moment.
>        */
>       lockdep_assert_held(&engine->active.lock);
> -     list_for_each_entry(request, &engine->active.requests, sched.link) {
> -             if (i915_request_completed(request))
> +
> +     rcu_read_lock();
> +     rq = execlists_active(&engine->execlists);
> +     if (rq) {
> +             struct intel_timeline *tl = rq->context->timeline;
> +
> +             list_for_each_entry_continue_reverse(rq, &tl->requests, link) {
> +                     if (i915_request_completed(rq))
> +                             break;

This will be active - 1. So it makes me think that
we should first check that the real active has not completed.

-Mika

> +
> +                     active = rq;
> +             }
> +     }
> +     rcu_read_unlock();
> +     if (active)
> +             return active;
> +
> +     list_for_each_entry(rq, &engine->active.requests, sched.link) {
> +             if (i915_request_completed(rq))
>                       continue;
>  
> -             if (!i915_request_started(request))
> +             if (!i915_request_started(rq))
>                       continue;
>  
>               /* More than one preemptible request may match! */
> -             if (!match_ring(request))
> +             if (!match_ring(rq))
>                       continue;
>  
> -             active = request;
> +             active = rq;
>               break;
>       }
>  
> -- 
> 2.25.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to