Re: [PATCH] sched/eevdf: Fix sched idle task group preemption

[Vincent Guittot]

On Mon, 19 Oct 2026 at 20:16, Vincent Guittot
<vincent.guittot@xxxxxxxxxx> wrote:
>
> Even with flat hierarchy which puts all runnable tasks in the same cfs rq,
> we need to parse the hierarchy to check for sched_idle task group.
>
> Reported-by: Jakub Okoński <jakub@xxxxxxxxxxx>
> Link: https://lore.kernel.org/all/58dc8e8a-7260-43ce-aebb-8d9c29a6afc9@xxxxxxxxxxx/
> Fixes: 85570f10a4c6 ("sched/eevdf: Move to a single runqueue")
> Signed-off-by: Vincent Guittot <vincent.guittot@xxxxxxxxxx>

This was sent too quickly. v2 coming soon

> ---
>  kernel/sched/fair.c | 81 ++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 69 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 0f2364479f9d..20c33d13a5e7 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -433,6 +433,38 @@ static inline struct sched_entity *parent_entity(const struct sched_entity *se)
>         return se->parent;
>  }
>
> +static void
> +find_matching_se(struct sched_entity **se, struct sched_entity **pse)
> +{
> +       int se_depth, pse_depth;
> +
> +       /*
> +        * preemption test can be made between sibling entities who are in the
> +        * same cfs_rq i.e who have a common parent. Walk up the hierarchy of
> +        * both tasks until we find their ancestors who are siblings of common
> +        * parent.
> +        */
> +
> +       /* First walk up until both entities are at same depth */
> +       se_depth = (*se)->depth;
> +       pse_depth = (*pse)->depth;
> +
> +       while (se_depth > pse_depth) {
> +               se_depth--;
> +               *se = parent_entity(*se);
> +       }
> +
> +       while (pse_depth > se_depth) {
> +               pse_depth--;
> +               *pse = parent_entity(*pse);
> +       }
> +
> +       while (!is_same_group(*se, *pse)) {
> +               *se = parent_entity(*se);
> +               *pse = parent_entity(*pse);
> +       }
> +}
> +
>  static int tg_is_idle(struct task_group *tg)
>  {
>         return tg->idle > 0;
> @@ -476,6 +508,11 @@ static inline struct sched_entity *parent_entity(struct sched_entity *se)
>         return NULL;
>  }
>
> +static inline void
> +find_matching_se(struct sched_entity **se, struct sched_entity **pse)
> +{
> +}
> +
>  static inline int tg_is_idle(struct task_group *tg)
>  {
>         return 0;
> @@ -10129,7 +10166,35 @@ preempt_sync(struct rq *rq, int wake_flags,
>  }
>
>  /*
> - * Preempt the current task with a newly woken task if needed:
> + * Preempt an idle entity in favor of a non-idle entity (and don't preempt
> + * in the inverse case).
> + */
> +static inline preempt_wakeup_action
> +wakeup_preempt_sched_idle(struct sched_entity *cse, struct sched_entity *pse)
> +{
> +       int cse_is_idle, pse_is_idle;
> +
> +       find_matching_se(&cse, &pse);
> +       WARN_ON_ONCE(!pse);
> +
> +       cse_is_idle = se_is_idle(cse);
> +       pse_is_idle = se_is_idle(pse);
> +
> +       /*
> +        * Preempt an idle entity in favor of a non-idle entity (and don't preempt
> +        * in the inverse case).
> +        */
> +       if (cse_is_idle && !pse_is_idle)
> +               return PREEMPT_WAKEUP_RESCHED;
> +
> +       if (cse_is_idle != pse_is_idle)
> +               return PREEMPT_WAKEUP_NONE;
> +
> +       return PREEMPT_WAKEUP_PICK;
> +}
> +
> +/*
> + * Preempt the current task with a newly woken task if needed.
>   */
>  static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_flags)
>  {
> @@ -10174,22 +10239,14 @@ static void wakeup_preempt_fair(struct rq *rq, struct task_struct *p, int wake_f
>         if (!sched_feat(WAKEUP_PREEMPTION))
>                 return;
>
> -       WARN_ON_ONCE(!pse);
> -
> -       cse_is_idle = se_is_idle(se);
> -       pse_is_idle = se_is_idle(pse);
> +       preempt_action = wakeup_preempt_sched_idle(se, pse);
>
> -       nse = se;
> -       /*
> -        * Preempt an idle entity in favor of a non-idle entity (and don't preempt
> -        * in the inverse case).
> -        */
> -       if (cse_is_idle && !pse_is_idle)
> +       if (preempt_action == PREEMPT_WAKEUP_RESCHED)
>                 goto preempt;
>
>         update_curr_fair(rq);
>
> -       if (cse_is_idle != pse_is_idle)
> +       if (preempt_action == PREEMPT_WAKEUP_NONE)
>                 goto update;
>
>         /*
> --
> 2.53.0
>