Concurrent cgroup knob writes update the core scheduler under its
internal locks and notify sched_ext only after those locks are released.

The notifications can therefore complete out of order, leaving the core
scheduler, the BPF scheduler and the cached sched_ext state with
different values.

Add a per-task-group mutex and hold it across both the core scheduler
update and the matching sched_ext notification. Serialize weight, idle
and bandwidth updates so that their callbacks and cached state follow
the same order as the core scheduler changes.

Cc: Tao Cui <cui.tao@linux.dev>
Fixes: 819513666966 ("sched_ext: Add cgroup support")
Fixes: 347ed2d566da ("sched/ext: Implement cgroup_set_idle() callback")
Fixes: ddceadce63d9 ("sched_ext: Add support for cgroup bandwidth control interface")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/r/20260825053648.EF7D41F000E9@smtp.kernel.org
Signed-off-by: Andrea Righi <arighi@nvidia.com>
---
 include/linux/sched/ext.h |  5 +++++
 kernel/sched/core.c       | 28 ++++++++++++++++++----------
 kernel/sched/ext/ext.c    |  1 +
 kernel/sched/ext/ext.h    | 14 ++++++++++++++
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/include/linux/sched/ext.h b/include/linux/sched/ext.h
index 582d7cd4a9839..380b4b96fd08f 100644
--- a/include/linux/sched/ext.h
+++ b/include/linux/sched/ext.h
@@ -317,6 +317,11 @@ struct scx_task_group {
 	 * cgroup1.
 	 */
 	struct scx_sched	*sched;
+	/*
+	 * Serialize cgroup knob updates from the core scheduler change through
+	 * the matching ops.cgroup_set_*() callback and cached state update.
+	 */
+	struct mutex		knob_mutex;
 
 	u32			flags;		/* SCX_TG_* */
 	u32			weight;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 145eea2d99a1c..2239ea8e78d15 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9820,14 +9820,16 @@ static unsigned long tg_weight(struct task_group *tg)
 static int cpu_shares_write_u64(struct cgroup_subsys_state *css,
 				struct cftype *cftype, u64 shareval)
 {
+	struct task_group *tg = css_tg(css);
 	int ret;
 
 	if (shareval > scale_load_down(ULONG_MAX))
 		shareval = MAX_SHARES;
-	ret = sched_group_set_shares(css_tg(css), scale_load(shareval));
+
+	guard(scx_group_knob)(tg);
+	ret = sched_group_set_shares(tg, scale_load(shareval));
 	if (!ret)
-		scx_group_set_weight(css_tg(css),
-				     sched_weight_to_cgroup(shareval));
+		scx_group_set_weight(tg, sched_weight_to_cgroup(shareval));
 	return ret;
 }
 
@@ -10159,6 +10161,7 @@ static int tg_set_bandwidth(struct task_group *tg,
 					burst_us + quota_us > max_bw_runtime_us))
 		return -EINVAL;
 
+	guard(scx_group_knob)(tg);
 #ifdef CONFIG_CFS_BANDWIDTH
 	ret = tg_set_cfs_bandwidth(tg, period_us, quota_us, burst_us);
 #endif /* CONFIG_CFS_BANDWIDTH */
@@ -10255,11 +10258,13 @@ static s64 cpu_idle_read_s64(struct cgroup_subsys_state *css,
 static int cpu_idle_write_s64(struct cgroup_subsys_state *css,
 				struct cftype *cft, s64 idle)
 {
+	struct task_group *tg = css_tg(css);
 	int ret;
 
-	ret = sched_group_set_idle(css_tg(css), idle);
+	guard(scx_group_knob)(tg);
+	ret = sched_group_set_idle(tg, idle);
 	if (!ret)
-		scx_group_set_idle(css_tg(css), idle);
+		scx_group_set_idle(tg, idle);
 	return ret;
 }
 #endif /* CONFIG_GROUP_SCHED_WEIGHT */
@@ -10425,6 +10430,7 @@ static u64 cpu_weight_read_u64(struct cgroup_subsys_state *css,
 static int cpu_weight_write_u64(struct cgroup_subsys_state *css,
 				struct cftype *cft, u64 cgrp_weight)
 {
+	struct task_group *tg = css_tg(css);
 	unsigned long weight;
 	int ret;
 
@@ -10433,9 +10439,10 @@ static int cpu_weight_write_u64(struct cgroup_subsys_state *css,
 
 	weight = sched_weight_from_cgroup(cgrp_weight);
 
-	ret = sched_group_set_shares(css_tg(css), scale_load(weight));
+	guard(scx_group_knob)(tg);
+	ret = sched_group_set_shares(tg, scale_load(weight));
 	if (!ret)
-		scx_group_set_weight(css_tg(css), cgrp_weight);
+		scx_group_set_weight(tg, cgrp_weight);
 	return ret;
 }
 
@@ -10460,6 +10467,7 @@ static s64 cpu_weight_nice_read_s64(struct cgroup_subsys_state *css,
 static int cpu_weight_nice_write_s64(struct cgroup_subsys_state *css,
 				     struct cftype *cft, s64 nice)
 {
+	struct task_group *tg = css_tg(css);
 	unsigned long weight;
 	int idx, ret;
 
@@ -10470,10 +10478,10 @@ static int cpu_weight_nice_write_s64(struct cgroup_subsys_state *css,
 	idx = array_index_nospec(idx, 40);
 	weight = sched_prio_to_weight[idx];
 
-	ret = sched_group_set_shares(css_tg(css), scale_load(weight));
+	guard(scx_group_knob)(tg);
+	ret = sched_group_set_shares(tg, scale_load(weight));
 	if (!ret)
-		scx_group_set_weight(css_tg(css),
-				     sched_weight_to_cgroup(weight));
+		scx_group_set_weight(tg, sched_weight_to_cgroup(weight));
 	return ret;
 }
 #endif /* CONFIG_GROUP_SCHED_WEIGHT */
diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index c539d15cda637..00926485e1b77 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -4679,6 +4679,7 @@ DEFINE_STATIC_PERCPU_RWSEM(scx_cgroup_ops_rwsem);
 
 void scx_tg_init(struct task_group *tg)
 {
+	mutex_init(&tg->scx.knob_mutex);
 	tg->scx.weight = CGROUP_WEIGHT_DFL;
 	tg->scx.bw_period_us = default_bw_period_us();
 	tg->scx.bw_quota_us = RUNTIME_INF;
diff --git a/kernel/sched/ext/ext.h b/kernel/sched/ext/ext.h
index 0b7fc46aee08c..68a455e23e3cf 100644
--- a/kernel/sched/ext/ext.h
+++ b/kernel/sched/ext/ext.h
@@ -78,6 +78,15 @@ void scx_tg_offline(struct task_group *tg);
 int scx_cgroup_can_attach(struct cgroup_taskset *tset);
 void scx_cgroup_move_task(struct task_struct *p);
 void scx_cgroup_cancel_attach(struct cgroup_taskset *tset);
+static inline void scx_group_knob_lock(struct task_group *tg)
+{
+	mutex_lock(&tg->scx.knob_mutex);
+}
+
+static inline void scx_group_knob_unlock(struct task_group *tg)
+{
+	mutex_unlock(&tg->scx.knob_mutex);
+}
 void scx_group_set_weight(struct task_group *tg, unsigned long cgrp_weight);
 void scx_group_set_idle(struct task_group *tg, bool idle);
 void scx_group_set_bandwidth(struct task_group *tg, u64 period_us, u64 quota_us, u64 burst_us);
@@ -88,8 +97,13 @@ static inline void scx_tg_offline(struct task_group *tg) {}
 static inline int scx_cgroup_can_attach(struct cgroup_taskset *tset) { return 0; }
 static inline void scx_cgroup_move_task(struct task_struct *p) {}
 static inline void scx_cgroup_cancel_attach(struct cgroup_taskset *tset) {}
+static inline void scx_group_knob_lock(struct task_group *tg) {}
+static inline void scx_group_knob_unlock(struct task_group *tg) {}
 static inline void scx_group_set_weight(struct task_group *tg, unsigned long cgrp_weight) {}
 static inline void scx_group_set_idle(struct task_group *tg, bool idle) {}
 static inline void scx_group_set_bandwidth(struct task_group *tg, u64 period_us, u64 quota_us, u64 burst_us) {}
 #endif	/* CONFIG_EXT_GROUP_SCHED */
+
+DEFINE_GUARD(scx_group_knob, struct task_group *,
+	     scx_group_knob_lock(_T), scx_group_knob_unlock(_T));
 #endif	/* CONFIG_CGROUP_SCHED */
-- 
2.55.0

Hello, Andrea.

On Tue, Aug 25, 2026 at 11:21:53AM +0200, Andrea Righi wrote:
> Concurrent cgroup knob writes update the core scheduler under its
> internal locks and notify sched_ext only after those locks are released.

This is the same race Michal's patch fixes by lifting the fair locking
into the core write handlers so that both updates run under the same
lock:

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

I'm inclined to go with that one.

Thanks.

--
tejun