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

Vincent Guittot
2026-04-17T17:19:02Z

On Thu, 2 Apr 2026 at 15:17, Vincent Guittot <vincent.guittot@linaro.org> wrote:
>
> On Thu, 2 Apr 2026 at 15:14, Peter Zijlstra <peterz@infradead.org> 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@linaro.org/

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