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

[Peter Zijlstra <peterz@infradead.org>]

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@infradead.org> 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.

Let me ponder this a bit...