* [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable()
@ 2026-09-19 0:28 Tejun Heo
2026-09-19 0:28 ` [PATCH 1/2] " Tejun Heo
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tejun Heo @ 2026-09-19 0:28 UTC (permalink / raw)
To: David Vernet, Andrea Righi, Changwoo Min
Cc: Emil Tsalapatis, David Dai, sched-ext, linux-kernel, Tejun Heo
Hello,
v3: - The initial set_cmask() is delivered before set_weight() so that the
mask is in place when weight-dependent state is derived (Andrea Righi).
- Added a selftest checking the cmask enable() receives, the set_cmask()
that follows it and their ordering (Andrea Righi).
v2: https://lore.kernel.org/r/2f0eb5c762d55edba5365f1006dbb395@kernel.org
v1: https://lore.kernel.org/r/c8343c2e457508a6cd3fcab66b0823b6@kernel.org
A cid-form scheduler has no p->cpus_ptr equivalent to read. A task's cid
mask is only visible through ops.set_cmask(), which doesn't fire when a task
enters a scheduler through fork, sub-sched enable or re-home, so schedulers
seed the mask from p->cpus_ptr in ops.init_task(), and that seed goes stale
when affinity changes between init_task() and enable().
The first patch passes the initial cmask to cid-form ops.enable() through a
new args struct and has set_cmask() repeat it right after, so a scheduler can
track affinity in set_cmask() alone. The second adds a selftest for it.
Verified by building the kernel, tools/sched_ext and the selftests. The new
selftest passes on a 16-CPU VM, standalone three times and inside the full
sched_ext suite. scx_qmap ran under fork churn with a throwaway enable()
check comparing the delivered mask against p->cpus_ptr on about 55k enables
with no mismatch.
Based on sched_ext/for-7.3-fixes (9ec7ba20c97d).
This patchset contains the following 2 patches.
0001 sched_ext: Pass the initial cmask to cid-form ops.enable()
0002 selftests/sched_ext: Check the cmask cid-form ops.enable() receives
The patchset is also available in the following git branch:
git://git.kernel.org/pub/scm/linux/kernel/git/tj/sched_ext.git cid-enable-args-v3
diffstat follows. Thanks.
kernel/sched/ext/ext.c | 92 ++++++++++++++-------
kernel/sched/ext/internal.h | 56 +++++++++++--
tools/sched_ext/scx_qmap.bpf.c | 3 -
tools/testing/selftests/sched_ext/Makefile | 1 +
tools/testing/selftests/sched_ext/enable_cmask.bpf.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++
tools/testing/selftests/sched_ext/enable_cmask.c | 138 ++++++++++++++++++++++++++++++++
6 files changed, 462 insertions(+), 41 deletions(-)
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] sched_ext: Pass the initial cmask to cid-form ops.enable() 2026-09-19 0:28 [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo @ 2026-09-19 0:28 ` Tejun Heo 2026-09-19 12:21 ` Andrea Righi 2026-09-19 0:28 ` [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives Tejun Heo 2026-09-19 14:25 ` [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo 2 siblings, 1 reply; 7+ messages in thread From: Tejun Heo @ 2026-09-19 0:28 UTC (permalink / raw) To: David Vernet, Andrea Righi, Changwoo Min Cc: Emil Tsalapatis, David Dai, sched-ext, linux-kernel, Tejun Heo The cid-form API has an obvious hole. A task's cid mask is only visible through ops.set_cmask(), which fires on affinity changes and class switches but not when a task enters a scheduler through fork, sub-sched enable or re-home, and there is no p->cpus_ptr equivalent to fall back on. Schedulers work around it by seeding the mask in ops.init_task() from p->cpus_ptr cid by cid, which is subtly wrong: on sub-sched enable and re-home, an affinity change between init_task() and enable() is delivered to the sched the task is still on, and nothing corrects the new sched's copy afterwards. Fix it by adding struct scx_enable_args to cid-form ops.enable() carrying the task's cmask, built in the per-cpu scratch under the rq lock as the task enters the scheduler, and calling set_cmask() with the same mask right after enable(), ahead of set_weight(). A scheduler can then track affinity in set_cmask() alone, and scx_qmap drops its init_task() seed. set_cmask() no longer fires for a cid-form task before it is enabled, and the class-switch republish in switching_to_scx() is limited to the cpu form. This changes the cid-form ops.enable() signature, which is fine as the cid-form API is still considered unreleased. An args struct rather than a bare cmask argument leaves room for more initial state without another signature change, and the cmask travels as a plain arena address because BTF can't type arena struct members yet. v2: The cmask travels as a u64 arena address, cmask_arena_addr, instead of a kernel-typed pointer, with the typing limitation and the planned typed alias documented (Sashiko review). v3: The initial set_cmask() is delivered before set_weight() so that the mask is in place when weight-dependent state is derived (Andrea Righi). Selftest added. Signed-off-by: Tejun Heo <tj@kernel.org> --- kernel/sched/ext/ext.c | 92 +++++++++++++++++++++++----------- kernel/sched/ext/internal.h | 56 ++++++++++++++++++--- tools/sched_ext/scx_qmap.bpf.c | 3 -- 3 files changed, 110 insertions(+), 41 deletions(-) diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c index f568fd9973f6..ad391a8cbd05 100644 --- a/kernel/sched/ext/ext.c +++ b/kernel/sched/ext/ext.c @@ -447,37 +447,45 @@ static void switch_rq_lock(struct rq *from, struct rq *to) DEFINE_STATIC_KEY_FALSE(__scx_is_cid_type); /** - * scx_call_op_set_cpumask - invoke ops.set_cpumask / ops_cid.set_cmask for @task + * scx_fill_cmask_scratch - Build this cpu's arena cmask from @cpumask + * @sch: scx_sched whose scratch to fill + * @cpumask: cpus to translate into cids + * + * The scratch lives in BPF-writable arena memory and its header can't be + * trusted, so it is rewritten from kernel geometry rather than read. Caller + * must hold an rq lock so this cpu is the sole kernel writer for as long as the + * returned address is in use. + */ +static struct scx_cmask *scx_fill_cmask_scratch(struct scx_sched *sch, + const struct cpumask *cpumask) +{ + struct scx_cmask *kern_va = *this_cpu_ptr(sch->set_cmask_scratch); + struct scx_cmask_ref ref; + + scx_cmask_ref_init_kern(sch, kern_va, 0, num_possible_cpus(), &ref); + scx_cmask_ref_from_cpumask(&ref, cpumask); + return kern_va; +} + +/** + * scx_call_op_set_cpumask - Invoke the set_cpumask or set_cmask op for @task * @sch: scx_sched being invoked * @rq: rq to update as the currently-locked rq, or NULL * @task: task whose affinity is changing * @cpumask: new cpumask * - * For cid-form schedulers, translate @cpumask to a cmask via the per-cpu - * scratch in cid.c and dispatch through the ops_cid union view. Caller - * must hold @rq's rq lock so this_cpu_ptr is stable across the call. + * For cid-form schedulers, translate @cpumask to a cmask in the per-cpu scratch + * and dispatch through the ops_cid union view. Caller must hold @rq's rq lock. */ static inline void scx_call_op_set_cpumask(struct scx_sched *sch, struct rq *rq, struct task_struct *task, const struct cpumask *cpumask) { - if (scx_is_cid_type()) { - struct scx_cmask *kern_va = *this_cpu_ptr(sch->set_cmask_scratch); - struct scx_cmask_ref ref; - - /* - * Build the per-cpu arena cmask from kernel geometry via @ref, - * never reading its BPF-writable header. set_cmask()'s __arena - * argument takes the kernel address and the struct_ops - * trampoline rebases it into BPF's arena pointer form. The rq - * lock makes this cpu the sole kernel writer. - */ - scx_cmask_ref_init_kern(sch, kern_va, 0, num_possible_cpus(), &ref); - scx_cmask_ref_from_cpumask(&ref, cpumask); - SCX_CALL_CID_OP_TASK(sch, set_cmask, rq, task, kern_va); - } else { + if (scx_is_cid_type()) + SCX_CALL_CID_OP_TASK(sch, set_cmask, rq, task, + scx_fill_cmask_scratch(sch, cpumask)); + else SCX_CALL_OP_TASK(sch, set_cpumask, rq, task, cpumask); - } } enum scx_dsq_iter_flags { @@ -3634,8 +3642,12 @@ static void set_cpus_allowed_scx(struct task_struct *p, * * Fine-grained memory write control is enforced by BPF making the const * designation pointless. Cast it away when calling the operation. + * + * The cid form receives the initial mask when the task is enabled and + * hears about changes only afterwards, see struct scx_enable_args. */ - if (SCX_HAS_OP(sch, set_cpumask)) + if (SCX_HAS_OP(sch, set_cpumask) && + (!scx_is_cid_type() || scx_get_task_state(p) == SCX_TASK_ENABLED)) scx_call_op_set_cpumask(sch, task_rq(p), p, (struct cpumask *)p->cpus_ptr); } @@ -3944,8 +3956,27 @@ static void __scx_enable_task(struct scx_sched *sch, struct task_struct *p) p->scx.weight = sched_weight_to_cgroup(weight); - if (SCX_HAS_OP(sch, enable)) - SCX_CALL_OP_TASK(sch, enable, rq, p); + if (SCX_HAS_OP(sch, enable)) { + if (scx_is_cid_type()) { + struct scx_cmask *cmask = scx_fill_cmask_scratch(sch, p->cpus_ptr); + struct scx_enable_args args = { + .cmask_arena_addr = scx_kaddr_to_arena(sch, cmask), + }; + + SCX_CALL_CID_OP_TASK(sch, enable, rq, p, &args); + } else { + SCX_CALL_OP_TASK(sch, enable, rq, p); + } + } + + /* + * The initial mask also goes out through set_cmask() so a scheduler can + * track affinity there alone, and before set_weight() so that the mask + * is in place when weight-dependent state is derived, see struct + * scx_enable_args. + */ + if (scx_is_cid_type() && SCX_HAS_OP(sch, set_cpumask)) + scx_call_op_set_cpumask(sch, rq, p, p->cpus_ptr); if (SCX_HAS_OP(sch, set_weight)) SCX_CALL_OP_TASK(sch, set_weight, rq, p, p->scx.weight); @@ -4288,9 +4319,10 @@ static void switching_to_scx(struct rq *rq, struct task_struct *p) /* * set_cpus_allowed_scx() is not called while @p is associated with a - * different scheduler class. Keep the BPF scheduler up-to-date. + * different scheduler class. Keep the BPF scheduler up-to-date. The cid + * form gets its mask from scx_enable_task(). */ - if (SCX_HAS_OP(sch, set_cpumask)) + if (!scx_is_cid_type() && SCX_HAS_OP(sch, set_cpumask)) scx_call_op_set_cpumask(sch, rq, p, (struct cpumask *)p->cpus_ptr); } @@ -8374,10 +8406,11 @@ static struct bpf_struct_ops bpf_sched_ext_ops = { /* * cid-form cfi stubs. Stubs whose signatures match the cpu-form (param types * identical, only param names differ across structs) are reused. Some need - * fresh stubs, set_cmask due to an argument type difference and the sub-sched - * notifiers because no cpu-form stub exists to reuse. + * fresh stubs, set_cmask and enable due to argument differences and the + * sub-sched notifiers because no cpu-form stub exists to reuse. */ static void sched_ext_ops_cid__set_cmask(struct task_struct *p, const struct scx_cmask *cmask__arena) {} +static void sched_ext_ops_cid__enable(struct task_struct *p, struct scx_enable_args *args) {} static void sched_ext_ops__sub_caps_updated(const struct scx_cmask *cmask__arena, u64 caps) {} static void sched_ext_ops__sub_ecaps_updated(s32 cid, u64 before, u64 after) {} @@ -8398,7 +8431,7 @@ static struct sched_ext_ops_cid __bpf_ops_sched_ext_ops_cid = { .update_idle = sched_ext_ops__update_idle, .init_task = sched_ext_ops__init_task, .exit_task = sched_ext_ops__exit_task, - .enable = sched_ext_ops__enable, + .enable = sched_ext_ops_cid__enable, .disable = sched_ext_ops__disable, #ifdef CONFIG_EXT_GROUP_SCHED .cpuctl_init = sched_ext_ops__cgroup_init, @@ -10421,8 +10454,7 @@ __bpf_kfunc const void *scx_bpf_online_cmask(const struct bpf_prog_aux *aux) if (unlikely(!online)) return NULL; - /* BPF rebases by the low 32 bits, like __arena callback args */ - return (void *)((unsigned long)online - sch->arena_kern_base); + return (void *)scx_kaddr_to_arena(sch, online); } /** diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h index d150de10a5c9..1df8f583b0ec 100644 --- a/kernel/sched/ext/internal.h +++ b/kernel/sched/ext/internal.h @@ -250,6 +250,31 @@ struct scx_exit_task_args { bool cancelled; }; +/** + * struct scx_enable_args - Argument container for cid-form ops.enable() + * @cmask_arena_addr: BPF arena address of the cmask of cids the task may run on + * + * @cmask_arena_addr is the task's affinity as it enters the scheduler. + * set_cmask() delivers the same mask right after enable(), before set_weight() + * and the first enqueue, then every affinity change afterwards, and is never + * called before enable(). A scheduler may therefore track affinity in + * set_cmask() alone. + * + * The kernel builds the mask in the scheduler arena from its own geometry, so + * the header is valid regardless of what the scheduler last wrote there. The + * memory is per-cpu scratch reused once the callback returns: copy the bits + * out, don't keep the address. The set_cmask() argument follows the same rules. + * + * The address is a plain value rather than a typed pointer because BTF can't + * mark a struct member as an arena pointer yet and a pointer member would reach + * the program typed as a kernel pointer. Cast it to struct scx_cmask __arena * + * before use. Once arena members can be typed, a typed alias will join this + * field in an anonymous union at the same offset. + */ +struct scx_enable_args { + u64 cmask_arena_addr; +}; + /* argument container for ops.cgroup_init() */ struct scx_cgroup_init_args { /* the weight of the cgroup [1..10000] */ @@ -1037,6 +1062,7 @@ struct sched_ext_ops { * - dispatch -> dispatch (cpu arg is now cid) * - update_idle -> update_idle (cpu arg is now cid) * - set_cpumask -> set_cmask (cmask instead of cpumask) + * - enable -> enable (takes struct scx_enable_args) * - cpu_online -> cid_online * - cpu_offline -> cid_offline * - dump_cpu -> dump_cid @@ -1070,7 +1096,7 @@ struct sched_ext_ops_cid { struct scx_init_task_args *args); void (*exit_task)(struct task_struct *p, struct scx_exit_task_args *args); - void (*enable)(struct task_struct *p); + void (*enable)(struct task_struct *p, struct scx_enable_args *args); void (*disable)(struct task_struct *p); void (*dump)(struct scx_dump_ctx *ctx); void (*dump_cid)(struct scx_dump_ctx *ctx, s32 cid, bool idle); @@ -1533,7 +1559,8 @@ struct scx_sched { * by BUILD_BUG_ON in scx_init()). The anonymous union lets the kernel * access either view of the same storage without function-pointer * casts: use .ops for cpu-form and shared fields, .ops_cid for the - * cid-renamed callbacks (set_cmask, select_cid, cid_online, ...). + * callbacks whose cid-form signature differs (set_cmask, enable, + * select_cid, cid_online, ...). */ union { struct sched_ext_ops ops; @@ -1556,9 +1583,9 @@ struct scx_sched { uintptr_t arena_kern_base; /* - * Per-CPU arena cmask used by scx_call_op_set_cpumask() to hand a cmask - * to ops_cid.set_cmask(). The kernel writes through the stored kern_va - * and passes it to the callback's __arena argument. + * Per-CPU arena cmask the kernel fills from a task's cpumask and hands + * to ops_cid.enable() and ops_cid.set_cmask(). The stored pointers are + * the kernel addresses. */ struct scx_cmask * __percpu *set_cmask_scratch; struct scx_cmask *online_cmask; @@ -1669,6 +1696,19 @@ static inline void *scx_arena_to_kaddr(struct scx_sched *sch, const void *bpf_pt return (void *)(sch->arena_kern_base + (u32)(uintptr_t)bpf_ptr); } +/** + * scx_kaddr_to_arena - Translate a kernel arena address to the BPF form + * @sch: scheduler whose arena hosts @kaddr + * @kaddr: kernel address inside @sch's arena + * + * __arena callback arguments need no translation. Addresses handed to BPF any + * other way, such as struct fields and kfunc return values, go through this. + */ +static inline uintptr_t scx_kaddr_to_arena(struct scx_sched *sch, const void *kaddr) +{ + return (uintptr_t)kaddr - sch->arena_kern_base; +} + enum scx_wake_flags { /* expose select WF_* flags as enums */ SCX_WAKE_FORK = WF_FORK, @@ -2302,9 +2342,9 @@ do { \ } while (0) /* - * Dispatch a task op through the cid-form ops_cid table. Only set_cmask() needs - * this: it takes an arena cmask address instead of a cpumask, so it cannot be - * invoked via its cpu-form set_cpumask() slot. + * Dispatch a task op through the cid-form ops_cid table, for the ops whose + * cid-form signature differs from the cpu-form slot: set_cmask() takes an arena + * cmask instead of a cpumask and enable() takes scx_enable_args. */ #define SCX_CALL_CID_OP_TASK(sch, op, locked_rq, task, args...) \ __SCX_CALL_OP_TASK(sch, ops_cid, op, locked_rq, task, ##args) diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c index 67b7c01cae55..2f3653199842 100644 --- a/tools/sched_ext/scx_qmap.bpf.c +++ b/tools/sched_ext/scx_qmap.bpf.c @@ -961,9 +961,6 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init_task, struct task_struct *p, taskc->highpri = false; taskc->core_sched_seq = 0; cmask_init(&taskc->cpus_allowed, 0, scx_bpf_nr_cids()); - bpf_rcu_read_lock(); - cmask_from_cpumask(&taskc->cpus_allowed, p->cpus_ptr); - bpf_rcu_read_unlock(); v = bpf_task_storage_get(&task_ctx_stor, p, NULL, BPF_LOCAL_STORAGE_GET_F_CREATE); -- 2.55.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] sched_ext: Pass the initial cmask to cid-form ops.enable() 2026-09-19 0:28 ` [PATCH 1/2] " Tejun Heo @ 2026-09-19 12:21 ` Andrea Righi 0 siblings, 0 replies; 7+ messages in thread From: Andrea Righi @ 2026-09-19 12:21 UTC (permalink / raw) To: Tejun Heo Cc: David Vernet, Changwoo Min, Emil Tsalapatis, David Dai, sched-ext, linux-kernel Hi Tejun, On Fri, Sep 18, 2026 at 02:28:37PM -1000, Tejun Heo wrote: > The cid-form API has an obvious hole. A task's cid mask is only visible > through ops.set_cmask(), which fires on affinity changes and class switches > but not when a task enters a scheduler through fork, sub-sched enable or > re-home, and there is no p->cpus_ptr equivalent to fall back on. Schedulers > work around it by seeding the mask in ops.init_task() from p->cpus_ptr cid > by cid, which is subtly wrong: on sub-sched enable and re-home, an affinity > change between init_task() and enable() is delivered to the sched the task > is still on, and nothing corrects the new sched's copy afterwards. > > Fix it by adding struct scx_enable_args to cid-form ops.enable() carrying > the task's cmask, built in the per-cpu scratch under the rq lock as the task > enters the scheduler, and calling set_cmask() with the same mask right after > enable(), ahead of set_weight(). A scheduler can then track affinity in > set_cmask() alone, and scx_qmap drops its init_task() seed. set_cmask() no > longer fires for a cid-form task before it is enabled, and the class-switch > republish in switching_to_scx() is limited to the cpu form. > > This changes the cid-form ops.enable() signature, which is fine as the > cid-form API is still considered unreleased. An args struct rather than a > bare cmask argument leaves room for more initial state without another > signature change, and the cmask travels as a plain arena address because BTF > can't type arena struct members yet. > > v2: The cmask travels as a u64 arena address, cmask_arena_addr, instead of a > kernel-typed pointer, with the typing limitation and the planned typed alias > documented (Sashiko review). > > v3: The initial set_cmask() is delivered before set_weight() so that the > mask is in place when weight-dependent state is derived (Andrea Righi). > Selftest added. > > Signed-off-by: Tejun Heo <tj@kernel.org> Looks good now. Reviewed-by: Andrea Righi <arighi@nvidia.com> Thanks, -Andrea > --- > kernel/sched/ext/ext.c | 92 +++++++++++++++++++++++----------- > kernel/sched/ext/internal.h | 56 ++++++++++++++++++--- > tools/sched_ext/scx_qmap.bpf.c | 3 -- > 3 files changed, 110 insertions(+), 41 deletions(-) > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c > index f568fd9973f6..ad391a8cbd05 100644 > --- a/kernel/sched/ext/ext.c > +++ b/kernel/sched/ext/ext.c > @@ -447,37 +447,45 @@ static void switch_rq_lock(struct rq *from, struct rq *to) > DEFINE_STATIC_KEY_FALSE(__scx_is_cid_type); > > /** > - * scx_call_op_set_cpumask - invoke ops.set_cpumask / ops_cid.set_cmask for @task > + * scx_fill_cmask_scratch - Build this cpu's arena cmask from @cpumask > + * @sch: scx_sched whose scratch to fill > + * @cpumask: cpus to translate into cids > + * > + * The scratch lives in BPF-writable arena memory and its header can't be > + * trusted, so it is rewritten from kernel geometry rather than read. Caller > + * must hold an rq lock so this cpu is the sole kernel writer for as long as the > + * returned address is in use. > + */ > +static struct scx_cmask *scx_fill_cmask_scratch(struct scx_sched *sch, > + const struct cpumask *cpumask) > +{ > + struct scx_cmask *kern_va = *this_cpu_ptr(sch->set_cmask_scratch); > + struct scx_cmask_ref ref; > + > + scx_cmask_ref_init_kern(sch, kern_va, 0, num_possible_cpus(), &ref); > + scx_cmask_ref_from_cpumask(&ref, cpumask); > + return kern_va; > +} > + > +/** > + * scx_call_op_set_cpumask - Invoke the set_cpumask or set_cmask op for @task > * @sch: scx_sched being invoked > * @rq: rq to update as the currently-locked rq, or NULL > * @task: task whose affinity is changing > * @cpumask: new cpumask > * > - * For cid-form schedulers, translate @cpumask to a cmask via the per-cpu > - * scratch in cid.c and dispatch through the ops_cid union view. Caller > - * must hold @rq's rq lock so this_cpu_ptr is stable across the call. > + * For cid-form schedulers, translate @cpumask to a cmask in the per-cpu scratch > + * and dispatch through the ops_cid union view. Caller must hold @rq's rq lock. > */ > static inline void scx_call_op_set_cpumask(struct scx_sched *sch, struct rq *rq, > struct task_struct *task, > const struct cpumask *cpumask) > { > - if (scx_is_cid_type()) { > - struct scx_cmask *kern_va = *this_cpu_ptr(sch->set_cmask_scratch); > - struct scx_cmask_ref ref; > - > - /* > - * Build the per-cpu arena cmask from kernel geometry via @ref, > - * never reading its BPF-writable header. set_cmask()'s __arena > - * argument takes the kernel address and the struct_ops > - * trampoline rebases it into BPF's arena pointer form. The rq > - * lock makes this cpu the sole kernel writer. > - */ > - scx_cmask_ref_init_kern(sch, kern_va, 0, num_possible_cpus(), &ref); > - scx_cmask_ref_from_cpumask(&ref, cpumask); > - SCX_CALL_CID_OP_TASK(sch, set_cmask, rq, task, kern_va); > - } else { > + if (scx_is_cid_type()) > + SCX_CALL_CID_OP_TASK(sch, set_cmask, rq, task, > + scx_fill_cmask_scratch(sch, cpumask)); > + else > SCX_CALL_OP_TASK(sch, set_cpumask, rq, task, cpumask); > - } > } > > enum scx_dsq_iter_flags { > @@ -3634,8 +3642,12 @@ static void set_cpus_allowed_scx(struct task_struct *p, > * > * Fine-grained memory write control is enforced by BPF making the const > * designation pointless. Cast it away when calling the operation. > + * > + * The cid form receives the initial mask when the task is enabled and > + * hears about changes only afterwards, see struct scx_enable_args. > */ > - if (SCX_HAS_OP(sch, set_cpumask)) > + if (SCX_HAS_OP(sch, set_cpumask) && > + (!scx_is_cid_type() || scx_get_task_state(p) == SCX_TASK_ENABLED)) > scx_call_op_set_cpumask(sch, task_rq(p), p, (struct cpumask *)p->cpus_ptr); > } > > @@ -3944,8 +3956,27 @@ static void __scx_enable_task(struct scx_sched *sch, struct task_struct *p) > > p->scx.weight = sched_weight_to_cgroup(weight); > > - if (SCX_HAS_OP(sch, enable)) > - SCX_CALL_OP_TASK(sch, enable, rq, p); > + if (SCX_HAS_OP(sch, enable)) { > + if (scx_is_cid_type()) { > + struct scx_cmask *cmask = scx_fill_cmask_scratch(sch, p->cpus_ptr); > + struct scx_enable_args args = { > + .cmask_arena_addr = scx_kaddr_to_arena(sch, cmask), > + }; > + > + SCX_CALL_CID_OP_TASK(sch, enable, rq, p, &args); > + } else { > + SCX_CALL_OP_TASK(sch, enable, rq, p); > + } > + } > + > + /* > + * The initial mask also goes out through set_cmask() so a scheduler can > + * track affinity there alone, and before set_weight() so that the mask > + * is in place when weight-dependent state is derived, see struct > + * scx_enable_args. > + */ > + if (scx_is_cid_type() && SCX_HAS_OP(sch, set_cpumask)) > + scx_call_op_set_cpumask(sch, rq, p, p->cpus_ptr); > > if (SCX_HAS_OP(sch, set_weight)) > SCX_CALL_OP_TASK(sch, set_weight, rq, p, p->scx.weight); > @@ -4288,9 +4319,10 @@ static void switching_to_scx(struct rq *rq, struct task_struct *p) > > /* > * set_cpus_allowed_scx() is not called while @p is associated with a > - * different scheduler class. Keep the BPF scheduler up-to-date. > + * different scheduler class. Keep the BPF scheduler up-to-date. The cid > + * form gets its mask from scx_enable_task(). > */ > - if (SCX_HAS_OP(sch, set_cpumask)) > + if (!scx_is_cid_type() && SCX_HAS_OP(sch, set_cpumask)) > scx_call_op_set_cpumask(sch, rq, p, (struct cpumask *)p->cpus_ptr); > } > > @@ -8374,10 +8406,11 @@ static struct bpf_struct_ops bpf_sched_ext_ops = { > /* > * cid-form cfi stubs. Stubs whose signatures match the cpu-form (param types > * identical, only param names differ across structs) are reused. Some need > - * fresh stubs, set_cmask due to an argument type difference and the sub-sched > - * notifiers because no cpu-form stub exists to reuse. > + * fresh stubs, set_cmask and enable due to argument differences and the > + * sub-sched notifiers because no cpu-form stub exists to reuse. > */ > static void sched_ext_ops_cid__set_cmask(struct task_struct *p, const struct scx_cmask *cmask__arena) {} > +static void sched_ext_ops_cid__enable(struct task_struct *p, struct scx_enable_args *args) {} > static void sched_ext_ops__sub_caps_updated(const struct scx_cmask *cmask__arena, u64 caps) {} > static void sched_ext_ops__sub_ecaps_updated(s32 cid, u64 before, u64 after) {} > > @@ -8398,7 +8431,7 @@ static struct sched_ext_ops_cid __bpf_ops_sched_ext_ops_cid = { > .update_idle = sched_ext_ops__update_idle, > .init_task = sched_ext_ops__init_task, > .exit_task = sched_ext_ops__exit_task, > - .enable = sched_ext_ops__enable, > + .enable = sched_ext_ops_cid__enable, > .disable = sched_ext_ops__disable, > #ifdef CONFIG_EXT_GROUP_SCHED > .cpuctl_init = sched_ext_ops__cgroup_init, > @@ -10421,8 +10454,7 @@ __bpf_kfunc const void *scx_bpf_online_cmask(const struct bpf_prog_aux *aux) > if (unlikely(!online)) > return NULL; > > - /* BPF rebases by the low 32 bits, like __arena callback args */ > - return (void *)((unsigned long)online - sch->arena_kern_base); > + return (void *)scx_kaddr_to_arena(sch, online); > } > > /** > diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h > index d150de10a5c9..1df8f583b0ec 100644 > --- a/kernel/sched/ext/internal.h > +++ b/kernel/sched/ext/internal.h > @@ -250,6 +250,31 @@ struct scx_exit_task_args { > bool cancelled; > }; > > +/** > + * struct scx_enable_args - Argument container for cid-form ops.enable() > + * @cmask_arena_addr: BPF arena address of the cmask of cids the task may run on > + * > + * @cmask_arena_addr is the task's affinity as it enters the scheduler. > + * set_cmask() delivers the same mask right after enable(), before set_weight() > + * and the first enqueue, then every affinity change afterwards, and is never > + * called before enable(). A scheduler may therefore track affinity in > + * set_cmask() alone. > + * > + * The kernel builds the mask in the scheduler arena from its own geometry, so > + * the header is valid regardless of what the scheduler last wrote there. The > + * memory is per-cpu scratch reused once the callback returns: copy the bits > + * out, don't keep the address. The set_cmask() argument follows the same rules. > + * > + * The address is a plain value rather than a typed pointer because BTF can't > + * mark a struct member as an arena pointer yet and a pointer member would reach > + * the program typed as a kernel pointer. Cast it to struct scx_cmask __arena * > + * before use. Once arena members can be typed, a typed alias will join this > + * field in an anonymous union at the same offset. > + */ > +struct scx_enable_args { > + u64 cmask_arena_addr; > +}; > + > /* argument container for ops.cgroup_init() */ > struct scx_cgroup_init_args { > /* the weight of the cgroup [1..10000] */ > @@ -1037,6 +1062,7 @@ struct sched_ext_ops { > * - dispatch -> dispatch (cpu arg is now cid) > * - update_idle -> update_idle (cpu arg is now cid) > * - set_cpumask -> set_cmask (cmask instead of cpumask) > + * - enable -> enable (takes struct scx_enable_args) > * - cpu_online -> cid_online > * - cpu_offline -> cid_offline > * - dump_cpu -> dump_cid > @@ -1070,7 +1096,7 @@ struct sched_ext_ops_cid { > struct scx_init_task_args *args); > void (*exit_task)(struct task_struct *p, > struct scx_exit_task_args *args); > - void (*enable)(struct task_struct *p); > + void (*enable)(struct task_struct *p, struct scx_enable_args *args); > void (*disable)(struct task_struct *p); > void (*dump)(struct scx_dump_ctx *ctx); > void (*dump_cid)(struct scx_dump_ctx *ctx, s32 cid, bool idle); > @@ -1533,7 +1559,8 @@ struct scx_sched { > * by BUILD_BUG_ON in scx_init()). The anonymous union lets the kernel > * access either view of the same storage without function-pointer > * casts: use .ops for cpu-form and shared fields, .ops_cid for the > - * cid-renamed callbacks (set_cmask, select_cid, cid_online, ...). > + * callbacks whose cid-form signature differs (set_cmask, enable, > + * select_cid, cid_online, ...). > */ > union { > struct sched_ext_ops ops; > @@ -1556,9 +1583,9 @@ struct scx_sched { > uintptr_t arena_kern_base; > > /* > - * Per-CPU arena cmask used by scx_call_op_set_cpumask() to hand a cmask > - * to ops_cid.set_cmask(). The kernel writes through the stored kern_va > - * and passes it to the callback's __arena argument. > + * Per-CPU arena cmask the kernel fills from a task's cpumask and hands > + * to ops_cid.enable() and ops_cid.set_cmask(). The stored pointers are > + * the kernel addresses. > */ > struct scx_cmask * __percpu *set_cmask_scratch; > struct scx_cmask *online_cmask; > @@ -1669,6 +1696,19 @@ static inline void *scx_arena_to_kaddr(struct scx_sched *sch, const void *bpf_pt > return (void *)(sch->arena_kern_base + (u32)(uintptr_t)bpf_ptr); > } > > +/** > + * scx_kaddr_to_arena - Translate a kernel arena address to the BPF form > + * @sch: scheduler whose arena hosts @kaddr > + * @kaddr: kernel address inside @sch's arena > + * > + * __arena callback arguments need no translation. Addresses handed to BPF any > + * other way, such as struct fields and kfunc return values, go through this. > + */ > +static inline uintptr_t scx_kaddr_to_arena(struct scx_sched *sch, const void *kaddr) > +{ > + return (uintptr_t)kaddr - sch->arena_kern_base; > +} > + > enum scx_wake_flags { > /* expose select WF_* flags as enums */ > SCX_WAKE_FORK = WF_FORK, > @@ -2302,9 +2342,9 @@ do { \ > } while (0) > > /* > - * Dispatch a task op through the cid-form ops_cid table. Only set_cmask() needs > - * this: it takes an arena cmask address instead of a cpumask, so it cannot be > - * invoked via its cpu-form set_cpumask() slot. > + * Dispatch a task op through the cid-form ops_cid table, for the ops whose > + * cid-form signature differs from the cpu-form slot: set_cmask() takes an arena > + * cmask instead of a cpumask and enable() takes scx_enable_args. > */ > #define SCX_CALL_CID_OP_TASK(sch, op, locked_rq, task, args...) \ > __SCX_CALL_OP_TASK(sch, ops_cid, op, locked_rq, task, ##args) > diff --git a/tools/sched_ext/scx_qmap.bpf.c b/tools/sched_ext/scx_qmap.bpf.c > index 67b7c01cae55..2f3653199842 100644 > --- a/tools/sched_ext/scx_qmap.bpf.c > +++ b/tools/sched_ext/scx_qmap.bpf.c > @@ -961,9 +961,6 @@ s32 BPF_STRUCT_OPS_SLEEPABLE(qmap_init_task, struct task_struct *p, > taskc->highpri = false; > taskc->core_sched_seq = 0; > cmask_init(&taskc->cpus_allowed, 0, scx_bpf_nr_cids()); > - bpf_rcu_read_lock(); > - cmask_from_cpumask(&taskc->cpus_allowed, p->cpus_ptr); > - bpf_rcu_read_unlock(); > > v = bpf_task_storage_get(&task_ctx_stor, p, NULL, > BPF_LOCAL_STORAGE_GET_F_CREATE); > -- > 2.55.0 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives 2026-09-19 0:28 [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo 2026-09-19 0:28 ` [PATCH 1/2] " Tejun Heo @ 2026-09-19 0:28 ` Tejun Heo 2026-09-19 12:31 ` Andrea Righi 2026-09-19 14:08 ` [PATCH v2 " Tejun Heo 2026-09-19 14:25 ` [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo 2 siblings, 2 replies; 7+ messages in thread From: Tejun Heo @ 2026-09-19 0:28 UTC (permalink / raw) To: David Vernet, Andrea Righi, Changwoo Min Cc: Emil Tsalapatis, David Dai, sched-ext, linux-kernel, Tejun Heo cid-form ops.enable() now hands the task's cmask to the scheduler and set_cmask() repeats it right after. Add a cid-form selftest that checks both against p->cpus_ptr, that they match each other, that the initial set_cmask() lands before set_weight() and before the task first becomes runnable, and that set_cmask() never precedes enable(), across class-switch enables, fork-path enables and live affinity changes. Signed-off-by: Tejun Heo <tj@kernel.org> --- tools/testing/selftests/sched_ext/Makefile | 1 + .../selftests/sched_ext/enable_cmask.bpf.c | 213 ++++++++++++++++++ .../selftests/sched_ext/enable_cmask.c | 138 ++++++++++++ 3 files changed, 352 insertions(+) create mode 100644 tools/testing/selftests/sched_ext/enable_cmask.bpf.c create mode 100644 tools/testing/selftests/sched_ext/enable_cmask.c diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile index 49897727f535..4e06d0baaeec 100644 --- a/tools/testing/selftests/sched_ext/Makefile +++ b/tools/testing/selftests/sched_ext/Makefile @@ -169,6 +169,7 @@ auto-test-targets := \ ddsp_bogus_dsq_fail \ ddsp_vtimelocal_fail \ dsp_local_on \ + enable_cmask \ enq_select_cpu \ exit \ hotplug \ diff --git a/tools/testing/selftests/sched_ext/enable_cmask.bpf.c b/tools/testing/selftests/sched_ext/enable_cmask.bpf.c new file mode 100644 index 000000000000..bbbf989d2c69 --- /dev/null +++ b/tools/testing/selftests/sched_ext/enable_cmask.bpf.c @@ -0,0 +1,213 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * A cid-form scheduler checking the cmask cid-form ops.enable() receives: the + * header, every cid bit against p->cpus_ptr, and that set_cmask() follows with + * the same mask before set_weight() and before the task first becomes runnable, + * and never runs before enable(). + * + * Copyright (c) 2026 Tejun Heo <tj@kernel.org> + */ +#include <scx/common.bpf.h> + +#define MAX_CPUS 1024 + +char _license[] SEC("license") = "GPL"; + +struct { + __uint(type, BPF_MAP_TYPE_ARENA); + __uint(map_flags, BPF_F_MMAPABLE); + __uint(max_entries, 1 << 16); +} arena SEC(".maps"); + +struct task_ctx { + u64 enable_fp; /* fingerprint of the mask enable() received */ + bool enabled; + bool pending; /* enable() ran, the initial set_cmask() hasn't */ +}; + +struct { + __uint(type, BPF_MAP_TYPE_TASK_STORAGE); + __uint(map_flags, BPF_F_NO_PREALLOC); + __type(key, int); + __type(value, struct task_ctx); +} task_ctx_stor SEC(".maps"); + +u64 nr_enable, nr_initial_set_cmask, nr_set_cmask, nr_set_weight; +s32 bad_cid; +bool bad_want, bad_got; + +UEI_DEFINE(uei); + +static struct task_ctx *lookup_task_ctx(struct task_struct *p) +{ + struct task_ctx *tctx; + + tctx = bpf_task_storage_get(&task_ctx_stor, p, 0, 0); + if (!tctx) + scx_bpf_error("task_ctx lookup failed for %s[%d]", p->comm, p->pid); + return tctx; +} + +/* + * Verify @m's header and every cid bit against @p's cpumask and fingerprint the + * bits into @fp. Return 0 on success, -EINVAL on a bad header, -ENOENT on a cid + * without a cpu and -EIO on a bit mismatch with the details in @bad_*. + */ +static int check_mask(struct task_struct *p, const struct scx_cmask __arena *m, u64 *fp) +{ + u32 nr_cids = scx_bpf_nr_cids(); + u64 h = 0; + s32 cid; + + if (m->base || m->nr_cids != nr_cids) + return -EINVAL; + + bpf_for(cid, 0, MAX_CPUS) { + bool want, got; + s32 cpu; + + if (cid >= nr_cids) + break; + cpu = scx_bpf_cid_to_cpu(cid); + if (cpu < 0) + return -ENOENT; + want = bpf_cpumask_test_cpu(cpu, p->cpus_ptr); + got = cmask_test(cid, m); + if (want != got) { + bad_cid = cid; + bad_want = want; + bad_got = got; + return -EIO; + } + h = h * 31 + got; + } + + *fp = h; + return 0; +} + +s32 BPF_STRUCT_OPS_SLEEPABLE(enable_cmask_init_task, struct task_struct *p, + struct scx_init_task_args *args) +{ + if (!bpf_task_storage_get(&task_ctx_stor, p, 0, BPF_LOCAL_STORAGE_GET_F_CREATE)) + return -ENOMEM; + return 0; +} + +void BPF_STRUCT_OPS(enable_cmask_enable, struct task_struct *p, struct scx_enable_args *args) +{ + struct scx_cmask __arena *m = (struct scx_cmask __arena *)args->cmask_arena_addr; + struct task_ctx *tctx; + int ret; + + asm volatile("" :: "r"(&arena)); + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + __sync_fetch_and_add(&nr_enable, 1); + if (tctx->enabled || tctx->pending) { + scx_bpf_error("enable: %s[%d] enabled twice", p->comm, p->pid); + return; + } + + ret = check_mask(p, m, &tctx->enable_fp); + if (ret) { + scx_bpf_error("enable: %s[%d] cmask check failed %d cid=%d want=%d got=%d", + p->comm, p->pid, ret, bad_cid, bad_want, bad_got); + return; + } + tctx->enabled = true; + tctx->pending = true; +} + +void BPF_STRUCT_OPS(enable_cmask_set_cmask, struct task_struct *p, + struct scx_cmask __arena *m) +{ + struct task_ctx *tctx; + u64 fp; + int ret; + + asm volatile("" :: "r"(&arena)); + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + __sync_fetch_and_add(&nr_set_cmask, 1); + if (!tctx->enabled) { + scx_bpf_error("set_cmask: %s[%d] not enabled", p->comm, p->pid); + return; + } + + ret = check_mask(p, m, &fp); + if (ret) { + scx_bpf_error("set_cmask: %s[%d] cmask check failed %d cid=%d want=%d got=%d", + p->comm, p->pid, ret, bad_cid, bad_want, bad_got); + return; + } + + if (tctx->pending) { + if (fp != tctx->enable_fp) { + scx_bpf_error("set_cmask: %s[%d] initial mask differs from enable()", + p->comm, p->pid); + return; + } + tctx->pending = false; + __sync_fetch_and_add(&nr_initial_set_cmask, 1); + } +} + +void BPF_STRUCT_OPS(enable_cmask_set_weight, struct task_struct *p, u32 weight) +{ + struct task_ctx *tctx; + + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + __sync_fetch_and_add(&nr_set_weight, 1); + if (tctx->pending) + scx_bpf_error("set_weight: %s[%d] before the initial set_cmask()", p->comm, + p->pid); +} + +void BPF_STRUCT_OPS(enable_cmask_runnable, struct task_struct *p, u64 enq_flags) +{ + struct task_ctx *tctx; + + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + if (tctx->pending) + scx_bpf_error("runnable: %s[%d] before the initial set_cmask()", p->comm, + p->pid); +} + +void BPF_STRUCT_OPS(enable_cmask_disable, struct task_struct *p) +{ + struct task_ctx *tctx; + + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + tctx->enabled = false; + tctx->pending = false; +} + +void BPF_STRUCT_OPS(enable_cmask_exit, struct scx_exit_info *ei) +{ + UEI_RECORD(uei, ei); +} + +SCX_OPS_CID_DEFINE(enable_cmask_ops, + .init_task = (void *)enable_cmask_init_task, + .enable = (void *)enable_cmask_enable, + .set_cmask = (void *)enable_cmask_set_cmask, + .set_weight = (void *)enable_cmask_set_weight, + .runnable = (void *)enable_cmask_runnable, + .disable = (void *)enable_cmask_disable, + .exit = (void *)enable_cmask_exit, + .flags = SCX_OPS_SWITCH_PARTIAL, + .name = "enable_cmask"); diff --git a/tools/testing/selftests/sched_ext/enable_cmask.c b/tools/testing/selftests/sched_ext/enable_cmask.c new file mode 100644 index 000000000000..556bcad4431d --- /dev/null +++ b/tools/testing/selftests/sched_ext/enable_cmask.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2026 Tejun Heo <tj@kernel.org> */ +#define _GNU_SOURCE +#include <sched.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <unistd.h> +#include <sys/wait.h> +#include <bpf/bpf.h> +#include <scx/common.h> +#include "enable_cmask.bpf.skel.h" +#include "scx_test.h" + +#define SCHED_EXT 7 +#define NR_CHILDREN 8 +#define MAX_CPUS 1024 + +static int cpus[MAX_CPUS]; +static int nr_cpus; + +static void spin_ms(int ms) +{ + struct timespec start, now; + + clock_gettime(CLOCK_MONOTONIC, &start); + do { + clock_gettime(CLOCK_MONOTONIC, &now); + } while ((now.tv_sec - start.tv_sec) * 1000 + + (now.tv_nsec - start.tv_nsec) / 1000000 < ms); +} + +static int pin(pid_t pid, int idx) +{ + cpu_set_t set; + + CPU_ZERO(&set); + CPU_SET(cpus[idx % nr_cpus], &set); + return sched_setaffinity(pid, sizeof(set), &set); +} + +/* + * Pin, switch to SCHED_EXT for a class-switch enable, fork a grandchild that + * inherits the policy for a fork-path enable, then change affinity a few times + * while running for set_cmask() on live tasks. + */ +static int child(int idx) +{ + struct sched_param param = {}; + int i, status; + pid_t pid; + + if (pin(0, idx) || sched_setscheduler(0, SCHED_EXT, ¶m)) + return 1; + + pid = fork(); + if (pid < 0) + return 1; + if (!pid) { + spin_ms(20); + return 0; + } + + for (i = 1; i <= 4; i++) { + if (pin(0, idx + i)) + return 1; + spin_ms(5); + } + + return waitpid(pid, &status, 0) == pid && !status ? 0 : 1; +} + +static enum scx_test_status run(void *ctx) +{ + struct enable_cmask *skel; + struct bpf_link *link; + pid_t pids[NR_CHILDREN]; + cpu_set_t set; + int i, status, failed = 0; + + if (!__COMPAT_struct_has_field("scx_enable_args", "cmask_arena_addr")) + return SCX_TEST_SKIP; + + SCX_FAIL_IF(sched_getaffinity(0, sizeof(set), &set), "Failed to read affinity"); + for (i = 0; i < MAX_CPUS && i < CPU_SETSIZE; i++) + if (CPU_ISSET(i, &set)) + cpus[nr_cpus++] = i; + if (nr_cpus < 2) + return SCX_TEST_SKIP; + + skel = enable_cmask__open(); + SCX_FAIL_IF(!skel, "Failed to open"); + SCX_ENUM_INIT(skel); + SCX_FAIL_IF(enable_cmask__load(skel), "Failed to load skel"); + + link = bpf_map__attach_struct_ops(skel->maps.enable_cmask_ops); + SCX_FAIL_IF(!link, "Failed to attach struct_ops"); + + for (i = 0; i < NR_CHILDREN; i++) { + pids[i] = fork(); + SCX_FAIL_IF(pids[i] < 0, "Failed to fork"); + if (!pids[i]) + exit(child(i)); + } + + /* affinity changes from the outside race with the children's own */ + for (i = 0; i < NR_CHILDREN; i++) + pin(pids[i], i + NR_CHILDREN); + + for (i = 0; i < NR_CHILDREN; i++) { + if (waitpid(pids[i], &status, 0) != pids[i] || status) + failed++; + } + + bpf_link__destroy(link); + + SCX_EQ(skel->data->uei.kind, EXIT_KIND(SCX_EXIT_UNREG)); + SCX_EQ(failed, 0); + SCX_GE(skel->bss->nr_enable, 2 * NR_CHILDREN); + SCX_EQ(skel->bss->nr_initial_set_cmask, skel->bss->nr_enable); + SCX_GT(skel->bss->nr_set_cmask, skel->bss->nr_initial_set_cmask); + SCX_GE(skel->bss->nr_set_weight, skel->bss->nr_enable); + printf("enable=%lu initial_set_cmask=%lu set_cmask=%lu set_weight=%lu\n", + (unsigned long)skel->bss->nr_enable, + (unsigned long)skel->bss->nr_initial_set_cmask, + (unsigned long)skel->bss->nr_set_cmask, + (unsigned long)skel->bss->nr_set_weight); + + enable_cmask__destroy(skel); + return SCX_TEST_PASS; +} + +struct scx_test enable_cmask = { + .name = "enable_cmask", + .description = "Check the cid-form ops.enable() cmask and the set_cmask() after it", + .run = run, +}; +REGISTER_SCX_TEST(&enable_cmask) -- 2.55.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives 2026-09-19 0:28 ` [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives Tejun Heo @ 2026-09-19 12:31 ` Andrea Righi 2026-09-19 14:08 ` [PATCH v2 " Tejun Heo 1 sibling, 0 replies; 7+ messages in thread From: Andrea Righi @ 2026-09-19 12:31 UTC (permalink / raw) To: Tejun Heo Cc: David Vernet, Changwoo Min, Emil Tsalapatis, David Dai, sched-ext, linux-kernel Hi Tejun, nits below. On Fri, Sep 18, 2026 at 02:28:38PM -1000, Tejun Heo wrote: > cid-form ops.enable() now hands the task's cmask to the scheduler and > set_cmask() repeats it right after. Add a cid-form selftest that checks both > against p->cpus_ptr, that they match each other, that the initial > set_cmask() lands before set_weight() and before the task first becomes > runnable, and that set_cmask() never precedes enable(), across class-switch > enables, fork-path enables and live affinity changes. > > Signed-off-by: Tejun Heo <tj@kernel.org> > --- > tools/testing/selftests/sched_ext/Makefile | 1 + > .../selftests/sched_ext/enable_cmask.bpf.c | 213 ++++++++++++++++++ > .../selftests/sched_ext/enable_cmask.c | 138 ++++++++++++ > 3 files changed, 352 insertions(+) > create mode 100644 tools/testing/selftests/sched_ext/enable_cmask.bpf.c > create mode 100644 tools/testing/selftests/sched_ext/enable_cmask.c > > diff --git a/tools/testing/selftests/sched_ext/Makefile b/tools/testing/selftests/sched_ext/Makefile > index 49897727f535..4e06d0baaeec 100644 > --- a/tools/testing/selftests/sched_ext/Makefile > +++ b/tools/testing/selftests/sched_ext/Makefile > @@ -169,6 +169,7 @@ auto-test-targets := \ > ddsp_bogus_dsq_fail \ > ddsp_vtimelocal_fail \ > dsp_local_on \ > + enable_cmask \ > enq_select_cpu \ > exit \ > hotplug \ > diff --git a/tools/testing/selftests/sched_ext/enable_cmask.bpf.c b/tools/testing/selftests/sched_ext/enable_cmask.bpf.c > new file mode 100644 > index 000000000000..bbbf989d2c69 > --- /dev/null > +++ b/tools/testing/selftests/sched_ext/enable_cmask.bpf.c > @@ -0,0 +1,213 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * A cid-form scheduler checking the cmask cid-form ops.enable() receives: the > + * header, every cid bit against p->cpus_ptr, and that set_cmask() follows with > + * the same mask before set_weight() and before the task first becomes runnable, > + * and never runs before enable(). > + * > + * Copyright (c) 2026 Tejun Heo <tj@kernel.org> > + */ > +#include <scx/common.bpf.h> > + > +#define MAX_CPUS 1024 > + > +char _license[] SEC("license") = "GPL"; > + > +struct { > + __uint(type, BPF_MAP_TYPE_ARENA); > + __uint(map_flags, BPF_F_MMAPABLE); > + __uint(max_entries, 1 << 16); > +} arena SEC(".maps"); > + > +struct task_ctx { > + u64 enable_fp; /* fingerprint of the mask enable() received */ > + bool enabled; > + bool pending; /* enable() ran, the initial set_cmask() hasn't */ > +}; > + > +struct { > + __uint(type, BPF_MAP_TYPE_TASK_STORAGE); > + __uint(map_flags, BPF_F_NO_PREALLOC); > + __type(key, int); > + __type(value, struct task_ctx); > +} task_ctx_stor SEC(".maps"); > + > +u64 nr_enable, nr_initial_set_cmask, nr_set_cmask, nr_set_weight; > +s32 bad_cid; > +bool bad_want, bad_got; As Sashiko also reported, these globals can be written concurrently by ops.enable() and ops.set_cmask() and could make a failure report misleading. It should be possible to move these in a caller-local stack struct. > + > +UEI_DEFINE(uei); > + > +static struct task_ctx *lookup_task_ctx(struct task_struct *p) > +{ > + struct task_ctx *tctx; > + > + tctx = bpf_task_storage_get(&task_ctx_stor, p, 0, 0); > + if (!tctx) > + scx_bpf_error("task_ctx lookup failed for %s[%d]", p->comm, p->pid); > + return tctx; > +} > + > +/* > + * Verify @m's header and every cid bit against @p's cpumask and fingerprint the > + * bits into @fp. Return 0 on success, -EINVAL on a bad header, -ENOENT on a cid > + * without a cpu and -EIO on a bit mismatch with the details in @bad_*. > + */ > +static int check_mask(struct task_struct *p, const struct scx_cmask __arena *m, u64 *fp) > +{ > + u32 nr_cids = scx_bpf_nr_cids(); > + u64 h = 0; > + s32 cid; > + > + if (m->base || m->nr_cids != nr_cids) > + return -EINVAL; Since this is checking the mask header, should this validate alloc_words too? if (m->base || m->nr_cids != nr_cids || m->alloc_words != CMASK_NR_WORDS(nr_cids)) return -EINVAL; > + > + bpf_for(cid, 0, MAX_CPUS) { > + bool want, got; > + s32 cpu; > + > + if (cid >= nr_cids) > + break; Can we iterate directly to nr_cids? > + cpu = scx_bpf_cid_to_cpu(cid); > + if (cpu < 0) > + return -ENOENT; > + want = bpf_cpumask_test_cpu(cpu, p->cpus_ptr); > + got = cmask_test(cid, m); > + if (want != got) { > + bad_cid = cid; > + bad_want = want; > + bad_got = got; > + return -EIO; > + } > + h = h * 31 + got; > + } > + > + *fp = h; > + return 0; > +} Thanks, -Andrea ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives 2026-09-19 0:28 ` [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives Tejun Heo 2026-09-19 12:31 ` Andrea Righi @ 2026-09-19 14:08 ` Tejun Heo 1 sibling, 0 replies; 7+ messages in thread From: Tejun Heo @ 2026-09-19 14:08 UTC (permalink / raw) To: David Vernet, Andrea Righi, Changwoo Min Cc: Emil Tsalapatis, David Dai, sched-ext, linux-kernel, Tejun Heo cid-form ops.enable() now hands the task's cmask to the scheduler and set_cmask() repeats it right after. Add a cid-form selftest that checks both against p->cpus_ptr, that they match each other, that the initial set_cmask() lands before set_weight() and before the task first becomes runnable, and that set_cmask() never precedes enable(), across class-switch enables, fork-path enables and live affinity changes. v2: Mismatch details returned through a caller-local struct instead of globals, alloc_words validated in the header check, loop bounded by nr_cids directly (Andrea Righi). Signed-off-by: Tejun Heo <tj@kernel.org> --- tools/testing/selftests/sched_ext/Makefile | 1 tools/testing/selftests/sched_ext/enable_cmask.bpf.c | 217 +++++++++++++++++++ tools/testing/selftests/sched_ext/enable_cmask.c | 138 ++++++++++++ 3 files changed, 356 insertions(+) --- a/tools/testing/selftests/sched_ext/Makefile +++ b/tools/testing/selftests/sched_ext/Makefile @@ -169,6 +169,7 @@ auto-test-targets := \ ddsp_bogus_dsq_fail \ ddsp_vtimelocal_fail \ dsp_local_on \ + enable_cmask \ enq_select_cpu \ exit \ hotplug \ --- /dev/null +++ b/tools/testing/selftests/sched_ext/enable_cmask.bpf.c @@ -0,0 +1,217 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * A cid-form scheduler checking the cmask cid-form ops.enable() receives: the + * header, every cid bit against p->cpus_ptr, and that set_cmask() follows with + * the same mask before set_weight() and before the task first becomes runnable, + * and never runs before enable(). + * + * Copyright (c) 2026 Tejun Heo <tj@kernel.org> + */ +#include <scx/common.bpf.h> + +char _license[] SEC("license") = "GPL"; + +struct { + __uint(type, BPF_MAP_TYPE_ARENA); + __uint(map_flags, BPF_F_MMAPABLE); + __uint(max_entries, 1 << 16); +} arena SEC(".maps"); + +struct task_ctx { + u64 enable_fp; /* fingerprint of the mask enable() received */ + bool enabled; + bool pending; /* enable() ran, the initial set_cmask() hasn't */ +}; + +struct { + __uint(type, BPF_MAP_TYPE_TASK_STORAGE); + __uint(map_flags, BPF_F_NO_PREALLOC); + __type(key, int); + __type(value, struct task_ctx); +} task_ctx_stor SEC(".maps"); + +/* details of a cid bit mismatch, filled by check_mask() */ +struct mask_mismatch { + s32 cid; + bool want; + bool got; +}; + +u64 nr_enable, nr_initial_set_cmask, nr_set_cmask, nr_set_weight; + +UEI_DEFINE(uei); + +static struct task_ctx *lookup_task_ctx(struct task_struct *p) +{ + struct task_ctx *tctx; + + tctx = bpf_task_storage_get(&task_ctx_stor, p, 0, 0); + if (!tctx) + scx_bpf_error("task_ctx lookup failed for %s[%d]", p->comm, p->pid); + return tctx; +} + +/* + * Verify @m's header and every cid bit against @p's cpumask and fingerprint the + * bits into @fp. Return 0 on success, -EINVAL on a bad header, -ENOENT on a cid + * without a cpu and -EIO on a bit mismatch with the details in @mm. + */ +static int check_mask(struct task_struct *p, const struct scx_cmask __arena *m, u64 *fp, + struct mask_mismatch *mm) +{ + u32 nr_cids = scx_bpf_nr_cids(); + u64 h = 0; + s32 cid; + + if (m->base || m->nr_cids != nr_cids || m->alloc_words != CMASK_NR_WORDS(nr_cids)) + return -EINVAL; + + bpf_for(cid, 0, nr_cids) { + bool want, got; + s32 cpu; + + cpu = scx_bpf_cid_to_cpu(cid); + if (cpu < 0) + return -ENOENT; + want = bpf_cpumask_test_cpu(cpu, p->cpus_ptr); + got = cmask_test(cid, m); + if (want != got) { + mm->cid = cid; + mm->want = want; + mm->got = got; + return -EIO; + } + h = h * 31 + got; + } + + *fp = h; + return 0; +} + +s32 BPF_STRUCT_OPS_SLEEPABLE(enable_cmask_init_task, struct task_struct *p, + struct scx_init_task_args *args) +{ + if (!bpf_task_storage_get(&task_ctx_stor, p, 0, BPF_LOCAL_STORAGE_GET_F_CREATE)) + return -ENOMEM; + return 0; +} + +void BPF_STRUCT_OPS(enable_cmask_enable, struct task_struct *p, struct scx_enable_args *args) +{ + struct scx_cmask __arena *m = (struct scx_cmask __arena *)args->cmask_arena_addr; + struct mask_mismatch mm = {}; + struct task_ctx *tctx; + int ret; + + asm volatile("" :: "r"(&arena)); + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + __sync_fetch_and_add(&nr_enable, 1); + if (tctx->enabled || tctx->pending) { + scx_bpf_error("enable: %s[%d] enabled twice", p->comm, p->pid); + return; + } + + ret = check_mask(p, m, &tctx->enable_fp, &mm); + if (ret) { + scx_bpf_error("enable: %s[%d] cmask check failed %d cid=%d want=%d got=%d", + p->comm, p->pid, ret, mm.cid, mm.want, mm.got); + return; + } + tctx->enabled = true; + tctx->pending = true; +} + +void BPF_STRUCT_OPS(enable_cmask_set_cmask, struct task_struct *p, + struct scx_cmask __arena *m) +{ + struct mask_mismatch mm = {}; + struct task_ctx *tctx; + u64 fp; + int ret; + + asm volatile("" :: "r"(&arena)); + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + __sync_fetch_and_add(&nr_set_cmask, 1); + if (!tctx->enabled) { + scx_bpf_error("set_cmask: %s[%d] not enabled", p->comm, p->pid); + return; + } + + ret = check_mask(p, m, &fp, &mm); + if (ret) { + scx_bpf_error("set_cmask: %s[%d] cmask check failed %d cid=%d want=%d got=%d", + p->comm, p->pid, ret, mm.cid, mm.want, mm.got); + return; + } + + if (tctx->pending) { + if (fp != tctx->enable_fp) { + scx_bpf_error("set_cmask: %s[%d] initial mask differs from enable()", + p->comm, p->pid); + return; + } + tctx->pending = false; + __sync_fetch_and_add(&nr_initial_set_cmask, 1); + } +} + +void BPF_STRUCT_OPS(enable_cmask_set_weight, struct task_struct *p, u32 weight) +{ + struct task_ctx *tctx; + + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + __sync_fetch_and_add(&nr_set_weight, 1); + if (tctx->pending) + scx_bpf_error("set_weight: %s[%d] before the initial set_cmask()", p->comm, + p->pid); +} + +void BPF_STRUCT_OPS(enable_cmask_runnable, struct task_struct *p, u64 enq_flags) +{ + struct task_ctx *tctx; + + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + if (tctx->pending) + scx_bpf_error("runnable: %s[%d] before the initial set_cmask()", p->comm, + p->pid); +} + +void BPF_STRUCT_OPS(enable_cmask_disable, struct task_struct *p) +{ + struct task_ctx *tctx; + + tctx = lookup_task_ctx(p); + if (!tctx) + return; + + tctx->enabled = false; + tctx->pending = false; +} + +void BPF_STRUCT_OPS(enable_cmask_exit, struct scx_exit_info *ei) +{ + UEI_RECORD(uei, ei); +} + +SCX_OPS_CID_DEFINE(enable_cmask_ops, + .init_task = (void *)enable_cmask_init_task, + .enable = (void *)enable_cmask_enable, + .set_cmask = (void *)enable_cmask_set_cmask, + .set_weight = (void *)enable_cmask_set_weight, + .runnable = (void *)enable_cmask_runnable, + .disable = (void *)enable_cmask_disable, + .exit = (void *)enable_cmask_exit, + .flags = SCX_OPS_SWITCH_PARTIAL, + .name = "enable_cmask"); --- /dev/null +++ b/tools/testing/selftests/sched_ext/enable_cmask.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2026 Tejun Heo <tj@kernel.org> */ +#define _GNU_SOURCE +#include <sched.h> +#include <stdio.h> +#include <stdlib.h> +#include <time.h> +#include <unistd.h> +#include <sys/wait.h> +#include <bpf/bpf.h> +#include <scx/common.h> +#include "enable_cmask.bpf.skel.h" +#include "scx_test.h" + +#define SCHED_EXT 7 +#define NR_CHILDREN 8 +#define MAX_CPUS 1024 + +static int cpus[MAX_CPUS]; +static int nr_cpus; + +static void spin_ms(int ms) +{ + struct timespec start, now; + + clock_gettime(CLOCK_MONOTONIC, &start); + do { + clock_gettime(CLOCK_MONOTONIC, &now); + } while ((now.tv_sec - start.tv_sec) * 1000 + + (now.tv_nsec - start.tv_nsec) / 1000000 < ms); +} + +static int pin(pid_t pid, int idx) +{ + cpu_set_t set; + + CPU_ZERO(&set); + CPU_SET(cpus[idx % nr_cpus], &set); + return sched_setaffinity(pid, sizeof(set), &set); +} + +/* + * Pin, switch to SCHED_EXT for a class-switch enable, fork a grandchild that + * inherits the policy for a fork-path enable, then change affinity a few times + * while running for set_cmask() on live tasks. + */ +static int child(int idx) +{ + struct sched_param param = {}; + int i, status; + pid_t pid; + + if (pin(0, idx) || sched_setscheduler(0, SCHED_EXT, ¶m)) + return 1; + + pid = fork(); + if (pid < 0) + return 1; + if (!pid) { + spin_ms(20); + return 0; + } + + for (i = 1; i <= 4; i++) { + if (pin(0, idx + i)) + return 1; + spin_ms(5); + } + + return waitpid(pid, &status, 0) == pid && !status ? 0 : 1; +} + +static enum scx_test_status run(void *ctx) +{ + struct enable_cmask *skel; + struct bpf_link *link; + pid_t pids[NR_CHILDREN]; + cpu_set_t set; + int i, status, failed = 0; + + if (!__COMPAT_struct_has_field("scx_enable_args", "cmask_arena_addr")) + return SCX_TEST_SKIP; + + SCX_FAIL_IF(sched_getaffinity(0, sizeof(set), &set), "Failed to read affinity"); + for (i = 0; i < MAX_CPUS && i < CPU_SETSIZE; i++) + if (CPU_ISSET(i, &set)) + cpus[nr_cpus++] = i; + if (nr_cpus < 2) + return SCX_TEST_SKIP; + + skel = enable_cmask__open(); + SCX_FAIL_IF(!skel, "Failed to open"); + SCX_ENUM_INIT(skel); + SCX_FAIL_IF(enable_cmask__load(skel), "Failed to load skel"); + + link = bpf_map__attach_struct_ops(skel->maps.enable_cmask_ops); + SCX_FAIL_IF(!link, "Failed to attach struct_ops"); + + for (i = 0; i < NR_CHILDREN; i++) { + pids[i] = fork(); + SCX_FAIL_IF(pids[i] < 0, "Failed to fork"); + if (!pids[i]) + exit(child(i)); + } + + /* affinity changes from the outside race with the children's own */ + for (i = 0; i < NR_CHILDREN; i++) + pin(pids[i], i + NR_CHILDREN); + + for (i = 0; i < NR_CHILDREN; i++) { + if (waitpid(pids[i], &status, 0) != pids[i] || status) + failed++; + } + + bpf_link__destroy(link); + + SCX_EQ(skel->data->uei.kind, EXIT_KIND(SCX_EXIT_UNREG)); + SCX_EQ(failed, 0); + SCX_GE(skel->bss->nr_enable, 2 * NR_CHILDREN); + SCX_EQ(skel->bss->nr_initial_set_cmask, skel->bss->nr_enable); + SCX_GT(skel->bss->nr_set_cmask, skel->bss->nr_initial_set_cmask); + SCX_GE(skel->bss->nr_set_weight, skel->bss->nr_enable); + printf("enable=%lu initial_set_cmask=%lu set_cmask=%lu set_weight=%lu\n", + (unsigned long)skel->bss->nr_enable, + (unsigned long)skel->bss->nr_initial_set_cmask, + (unsigned long)skel->bss->nr_set_cmask, + (unsigned long)skel->bss->nr_set_weight); + + enable_cmask__destroy(skel); + return SCX_TEST_PASS; +} + +struct scx_test enable_cmask = { + .name = "enable_cmask", + .description = "Check the cid-form ops.enable() cmask and the set_cmask() after it", + .run = run, +}; +REGISTER_SCX_TEST(&enable_cmask) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() 2026-09-19 0:28 [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo 2026-09-19 0:28 ` [PATCH 1/2] " Tejun Heo 2026-09-19 0:28 ` [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives Tejun Heo @ 2026-09-19 14:25 ` Tejun Heo 2 siblings, 0 replies; 7+ messages in thread From: Tejun Heo @ 2026-09-19 14:25 UTC (permalink / raw) To: David Vernet, Andrea Righi, Changwoo Min Cc: Tejun Heo, Emil Tsalapatis, David Dai, sched-ext, linux-kernel, sashiko-bot Hello, On Fri, Sep 18, 2026 at 02:28:36PM -1000, Tejun Heo wrote: > This patchset contains the following 2 patches. > > 0001 sched_ext: Pass the initial cmask to cid-form ops.enable() > 0002 selftests/sched_ext: Check the cmask cid-form ops.enable() receives Applied 1-2 to sched_ext/for-7.3-fixes, 1 with Andrea's Reviewed-by added and 2 as the v2 posted in reply to it. Thanks. -- tejun ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-19 14:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-19 0:28 [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo 2026-09-19 0:28 ` [PATCH 1/2] " Tejun Heo 2026-09-19 12:21 ` Andrea Righi 2026-09-19 0:28 ` [PATCH 2/2] selftests/sched_ext: Check the cmask cid-form ops.enable() receives Tejun Heo 2026-09-19 12:31 ` Andrea Righi 2026-09-19 14:08 ` [PATCH v2 " Tejun Heo 2026-09-19 14:25 ` [PATCHSET v3 sched_ext/for-7.3-fixes] sched_ext: Pass the initial cmask to cid-form ops.enable() Tejun Heo
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®