Re: [PATCH] [RFC]: sched/deadline: Avoid double enqueue_pushable_dl_task() warning

[Juri Lelli <juri.lelli@redhat.com>]

On 04/03/26 10:51, Peter Zijlstra wrote:
> On Wed, Mar 04, 2026 at 08:06:53AM +0100, Juri Lelli wrote:
> > Hello,
> > 
> > On 03/03/26 19:41, John Stultz wrote:
> > > In testing with the full Proxy Execution patch stack, I found
> > > I would occasionally trip over the !RB_EMPTY_NODE() WARN_ON in
> > > enqueue_pushable_dl_task(), where the task we're adding to the
> > > pushable list is already enqueued.
> > > 
> > > This triggers from put_prev_task_dl(), where it seems we go into
> > > put_prev_task_dl()
> > > -> update_curr_dl()
> > >    -> update_curr_dl_se() [hitting the dl_runtime_exceeded() case]
> > >       -> enqueue_task_dl()
> > >          -> enqueue_pushable_dl_task()
> > > 
> > > Adding the task to the pushable the first time.
> > 
> > Ah, so in case the task is boosted (or we fail to start the
> > replenishment timer).
> > 
> > > Then we back up the call stack to put_prev_task_dl(), which at
> > > the end again calls enqueue_pushable_dl_task(), trying to add it
> > > a second time, tripping the warning.
> > > 
> > > To avoid this, add a dl_task_pushable() helper which we can use
> > > to replace the RB_EMPTY_NODE checks elsewhere, and then before
> > > enqueueing in put_prev_task_dl(), we can first check
> > > dl_task_pushable() to avoid the double enqueue.
> > 
> > Can't we just return early (as we do already in dequeue_pushable
> > _dl_task()) in enqueue_pushable_dl_task() instead of checking before
> > calling that function?
> 
> So I was mightily confused for a moment by all this.
> 
> But it turns out DL keeps current *in* the tree; since ->deadline is a
> lot less mutable than ->vruntime this is possible.
> 
> But that also means that set_next_task() / put_prev_task() are
> 'simpler'.
> 
> However, in this case I think they are too simple, and its leading to
> problems.
> 
> So update_curr_dl() needs to dequeue+enqueue because it is pushing
> ->deadline (because current is in tree). Then because of that, it also
> ends up doing enqueue_pushable, but that is actively wrong, you must not
> do that on current.
> 
> Only once you're doing put_prev_task() must you do that.
> 
> Imagine this happens because of an update_curr() that is not from
> put_prev_task(), then you end up with current on the pushable list,
> which is a big no-no.
> 
> So I think I'd like to see dl_rq->curr tracking, similar to what we have
> for fair.
> 
> If you do that (see below), you'll probably find this case was already
> handled 'correctly' but was broken by the PE thing ;-)
> 
> Note the !task_current() clause in enqueue_task_dl() guarding
> enqueue_pushable_dl_task().

Ah, indeed. The below makes sense to me. Let's see if John's tests are
happy as well.

Thanks,
Juri