All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	sched-ext@lists.linux.dev, Emil Tsalapatis <emil@etsalapatis.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH sched_ext/for-7.1-fixes] tools/sched_ext: scx_qmap: Silence task_ctx lookup miss
Date: Tue, 21 Apr 2026 08:47:03 +0200	[thread overview]
Message-ID: <aecdZ0k0c02i4t3w@gpd4> (raw)
In-Reply-To: <59bc5171ee5aa02746c2f576d0f1e14f@kernel.org>

Hi Tejun,

On Mon, Apr 20, 2026 at 08:13:09PM -1000, Tejun Heo wrote:
> scx_fork() dispatches ops.init_task to exactly one scheduler - the one
> owning the forking task's cgroup. A task forked inside a sub-scheduler's
> cgroup is init'd into the sub only; the root scheduler has no task_ctx
> entry for it. When that task later appears as @prev in the root's
> qmap_dispatch() (or flows through core-sched comparison via task_qdist),
> the bpf_task_storage_get() legitimately misses.
> 
> qmap treated those misses as fatal via scx_bpf_error("task_ctx lookup
> failed") and aborted the scheduler as soon as the first cross-sched
> task hit the root. Drop the error in the three sites where the miss is
> legitimate: lookup_task_ctx() (helper; callers already check for NULL),
> qmap_dispatch()'s @prev branch (bookkeeping-only), and task_qdist()
> (returns 0 which makes the comparison a no-op). The existing scx_error
> was a paranoid guard from the pre-sub-sched world where every task was
> owned by the one and only scheduler.
> 
> Fixes: 4f8b122848db ("sched_ext: Add basic building blocks for nested sub-scheduler dispatching")
> Signed-off-by: Tejun Heo <tj@kernel.org>

Should we return prev_cpu in qmap_select_cpu() instead of -ESRCH when
lookup_task_ctx() returns NULL? Otherwise the scheduler would error.

Apart than that looks good to me.

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

Thanks,
-Andrea

> ---
>  tools/sched_ext/scx_qmap.bpf.c | 22 +++++-----------------
>  1 file changed, 5 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c
> index b68abb9e760b..0330ba15210d 100644
> --- a/tools/sched_ext/scx_qmap.bpf.c
> +++ b/tools/sched_ext/scx_qmap.bpf.c
> @@ -159,13 +159,7 @@ static s32 pick_direct_dispatch_cpu(struct task_struct *p, s32 prev_cpu)
> 
>  static struct task_ctx *lookup_task_ctx(struct task_struct *p)
>  {
> -	struct task_ctx *tctx;
> -
> -	if (!(tctx = bpf_task_storage_get(&task_ctx_stor, p, 0, 0))) {
> -		scx_bpf_error("task_ctx lookup failed");
> -		return NULL;
> -	}
> -	return tctx;
> +	return bpf_task_storage_get(&task_ctx_stor, p, 0, 0);
>  }
> 
>  s32 BPF_STRUCT_OPS(qmap_select_cpu, struct task_struct *p,
> @@ -540,13 +534,9 @@ void BPF_STRUCT_OPS(qmap_dispatch, s32 cpu, struct task_struct *prev)
>  	 */
>  	if (prev) {
>  		tctx = bpf_task_storage_get(&task_ctx_stor, prev, 0, 0);
> -		if (!tctx) {
> -			scx_bpf_error("task_ctx lookup failed");
> -			return;
> -		}
> -
> -		tctx->core_sched_seq =
> -			core_sched_tail_seqs[weight_to_idx(prev->scx.weight)]++;
> +		if (tctx)
> +			tctx->core_sched_seq =
> +				core_sched_tail_seqs[weight_to_idx(prev->scx.weight)]++;
>  	}
>  }
> 
> @@ -584,10 +574,8 @@ static s64 task_qdist(struct task_struct *p)
>  	s64 qdist;
> 
>  	tctx = bpf_task_storage_get(&task_ctx_stor, p, 0, 0);
> -	if (!tctx) {
> -		scx_bpf_error("task_ctx lookup failed");
> +	if (!tctx)
>  		return 0;
> -	}
> 
>  	qdist = tctx->core_sched_seq - core_sched_head_seqs[idx];
> 
> --
> 2.53.0

  reply	other threads:[~2026-04-21  6:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21  6:13 [PATCH sched_ext/for-7.1-fixes] tools/sched_ext: scx_qmap: Silence task_ctx lookup miss Tejun Heo
2026-04-21  6:47 ` Andrea Righi [this message]
2026-04-21  7:17 ` [PATCH v2 " Tejun Heo
2026-04-21  8:40   ` Zhao Mengmeng
2026-04-21 16:39   ` 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=aecdZ0k0c02i4t3w@gpd4 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.