[Intel-gfx] [PATCH] drm/i915/gt: Decouple completed requests on unwind

[Abodunrin, Akeem G]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Chris
> Wilson
> Sent: Tuesday, June 16, 2020 12:02 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Subject: [Intel-gfx] [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 at chris-wilson.co.uk>
> ---
>  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?

Regards,
~Akeem
> +			continue;
> +		}
> 
>  		__i915_request_unsubmit(rq);
> 
> --
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx