From: Andrea Righi <arighi@nvidia.com>
To: Hui Su <sh_def@163.com>
Cc: tj@kernel.org, void@manifault.com, changwoo@igalia.com,
mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com,
vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de,
vschneid@redhat.com, kprateek.nayak@amd.com,
sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org,
bpf@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] sched_ext: idle: Respect task affinity in restricted CPU selection
Date: Sun, 11 Oct 2026 09:02:13 +0200 [thread overview]
Message-ID: <ass0dexwariQCOtC@gpd4> (raw)
In-Reply-To: <20261011025256.4180374-1-sh_def@163.com>
Hi Hui,
On Sun, Oct 11, 2026 at 11:52:56AM +0900, Hui Su wrote:
> The restricted select_cpu kfunc path handles single-CPU and
> migration-disabled tasks without going through scx_select_cpu_dfl().
>
> When a caller supplies @allowed, the path currently checks prev_cpu only
> against that mask. It can therefore return an idle prev_cpu which isn't
> in p->cpus_ptr.
>
> Require prev_cpu to belong to both p->cpus_ptr and the caller-provided
> mask before consuming its idle state.
>
> Fixes: 683d2d0faba1 ("sched_ext: idle: Introduce scx_bpf_select_cpu_and()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hui Su <sh_def@163.com>
The fix makes sense, but I'm wondering if we should derive the candidate CPU
from the task's state instead of relying on prev_cpu. In practice prev_cpu
should be always considered a hint rather than a guarantee that the task can
still run there.
Per-CPU tasks could use cpumask_first(p->cpus_ptr) and migration-disabled tasks
could use task_cpu(p) directly.
Something like this maybe:
migration_disabled = is_bpf_migration_disabled(p);
if (p->nr_cpus_allowed == 1 || migration_disabled) {
if (migration_disabled)
cpu = task_cpu(p);
else
cpu = cpumask_first(p->cpus_ptr);
if ((migration_disabled &&
!cpumask_test_cpu(cpu, p->cpus_ptr)) ||
(allowed && !cpumask_test_cpu(cpu, allowed)) ||
!scx_idle_test_and_clear_cpu(cpu))
cpu = -EBUSY;
} else {
cpu = scx_select_cpu_dfl(p, prev_cpu, wake_flags,
allowed ?: p->cpus_ptr, flags);
}
This would let us try the task's only allowed CPU after an affinity change,
instead of rejecting a stale/invalid prev_cpu.
And could you also clarify in the patch description that selecting the CPU
consumes its idle bit? Without the affinity check, we can claim an idle CPU that
the task cannot use and later dispatch validation doesn't undo that side effect.
It would also help to mention that per-CPU and migration-disabled tasks skip
ops.select_cpu(), so this affects calls from other contexts, such as
ops.enqueue(), leaving the normal ops.select_cpu() path unaffected.
What do you think?
Thanks,
-Andrea
> ---
> Testing:
> - x86_64 KVM/QEMU.
> - A private regression harness pinned the task to CPU 0 and called
> scx_bpf_select_cpu_and() with an idle CPU 1 outside the task affinity
> but inside the caller-provided mask.
> - On the baseline kernel, the helper returned CPU 1.
> - With this fix, the helper rejected the out-of-affinity CPU.
> - The existing allowed_cpus selftest passed with the fix.
>
> kernel/sched/ext/idle.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/sched/ext/idle.c b/kernel/sched/ext/idle.c
> index aa9fb6de0ad6..20efa463cafa 100644
> --- a/kernel/sched/ext/idle.c
> +++ b/kernel/sched/ext/idle.c
> @@ -999,7 +999,8 @@ static s32 select_cpu_from_kfunc(struct scx_sched *sch, struct task_struct *p,
> * used CPU is idle and within the allowed cpumask.
> */
> if (p->nr_cpus_allowed == 1 || is_bpf_migration_disabled(p)) {
> - if (cpumask_test_cpu(prev_cpu, allowed ?: p->cpus_ptr) &&
> + if (cpumask_test_cpu(prev_cpu, p->cpus_ptr) &&
> + (!allowed || cpumask_test_cpu(prev_cpu, allowed)) &&
> scx_idle_test_and_clear_cpu(prev_cpu))
> cpu = prev_cpu;
> else
>
> base-commit: 7b63ef2d55f24519e7e9e5f4d15dbea03f126e40
>
next prev parent reply other threads:[~2026-10-11 7:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-11 2:52 [PATCH] sched_ext: idle: Respect task affinity in restricted CPU selection Hui Su
2026-10-11 7:02 ` Andrea Righi [this message]
2026-10-11 9:10 ` Hui Su
2026-10-11 13:17 ` [PATCH v2] " Hui Su
2026-10-11 14:39 ` Andrea Righi
2026-10-11 17:09 ` Hui Su
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=ass0dexwariQCOtC@gpd4 \
--to=arighi@nvidia.com \
--cc=bpf@vger.kernel.org \
--cc=bsegall@google.com \
--cc=changwoo@igalia.com \
--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=sched-ext@lists.linux.dev \
--cc=sh_def@163.com \
--cc=stable@vger.kernel.org \
--cc=tj@kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=void@manifault.com \
--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 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.