[PATCH v2] sched/rt: Push RT tasks when preempted by a deadline task

[PATCH v2] sched/rt: Push RT tasks when preempted by a deadline task

[yangsonghua]

Commit 704069649b5b ("sched/core: Rework sched_class::wakeup_preempt()
and rq_modified_*()") made wakeup_preempt_rt() callable for cross-class
wakeups, leaving a "XXX If we're preempted by DL, queue a push?" comment
with no implementation.

When a SCHED_DEADLINE task preempts a running SCHED_FIFO/SCHED_RR task,
the RT class's put_prev_task_rt() is the natural place to trigger a push:
at that point the preempted task is (conditionally) added to
pushable_tasks, and we have 'next' available to identify the reason for
preemption.

The earlier wakeup_preempt_rt() call site is too early: the running RT
task has not yet been added to pushable_tasks, so rt_queue_push_tasks()
would be a no-op for the common single-RT-task-per-CPU case.

Guard the dl_task(next) check with a NULL test.  put_prev_task_rt() is
reachable with next == NULL when called via the bare put_prev_task()
wrapper (sched.h), which always passes NULL:

  sched_change_begin()
    put_prev_task(rq, p)              /* next = NULL */
      put_prev_task_rt(rq, p, NULL)
        dl_task(NULL) -> NULL deref   /* BUG in v1 */

The actual DL-preemption path goes through put_prev_set_next_task(),
which supplies the real next task, so the NULL guard does not change
the intended trigger condition.

Regarding potentially unnecessary push/pull cycles for short-lived DL
tasks: this is an inherent property of any asynchronous push mechanism.
rt_queue_push_tasks() enqueues a balance callback rather than migrating
synchronously, so by the time it fires the DL task may have already
yielded; in that case the RT task may need to be pulled back.  This
trade-off is acceptable: leaving RT tasks stranded on a busy CPU
while peer CPUs sit idle is worse than an occasional redundant
migration.  The same concern applies to the existing triggers of
rt_queue_push_tasks() (e.g. after dequeue_task_rt), so this path is
consistent with the established policy.

Fix this by adding a dl_task(next) check in put_prev_task_rt(), guarded
against NULL.  This is intentionally placed outside and after the
enqueue_pushable_task() condition, so that a push is triggered even when
the preempted RT task is pinned (nr_cpus_allowed == 1) or blocked
(proxy-exec): in those cases there may be other migratable RT tasks
already sitting in pushable_tasks.

The task_is_blocked() early-return is folded into the
enqueue_pushable_task() condition to avoid inadvertently suppressing the
new push logic.

Note: this does not cover the DL-server case where a fair-class task
running under DL bandwidth budget displaces an RT task; that is a
separate concern.

Signed-off-by: yangsonghua <yangsonghua@lixiang.com>
---
Changes in v2:
 - Guard dl_task(next) with a 'next != NULL' check to fix a NULL pointer
   dereference when put_prev_task_rt() is called from sched_change_begin()
   via the put_prev_task() wrapper, which always passes next = NULL.
 - Extend the commit message to explain the NULL call path and address the
   reviewer concern about unnecessary push/pull cycles for short-lived DL
   tasks.
 - Improve wakeup_preempt_rt() comment to explicitly point readers to
   put_prev_task_rt() where the DL push is now handled.

 kernel/sched/rt.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index e474c31d8fe6..ef8cdc4c25d2 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1627,7 +1627,10 @@ static void wakeup_preempt_rt(struct rq *rq, struct task_struct *p, int flags)
 	struct task_struct *donor = rq->donor;
 
 	/*
-	 * XXX If we're preempted by DL, queue a push?
+	 * If the incoming task belongs to a higher-priority class (e.g. DL),
+	 * we only handle same-class RT preemption here.  The DL case is
+	 * handled in put_prev_task_rt() once the current task has been
+	 * added to pushable_tasks.
 	 */
 	if (p->sched_class != &rt_sched_class)
 		return;
@@ -1736,14 +1739,25 @@ static void put_prev_task_rt(struct rq *rq, struct task_struct *p, struct task_s
 
 	update_rt_rq_load_avg(rq_clock_pelt(rq), rq, 1);
 
-	if (task_is_blocked(p))
-		return;
 	/*
 	 * The previous task needs to be made eligible for pushing
-	 * if it is still active
+	 * if it is still active and migratable.
 	 */
-	if (on_rt_rq(&p->rt) && p->nr_cpus_allowed > 1)
+	if (!task_is_blocked(p) && on_rt_rq(&p->rt) && p->nr_cpus_allowed > 1)
 		enqueue_pushable_task(rq, p);
+
+	/*
+	 * When a deadline task takes over this CPU, try to push any queued
+	 * RT tasks to CPUs running lower-priority work. This is independent
+	 * of whether p itself is pushable: even if p is pinned or blocked,
+	 * there may be other migratable RT tasks already in pushable_tasks.
+	 *
+	 * next is NULL when called from put_prev_task() (e.g. sched_change_begin);
+	 * guard accordingly.  rt_queue_push_tasks() checks has_pushable_tasks()
+	 * internally, so this is a no-op if nothing is queued.
+	 */
+	if (next && dl_task(next))
+		rt_queue_push_tasks(rq);
 }
 
 /* Only try algorithms three times */
-- 
2.34.1

Re: [PATCH v2] sched/rt: Push RT tasks when preempted by a deadline task

[K Prateek Nayak]

Hello Yangsonghua, 

On 6/26/2026 8:53 AM, yangsonghua wrote:
> [You don't often get email from jluyangsonghua@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Commit 704069649b5b ("sched/core: Rework sched_class::wakeup_preempt()
> and rq_modified_*()") made wakeup_preempt_rt() callable for cross-class
> wakeups, leaving a "XXX If we're preempted by DL, queue a push?" comment
> with no implementation.
> 
> When a SCHED_DEADLINE task preempts a running SCHED_FIFO/SCHED_RR task,
> the RT class's put_prev_task_rt() is the natural place to trigger a push:
> at that point the preempted task is (conditionally) added to
> pushable_tasks, and we have 'next' available to identify the reason for
> preemption.
> 
> The earlier wakeup_preempt_rt() call site is too early: the running RT
> task has not yet been added to pushable_tasks, so rt_queue_push_tasks()
> would be a no-op for the common single-RT-task-per-CPU case.
> 
> Guard the dl_task(next) check with a NULL test.  put_prev_task_rt() is
> reachable with next == NULL when called via the bare put_prev_task()
> wrapper (sched.h), which always passes NULL:
> 
>   sched_change_begin()
>     put_prev_task(rq, p)              /* next = NULL */
>       put_prev_task_rt(rq, p, NULL)
>         dl_task(NULL) -> NULL deref   /* BUG in v1 */
> 
> The actual DL-preemption path goes through put_prev_set_next_task(),
> which supplies the real next task, so the NULL guard does not change
> the intended trigger condition.
> 
> Regarding potentially unnecessary push/pull cycles for short-lived DL
> tasks: this is an inherent property of any asynchronous push mechanism.
> rt_queue_push_tasks() enqueues a balance callback rather than migrating
> synchronously, so by the time it fires the DL task may have already
> yielded; in that case the RT task may need to be pulled back.  This
> trade-off is acceptable: leaving RT tasks stranded on a busy CPU
> while peer CPUs sit idle is worse than an occasional redundant
> migration.  The same concern applies to the existing triggers of
> rt_queue_push_tasks() (e.g. after dequeue_task_rt), so this path is
> consistent with the established policy.
> 
> Fix this by adding a dl_task(next) check in put_prev_task_rt(), guarded
> against NULL.  This is intentionally placed outside and after the
> enqueue_pushable_task() condition, so that a push is triggered even when
> the preempted RT task is pinned (nr_cpus_allowed == 1) or blocked
> (proxy-exec): in those cases there may be other migratable RT tasks
> already sitting in pushable_tasks.

That cannot be the case. More details below.

> 
> The task_is_blocked() early-return is folded into the
> enqueue_pushable_task() condition to avoid inadvertently suppressing the
> new push logic.
> 
> Note: this does not cover the DL-server case where a fair-class task
> running under DL bandwidth budget displaces an RT task; that is a
> separate concern.
> 
> Signed-off-by: yangsonghua <yangsonghua@lixiang.com>
> ---
> Changes in v2:
>  - Guard dl_task(next) with a 'next != NULL' check to fix a NULL pointer
>    dereference when put_prev_task_rt() is called from sched_change_begin()
>    via the put_prev_task() wrapper, which always passes next = NULL.
>  - Extend the commit message to explain the NULL call path and address the
>    reviewer concern about unnecessary push/pull cycles for short-lived DL
>    tasks.
>  - Improve wakeup_preempt_rt() comment to explicitly point readers to
>    put_prev_task_rt() where the DL push is now handled.
> 
>  kernel/sched/rt.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index e474c31d8fe6..ef8cdc4c25d2 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1627,7 +1627,10 @@ static void wakeup_preempt_rt(struct rq *rq, struct task_struct *p, int flags)
>         struct task_struct *donor = rq->donor;
> 
>         /*
> -        * XXX If we're preempted by DL, queue a push?
> +        * If the incoming task belongs to a higher-priority class (e.g. DL),
> +        * we only handle same-class RT preemption here.  The DL case is
> +        * handled in put_prev_task_rt() once the current task has been
> +        * added to pushable_tasks.
>          */
>         if (p->sched_class != &rt_sched_class)
>                 return;
> @@ -1736,14 +1739,25 @@ static void put_prev_task_rt(struct rq *rq, struct task_struct *p, struct task_s
> 
>         update_rt_rq_load_avg(rq_clock_pelt(rq), rq, 1);
> 
> -       if (task_is_blocked(p))
> -               return;

I still don't get why we are removing this early return. When "p" was
selected to run, it would have already queued a balance callback via
set_nex_task_rt() and the balance would have been done during last
__schedule().

If there are pushable tasks on the list still, they'll still remain
non-pushable since they haven't run on CPU and nothing has changed for
them to suddenly enable a movement.

"p" itself is a blocked task so it cannot be moved - it'll simply
follow the lock owner via proxy migration path whenever it gets a
chance.

So the gist is: If there are pushable tasks, then balance failed to
move them previously. If the state of the system had changed,
balance_rt() on a remote CPU would have already pulled the highest
priority task from this rq.

I feel that early return for blocked task should stay as is since
there is no opportunity for balance yet unless I'm missing something.

>         /*
>          * The previous task needs to be made eligible for pushing
> -        * if it is still active
> +        * if it is still active and migratable.
>          */
> -       if (on_rt_rq(&p->rt) && p->nr_cpus_allowed > 1)
> +       if (!task_is_blocked(p) && on_rt_rq(&p->rt) && p->nr_cpus_allowed > 1)
>                 enqueue_pushable_task(rq, p);
> +
> +       /*
> +        * When a deadline task takes over this CPU, try to push any queued
> +        * RT tasks to CPUs running lower-priority work. This is independent
> +        * of whether p itself is pushable: even if p is pinned or blocked,
> +        * there may be other migratable RT tasks already in pushable_tasks.
> +        *
> +        * next is NULL when called from put_prev_task() (e.g. sched_change_begin);
> +        * guard accordingly.  rt_queue_push_tasks() checks has_pushable_tasks()
> +        * internally, so this is a no-op if nothing is queued.
> +        */
> +       if (next && dl_task(next))
> +               rt_queue_push_tasks(rq);

Based on some quick look at the cpupri bits, I think this makes sense.

Since we try to move "p" to a CPU running a lower priority task and a
deadline task would have increased the priority of this CPU anyways, it
is fair to attempt a push.

I'll let Steve finish his metamorphosis and see if that estimate is
wrong or not ;-)

>  }
> 
>  /* Only try algorithms three times */
> --
> 2.34.1
> 

-- 
Thanks and Regards,
Prateek

Re: [PATCH v2] sched/rt: Push RT tasks when preempted by a deadline task

[yangsonghua]

Hi Prateek,

Thank you for the thorough review!

You are right about the task_is_blocked() early return — the blocked-task
case does not create new migration opportunities, since balance was already
attempted when p was last scheduled in, and a DL arrival on this CPU does
not change the migrability of tasks already sitting in pushable_tasks.
I will restore the early return in v3.


Will send v3 shortly.

Best regards,
Yangsonghua

K Prateek Nayak <kprateek.nayak@amd.com> 于2026年6月26日周五 13:52写道：
>
> Hello Yangsonghua,
>
> On 6/26/2026 8:53 AM, yangsonghua wrote:
> > [You don't often get email from jluyangsonghua@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Commit 704069649b5b ("sched/core: Rework sched_class::wakeup_preempt()
> > and rq_modified_*()") made wakeup_preempt_rt() callable for cross-class
> > wakeups, leaving a "XXX If we're preempted by DL, queue a push?" comment
> > with no implementation.
> >
> > When a SCHED_DEADLINE task preempts a running SCHED_FIFO/SCHED_RR task,
> > the RT class's put_prev_task_rt() is the natural place to trigger a push:
> > at that point the preempted task is (conditionally) added to
> > pushable_tasks, and we have 'next' available to identify the reason for
> > preemption.
> >
> > The earlier wakeup_preempt_rt() call site is too early: the running RT
> > task has not yet been added to pushable_tasks, so rt_queue_push_tasks()
> > would be a no-op for the common single-RT-task-per-CPU case.
> >
> > Guard the dl_task(next) check with a NULL test.  put_prev_task_rt() is
> > reachable with next == NULL when called via the bare put_prev_task()
> > wrapper (sched.h), which always passes NULL:
> >
> >   sched_change_begin()
> >     put_prev_task(rq, p)              /* next = NULL */
> >       put_prev_task_rt(rq, p, NULL)
> >         dl_task(NULL) -> NULL deref   /* BUG in v1 */
> >
> > The actual DL-preemption path goes through put_prev_set_next_task(),
> > which supplies the real next task, so the NULL guard does not change
> > the intended trigger condition.
> >
> > Regarding potentially unnecessary push/pull cycles for short-lived DL
> > tasks: this is an inherent property of any asynchronous push mechanism.
> > rt_queue_push_tasks() enqueues a balance callback rather than migrating
> > synchronously, so by the time it fires the DL task may have already
> > yielded; in that case the RT task may need to be pulled back.  This
> > trade-off is acceptable: leaving RT tasks stranded on a busy CPU
> > while peer CPUs sit idle is worse than an occasional redundant
> > migration.  The same concern applies to the existing triggers of
> > rt_queue_push_tasks() (e.g. after dequeue_task_rt), so this path is
> > consistent with the established policy.
> >
> > Fix this by adding a dl_task(next) check in put_prev_task_rt(), guarded
> > against NULL.  This is intentionally placed outside and after the
> > enqueue_pushable_task() condition, so that a push is triggered even when
> > the preempted RT task is pinned (nr_cpus_allowed == 1) or blocked
> > (proxy-exec): in those cases there may be other migratable RT tasks
> > already sitting in pushable_tasks.
>
> That cannot be the case. More details below.
>
> >
> > The task_is_blocked() early-return is folded into the
> > enqueue_pushable_task() condition to avoid inadvertently suppressing the
> > new push logic.
> >
> > Note: this does not cover the DL-server case where a fair-class task
> > running under DL bandwidth budget displaces an RT task; that is a
> > separate concern.
> >
> > Signed-off-by: yangsonghua <yangsonghua@lixiang.com>
> > ---
> > Changes in v2:
> >  - Guard dl_task(next) with a 'next != NULL' check to fix a NULL pointer
> >    dereference when put_prev_task_rt() is called from sched_change_begin()
> >    via the put_prev_task() wrapper, which always passes next = NULL.
> >  - Extend the commit message to explain the NULL call path and address the
> >    reviewer concern about unnecessary push/pull cycles for short-lived DL
> >    tasks.
> >  - Improve wakeup_preempt_rt() comment to explicitly point readers to
> >    put_prev_task_rt() where the DL push is now handled.
> >
> >  kernel/sched/rt.c | 24 +++++++++++++++++++-----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> > index e474c31d8fe6..ef8cdc4c25d2 100644
> > --- a/kernel/sched/rt.c
> > +++ b/kernel/sched/rt.c
> > @@ -1627,7 +1627,10 @@ static void wakeup_preempt_rt(struct rq *rq, struct task_struct *p, int flags)
> >         struct task_struct *donor = rq->donor;
> >
> >         /*
> > -        * XXX If we're preempted by DL, queue a push?
> > +        * If the incoming task belongs to a higher-priority class (e.g. DL),
> > +        * we only handle same-class RT preemption here.  The DL case is
> > +        * handled in put_prev_task_rt() once the current task has been
> > +        * added to pushable_tasks.
> >          */
> >         if (p->sched_class != &rt_sched_class)
> >                 return;
> > @@ -1736,14 +1739,25 @@ static void put_prev_task_rt(struct rq *rq, struct task_struct *p, struct task_s
> >
> >         update_rt_rq_load_avg(rq_clock_pelt(rq), rq, 1);
> >
> > -       if (task_is_blocked(p))
> > -               return;
>
> I still don't get why we are removing this early return. When "p" was
> selected to run, it would have already queued a balance callback via
> set_nex_task_rt() and the balance would have been done during last
> __schedule().
>
> If there are pushable tasks on the list still, they'll still remain
> non-pushable since they haven't run on CPU and nothing has changed for
> them to suddenly enable a movement.
>
> "p" itself is a blocked task so it cannot be moved - it'll simply
> follow the lock owner via proxy migration path whenever it gets a
> chance.
>
> So the gist is: If there are pushable tasks, then balance failed to
> move them previously. If the state of the system had changed,
> balance_rt() on a remote CPU would have already pulled the highest
> priority task from this rq.
>
> I feel that early return for blocked task should stay as is since
> there is no opportunity for balance yet unless I'm missing something.
>
> >         /*
> >          * The previous task needs to be made eligible for pushing
> > -        * if it is still active
> > +        * if it is still active and migratable.
> >          */
> > -       if (on_rt_rq(&p->rt) && p->nr_cpus_allowed > 1)
> > +       if (!task_is_blocked(p) && on_rt_rq(&p->rt) && p->nr_cpus_allowed > 1)
> >                 enqueue_pushable_task(rq, p);
> > +
> > +       /*
> > +        * When a deadline task takes over this CPU, try to push any queued
> > +        * RT tasks to CPUs running lower-priority work. This is independent
> > +        * of whether p itself is pushable: even if p is pinned or blocked,
> > +        * there may be other migratable RT tasks already in pushable_tasks.
> > +        *
> > +        * next is NULL when called from put_prev_task() (e.g. sched_change_begin);
> > +        * guard accordingly.  rt_queue_push_tasks() checks has_pushable_tasks()
> > +        * internally, so this is a no-op if nothing is queued.
> > +        */
> > +       if (next && dl_task(next))
> > +               rt_queue_push_tasks(rq);
>
> Based on some quick look at the cpupri bits, I think this makes sense.
>
> Since we try to move "p" to a CPU running a lower priority task and a
> deadline task would have increased the priority of this CPU anyways, it
> is fair to attempt a push.
>
> I'll let Steve finish his metamorphosis and see if that estimate is
> wrong or not ;-)
>
> >  }
> >
> >  /* Only try algorithms three times */
> > --
> > 2.34.1
> >
>
> --
> Thanks and Regards,
> Prateek
>