All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.