Re: [PATCH v2] sched: fair: Prevent negative lag increase during delayed dequeue

[Vincent Guittot]

On Thu, 2 Apr 2026 at 15:17, Vincent Guittot <vincent.guittot@xxxxxxxxxx> wrote:
>
> On Thu, 2 Apr 2026 at 15:14, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > On Tue, Mar 31, 2026 at 06:23:52PM +0200, Vincent Guittot wrote:
> > > +/*
> > > + * Delayed dequeue aims to reduce the negative lag of a dequeued task.
> > > + * While updating the lag of an entity, check that negative lag didn't increase
> > > + * during the delayed dequeue period which would be unfair.
> > > + * Similarly, check that the entity didn't gain positive lag when DELAY_ZERO is
> > > + * set.
> > > + *
> > > + * Return true if the lag has been adjusted.
> > > + */
> > > +static bool update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> > >  {
> > > +     s64 vlag;
> > > +
> > >       WARN_ON_ONCE(!se->on_rq);
> > >
> > > -     se->vlag = entity_lag(cfs_rq, se, avg_vruntime(cfs_rq));
> > > +     vlag = entity_lag(cfs_rq, se, avg_vruntime(cfs_rq));
> > > +
> > > +     if (se->sched_delayed)
> > > +             /* previous vlag < 0 otherwise se would not be delayed */
> > > +             se->vlag = clamp(vlag, se->vlag, sched_feat(DELAY_ZERO) ? 0 : S64_MAX);
> > > +     else
> > > +             se->vlag = vlag;
> > > +
> > > +     return (vlag != se->vlag);
> > >  }
> >
> > Would you mind terribly if I write this like so?
>
> np, that's looks good too for me
>
> >
> > ---
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -841,29 +841,32 @@ static s64 entity_lag(struct cfs_rq *cfs
> >  }
> >
> >  /*
> > - * Delayed dequeue aims to reduce the negative lag of a dequeued task.
> > - * While updating the lag of an entity, check that negative lag didn't increase
> > + * Delayed dequeue aims to reduce the negative lag of a dequeued task. While
> > + * updating the lag of an entity, check that negative lag didn't increase
> >   * during the delayed dequeue period which would be unfair.
> > - * Similarly, check that the entity didn't gain positive lag when DELAY_ZERO is
> > - * set.
> > + * Similarly, check that the entity didn't gain positive lag when DELAY_ZERO
> > + * is set.
> >   *
> >   * Return true if the lag has been adjusted.
> >   */
> > -static bool update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> > +static __always_inline
> > +bool update_entity_lag(struct cfs_rq *cfs_rq, struct sched_entity *se)
> >  {
> > -       s64 vlag;
> > +       s64 vlag = entity_lag(cfs_rq, se, avg_vruntime(cfs_rq));
> > +       bool ret;
> >
> >         WARN_ON_ONCE(!se->on_rq);
> >
> > -       vlag = entity_lag(cfs_rq, se, avg_vruntime(cfs_rq));
> > -
> > -       if (se->sched_delayed)
> > +       if (se->sched_delayed) {
> >                 /* previous vlag < 0 otherwise se would not be delayed */
> > -               se->vlag = clamp(vlag, se->vlag, sched_feat(DELAY_ZERO) ? 0 : S64_MAX);
> > -       else
> > -               se->vlag = vlag;
> > +               vlag = max(vlag, se->vlag);
> > +               if (sched_feat(DELAY_ZERO))
> > +                       vlag = min(vlag, 0);
> > +       }
> > +       ret = (vlag == se->vlag);

hmm, I missed that we now return false when the lag has been updated
instead of true

I sent a fix
https://lore.kernel.org/all/20260417171642.3539914-1-vincent.guittot@xxxxxxxxxx/

> > +       se->vlag = vlag;
> >
> > -       return (vlag != se->vlag);
> > +       return ret;
> >  }
> >
> >  /*
> >