Re: [Intel-gfx] [PATCH 19/57] drm/i915: Fix the iterative dfs for defering requests
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
---
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 = >sched.waiters_list;
+ const int prio = rq_prio(rq);
+ struct i915_request *rn;
+ LIST_HEAD(dfs);
lockdep_assert_held(>active.lock);
GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_PQUEUE, >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(>sched.link, pl);
-
- for_each_waiter(p, rq) {
+ list_for_each_continue(pos, >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(>sched.link, );
+ GEM_BUG_ON(rq_prio(w) != prio);
+
+ /* Remember our position along this branch */
+ rq = stack_push(w, rq, pos);
+ pos = >sched.waiters_list;
}
- rq = list_first_entry_or_null(, 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(>sched.link, );
+
+ /* Track our visit, and prevent duplicate processing */
+ clear_bit(I915_FENCE_FLAG_PQUEUE, >fence.flags);
+ } while ((rq = stack_pop(rq, )));
+
+ pos = lookup_priolist(engine, prio);
+ list_for_each_entry_safe(rq, rn, , sched.link) {
+ set_bit(I915_FENCE_FLAG_PQUEUE, >fence.flags);
+ list_add_tail(>sched.link, pos);
+ }
}
static void queue_request(struct intel_engine_cs *engine,
Reviewed-by: Tvrtko Ursulin
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 19/57] drm/i915: Fix the iterative dfs for defering requests
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
---
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 = >sched.waiters_list;
+ const int prio = rq_prio(rq);
+ struct i915_request *rn;
+ LIST_HEAD(dfs);
lockdep_assert_held(>active.lock);
GEM_BUG_ON(!test_bit(I915_FENCE_FLAG_PQUEUE, >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(>sched.link, pl);
-
- for_each_waiter(p, rq) {
+ list_for_each_continue(pos, >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.
+*/
+ 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(>sched.link, );
+ GEM_BUG_ON(rq_prio(w) != prio);
+
+ /* Remember our position along this branch */
+ rq = stack_push(w, rq, pos);
+ pos = >sched.waiters_list;
}
- rq = list_first_entry_or_null(, 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(>sched.link, );
+
+ /* Track our visit, and prevent duplicate processing */
+ clear_bit(I915_FENCE_FLAG_PQUEUE, >fence.flags);
+ } while ((rq = stack_pop(rq, )));
+
+ pos = lookup_priolist(engine, prio);
+ list_for_each_entry_safe(rq, rn, , sched.link) {
+ set_bit(I915_FENCE_FLAG_PQUEUE, >fence.flags);
+ list_add_tail(>sched.link, pos);
+ }
}
static void queue_request(struct intel_engine_cs *engine,
--
2.20.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
