1. Patchew
  2. linux
  3. View series

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

Changwoo Min posted 1 patch 1 month, 1 week ago 
kernel/sched/ext/ext.c | 9 +++++++++
1 file changed, 9 insertions(+)
[PATCH] sched_ext: serialize concurrent cpu.max writers in scx_group_set_bandwidth()
Posted by Changwoo Min 1 month, 1 week ago 
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()
Posted by Tejun Heo 1 month, 1 week ago 
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()
Posted by Changwoo Min 1 month, 1 week ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.