Re: [PATCH v3] sched/fair: Fix enqueue_task_fair warning some more
On Tue, May 12, 2020 at 04:10:48PM +0200 Peter Zijlstra wrote: > On Tue, May 12, 2020 at 09:52:22AM -0400, Phil Auld wrote: > > sched/fair: Fix enqueue_task_fair warning some more > > > > The recent patch, fe61468b2cb (sched/fair: Fix enqueue_task_fair warning) > > did not fully resolve the issues with the rq->tmp_alone_branch != > > >leaf_cfs_rq_list warning in enqueue_task_fair. There is a case where > > the first for_each_sched_entity loop exits due to on_rq, having incompletely > > updated the list. In this case the second for_each_sched_entity loop can > > further modify se. The later code to fix up the list management fails to do > > what is needed because se does not point to the sched_entity which broke out > > of the first loop. The list is not fixed up because the throttled parent was > > already added back to the list by a task enqueue in a parallel child > > hierarchy. > > > > Address this by calling list_add_leaf_cfs_rq if there are throttled parents > > while doing the second for_each_sched_entity loop. > > > > v3: clean up commit message and add fixes and review tags. > > Excellent, ignore what I just sent, I now have this one. > Thank you! Cheers, Phil --
Re: [PATCH v3] sched/fair: Fix enqueue_task_fair warning some more
On Tue, May 12, 2020 at 09:52:22AM -0400, Phil Auld wrote: > sched/fair: Fix enqueue_task_fair warning some more > > The recent patch, fe61468b2cb (sched/fair: Fix enqueue_task_fair warning) > did not fully resolve the issues with the rq->tmp_alone_branch != > >leaf_cfs_rq_list warning in enqueue_task_fair. There is a case where > the first for_each_sched_entity loop exits due to on_rq, having incompletely > updated the list. In this case the second for_each_sched_entity loop can > further modify se. The later code to fix up the list management fails to do > what is needed because se does not point to the sched_entity which broke out > of the first loop. The list is not fixed up because the throttled parent was > already added back to the list by a task enqueue in a parallel child > hierarchy. > > Address this by calling list_add_leaf_cfs_rq if there are throttled parents > while doing the second for_each_sched_entity loop. > > v3: clean up commit message and add fixes and review tags. Excellent, ignore what I just sent, I now have this one.
Re: [PATCH v3] sched/fair: Fix enqueue_task_fair warning some more
sched/fair: Fix enqueue_task_fair warning some more The recent patch, fe61468b2cb (sched/fair: Fix enqueue_task_fair warning) did not fully resolve the issues with the rq->tmp_alone_branch != >leaf_cfs_rq_list warning in enqueue_task_fair. There is a case where the first for_each_sched_entity loop exits due to on_rq, having incompletely updated the list. In this case the second for_each_sched_entity loop can further modify se. The later code to fix up the list management fails to do what is needed because se does not point to the sched_entity which broke out of the first loop. The list is not fixed up because the throttled parent was already added back to the list by a task enqueue in a parallel child hierarchy. Address this by calling list_add_leaf_cfs_rq if there are throttled parents while doing the second for_each_sched_entity loop. v3: clean up commit message and add fixes and review tags. Suggested-by: Vincent Guittot Signed-off-by: Phil Auld Cc: Peter Zijlstra (Intel) Cc: Vincent Guittot Cc: Ingo Molnar Cc: Juri Lelli Reviewed-by: Vincent Guittot Reviewed-by: Dietmar Eggemann Fixes: fe61468b2cb (sched/fair: Fix enqueue_task_fair warning) --- kernel/sched/fair.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 02f323b85b6d..c6d57c334d51 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -5479,6 +5479,13 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags) /* end evaluation on encountering a throttled cfs_rq */ if (cfs_rq_throttled(cfs_rq)) goto enqueue_throttle; + + /* +* One parent has been throttled and cfs_rq removed from the +* list. Add it back to not break the leaf list. +*/ + if (throttled_hierarchy(cfs_rq)) + list_add_leaf_cfs_rq(cfs_rq); } enqueue_throttle: -- 2.18.0 --
