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

[Kayra Cizmeci <kayracizmeci@gmail.com>]

Hi Josh!

On Mon, Nov 09, 2026 at 04:10:30PM -0800, Josh Don wrote:

> >  /*
> > - * 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 enum preempt_wakeup_action
> > +wakeup_preempt_sched_idle(struct sched_entity *cse, struct sched_entity *pse)
> > +{
> > +       int cse_is_idle, pse_is_idle;
> > +
> > +       cse_is_idle = se_is_idle(cse);
> > +       pse_is_idle = se_is_idle(pse);
> > +
> > +       if (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);
> 
> now that se_is_idle() is checking for any ancestors with idle, this
> doesn't appear to be correct. Some motivaing examples:

So we want to keep the idle difference between groups with idle
ancestors? FWIW, I asked about this but then thought it was knowingly
and that was what we wanted. I'm still not sure what we want...



> 
> Ex 1:
> regular task and SCHED_IDLE task both inside an idle cgroup.
> se_is_idle() will be true for both se's, and so the regular task will
> not end up preempting the SCHED_IDLE task, which would be the
> desirable behavior (the task is not idle relative to the SCHED_IDLE
> task)
> 
>     cgrp A (idle)
>      /                 \
> regular_task     SCHED_IDLE task
> 
> Ex2:
> more complex example with a hierarchy of cgroups
> 
>           root
>             |
>           cgrp A (idle)
>          /            \
>      cgrp B           cgrp C (idle)
>         |                   |
>       task_1            task_2
> 
> task_1 should preempt task_2, and the inverse should also never
> happen. However, the matching_se here are cgroups B and C, and both
> show se_is_idle() as true
> 
> I can think of two simple ways to fix that, either seems fine
> 1) use the existing se_is_idle() check for knowing when we can
> fast-path skip the whole find_matching_se() business, but when we do
> find the matching se, rather than use se_is_idle, use a more targeted
> helper that directly compares just those two entities without any of
> their ancestors (ie. for the task case just check if it has an idle
> policy, and for the group case, just check tg_is_idle())
> 2) once you find the matching se, just compare the ->idle values,
> since if one of them is bigger (by at most 1?), then that one must be
> the "idle" one relatively speaking. Handling the task vs group case
> here seems a bit messier compared to the first option, so my
> suggestion would be to go with the above
> 
> 
> Best,
> Josh

Cheers,

Kayra :-).