mirror of https://lore.kernel.org/lkml/
 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:53 ` Tejun Heo
  0 siblings, 1 reply; 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	[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:53 ` Tejun Heo
  2026-08-18 14:15   ` Changwoo Min
  0 siblings, 1 reply; 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

* Re: [PATCH] sched_ext: allow ops.cgroup_set_bandwidth() to be sleepable
  2026-08-17 17:53 ` Tejun Heo
@ 2026-08-18 14:15   ` Changwoo Min
  0 siblings, 0 replies; 3+ messages in thread
From: Changwoo Min @ 2026-08-18 14:15 UTC (permalink / raw)
  To: Tejun Heo; +Cc: void, arighi, emil, kernel-dev, sched-ext, linux-kernel

Hello,

On 8/18/26 2:53 AM, Tejun Heo wrote:
> 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 for the comments. I will address these two and send v2 shortly.
Regarding the sashiko review, I think that needs to be addressed
separately.

Regards,
Changwoo Min

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-18 14:16 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:53 ` Tejun Heo
2026-08-18 14:15   ` Changwoo Min

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®