[PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()

[PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()

[Changwoo Min]

Concurrent writes to a cgroup's cpu.max are not serialized by the cgroup or
kernfs layer -- cgroup_file_write() calls cft->write without cgroup_mutex, and
kernfs only serializes per open file -- so two writers to the same cgroup
through separate open files can reach tg_set_bandwidth() concurrently.

tg_set_cfs_bandwidth() serializes the CFS side under cfs_constraints_mutex,
but scx_group_set_bandwidth() runs afterwards with only
percpu_down_read(&scx_cgroup_ops_rwsem) held, a read lock, so it does not
serialize concurrent writers.

The ops.cgroup_set_bandwidth() callback and the cached tg->scx.bw_* stores can
then interleave between writers:

  CPU1 (writer A)                 CPU2 (writer B)
  scx_group_set_bandwidth()
    SCX_CALL_OP(...)  /* A */
                                  scx_group_set_bandwidth()
                                    SCX_CALL_OP(...)  /* B */
                                    tg->scx.bw_* = B
    tg->scx.bw_* = A

The scheduler's cgroup_set_bandwidth() op is invoked out of order and the cached
state is left inconsistent with the last writer; the 64-bit bw_* stores can also
tear on 32-bit.

Serialize the SCX-side update with a new scx_cgroup_set_bw_mutex held across the
callback and the stores, so each writer applies its update atomically and in one
order -- the SCX counterpart to cfs_constraints_mutex on the CFS side.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/sched-ext/20260817172131.BCDA51F000E9@smtp.kernel.org/
Signed-off-by: Changwoo Min <changwoo@igalia.com>
---
 kernel/sched/ext/ext.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index b646711a45fe..a2fc581d9636 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -4677,6 +4677,13 @@ bool scx_can_stop_tick(struct rq *rq)
 
 DEFINE_STATIC_PERCPU_RWSEM(scx_cgroup_ops_rwsem);
 
+/*
+ * Serialize concurrent cpu.max writers to the same cgroup so the
+ * ops.cgroup_set_bandwidth() callback and the cached tg->scx.bw_* values update
+ * atomically in one order -- the SCX-side counterpart to cfs_constraints_mutex.
+ */
+static DEFINE_MUTEX(scx_cgroup_set_bw_mutex);
+
 void scx_tg_init(struct task_group *tg)
 {
 	tg->scx.weight = CGROUP_WEIGHT_DFL;
@@ -4945,6 +4952,7 @@ void scx_group_set_bandwidth(struct task_group *tg,
 	struct scx_sched *sch;
 
 	percpu_down_read(&scx_cgroup_ops_rwsem);
+	mutex_lock(&scx_cgroup_set_bw_mutex);
 	sch = scx_tg_knob_sched(tg);
 
 	if (scx_cgroup_enabled && sch && SCX_HAS_OP(sch, cgroup_set_bandwidth) &&
@@ -4958,6 +4966,7 @@ void scx_group_set_bandwidth(struct task_group *tg,
 	tg->scx.bw_quota_us = quota_us;
 	tg->scx.bw_burst_us = burst_us;
 
+	mutex_unlock(&scx_cgroup_set_bw_mutex);
 	percpu_up_read(&scx_cgroup_ops_rwsem);
 }
 #endif	/* CONFIG_EXT_GROUP_SCHED */
-- 
2.55.0

Re: [PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Acquiring a global mutex inside a per-CPU rwsem read-side critical section causes severe starvation for scheduler operations.

Pre-existing issues:
- [High] Concurrent writers to cgroup bandwidth limits cause the CFS and SCX states to permanently diverge due to split-lock domains.
- [High] The write-side mutex fails to protect the lockless Read-Modify-Write cycle, resulting in lost updates and permanently committed torn reads.
- [High] Missing serialization for other unprotected cgroup attributes (`cpu.weight` and `cpu.idle`).
--

--- Patch [1]: [PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth() ---
commit bbcc7e2a9caf9650a3bf7bd4d7036631840fbfd4
Author: Changwoo Min <changwoo@igalia.com>

sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()

Concurrent writes to a cgroup's cpu.max are not serialized by the cgroup or
kernfs layer. This patch adds scx_cgroup_set_bw_mutex to serialize SCX-side
updates, preventing interleaved writers and torn 64-bit stores.

Link: https://lore.kernel.org/sched-ext/20260817172131.BCDA51F000E9@smtp.kernel.org/

[Severity: High]
This is a pre-existing issue, but does the split-lock design between CFS
and SCX cause their states to permanently diverge?

When two concurrent writers modify cpu.max, tg_set_bandwidth() updates
the CFS subsystem sequentially under cfs_constraints_mutex, and then the
SCX subsystem under the new scx_cgroup_set_bw_mutex:

kernel/sched/core.c:tg_set_bandwidth()
#ifdef CONFIG_CFS_BANDWIDTH
    ret = tg_set_cfs_bandwidth(tg, period_us, quota_us, burst_us);
#endif /* CONFIG_CFS_BANDWIDTH */
    if (!ret)
        scx_group_set_bandwidth(tg, period_us, quota_us, burst_us);

Because these locks are not held atomically across both updates, writers
can interleave. CFS might commit the second writer's limits while SCX
commits the first writer's limits.

[Severity: High]
This isn't a bug introduced by this patch, but does the write-side mutex
fail to protect the lockless Read-Modify-Write cycle?

Handlers like cpu_max_write() and cpu_burst_write_u64() perform an RMW
cycle by calling tg_bandwidth() to read existing limits locklessly:

kernel/sched/core.c:tg_bandwidth()
#else /* !CONFIG_CFS_BANDWIDTH */
    if (period_us_p)
        *period_us_p = tg->scx.bw_period_us;
    if (quota_us_p)
        *quota_us_p = tg->scx.bw_quota_us;

They then modify one field and write everything back via tg_set_bandwidth().

Concurrent writers to different fields will read each other's stale data
and overwrite the other's update. Since the new scx_cgroup_set_bw_mutex
only protects the final write, doesn't this leave the RMW cycle broken,
allowing corrupted or torn 64-bit limits to be written back on 32-bit
architectures?

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index b646711a45fee..a2fc581d9636d 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
[ ... ]
> @@ -4945,6 +4952,7 @@ void scx_group_set_bandwidth(struct task_group *tg,
>  	struct scx_sched *sch;
>  
>  	percpu_down_read(&scx_cgroup_ops_rwsem);
> +	mutex_lock(&scx_cgroup_set_bw_mutex);

[Severity: Medium]
Does acquiring this global mutex inside a per-CPU rwsem read-side locking
section cause starvation for scheduler operations?

If multiple threads write to cpu.max across any cgroups system-wide, they
will all acquire the read lock and queue up sequentially on the global
mutex. If a system thread then attempts an operation requiring the write
lock, it must wait for every queued thread to individually acquire the
mutex, run a potentially slow BPF operation, and release the read lock.

Would reversing the lock order avoid inflating the read-side hold time?

[ ... ]

[Severity: High]
This is a pre-existing issue, but do other cgroup attributes like
cpu.weight and cpu.idle lack serialization?

For example, in scx_group_set_weight(), concurrent writers can
race through the SCX_CALL_OP() dispatch and the subsequent assignment
to tg->scx.weight.

kernel/sched/ext/ext.c:scx_group_set_weight()
    if (scx_cgroup_enabled && sch && SCX_HAS_OP(sch, cgroup_set_weight) &&
        tg->scx.weight != weight)
        SCX_CALL_OP(sch, cgroup_set_weight, NULL, tg_cgrp(tg), weight);

    tg->scx.weight = weight;

Since the BPF dispatch and the cache update are not atomically linked,
writers can interleave, leaving the BPF scheduler applying one weight
while the kernel caches another. Should similar serialization be applied
to scx_group_set_weight() and scx_group_set_idle() to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821103519.535987-1-changwoo@igalia.com?part=1

Re: [PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()

[Tejun Heo]

On Fri, Aug 21, 2026 at 07:35:19PM +0900, Changwoo Min wrote:
> Concurrent writes to a cgroup's cpu.max are not serialized by the cgroup or
> kernfs layer -- cgroup_file_write() calls cft->write without cgroup_mutex, and
> kernfs only serializes per open file -- so two writers to the same cgroup
> through separate open files can reach tg_set_bandwidth() concurrently.
> 
> tg_set_cfs_bandwidth() serializes the CFS side under cfs_constraints_mutex,
> but scx_group_set_bandwidth() runs afterwards with only
> percpu_down_read(&scx_cgroup_ops_rwsem) held, a read lock, so it does not
> serialize concurrent writers.
> 
> The ops.cgroup_set_bandwidth() callback and the cached tg->scx.bw_* stores can
> then interleave between writers:
> 
>   CPU1 (writer A)                 CPU2 (writer B)
>   scx_group_set_bandwidth()
>     SCX_CALL_OP(...)  /* A */
>                                   scx_group_set_bandwidth()
>                                     SCX_CALL_OP(...)  /* B */
>                                     tg->scx.bw_* = B
>     tg->scx.bw_* = A
> 
> The scheduler's cgroup_set_bandwidth() op is invoked out of order and the cached
> state is left inconsistent with the last writer; the 64-bit bw_* stores can also
> tear on 32-bit.
> 
> Serialize the SCX-side update with a new scx_cgroup_set_bw_mutex held across the
> callback and the stores, so each writer applies its update atomically and in one
> order -- the SCX counterpart to cfs_constraints_mutex on the CFS side.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/sched-ext/20260817172131.BCDA51F000E9@smtp.kernel.org/
> Signed-off-by: Changwoo Min <changwoo@igalia.com>

I think the following is a better way to resolve this:

  http://lkml.kernel.org/r/20260821140818.1559100-1-michalblk@google.com

Thanks.

-- 
tejun

Re: [PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()

[Changwoo Min]

Hello,

On 8/22/26 4:13 AM, Tejun Heo wrote:
> On Fri, Aug 21, 2026 at 07:35:19PM +0900, Changwoo Min wrote:
>> Concurrent writes to a cgroup's cpu.max are not serialized by the cgroup or
>> kernfs layer -- cgroup_file_write() calls cft->write without cgroup_mutex, and
>> kernfs only serializes per open file -- so two writers to the same cgroup
>> through separate open files can reach tg_set_bandwidth() concurrently.
>>
>> tg_set_cfs_bandwidth() serializes the CFS side under cfs_constraints_mutex,
>> but scx_group_set_bandwidth() runs afterwards with only
>> percpu_down_read(&scx_cgroup_ops_rwsem) held, a read lock, so it does not
>> serialize concurrent writers.
>>
>> The ops.cgroup_set_bandwidth() callback and the cached tg->scx.bw_* stores can
>> then interleave between writers:
>>
>>    CPU1 (writer A)                 CPU2 (writer B)
>>    scx_group_set_bandwidth()
>>      SCX_CALL_OP(...)  /* A */
>>                                    scx_group_set_bandwidth()
>>                                      SCX_CALL_OP(...)  /* B */
>>                                      tg->scx.bw_* = B
>>      tg->scx.bw_* = A
>>
>> The scheduler's cgroup_set_bandwidth() op is invoked out of order and the cached
>> state is left inconsistent with the last writer; the 64-bit bw_* stores can also
>> tear on 32-bit.
>>
>> Serialize the SCX-side update with a new scx_cgroup_set_bw_mutex held across the
>> callback and the stores, so each writer applies its update atomically and in one
>> order -- the SCX counterpart to cfs_constraints_mutex on the CFS side.
>>
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Link: https://lore.kernel.org/sched-ext/20260817172131.BCDA51F000E9@smtp.kernel.org/
>> Signed-off-by: Changwoo Min <changwoo@igalia.com>
> 
> I think the following is a better way to resolve this:
> 
>    http://lkml.kernel.org/r/20260821140818.1559100-1-michalblk@google.com

Thanks for pointing this out. I missed the Michal's patch. Will take a look.

Regards,
Changwoo Min