Hi Phil,
- reply to all this time
On Wed, 6 May 2020 at 16:18, Phil Auld <pa...@redhat.com> 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 !=
> &rq->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
But for the 2nd for_each_sched_entity, the cfs_rq should already be
in the list, isn't it ?
The third for_each_entity loop is there for the throttled case but is
useless for other case
Could you provide us some details about the use case that creates such
a situation ?
> what is needed because se does not point to the sched_entity which broke out
> of the first loop.
>
> Address this issue by saving the se pointer when the first loop exits and
> resetting it before doing the fix up, if needed.
>
> Signed-off-by: Phil Auld <pa...@redhat.com>
> Cc: Peter Zijlstra (Intel) <pet...@infradead.org>
> Cc: Vincent Guittot <vincent.guit...@linaro.org>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Juri Lelli <juri.le...@redhat.com>
> ---
> kernel/sched/fair.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 02f323b85b6d..719c996317e3 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5432,6 +5432,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p,
> int flags)
> {
> struct cfs_rq *cfs_rq;
> struct sched_entity *se = &p->se;
> + struct sched_entity *saved_se = NULL;
> int idle_h_nr_running = task_has_idle_policy(p);
>
> /*
> @@ -5466,6 +5467,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p,
> int flags)
> flags = ENQUEUE_WAKEUP;
> }
>
> + saved_se = se;
> for_each_sched_entity(se) {
> cfs_rq = cfs_rq_of(se);
>
> @@ -5510,6 +5512,8 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p,
> int flags)
> * leaf list maintenance, resulting in triggering the
> assertion
> * below.
> */
> + if (saved_se)
> + se = saved_se;
> for_each_sched_entity(se) {
> cfs_rq = cfs_rq_of(se);
>
> --
> 2.18.0
>
>
> Cheers,
> Phil
>
