mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kayra Cizmeci <kayracizmeci@gmail.com>
To: "Shubhang Kaushik (Ampere)" <sh@gentwo.org>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	 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>, Mel Gorman <mgorman@suse.de>,
	 Valentin Schneider <vschneid@redhat.com>,
	K Prateek Nayak <kprateek.nayak@amd.com>,
	 Shubhang Kaushik <shubhang@os.amperecomputing.com>,
	"Christoph Lameter (Ampere)" <cl@gentwo.org>,
	 linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] sched/rt,dl: Remove unreachable migration-disabled push checks
Date: Mon, 19 Oct 2026 20:40:39 +0300	[thread overview]
Message-ID: <atZR51V-xwaRL6OL@scheduler.schedules> (raw)
In-Reply-To: <20261014-sched-rt-dead-md-push-v2-1-9f57f1a530dc@gentwo.org>

Hi Shubhang :-),

On Wed, Oct 14, 2026 at 03:32:10PM -0700, Shubhang Kaushik (Ampere) wrote:
> push_rt_task() and push_dl_task() select their candidates with
> pick_next_pushable_task() and pick_next_pushable_dl_task(),
> respectively. Both helpers skip migration-disabled tasks.
> 
> The source rq lock is held after candidate selection. The selected task
> is queued, not executing, and cannot enter a migration-disabled region
> while that lock is held.
> 
> find_lock_lowest_rq() and find_lock_later_rq() can drop the source rq
> lock. If that happens, they recheck is_migration_disabled() after
> reacquiring the locks and reject a task that changed state. Their
> callers then either exit or retry with a task selected by the same
> helper, which again excludes migration-disabled tasks.
> 
> The migration-disabled fallback in push_rt_task() and the corresponding
> check in push_dl_task() are therefore unreachable. Remove them. The RT
> fallback is the only user of the @pull argument, so remove that
> argument too.
> 
> This does not change the migration-disabled handling in pull_rt_task()
> or pull_dl_task(), which use different candidate selectors and retain
> their existing donor-push recovery.
> 
> Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
> ---
> Tested on 80-core SMP, non-SMT Ampere Altra system:
> 
>   - Built and booted with CONFIG_IRQ_WORK=y.
>   - Ran repeated SCHED_FIFO affinity changes among CPUs 78 and 79 with
>     RT_PUSH_IPI enabled and disabled; both runs completed and cleaned
>     up all worker tasks.
>   - With RT_PUSH_IPI enabled, function tracing confirmed repeated
>     execution of rto_push_irq_work_func() via irq_work_single().
> 
> Changes in v2:
>   - Also remove the unreachable migration-disabled check in push_dl_task().
>   - Clarify that both push paths exclude migration-disabled tasks.
> 
> Link to v1: https://lore.kernel.org/r/20261014-sched-rt-dead-md-push-v1-1-5f9757be3c80@gentwo.org
> ---
>  kernel/sched/deadline.c |  3 ---
>  kernel/sched/rt.c       | 48 +++---------------------------------------------
>  2 files changed, 3 insertions(+), 48 deletions(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 0663c00c41c04213fda2ebaf16a0bdf34e0849a8..e9d952853f14aaf48cdca6f89135090adee5f6ba 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -3158,9 +3158,6 @@ static int push_dl_task(struct rq *rq)
>  		return 0;
>  	}
>  
> -	if (is_migration_disabled(next_task))
> -		return 0;
> -
>  	if (WARN_ON(next_task == rq->curr))
>  		return 0;

I sadly started writing from the diff below. But again,
pick_next_pushable_dl_task() already can't return a task with
is_migration_disabled() true.

>  
> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
> index 85303add726df4c2da846c427aab2d627fca9589..f85edc6f896c72d2251b5eb59ee7ffd0e23934f5 100644
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1957,7 +1957,7 @@ static struct rq *find_lock_lowest_rq(struct task_struct *task, struct rq *rq)
>   * running task can migrate over to a CPU that is running a task
>   * of lesser priority.
>   */
> -static int push_rt_task(struct rq *rq, bool pull)
> +static int push_rt_task(struct rq *rq)
>  {
>  	struct task_struct *next_task;
>  	struct rq *lowest_rq;
> @@ -1981,48 +1981,6 @@ static int push_rt_task(struct rq *rq, bool pull)
>  		return 0;
>  	}
>  
> -	if (is_migration_disabled(next_task)) {
> -		struct task_struct *push_task = NULL;
> -		int cpu;

Yeah well. This is never true since pick_next_pushable_task() does not
returns a task with is_migration_disabled() true AFAICT tho.

> -
> -		if (!pull || rq->push_busy)
> -			return 0;
> -
> -		/*
> -		 * Invoking find_lowest_rq() on anything but an RT task doesn't
> -		 * make sense. Per the above priority check, curr has to
> -		 * be of higher priority than next_task, so no need to
> -		 * reschedule when bailing out.
> -		 *
> -		 * Note that the stoppers are masqueraded as SCHED_FIFO
> -		 * (cf. sched_set_stop_task()), so we can't rely on rt_task().
> -		 */
> -		if (rq->donor->sched_class != &rt_sched_class)
> -			return 0;
> -
> -		cpu = find_lowest_rq(rq->curr);
> -		if (cpu == -1 || cpu == rq->cpu)
> -			return 0;
> -
> -		/*
> -		 * Given we found a CPU with lower priority than @next_task,
> -		 * therefore it should be running. However we cannot migrate it
> -		 * to this other CPU, instead attempt to push the current
> -		 * running task on this CPU away.
> -		 */
> -		push_task = get_push_task(rq);
> -		if (push_task) {
> -			preempt_disable();
> -			raw_spin_rq_unlock(rq);
> -			stop_one_cpu_nowait(rq->cpu, push_cpu_stop,
> -					    push_task, &rq->push_work);
> -			preempt_enable();
> -			raw_spin_rq_lock(rq);
> -		}
> -
> -		return 0;
> -	}
> -
>  	if (WARN_ON(next_task == rq->curr))
>  		return 0;
>  
> @@ -2078,7 +2036,7 @@ static int push_rt_task(struct rq *rq, bool pull)
>  static void push_rt_tasks(struct rq *rq)
>  {
>  	/* push_rt_task will return true if it moved an RT */
> -	while (push_rt_task(rq, false))
> +	while (push_rt_task(rq))
>  		;
>  }
>  
> @@ -2236,7 +2194,7 @@ void rto_push_irq_work_func(struct irq_work *work)
>  	 */
>  	if (has_pushable_tasks(rq)) {
>  		raw_spin_rq_lock(rq);
> -		while (push_rt_task(rq, true))
> +		while (push_rt_task(rq))
>  			;
>  		raw_spin_rq_unlock(rq);
>  	}
> 
> ---
> base-commit: 0fcb05b72344f40806cbe810de8872f1698bf053

Someone who writes their base :-)

> change-id: 20261014-sched-rt-dead-md-push-c88b47c3e2f0
> 
> Best regards,
> -- 
> Shubhang Kaushik (Ampere) <sh@gentwo.org>
>

Reviewed-by: Kayra Cizmeci <kayracizmeci@gmail.com>


      parent reply	other threads:[~2026-10-19 17:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 22:32 Shubhang Kaushik (Ampere)
2026-10-19  7:35 ` Christian Loehle
2026-10-19 17:40 ` Kayra Cizmeci [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atZR51V-xwaRL6OL@scheduler.schedules \
    --to=kayracizmeci@gmail.com \
    --cc=bsegall@google.com \
    --cc=cl@gentwo.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sh@gentwo.org \
    --cc=shubhang@os.amperecomputing.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®