Re: [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable

[Tao Cui <cui.tao@linux.dev>]

Hello,

在 2026/8/25 13:36, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Concurrent cgroup writes to `cpu.weight`, `cpu.shares`, or `cpu.idle` can permanently desynchronize the core scheduler and BPF scheduler state, and cause data races on SCX fields.
> --
> 
> --- Patch [1]: [PATCH] sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable ---
> commit 1a01a0e2918fcf8ca79c6152b56ecfa3e223935c
> Author: Tao Cui <cuitao@kylinos.cn>
> 
> sched_ext: Allow ops.cgroup_set_weight/idle() to be sleepable
> 
> 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 cpu.idle cgroup interface write paths. This patch adds
> them to the allow-list so these callbacks can allocate on demand.
> 
>> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
>> index c539d15cda637..dd4ecc0b5201d 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):
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does making these operations
> sleepable widen a race condition where concurrent cgroup writes could
> permanently desynchronize the core scheduler and BPF scheduler state?
> 
> Looking at kernel/sched/core.c:cpu_weight_write_u64(), the core scheduler is
> updated inside sched_group_set_shares() (which takes and drops shares_mutex),
> leaving a gap before the BPF scheduler is updated:
> 
> kernel/sched/core.c:cpu_weight_write_u64() {
>     ...
> 	ret = sched_group_set_shares(css_tg(css), scale_load(weight));
> 	if (!ret)
> 		scx_group_set_weight(css_tg(css), cgrp_weight);
>     ...
> }
> 
> Inside scx_group_set_weight(), the SCX scheduler state and tg->scx.weight
> are updated without serialization (holding only a read lock):
> 
> kernel/sched/ext/ext.c:scx_group_set_weight() {
>     ...
> 	SCX_CALL_OP(sch, cgroup_set_weight, NULL, tg_cgrp(tg), weight);
> 
> 	tg->scx.weight = weight;
>     ...
> }
> 
> The same pattern exists in kernel/sched/core.c:cpu_idle_write_s64() calling
> scx_group_set_idle() and updating tg->scx.idle.
> 
> If two threads concurrently write to cpu.weight or cpu.idle, they serialize on
> shares_mutex but drop it before calling the SCX functions. Because this patch
> allows ops.cgroup_set_weight and ops.cgroup_set_idle to sleep, could the
> threads execute out of order? 
> 
> If Thread B's core scheduler update runs after Thread A's, but Thread B's BPF
> callback finishes before Thread A's, the final state in the core scheduler
> would be B, while the SCX state ends up as A. Can this lead to incorrect
> functional behavior and data races on tg->scx.weight and tg->scx.idle?
> 

That race is pre-existing: the window between
sched_group_set_shares() dropping its lock and scx_group_set_weight()
running is unserialized even without a sleepable callback. Making
the callbacks sleepable only widens the window.

Michal's "Lift cgroup update locking to core to prevent CFS/SCX
divergence" series covers both updates with core-side locking and
fixes it, so this patch doesn't grow a fix for it.

Thanks.

>>  #endif
>>  	case offsetof(struct sched_ext_ops, cpu_online):
> 
> [ ... ]
>