* [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable @ 2026-08-25 5:23 Tao Cui 2026-08-25 8:27 ` Andrea Righi 0 siblings, 1 reply; 3+ messages in thread From: Tao Cui @ 2026-08-25 5:23 UTC (permalink / raw) To: tj, void Cc: arighi, changwoo, michalblk, suzhidao, sched-ext, linux-kernel, bpf, cui.tao, Tao Cui From: Tao Cui <cuitao@kylinos.cn> ops.cgroup_set_weight() and ops.cgroup_set_idle() are delivered from scx_group_set_weight() and scx_group_set_idle(), which run from the cpu.weight (and v1 cpu.shares) and cpu.idle cgroup interface write paths in process context. Both hold percpu_down_read(&scx_cgroup_ops_rwsem), whose read side may sleep. The call sites are therefore sleepable, like ops.cgroup_set_bandwidth(), which was recently added to the sleepable allow-list. 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's weight or idle state changes at runtime. Add cgroup_set_weight() and cgroup_set_idle() to the allow-list so these callbacks can allocate on demand, and document that they may block. Also add the matching compatibility markers, so userspace can detect this support via BTF, mirroring scx_compat_marker_cgroup_set_bandwidth_may_sleep(). To size the alternative, a scheduler that gives each cgroup a dedicated idle DSQ must create it in ops.cgroup_init() for every cgroup up front. In a VM with 2000 cgroups that is 2000+ standing DSQs, each a struct scx_dispatch_q plus a per-CPU area. With the allow-list entries the same scheduler can create the DSQ lazily on the first cpu.idle=1 write of a cgroup: 4 allocations for the 4 cgroups marked idle at runtime, and repeated cpu.idle writes do not re-allocate. Verified with a probe scheduler on an unpatched kernel (load rejected with -EINVAL) and on a patched one. Signed-off-by: Tao Cui <cuitao@kylinos.cn> --- kernel/sched/ext/ext.c | 18 ++++++++++++++++++ kernel/sched/ext/internal.h | 9 +++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c index b646711a45fe..53b888f5a6c5 100644 --- a/kernel/sched/ext/ext.c +++ b/kernel/sched/ext/ext.c @@ -8079,7 +8079,9 @@ 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_weight): case offsetof(struct sched_ext_ops, cgroup_set_bandwidth): + case offsetof(struct sched_ext_ops, cgroup_set_idle): #endif case offsetof(struct sched_ext_ops, cpu_online): case offsetof(struct sched_ext_ops, cpu_offline): @@ -11055,3 +11057,19 @@ __initcall(scx_init); #ifdef CONFIG_EXT_GROUP_SCHED DEFINE_SCX_COMPAT_MARKER(cgroup_set_bandwidth_may_sleep); #endif /* CONFIG_EXT_GROUP_SCHED */ + +/* + * scx_compat_marker_cgroup_set_weight_may_sleep: advertises that + * ops.cgroup_set_weight() may be implemented as a sleepable callback. + */ +#ifdef CONFIG_EXT_GROUP_SCHED +DEFINE_SCX_COMPAT_MARKER(cgroup_set_weight_may_sleep); +#endif /* CONFIG_EXT_GROUP_SCHED */ + +/* + * scx_compat_marker_cgroup_set_idle_may_sleep: advertises that + * ops.cgroup_set_idle() may be implemented as a sleepable callback. + */ +#ifdef CONFIG_EXT_GROUP_SCHED +DEFINE_SCX_COMPAT_MARKER(cgroup_set_idle_may_sleep); +#endif /* CONFIG_EXT_GROUP_SCHED */ diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h index 53e136a47924..f81d03de2d3c 100644 --- a/kernel/sched/ext/internal.h +++ b/kernel/sched/ext/internal.h @@ -736,7 +736,7 @@ struct sched_ext_ops { * @cgrp: cgroup whose weight is being updated * @weight: new weight [1..10000] * - * Update @cgrp's weight to @weight. + * Update @cgrp's weight to @weight. This operation may block. * * Knobs of a cgroup belong to the parent, so the set_* ops are * delivered to @cgrp's parent's sched. That sched may never have seen @@ -773,9 +773,10 @@ struct sched_ext_ops { * @cgrp: cgroup whose idle state is being updated * @idle: whether the cgroup is entering or exiting idle state * - * Update @cgrp's idle state to @idle. This callback is invoked when - * a cgroup transitions between idle and non-idle states, allowing the - * BPF scheduler to adjust its behavior accordingly. + * Update @cgrp's idle state to @idle. This operation may block. This + * callback is invoked when a cgroup transitions between idle and + * non-idle states, allowing the BPF scheduler to adjust its behavior + * accordingly. * * Delivery follows the same rule as cgroup_set_weight(). */ -- 2.43.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable 2026-08-25 5:23 [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable Tao Cui @ 2026-08-25 8:27 ` Andrea Righi 2026-08-25 9:28 ` Tao Cui 0 siblings, 1 reply; 3+ messages in thread From: Andrea Righi @ 2026-08-25 8:27 UTC (permalink / raw) To: Tao Cui Cc: tj, void, changwoo, michalblk, suzhidao, sched-ext, linux-kernel, bpf, Tao Cui Hi Tao, On Tue, Aug 25, 2026 at 01:23:36PM +0800, Tao Cui wrote: > From: Tao Cui <cuitao@kylinos.cn> > > ops.cgroup_set_weight() and ops.cgroup_set_idle() are delivered from > scx_group_set_weight() and scx_group_set_idle(), which run from the > cpu.weight (and v1 cpu.shares) and cpu.idle cgroup interface write paths > in process context. Both hold percpu_down_read(&scx_cgroup_ops_rwsem), > whose read side may sleep. The call sites are therefore sleepable, like > ops.cgroup_set_bandwidth(), which was recently added to the sleepable > allow-list. The change itself makes sense, but I think the serialization issue pointed out by sashiko is valid. Concurrent cgroup knob updates can complete their sched_ext notifications out of order, potentially leaving the core scheduler and BPF scheduler with inconsistent state. We should probably address the race first. I may have a fix and will post it shortly (testing right now). Then we can apply this sleepable callback change on top. > > 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's weight or idle state changes at runtime. Add > cgroup_set_weight() and cgroup_set_idle() to the allow-list so these > callbacks can allocate on demand, and document that they may block. > > Also add the matching compatibility markers, so userspace can detect > this support via BTF, mirroring > scx_compat_marker_cgroup_set_bandwidth_may_sleep(). > > To size the alternative, a scheduler that gives each cgroup a > dedicated idle DSQ must create it in ops.cgroup_init() for every > cgroup up front. In a VM with 2000 cgroups that is 2000+ standing > DSQs, each a struct scx_dispatch_q plus a per-CPU area. With the > allow-list entries the same scheduler can create the DSQ lazily on > the first cpu.idle=1 write of a cgroup: 4 allocations for the 4 > cgroups marked idle at runtime, and repeated cpu.idle writes do not re-allocate. Verified with a probe scheduler on an > unpatched kernel (load rejected with -EINVAL) and on a patched one. nit: this paragraph needs a rewrapping (line too long). Besides rewrapping, could we simplify this part and rephrase it in terms of DSQs rather than allocations? For example (something along these lines): Without this support, a scheduler that uses a dedicated idle DSQ for each cgroup must create it eagerly from ops.cgroup_init(). With a sleepable ops.cgroup_set_idle() callback, it can instead create the DSQ lazily when the cgroup is first marked idle, avoiding unnecessary DSQs for cgroups that never use cpu.idle. Thanks, -Andrea > > Signed-off-by: Tao Cui <cuitao@kylinos.cn> > --- > kernel/sched/ext/ext.c | 18 ++++++++++++++++++ > kernel/sched/ext/internal.h | 9 +++++---- > 2 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c > index b646711a45fe..53b888f5a6c5 100644 > --- a/kernel/sched/ext/ext.c > +++ b/kernel/sched/ext/ext.c > @@ -8079,7 +8079,9 @@ 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_weight): > case offsetof(struct sched_ext_ops, cgroup_set_bandwidth): > + case offsetof(struct sched_ext_ops, cgroup_set_idle): > #endif > case offsetof(struct sched_ext_ops, cpu_online): > case offsetof(struct sched_ext_ops, cpu_offline): > @@ -11055,3 +11057,19 @@ __initcall(scx_init); > #ifdef CONFIG_EXT_GROUP_SCHED > DEFINE_SCX_COMPAT_MARKER(cgroup_set_bandwidth_may_sleep); > #endif /* CONFIG_EXT_GROUP_SCHED */ > + > +/* > + * scx_compat_marker_cgroup_set_weight_may_sleep: advertises that > + * ops.cgroup_set_weight() may be implemented as a sleepable callback. > + */ > +#ifdef CONFIG_EXT_GROUP_SCHED > +DEFINE_SCX_COMPAT_MARKER(cgroup_set_weight_may_sleep); > +#endif /* CONFIG_EXT_GROUP_SCHED */ > + > +/* > + * scx_compat_marker_cgroup_set_idle_may_sleep: advertises that > + * ops.cgroup_set_idle() may be implemented as a sleepable callback. > + */ > +#ifdef CONFIG_EXT_GROUP_SCHED > +DEFINE_SCX_COMPAT_MARKER(cgroup_set_idle_may_sleep); > +#endif /* CONFIG_EXT_GROUP_SCHED */ > diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h > index 53e136a47924..f81d03de2d3c 100644 > --- a/kernel/sched/ext/internal.h > +++ b/kernel/sched/ext/internal.h > @@ -736,7 +736,7 @@ struct sched_ext_ops { > * @cgrp: cgroup whose weight is being updated > * @weight: new weight [1..10000] > * > - * Update @cgrp's weight to @weight. > + * Update @cgrp's weight to @weight. This operation may block. > * > * Knobs of a cgroup belong to the parent, so the set_* ops are > * delivered to @cgrp's parent's sched. That sched may never have seen > @@ -773,9 +773,10 @@ struct sched_ext_ops { > * @cgrp: cgroup whose idle state is being updated > * @idle: whether the cgroup is entering or exiting idle state > * > - * Update @cgrp's idle state to @idle. This callback is invoked when > - * a cgroup transitions between idle and non-idle states, allowing the > - * BPF scheduler to adjust its behavior accordingly. > + * Update @cgrp's idle state to @idle. This operation may block. This > + * callback is invoked when a cgroup transitions between idle and > + * non-idle states, allowing the BPF scheduler to adjust its behavior > + * accordingly. > * > * Delivery follows the same rule as cgroup_set_weight(). > */ > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable 2026-08-25 8:27 ` Andrea Righi @ 2026-08-25 9:28 ` Tao Cui 0 siblings, 0 replies; 3+ messages in thread From: Tao Cui @ 2026-08-25 9:28 UTC (permalink / raw) To: Andrea Righi Cc: cui.tao, tj, void, changwoo, michalblk, suzhidao, sched-ext, linux-kernel, bpf, Tao Cui Hi Andrea, 在 2026/8/25 16:27, Andrea Righi 写道: > Hi Tao, > > On Tue, Aug 25, 2026 at 01:23:36PM +0800, Tao Cui wrote: >> From: Tao Cui <cuitao@kylinos.cn> >> >> ops.cgroup_set_weight() and ops.cgroup_set_idle() are delivered from >> scx_group_set_weight() and scx_group_set_idle(), which run from the >> cpu.weight (and v1 cpu.shares) and cpu.idle cgroup interface write paths >> in process context. Both hold percpu_down_read(&scx_cgroup_ops_rwsem), >> whose read side may sleep. The call sites are therefore sleepable, like >> ops.cgroup_set_bandwidth(), which was recently added to the sleepable >> allow-list. > > The change itself makes sense, but I think the serialization issue pointed out > by sashiko is valid. Concurrent cgroup knob updates can complete their sched_ext > notifications out of order, potentially leaving the core scheduler and BPF > scheduler with inconsistent state. > > We should probably address the race first. I may have a fix and will post it > shortly (testing right now). Then we can apply this sleepable callback change on > top. > Thanks for the review. Agreed on the ordering: I'll wait for the serialization fix to land and rebase this on top before sending v2. >> >> 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's weight or idle state changes at runtime. Add >> cgroup_set_weight() and cgroup_set_idle() to the allow-list so these >> callbacks can allocate on demand, and document that they may block. >> >> Also add the matching compatibility markers, so userspace can detect >> this support via BTF, mirroring >> scx_compat_marker_cgroup_set_bandwidth_may_sleep(). >> >> To size the alternative, a scheduler that gives each cgroup a >> dedicated idle DSQ must create it in ops.cgroup_init() for every >> cgroup up front. In a VM with 2000 cgroups that is 2000+ standing >> DSQs, each a struct scx_dispatch_q plus a per-CPU area. With the >> allow-list entries the same scheduler can create the DSQ lazily on >> the first cpu.idle=1 write of a cgroup: 4 allocations for the 4 >> cgroups marked idle at runtime, and repeated cpu.idle writes do not re-allocate. Verified with a probe scheduler on an >> unpatched kernel (load rejected with -EINVAL) and on a patched one. > > nit: this paragraph needs a rewrapping (line too long). > > Besides rewrapping, could we simplify this part and rephrase it in terms of DSQs > rather than allocations? For example (something along these lines): > > Without this support, a scheduler that uses a dedicated idle DSQ for each > cgroup must create it eagerly from ops.cgroup_init(). With a sleepable > ops.cgroup_set_idle() callback, it can instead create the DSQ > lazily when the cgroup is first marked idle, avoiding unnecessary DSQs > for cgroups that never use cpu.idle. > For the commit message, v2 will rewrap the last paragraph and rephrase it in terms of DSQs following your suggestion. Thanks, Tao > Thanks, > -Andrea > >> >> Signed-off-by: Tao Cui <cuitao@kylinos.cn> >> --- >> kernel/sched/ext/ext.c | 18 ++++++++++++++++++ >> kernel/sched/ext/internal.h | 9 +++++---- >> 2 files changed, 23 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c >> index b646711a45fe..53b888f5a6c5 100644 >> --- a/kernel/sched/ext/ext.c >> +++ b/kernel/sched/ext/ext.c >> @@ -8079,7 +8079,9 @@ 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_weight): >> case offsetof(struct sched_ext_ops, cgroup_set_bandwidth): >> + case offsetof(struct sched_ext_ops, cgroup_set_idle): >> #endif >> case offsetof(struct sched_ext_ops, cpu_online): >> case offsetof(struct sched_ext_ops, cpu_offline): >> @@ -11055,3 +11057,19 @@ __initcall(scx_init); >> #ifdef CONFIG_EXT_GROUP_SCHED >> DEFINE_SCX_COMPAT_MARKER(cgroup_set_bandwidth_may_sleep); >> #endif /* CONFIG_EXT_GROUP_SCHED */ >> + >> +/* >> + * scx_compat_marker_cgroup_set_weight_may_sleep: advertises that >> + * ops.cgroup_set_weight() may be implemented as a sleepable callback. >> + */ >> +#ifdef CONFIG_EXT_GROUP_SCHED >> +DEFINE_SCX_COMPAT_MARKER(cgroup_set_weight_may_sleep); >> +#endif /* CONFIG_EXT_GROUP_SCHED */ >> + >> +/* >> + * scx_compat_marker_cgroup_set_idle_may_sleep: advertises that >> + * ops.cgroup_set_idle() may be implemented as a sleepable callback. >> + */ >> +#ifdef CONFIG_EXT_GROUP_SCHED >> +DEFINE_SCX_COMPAT_MARKER(cgroup_set_idle_may_sleep); >> +#endif /* CONFIG_EXT_GROUP_SCHED */ >> diff --git a/kernel/sched/ext/internal.h b/kernel/sched/ext/internal.h >> index 53e136a47924..f81d03de2d3c 100644 >> --- a/kernel/sched/ext/internal.h >> +++ b/kernel/sched/ext/internal.h >> @@ -736,7 +736,7 @@ struct sched_ext_ops { >> * @cgrp: cgroup whose weight is being updated >> * @weight: new weight [1..10000] >> * >> - * Update @cgrp's weight to @weight. >> + * Update @cgrp's weight to @weight. This operation may block. >> * >> * Knobs of a cgroup belong to the parent, so the set_* ops are >> * delivered to @cgrp's parent's sched. That sched may never have seen >> @@ -773,9 +773,10 @@ struct sched_ext_ops { >> * @cgrp: cgroup whose idle state is being updated >> * @idle: whether the cgroup is entering or exiting idle state >> * >> - * Update @cgrp's idle state to @idle. This callback is invoked when >> - * a cgroup transitions between idle and non-idle states, allowing the >> - * BPF scheduler to adjust its behavior accordingly. >> + * Update @cgrp's idle state to @idle. This operation may block. This >> + * callback is invoked when a cgroup transitions between idle and >> + * non-idle states, allowing the BPF scheduler to adjust its behavior >> + * accordingly. >> * >> * Delivery follows the same rule as cgroup_set_weight(). >> */ >> -- >> 2.43.0 >> ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-25 9:28 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-25 5:23 [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable Tao Cui 2026-08-25 8:27 ` Andrea Righi 2026-08-25 9:28 ` Tao Cui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox