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

[Josh Don]

On Tue, Nov 10, 2026 at 6:20 AM Kayra Cizmeci <kayracizmeci@xxxxxxxxx> wrote:
>
> 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...

cgroup properties are cgroup hierarchy position relative, this just
maintins that behavior for cpu.idle as well. There was a more
extensive discussion on this in the previous RFC, which is how we
landed back on the find_matching_se() restoration.

ie. user given a cgroup can sub-delegate portions of their job as idle
relative to their main workload, but all of this still runs at the
user's priority vs other users on the system

>
> >
> > 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 :-).