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

[John Stultz]

On Wed, Mar 4, 2026 at 1:51 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Wed, Mar 04, 2026 at 08:06:53AM +0100, Juri Lelli wrote:
> > On 03/03/26 19:41, John Stultz wrote:
> > > 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().
>
> Hmm?
>
> ---
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 9869025941a0..a7db81d17082 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -2565,6 +2571,10 @@ static void start_hrtick_dl(struct rq *rq, struct sched_dl_entity *dl_se)
>  }
>  #endif /* !CONFIG_SCHED_HRTICK */
>
> +/*
> + * DL keeps current in tree, because ->deadline is not typically changed while
> + * a task is runnable.
> + */
>  static void set_next_task_dl(struct rq *rq, struct task_struct *p, bool first)
>  {
>         struct sched_dl_entity *dl_se = &p->dl;
> @@ -2585,6 +2595,9 @@ static void set_next_task_dl(struct rq *rq, struct task_struct *p, bool first)
>
>         deadline_queue_push_tasks(rq);
>
> +       WARN_ON_ONCE(dl_rq->curr);
> +       dl_rq->curr = dl_se;
> +

So I think this part assigning the curr ptr needs to be set above the
"if (!first) return;" logic, otherwise when a running task swtiches to
DL (via __sched_setscheduler()), we will trip the newly added warning
when we later put_prev and dl_rq->curr is still NULL.

But with that tweak, so far this is looking ok.   I'll continue
testing tonight and then resend it to the list.

thanks
-john