* [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable
@ 2026-08-17 17:09 Changwoo Min
2026-08-17 17:21 ` sashiko-bot
2026-08-17 17:53 ` Tejun Heo
0 siblings, 2 replies; 3+ messages in thread
From: Changwoo Min @ 2026-08-17 17:09 UTC (permalink / raw)
To: tj, void, arighi, changwoo; +Cc: kernel-dev, sched-ext, linux-kernel
ops.cgroup_set_bandwidth() is delivered from scx_group_set_bandwidth(),
which runs from the cpu.max cgroup interface write path (tg_set_bandwidth())
in process context. tg_set_cfs_bandwidth() has already returned by then, so
its cpus_read_lock and cfs_constraints_mutex are released, and the only lock
held is percpu_down_read(&scx_cgroup_ops_rwsem), whose read side may sleep.
The call site is therefore sleepable, like ops.cgroup_init().
bpf_scx_check_member() rejects a sleepable program on any member not on its
allow-list, so a BPF scheduler cannot allocate -- which is sleepable -- when
a cgroup gains a cpu.max limit at runtime; it must instead pre-reserve memory
for a callback that cannot allocate. Add cgroup_set_bandwidth() to the
allow-list so the callback can allocate on demand, and document that it may
block.
A scheduler must decide at load time whether to mark the callback sleepable,
but the allow-list entry is a verifier property with no symbol to probe. Add
scx_cgroup_set_bandwidth_may_sleep() as a marker whose presence in the
kernel's BTF lets userspace detect this support; it has no callers and does
nothing.
Signed-off-by: Changwoo Min <changwoo@igalia.com>
---
kernel/sched/ext/ext.c | 10 ++++++++++
kernel/sched/ext/ext.h | 1 +
kernel/sched/ext/internal.h | 2 +-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 10af28a9f2c0..e25a2e9f4eca 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -4960,6 +4960,15 @@ void scx_group_set_bandwidth(struct task_group *tg,
percpu_up_read(&scx_cgroup_ops_rwsem);
}
+
+/*
+ * Capability marker for userspace. The sleepable allowance for
+ * ops.cgroup_set_bandwidth() (see bpf_scx_check_member()) is a verifier
+ * property with no other symbol a scheduler can probe, so this no-op function
+ * exists solely so its presence in the kernel's BTF can be detected. It has no
+ * callers; __used keeps it from being optimized away.
+ */
+__used void scx_cgroup_set_bandwidth_may_sleep(void) {}
#endif /* CONFIG_EXT_GROUP_SCHED */
#if defined(CONFIG_EXT_GROUP_SCHED) || defined(CONFIG_EXT_SUB_SCHED)
@@ -8079,6 +8088,7 @@ static int bpf_scx_check_member(const struct btf_type *t,
case offsetof(struct sched_ext_ops, cgroup_init):
case offsetof(struct sched_ext_ops, cgroup_exit):
case offsetof(struct sched_ext_ops, cgroup_prep_move):
+ case offsetof(struct sched_ext_ops, cgroup_set_bandwidth):
#endif
case offsetof(struct sched_ext_ops, cpu_online):
case offsetof(struct sched_ext_ops, cpu_offline):
diff --git a/kernel/sched/ext/ext.h b/kernel/sched/ext/ext.h
index 0b7fc46aee08..e6fcfadc25aa 100644
--- a/kernel/sched/ext/ext.h
+++ b/kernel/sched/ext/ext.h
@@ -81,6 +81,7 @@ void scx_cgroup_cancel_attach(struct cgroup_taskset *tset);
void scx_group_set_weight(struct task_group *tg, unsigned long cgrp_weight);
void scx_group_set_idle(struct task_group *tg, bool idle);
void scx_group_set_bandwidth(struct task_group *tg, u64 period_us, u64 quota_us, u64 burst_us);
+void scx_cgroup_set_bandwidth_may_sleep(void);
#else /* CONFIG_EXT_GROUP_SCHED */
static inline void scx_tg_init(struct task_group *tg) {}
static inline int scx_tg_online(struct task_group *tg) { return 0; }
diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h
index 27bbf5e04d90..e2d553d13c49 100644
--- a/kernel/sched/ext/internal.h
+++ b/kernel/sched/ext/internal.h
@@ -753,7 +753,7 @@ struct sched_ext_ops {
* @burst_us: bandwidth control burst
*
* Update @cgrp's bandwidth control parameters. This is from the cpu.max
- * cgroup interface.
+ * cgroup interface. This operation may block.
*
* @quota_us / @period_us determines the CPU bandwidth @cgrp is entitled
* to. For example, if @period_us is 1_000_000 and @quota_us is
--
2.55.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable
2026-08-17 17:09 [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable Changwoo Min
@ 2026-08-17 17:21 ` sashiko-bot
2026-08-17 17:53 ` Tejun Heo
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-17 17:21 UTC (permalink / raw)
To: Changwoo Min; +Cc: sched-ext
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent writes to the cgroup `cpu.max` interface can race, causing desynchronization between CFS and SCX bandwidth states and out-of-order invocations of the BPF callback.
--
--- Patch [1]: [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable ---
commit 1a287753fea54decaf90a63415735694726dc348
Author: Changwoo Min <changwoo@igalia.com>
sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable
This commit allows the cgroup_set_bandwidth BPF scheduler operation to
sleep. It adds a BTF marker function for userspace to detect this
capability and documents the potential blocking behavior.
> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index 18183062f751c..c48561bf108be 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -4572,6 +4572,15 @@ void scx_group_set_bandwidth(struct task_group *tg,
>
> percpu_up_read(&scx_cgroup_ops_rwsem);
> }
[Severity: High]
This is a pre-existing issue, but does making this operation sleepable widen
the window for a race condition between concurrent cpu.max writers?
The commit message notes that cfs_constraints_mutex has already been released
by tg_set_cfs_bandwidth(), leaving only percpu_down_read(&scx_cgroup_ops_rwsem)
held. Since this is a read lock, concurrent writers to the same cgroup's
cpu.max interface can execute scx_group_set_bandwidth() at the same time.
If two threads write to cpu.max, they are serialized in tg_set_cfs_bandwidth(),
but can race here:
CPU1 CPU2
tg_set_cfs_bandwidth() (writer A)
tg_set_cfs_bandwidth() (writer B)
scx_group_set_bandwidth()
percpu_down_read()
SCX_CALL_OP(...) // blocks
scx_group_set_bandwidth()
percpu_down_read()
SCX_CALL_OP(...) // returns quickly
tg->scx.bw_quota_us = ...
tg->scx.bw_quota_us = ...
Can this lead to the BPF callback executing out of order, causing
desynchronization between the CFS bandwidth state and the SCX scheduler state?
Additionally, on 32-bit architectures, could updating the 64-bit
tg->scx.bw_period_us, bw_quota_us, and bw_burst_us fields without a write
lock result in torn writes?
> +
> +/*
> + * Capability marker for userspace. The sleepable allowance for
> + * ops.cgroup_set_bandwidth() (see bpf_scx_check_member()) is a verifier
> + * property with no other symbol a scheduler can probe, so this no-op function
> + * exists solely so its presence in the kernel's BTF can be detected. It has no
> + * callers; __used keeps it from being optimized away.
> + */
> +__used void scx_cgroup_set_bandwidth_may_sleep(void) {}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817170941.668571-1-changwoo@igalia.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable
2026-08-17 17:09 [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable Changwoo Min
2026-08-17 17:21 ` sashiko-bot
@ 2026-08-17 17:53 ` Tejun Heo
1 sibling, 0 replies; 3+ messages in thread
From: Tejun Heo @ 2026-08-17 17:53 UTC (permalink / raw)
To: Changwoo Min; +Cc: void, arighi, emil, kernel-dev, sched-ext, linux-kernel
Hello,
On Tue, Aug 18, 2026 at 02:09:41AM +0900, Changwoo Min wrote:
> +/*
> + * Capability marker for userspace. The sleepable allowance for
> + * ops.cgroup_set_bandwidth() (see bpf_scx_check_member()) is a verifier
> + * property with no other symbol a scheduler can probe, so this no-op function
> + * exists solely so its presence in the kernel's BTF can be detected. It has no
> + * callers; __used keeps it from being optimized away.
> + */
> +__used void scx_cgroup_set_bandwidth_may_sleep(void) {}
__used only prevents the compiler from dropping the function. With
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION, the linker can still garbage
collect its section, so please add __retain too (cf. __bpf_kfunc).
Also, we'll likely accumulate more of these markers over time, so let's
give them a shared prefix - scx_compat_marker_ or something like that -
and collect them in one place, maybe towards the end of ext.c next to
the module init code.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-17 17:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17 17:09 [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable Changwoo Min
2026-08-17 17:21 ` sashiko-bot
2026-08-17 17:53 ` Tejun Heo
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.