[Intel-gfx] [PATCH] drm/i915/gt: Report the currently active execlists request

[Chris Wilson]

Quoting Mika Kuoppala (2020-01-17 11:23:01)
> Chris Wilson <chris at 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 <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at 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.

Hmm, you want from_reverse in this case.
-Chris