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

[Vincent Guittot]

On Wed, 22 Apr 2026 at 16:28, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>
> On Wed, Apr 22, 2026 at 04:24:11PM +0200, Peter Zijlstra wrote:
> > On Wed, Apr 22, 2026 at 04:06:42PM +0200, Vincent Guittot wrote:
> > > On Wed, 22 Apr 2026 at 15:39, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> >
> > > > That said, on IRQ you mentioned that this wasn't quite good enough and
> > > > that your original patch is best.
> > >
> > > Yes, it fixes one case but breaks the other one :-(
> > >
> > > >
> > > > The trouble is, your original patch can update vlag (!se->sched_delayed)
> > > > and report it hasn't changed; because then vlag == se->clag, obviously.
> > >
> > > In the  (!se->sched_delayed),  we don't care because the entity is not
> > > enqueued so we don't need to place it with new vlag
> >
> > I am confused more :-) this could be dequeue_entity() doing a normal
> > dequeue, in which case it very much is enqueued. We only delay for
> > !eligible, !special etc..
> >
> > > > This invalidates the comment on the return value of the function. In
> > > > fact, it makes the function have a very non-obvious return meaning.
> > > >
> > > > So I'm a little confused -- what do we actually want this function to
> > > > do?
> > >
> > > I want update_entity_lag() to return true if we have modified the vlag
> > > of an enqueued entity. In this case we need to dequeue, place entity
> > > with new vlag and enqueue it.
> > >
> > > we don't need to test se->on_rq because update_entity_lag() is called
> > > for enqueued task only with delayed dequeue entity so
> > > se->sched_delayed implies se->on_rq
> > >
> > > In fact we should test :
> > > (vlag != se->vlag) && se->on_rq
> > > but  && se->on_rq is useless
> > >
> > > That being said, this probably deserves a comment
> >
> > But but, dequeue_entity()'s second update_entity_lag() call can have:
> >
> >   se->sched_delayed == 0 && se->on_rq == 1
> >
> > Think dequeue of eligible or special or...
>
> So only requeue_delayed_entity() cares about the return value, and there
> it holds what you say, but that is a very narrow case and doesn't
> reflect all the callsites.

Yes and only in the case of if(se->sched_delayed), se->vlag can be
different from the value return by entity_lag() which return current
lag

>
> Let me ponder this a bit...