The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [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