mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] sched/rt: Push RT tasks when preempted by a deadline task
@ 2026-06-24  3:04 yangsonghua
  2026-06-24  3:53 ` K Prateek Nayak
  0 siblings, 1 reply; 4+ messages in thread
From: yangsonghua @ 2026-06-24  3:04 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra
  Cc: Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Valentin Schneider, K Prateek Nayak, linux-kernel,
	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.

Fix this by adding a dl_task(next) check in put_prev_task_rt(). 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>
---
 kernel/sched/rt.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index e474c31d8fe6..299ce6c9699c 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1627,7 +1627,8 @@ 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 we're preempted by a higher-priority class (e.g. deadline),
+	 * nothing to do here for the RT class itself.
 	 */
 	if (p->sched_class != &rt_sched_class)
 		return;
@@ -1736,14 +1737,24 @@ 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.
+	 *
+	 * rt_queue_push_tasks() guards on has_pushable_tasks() internally,
+	 * so this is a no-op if nothing is queued.
+	 */
+	if (dl_task(next))
+		rt_queue_push_tasks(rq);
 }
 
 /* Only try algorithms three times */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/rt: Push RT tasks when preempted by a deadline task
  2026-06-24  3:04 [PATCH] sched/rt: Push RT tasks when preempted by a deadline task yangsonghua
@ 2026-06-24  3:53 ` K Prateek Nayak
       [not found]   ` <5213745e18453d424bea3d94af6bd31c5fc87620.a5e26565.899d.42c6.84f0.cf74cdb1acdd@feishu.cn>
  2026-06-25  3:09   ` yangsonghua
  0 siblings, 2 replies; 4+ messages in thread
From: K Prateek Nayak @ 2026-06-24  3:53 UTC (permalink / raw)
  To: yangsonghua, Ingo Molnar, Peter Zijlstra
  Cc: Juri Lelli, Vincent Guittot, Dietmar Eggemann, Steven Rostedt,
	Ben Segall, Valentin Schneider, linux-kernel, yangsonghua

Hello Yangsonghua, 

On 6/24/2026 8:34 AM, yangsonghua wrote:
> @@ -1736,14 +1737,24 @@ 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.
> +        *
> +        * rt_queue_push_tasks() guards on has_pushable_tasks() internally,
> +        * so this is a no-op if nothing is queued.
> +        */
> +       if (dl_task(next))
> +               rt_queue_push_tasks(rq);

next can be NULL if we are are coming here from sched_change_begin()
for the current task:

  sched_change_begin()
    put_prev_task()
      put_prev_task_rt(rq, prev, next = NULL)
        dl_task(next)
          dl_prio(next->prio) !!! NULL pointer dereference !!!

Do we avoid this in some way?

I don't know if this push was intentionally avoided or not. If there
are short running deadline task, we'll unnecessarily push a task and
pull it back later. Is that not a concern?

-- 
Thanks and Regards,
Prateek


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/rt: Push RT tasks when preempted by a deadline task
       [not found]   ` <5213745e18453d424bea3d94af6bd31c5fc87620.a5e26565.899d.42c6.84f0.cf74cdb1acdd@feishu.cn>
@ 2026-06-25  2:26     ` Eric yang
  0 siblings, 0 replies; 4+ messages in thread
From: Eric yang @ 2026-06-25  2:26 UTC (permalink / raw)
  To: 杨送华
  Cc: K Prateek Nayak, Ingo Molnar, Peter Zijlstra, Juri Lelli,
	Vincent Guittot, Dietmar Eggemann, Steven Rostedt, Ben Segall,
	Valentin Schneider, linux-kernel

Hi Prateek,

Thank you for the review!

> next can be NULL if we are coming here from sched_change_begin()
> for the current task [...] NULL pointer dereference !!!

You are absolutely right. put_prev_task() in sched.h always passes
NULL as 'next':

  static inline void put_prev_task(struct rq *rq, struct task_struct *prev)
  {
      prev->sched_class->put_prev_task(rq, prev, NULL);
  }

So when sched_change_begin() calls put_prev_task() for the currently
running RT task (e.g. during a priority/affinity change), next is NULL
and dl_task(next) will dereference it. I'll fix this with a NULL guard:

  if (next && dl_task(next))
      rt_queue_push_tasks(rq);

> If there are short running deadline tasks, we'll unnecessarily push
> a task and pull it back later. Is that not a concern?

This is a valid concern. The push is asynchronous (queued via IPI), so
there is an inherent delay before the RT task actually migrates. If the
DL task finishes before the push executes, the push becomes a no-op
since rt_queue_push_tasks() checks has_pushable_tasks() and the RT
task will have already been re-selected on this CPU.

If the RT task does get migrated and the DL task then exits shortly
after, pull_rt_task() triggered from the DL task's dequeue path will
bring it back. This round-trip is not free, but it is the same
trade-off already accepted in __schedule() when a DL task preempts
an RT task there.

The alternative -- never pushing -- means that while a DL task runs,
other CPUs may sit idle or run lower-priority work while a migratable
RT task is stuck waiting. For real-time workloads, that latency cost
is typically worse than an occasional unnecessary migration.

That said, I agree this is worth calling out explicitly in the commit
message. I'll update it accordingly in v2.

Thanks again for the careful review.

Best regards,
yangsonghua

杨送华 <yangsonghua@lixiang.com> 于2026年6月25日周四 10:13写道:
>
> Hi Prateek,   Thank you for the review!   > next can be NULL if we are coming here from sched_change_begin()   > for the current task [...] NULL pointer dereference !!!   You are absolutely right. put_prev_task() in sched.h always passes   NULL as 'next':     static inline void put_prev_task(struct rq *rq, struct task_struct *prev)     {         prev->sched_class->put_prev_task(rq, prev, NULL);     }   So when sched_change_begin() calls put_prev_task() for the currently   running RT task (e.g. during a priority/affinity change), next is NULL   and dl_task(next) will dereference it. I'll fix this with a NULL guard:     if (next && dl_task(next))         rt_queue_push_tasks(rq);   > If there are short running deadline tasks, we'll unnecessarily push   > a task and pull it back later. Is that not a concern?   This is a valid concern. The push is asynchronous (queued via IPI), so   there is an inherent delay before the RT task actually migrates. If the   DL task finishes before the push executes, the push becomes a no-op   since rt_queue_push_tasks() checks has_pushable_tasks() and the RT   task will have already been re-selected on this CPU.   If the RT task does get migrated and the DL task then exits shortly   after, pull_rt_task() triggered from the DL task's dequeue path will   bring it back. This round-trip is not free, but it is the same   trade-off already accepted in __schedule() when a DL task preempts   an RT task there.   The alternative -- never pushing -- means that while a DL task runs,   other CPUs may sit idle or run lower-priority work while a migratable   RT task is stuck waiting. For real-time workloads, that latency cost   is typically worse than an occasional unnecessary migration.   That said, I agree this is worth calling out explicitly in the commit   message. I'll update it accordingly in v2.   Thanks again for the careful review.   Best regards,   yangsonghua > From: "K Prateek Nayak"<kprateek.nayak@amd.com> > Date:  Wed, Jun 24, 2026, 11:53 > Subject:  Re: [PATCH] sched/rt: Push RT tasks when preempted by a deadline task > To: "yangsonghua"<jluyangsonghua@gmail.com>, "Ingo Molnar"<mingo@redhat.com>, "Peter Zijlstra"<peterz@infradead.org> > Cc: "Juri Lelli"<juri.lelli@redhat.com>, "Vincent Guittot"<vincent.guittot@linaro.org>, "Dietmar Eggemann"<dietmar.eggemann@arm.com>, "Steven Rostedt"<rostedt@goodmis.org>, "Ben Segall"<bsegall@google.com>, "Valentin Schneider"<vschneid@redhat.com>, <linux-kernel@vger.kernel.org>, "yangsonghua"<yangsonghua@lixiang.com> > Hello Yangsonghua,  >  > On 6/24/2026 8:34 AM, yangsonghua wrote: > > @@ -1736,14 +1737,24 @@ 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. > > +        * > > +        * rt_queue_push_tasks() guards on has_pushable_tasks() internally, > > +        * so this is a no-op if nothing is queued. > > +        */ > > +       if (dl_task(next)) > > +               rt_queue_push_tasks(rq); >  > next can be NULL if we are are coming here from sched_change_begin() > for the current task: >  >   sched_change_begin() >     put_prev_task() >       put_prev_task_rt(rq, prev, next = NULL) >         dl_task(next) >           dl_prio(next->prio) !!! NULL pointer dereference !!! >  > Do we avoid this in some way? >  > I don't know if this push was intentionally avoided or not. If there > are short running deadline task, we'll unnecessarily push a task and > pull it back later. Is that not a concern? >  > --  > Thanks and Regards, > Prateek >
>
> 声明:这封邮件只允许文件接收者阅读,有很高的机密性要求。禁止其他人使用、打开、复制或转发里面的任何内容。如果本邮件错误地发给了你,请联系邮件发出者并删除这个文件。机密及法律的特权并不因为误发邮件而放弃或丧失。任何提出的观点或意见只属于作者的个人见解,并不一定代表本公司。
> Disclaimer: This email is intended to be read only by the designated recipient of the document and has high confidentiality requirements. Anyone else is prohibited from using, opening, copying or forwarding any of the contents inside. If this email was sent to you by mistake, please contact the sender of the email and delete this file immediately. Confidentiality and legal privileges are not waived or lost by misdirected emails. Any views or opinions expressed in the email are those of the author and do not necessarily represent those of the Company.
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/rt: Push RT tasks when preempted by a deadline task
  2026-06-24  3:53 ` K Prateek Nayak
       [not found]   ` <5213745e18453d424bea3d94af6bd31c5fc87620.a5e26565.899d.42c6.84f0.cf74cdb1acdd@feishu.cn>
@ 2026-06-25  3:09   ` yangsonghua
  1 sibling, 0 replies; 4+ messages in thread
From: yangsonghua @ 2026-06-25  3:09 UTC (permalink / raw)
  To: K Prateek Nayak
  Cc: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Valentin Schneider,
	linux-kernel, yangsonghua

Hi Prateek,

Sorry, my previous reply had formatting issues. Please ignore that email.

You are absolutely right. put_prev_task() can reach put_prev_task_rt()
with next == NULL via sched_change_begin(), so dl_task(next) would indeed
be unsafe. I'll fix this with a NULL guard:

  if (next && dl_task(next))
      rt_queue_push_tasks(rq);

Your concern about short-lived DL tasks causing an unnecessary push/pull
cycle is also valid. My understanding is that this is the same trade-off
already accepted in __schedule() when a DL task preempts an RT task there,
but I agree this should be made explicit in the commit message. I'll update
that in v2.

Thanks again for the careful review.

Best regards,
Yangsonghua

K Prateek Nayak <kprateek.nayak@amd.com> 于2026年6月24日周三 11:53写道:
>
> Hello Yangsonghua,
>
> On 6/24/2026 8:34 AM, yangsonghua wrote:
> > @@ -1736,14 +1737,24 @@ 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.
> > +        *
> > +        * rt_queue_push_tasks() guards on has_pushable_tasks() internally,
> > +        * so this is a no-op if nothing is queued.
> > +        */
> > +       if (dl_task(next))
> > +               rt_queue_push_tasks(rq);
>
> next can be NULL if we are are coming here from sched_change_begin()
> for the current task:
>
>   sched_change_begin()
>     put_prev_task()
>       put_prev_task_rt(rq, prev, next = NULL)
>         dl_task(next)
>           dl_prio(next->prio) !!! NULL pointer dereference !!!
>
> Do we avoid this in some way?
>
> I don't know if this push was intentionally avoided or not. If there
> are short running deadline task, we'll unnecessarily push a task and
> pull it back later. Is that not a concern?
>
> --
> Thanks and Regards,
> Prateek
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-25  3:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-24  3:04 [PATCH] sched/rt: Push RT tasks when preempted by a deadline task yangsonghua
2026-06-24  3:53 ` K Prateek Nayak
     [not found]   ` <5213745e18453d424bea3d94af6bd31c5fc87620.a5e26565.899d.42c6.84f0.cf74cdb1acdd@feishu.cn>
2026-06-25  2:26     ` Eric yang
2026-06-25  3:09   ` yangsonghua

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®