mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: sched-ext@lists.linux.dev, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	Emil Tsalapatis <emil@etsalapatis.com>,
	David Dai <david.dai@linux.dev>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH sched_ext/for-7.4-fixes] sched_ext: Fix the stall watchdog testing and blaming the wrong sched under bypass
Date: Tue, 27 Oct 2026 03:58:22 +0100	[thread overview]
Message-ID: <auATTu6UWiFYGj5e@gpd4> (raw)
In-Reply-To: <20261026204300.3618150-1-tj@kernel.org>

On Mon, Oct 26, 2026 at 02:08:59PM -1000, Tejun Heo wrote:
> While a sched is bypassing, its tasks are run by its host, the nearest
> ancestor that isn't bypassing. check_rq_for_timeouts() tests a runnable task
> against its owner's timeout. Since 2c91169377f4 ("sched_ext: Blame the DSQ's
> owning scheduler for a runnable stall"), it ejects the sched owning the DSQ
> the task waits on. The sched tested, the sched ejected and the sched running
> the task can all differ.
> 
> - Timeouts are per sched. A bypassing sub-scheduler with a 2s timeout under
>   a root with the default 30s has its tasks on the root's bypass DSQ. The
>   root, not bypassing itself, consumes that DSQ before calling
>   ops.dispatch() only on every other pass, and on the others only after
>   ops.dispatch() yields nothing. A task that waits 2s there ejects the root,
>   disabling all of sched_ext.
> 
> - A bypassing owner's task on the local DSQ is blamed on the owner, which
>   places nothing while bypassing. If the owner is already exiting, e.g.
>   during its own disable, the exit is a no-op and nothing is ejected. If the
>   host isn't running the task, the owner's disable, which waits on it, may
>   never finish.
> 
> - The wait isn't restarted when bypass moves a task. The time it waited
>   under its owner counts against the host once it is on the host's bypass
>   DSQ, and its time there against the owner once bypass lifts.
> 
> The sched running a task is its owner, or the host while the owner is
> bypassing. Test the task against that sched's timeout and blame it. A bypass
> requeue can change the sched running a task, so restart the wait of every
> task scx_bypass() requeues except the root's own, which the root runs
> bypassing or not. Single-scheduler behavior is unchanged.
> 
> A task that migrates while scx_bypass() works through the cpus, leaving one
> before its requeue and arriving on another after it, is requeued by neither
> and keeps its wait, which then counts against the sched now running it. That
> is what happened to every task before this patch. This might be worth
> addressing in the future, but the race is rare and the real-world
> implications should be limited.
> 
> Fixes: 2c91169377f4 ("sched_ext: Blame the DSQ's owning scheduler for a runnable stall")
> Cc: stable@vger.kernel.org # v7.3+: 30fb2b1e4d41: sched_ext: Fix the running task keeping its protection into bypass
> Cc: stable@vger.kernel.org # v7.3+: 5809f749fbdb: sched_ext: Fix a bypassed task stalling on a cpu its scheduler has no caps on
> Cc: stable@vger.kernel.org # v7.3+
> Reported-by: Andrea Righi <arighi@nvidia.com>
> Link: https://lore.kernel.org/all/at4l0yXTk7T_ouk0@gpd4/
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Tejun Heo <tj@kernel.org>

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> ---
>  kernel/sched/ext/ext.c | 38 +++++++++++++++++++++++---------------
>  1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 4b7d2602b4d4..a9621bbdb183 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -2458,7 +2458,8 @@ static void ops_dequeue(struct rq *rq, struct task_struct *p, u64 deq_flags)
>  	/*
>  	 * Don't reset runnable_at. If @p is waiting, the wait continues across
>  	 * temporary dequeues, e.g. for a migration or an attribute change.
> -	 * scx_disable_task() ends it if @p leaves its scheduler.
> +	 * scx_disable_task() ends it if @p leaves its scheduler. scx_bypass()
> +	 * restarts it when its requeue can change the sched running @p.
>  	 */
>  	clr_task_runnable(p, false);
>  
> @@ -4298,24 +4299,17 @@ static bool check_rq_for_timeouts(struct rq *rq)
>  		goto out_unlock;
>  
>  	list_for_each_entry(p, &rq->scx.runnable_list, scx.runnable_node) {
> -		struct scx_sched *sch = scx_task_sched(p);
> +		/*
> +		 * @p's owner schedules @p, or its host while the owner is
> +		 * bypassing. Test @p against that sched's timeout and blame it.
> +		 */
> +		struct scx_sched *sch = scx_exec_sched(scx_task_sched(p), cpu_of(rq));
>  		unsigned long last_runnable = p->scx.runnable_at;
>  
>  		if (unlikely(time_after(jiffies,
>  					last_runnable + READ_ONCE(sch->watchdog_timeout)))) {
> -			struct scx_dispatch_q *dsq = READ_ONCE(p->scx.dsq);
>  			u32 dur_ms = jiffies_to_msecs(jiffies - last_runnable);
>  
> -			/*
> -			 * A task can be stuck on a DSQ that a sched other than
> -			 * its owner is responsible for draining, e.g. an
> -			 * ancestor's bypass DSQ while the owner is bypassing.
> -			 * Blame the drainer. The local DSQ is consumed by the
> -			 * cpu itself and keeps blame on the owner.
> -			 */
> -			if (dsq && dsq->sched && dsq->id != SCX_DSQ_LOCAL)
> -				sch = dsq->sched;
> -
>  			__scx_exit(sch, SCX_EXIT_ERROR_STALL, 0, cpu_of(rq),
>  				   "%s[%d] failed to run for %u.%03us",
>  				   p->comm, p->pid, dur_ms / 1000,
> @@ -6976,12 +6970,26 @@ void scx_bypass(struct scx_sched *sch, bool bypass)
>  		 */
>  		list_for_each_entry_safe_reverse(p, n, &rq->scx.runnable_list,
>  						 scx.runnable_node) {
> -			if (!scx_is_descendant(scx_task_sched(p), sch))
> +			struct scx_sched *owner = scx_task_sched(p);
> +
> +			if (!scx_is_descendant(owner, sch))
>  				continue;
>  
>  			/* cycling deq/enq is enough, see the function comment */
>  			scoped_guard (sched_change, p, DEQUEUE_SAVE | DEQUEUE_MOVE) {
> -				/* nothing */ ;
> +				/*
> +				 * The requeue can change the sched running @p,
> +				 * and the watchdog counts @p's wait against
> +				 * that sched. Restart the wait unless @owner is
> +				 * the root, which runs its own tasks bypassing
> +				 * or not.
> +				 *
> +				 * A dispatch in flight writes p->scx.flags
> +				 * without @p's rq lock. The dequeue waits it
> +				 * out, so set the flag after it.
> +				 */
> +				if (scx_parent(owner))
> +					p->scx.flags |= SCX_TASK_RESET_RUNNABLE_AT;
>  			}
>  		}
>  
> -- 
> 2.56.0
> 
> 

  reply	other threads:[~2026-10-27  2:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-27  0:08 Tejun Heo
2026-10-27  2:58 ` Andrea Righi [this message]
2026-10-27  7:11 ` Tejun Heo

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=auATTu6UWiFYGj5e@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=david.dai@linux.dev \
    --cc=emil@etsalapatis.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=void@manifault.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®