Quoting Abodunrin, Akeem G (2020-06-16 22:31:45)
>
>
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Chris
> > Wilson
> > Sent: Tuesday, June 16, 2020 12:02 PM
> > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Subject: [PATCH] drm/i915/gt: Decouple completed requests on
> > unwind
> >
> > Since the introduction of preempt-to-busy, requests can complete in the
> > background, even while they are not on the engine->active.requests list.
> > As such, the engine->active.request list itself is not in strict retirement order,
> > and we have to scan the entire list while unwinding to not miss any.
> > However, if the request is completed we currently leave it on the list [until
> > retirement], but we could just as simply remove it and stop treating it as
> > active. We would only have to then traverse it once while unwinding in quick
> > succession.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > index e866b8d721ed..4eb397b0e14d 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > @@ -1114,8 +1114,10 @@ __unwind_incomplete_requests(struct
> > intel_engine_cs *engine)
> > list_for_each_entry_safe_reverse(rq, rn,
> > &engine->active.requests,
> > sched.link) {
> > - if (i915_request_completed(rq))
> > - continue; /* XXX */
> > + if (i915_request_completed(rq)) {
> > + list_del_init(&rq->sched.link);
>
> Albeit this seems like a valid approach to resolve inconsistence in the list of requests that are active or retired, but we can't just delete completed requests from the list until full retirement is done - otherwise we stand the risk of out-of-the-order list, and could lead to inconsistence (which is the original problem you intend to resolve). Have you thought about locking mechanism?
The list is always in execution [context] order. Within a context it is
in retirement order. It is irrelevant whether it is removed here or in
remove_from_engine().
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
