[Intel-gfx] [PATCH 03/27] drm/i915/guc: Unwind context requests in reverse order

[Daniele Ceraolo Spurio]

On 8/18/2021 11:16 PM, Matthew Brost wrote:
> When unwinding requests on a reset context, if other requests in the
> context are in the priority list the requests could be resubmitted out
> of seqno order. Traverse the list of active requests in reverse and
> append to the head of the priority list to fix this.
>
> Fixes: eb5e7da736f3 ("drm/i915/guc: Reset implementation for new GuC interface")
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> Cc: <stable at vger.kernel.org>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 32c414aa9009..9ca0ba4ea85a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -805,9 +805,9 @@ __unwind_incomplete_requests(struct intel_context *ce)
>   
>   	spin_lock_irqsave(&sched_engine->lock, flags);
>   	spin_lock(&ce->guc_active.lock);
> -	list_for_each_entry_safe(rq, rn,
> -				 &ce->guc_active.requests,
> -				 sched.link) {
> +	list_for_each_entry_safe_reverse(rq, rn,
> +					 &ce->guc_active.requests,
> +					 sched.link) {
>   		if (i915_request_completed(rq))

The execlists unwind function has a list_del if the request is 
completed. Any reason not to do that here?

>   			continue;
>   
> @@ -824,7 +824,7 @@ __unwind_incomplete_requests(struct intel_context *ce)
>   		}
>   		GEM_BUG_ON(i915_sched_engine_is_empty(sched_engine));
>   
> -		list_add_tail(&rq->sched.link, pl);
> +		list_add(&rq->sched.link, pl);

Since you always do both list_del and list_add and it doesn't look like 
you use the fact that the list is empty between the 2 calls, you can 
merge them in a list_move.

Apart from these nits, the change to navigate the list in reverse and 
append here at the top LGTM.

Daniele

>   		set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>   
>   		spin_lock(&ce->guc_active.lock);