[Intel-gfx] [PATCH 19/57] drm/i915: Fix the iterative dfs for defering requests

[Tvrtko Ursulin]

On 01/02/2021 08:56, Chris Wilson wrote:
> The current implementation of walking the children of a deferred
> requests lacks the backtracking required to reduce the dfs to linear.
> Having pulled it from execlists into the common layer, we can reuse the
> dfs code for priority inheritance.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_scheduler.c | 56 +++++++++++++++++++--------
>   1 file changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_scheduler.c b/drivers/gpu/drm/i915/i915_scheduler.c
> index bfd37ee801fd..694ca3a3b563 100644
> --- a/drivers/gpu/drm/i915/i915_scheduler.c
> +++ b/drivers/gpu/drm/i915/i915_scheduler.c
> @@ -466,8 +466,10 @@ void i915_request_set_priority(struct i915_request *rq, int prio)
>   void __i915_sched_defer_request(struct intel_engine_cs *engine,
>   				struct i915_request *rq)
>   {
> -	struct list_head *pl;
> -	LIST_HEAD(list);
> +	struct list_head *pos = &rq->sched.waiters_list;
> +	const int prio = rq_prio(rq);
> +	struct i915_request *rn;
> +	LIST_HEAD(dfs);
>   
>   	lockdep_assert_held(&engine->active.lock);
>   	GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags));
> @@ -477,14 +479,11 @@ void __i915_sched_defer_request(struct intel_engine_cs *engine,
>   	 * to those that are waiting upon it. So we traverse its chain of
>   	 * waiters and move any that are earlier than the request to after it.
>   	 */
> -	pl = lookup_priolist(engine, rq_prio(rq));
> +	rq->sched.dfs.prev = NULL;
>   	do {
> -		struct i915_dependency *p;
> -
> -		GEM_BUG_ON(i915_request_is_active(rq));
> -		list_move_tail(&rq->sched.link, pl);
> -
> -		for_each_waiter(p, rq) {
> +		list_for_each_continue(pos, &rq->sched.waiters_list) {
> +			struct i915_dependency *p =
> +				list_entry(pos, typeof(*p), wait_link);
>   			struct i915_request *w =
>   				container_of(p->waiter, typeof(*w), sched);
>   
> @@ -500,19 +499,44 @@ void __i915_sched_defer_request(struct intel_engine_cs *engine,
>   				   __i915_request_has_started(w) &&
>   				   !__i915_request_is_complete(rq));
>   
> -			if (!i915_request_is_ready(w))
> +			if (!i915_request_in_priority_queue(w))
>   				continue;
>   
> -			if (rq_prio(w) < rq_prio(rq))
> +			/*
> +			 * We also need to reorder within the same priority.
> +			 *
> +			 * This is unlike priority-inheritance, where if the
> +			 * signaler already has a higher priority [earlier
> +			 * deadline] than us, we can ignore as it will be
> +			 * scheduled first. If a waiter already has the
> +			 * same priority, we still have to push it to the end
> +			 * of the list. This unfortunately means we cannot
> +			 * use the rq_deadline() itself as a 'visited' bit.

rq_deadline only appears later but never mind.

> +			 */
> +			if (rq_prio(w) < prio)
>   				continue;
>   
> -			GEM_BUG_ON(rq_prio(w) > rq_prio(rq));
> -			GEM_BUG_ON(i915_request_is_active(w));
> -			list_move_tail(&w->sched.link, &list);
> +			GEM_BUG_ON(rq_prio(w) != prio);
> +
> +			/* Remember our position along this branch */
> +			rq = stack_push(w, rq, pos);
> +			pos = &rq->sched.waiters_list;
>   		}
>   
> -		rq = list_first_entry_or_null(&list, typeof(*rq), sched.link);
> -	} while (rq);
> +		/* Note list is reversed for waiters wrt signal hierarchy */
> +		GEM_BUG_ON(rq->engine != engine);
> +		GEM_BUG_ON(!i915_request_in_priority_queue(rq));
> +		list_move(&rq->sched.link, &dfs);
> +
> +		/* Track our visit, and prevent duplicate processing */
> +		clear_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
> +	} while ((rq = stack_pop(rq, &pos)));
> +
> +	pos = lookup_priolist(engine, prio);
> +	list_for_each_entry_safe(rq, rn, &dfs, sched.link) {
> +		set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
> +		list_add_tail(&rq->sched.link, pos);
> +	}
>   }
>   
>   static void queue_request(struct intel_engine_cs *engine,
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Regards,

Tvrtko