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

Changwoo Min <[email protected]>
Newsgroups dev.linux.lists.sched-ext,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
Link: https://lore.kernel.org/sched-ext/[email protected]/
Signed-off-by: Changwoo Min <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.