All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sched: Serialize cgroup updates to prevent CFS/SCX state divergence
@ 2026-08-20 16:09 Michal Blaszczyk
  2026-08-20 17:21 ` Tejun Heo
  2026-08-21 14:08 ` [PATCH v2] sched: Lift cgroup update locking to core to prevent CFS/SCX divergence Michal Blaszczyk
  0 siblings, 2 replies; 5+ messages in thread
From: Michal Blaszczyk @ 2026-08-20 16:09 UTC (permalink / raw)
  To: Peter Zijlstra, Tejun Heo, David Vernet, Andrea Righi,
	Changwoo Min
  Cc: Michal Blaszczyk, Kuba Piecuch, sched-ext, linux-kernel

Concurrent writes to cgroup control files (such as cpu.shares or
cpu.weight) can lead to state divergence between CFS and SCX.

For instance, in cpu_shares_write_u64(), the CFS update is serialized
by shares_mutex (internal to fair.c), but this lock is dropped before
scx_group_set_weight() is called. The latter only acquires a read
semaphore (scx_cgroup_ops_rwsem), allowing multiple threads to evaluate
and act on the sched_ext update concurrently.

This serialization gap allows concurrent writes to interleave.
As a result, the recorded state in CFS, the SCX internal bookkeeping
(e.g., tg->scx.weight), and the BPF scheduler itself can end up operating
on completely distinct parameters (pairwise distinct values).

Similar races are present in tg_set_bandwidth(), cpu_idle_write_s64(),
cpu_weight_write_u64(), and cpu_weight_nice_write_s64().

Fix this by introducing scx_cgroup_mutex in kernel/sched/core.c to
serialize these file write operations.

Fixes: 819513666966 ("sched_ext: Add cgroup support")
Signed-off-by: Michal Blaszczyk <michalblk@google.com>
---
 kernel/sched/core.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f5f7ff8c680a..e3e84f7b9da9 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9789,6 +9789,11 @@ static unsigned long tg_weight(struct task_group *tg)
 #endif
 }
 
+/* Serializes concurrent cgroup updates to keep CFS and SCX state consistent */
+#ifdef CONFIG_EXT_GROUP_SCHED
+static DEFINE_MUTEX(scx_cgroup_mutex);
+#endif
+
 static int cpu_shares_write_u64(struct cgroup_subsys_state *css,
 				struct cftype *cftype, u64 shareval)
 {
@@ -9796,6 +9801,10 @@ static int cpu_shares_write_u64(struct cgroup_subsys_state *css,
 
 	if (shareval > scale_load_down(ULONG_MAX))
 		shareval = MAX_SHARES;
+
+#ifdef CONFIG_EXT_GROUP_SCHED
+	guard(mutex)(&scx_cgroup_mutex);
+#endif
 	ret = sched_group_set_shares(css_tg(css), scale_load(shareval));
 	if (!ret)
 		scx_group_set_weight(css_tg(css),
@@ -10131,6 +10140,9 @@ static int tg_set_bandwidth(struct task_group *tg,
 					burst_us + quota_us > max_bw_runtime_us))
 		return -EINVAL;
 
+#ifdef CONFIG_EXT_GROUP_SCHED
+	guard(mutex)(&scx_cgroup_mutex);
+#endif
 #ifdef CONFIG_CFS_BANDWIDTH
 	ret = tg_set_cfs_bandwidth(tg, period_us, quota_us, burst_us);
 #endif /* CONFIG_CFS_BANDWIDTH */
@@ -10229,6 +10241,9 @@ static int cpu_idle_write_s64(struct cgroup_subsys_state *css,
 {
 	int ret;
 
+#ifdef CONFIG_EXT_GROUP_SCHED
+	guard(mutex)(&scx_cgroup_mutex);
+#endif
 	ret = sched_group_set_idle(css_tg(css), idle);
 	if (!ret)
 		scx_group_set_idle(css_tg(css), idle);
@@ -10405,6 +10420,9 @@ static int cpu_weight_write_u64(struct cgroup_subsys_state *css,
 
 	weight = sched_weight_from_cgroup(cgrp_weight);
 
+#ifdef CONFIG_EXT_GROUP_SCHED
+	guard(mutex)(&scx_cgroup_mutex);
+#endif
 	ret = sched_group_set_shares(css_tg(css), scale_load(weight));
 	if (!ret)
 		scx_group_set_weight(css_tg(css), cgrp_weight);
@@ -10442,6 +10460,9 @@ static int cpu_weight_nice_write_s64(struct cgroup_subsys_state *css,
 	idx = array_index_nospec(idx, 40);
 	weight = sched_prio_to_weight[idx];
 
+#ifdef CONFIG_EXT_GROUP_SCHED
+	guard(mutex)(&scx_cgroup_mutex);
+#endif
 	ret = sched_group_set_shares(css_tg(css), scale_load(weight));
 	if (!ret)
 		scx_group_set_weight(css_tg(css),
-- 
2.55.0.737.g08866a6d13-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] sched: Serialize cgroup updates to prevent CFS/SCX state divergence
  2026-08-20 16:09 [PATCH] sched: Serialize cgroup updates to prevent CFS/SCX state divergence Michal Blaszczyk
@ 2026-08-20 17:21 ` Tejun Heo
  2026-08-21  7:26   ` Peter Zijlstra
  2026-08-21 14:08 ` [PATCH v2] sched: Lift cgroup update locking to core to prevent CFS/SCX divergence Michal Blaszczyk
  1 sibling, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2026-08-20 17:21 UTC (permalink / raw)
  To: Michal Blaszczyk
  Cc: Peter Zijlstra, David Vernet, Andrea Righi, Changwoo Min,
	Kuba Piecuch, sched-ext, linux-kernel

Hello,

On Thu, Aug 20, 2026 at 04:09:56PM +0000, Michal Blaszczyk wrote:
> Concurrent writes to cgroup control files (such as cpu.shares or
> cpu.weight) can lead to state divergence between CFS and SCX.
> 
> For instance, in cpu_shares_write_u64(), the CFS update is serialized
> by shares_mutex (internal to fair.c), but this lock is dropped before
> scx_group_set_weight() is called. The latter only acquires a read
> semaphore (scx_cgroup_ops_rwsem), allowing multiple threads to evaluate
> and act on the sched_ext update concurrently.
> 
> This serialization gap allows concurrent writes to interleave.
> As a result, the recorded state in CFS, the SCX internal bookkeeping
> (e.g., tg->scx.weight), and the BPF scheduler itself can end up operating
> on completely distinct parameters (pairwise distinct values).
> 
> Similar races are present in tg_set_bandwidth(), cpu_idle_write_s64(),
> cpu_weight_write_u64(), and cpu_weight_nice_write_s64().
> 
> Fix this by introducing scx_cgroup_mutex in kernel/sched/core.c to
> serialize these file write operations.

I wonder whether a better way to do this is just taking out fair's
cpu.weight and .max locking into the core layer so that both callbacks are
called under the same locking. Peter, what do you think?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] sched: Serialize cgroup updates to prevent CFS/SCX state divergence
  2026-08-20 17:21 ` Tejun Heo
@ 2026-08-21  7:26   ` Peter Zijlstra
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2026-08-21  7:26 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Michal Blaszczyk, David Vernet, Andrea Righi, Changwoo Min,
	Kuba Piecuch, sched-ext, linux-kernel

On Thu, Aug 20, 2026 at 07:21:01AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Thu, Aug 20, 2026 at 04:09:56PM +0000, Michal Blaszczyk wrote:
> > Concurrent writes to cgroup control files (such as cpu.shares or
> > cpu.weight) can lead to state divergence between CFS and SCX.
> > 
> > For instance, in cpu_shares_write_u64(), the CFS update is serialized
> > by shares_mutex (internal to fair.c), but this lock is dropped before
> > scx_group_set_weight() is called. The latter only acquires a read
> > semaphore (scx_cgroup_ops_rwsem), allowing multiple threads to evaluate
> > and act on the sched_ext update concurrently.
> > 
> > This serialization gap allows concurrent writes to interleave.
> > As a result, the recorded state in CFS, the SCX internal bookkeeping
> > (e.g., tg->scx.weight), and the BPF scheduler itself can end up operating
> > on completely distinct parameters (pairwise distinct values).
> > 
> > Similar races are present in tg_set_bandwidth(), cpu_idle_write_s64(),
> > cpu_weight_write_u64(), and cpu_weight_nice_write_s64().
> > 
> > Fix this by introducing scx_cgroup_mutex in kernel/sched/core.c to
> > serialize these file write operations.
> 
> I wonder whether a better way to do this is just taking out fair's
> cpu.weight and .max locking into the core layer so that both callbacks are
> called under the same locking. Peter, what do you think?

Yeah, makes sense, no point in fair having an extra/superfluous layer of
locking if it is (also) needed in core.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] sched: Lift cgroup update locking to core to prevent CFS/SCX divergence
  2026-08-20 16:09 [PATCH] sched: Serialize cgroup updates to prevent CFS/SCX state divergence Michal Blaszczyk
  2026-08-20 17:21 ` Tejun Heo
@ 2026-08-21 14:08 ` Michal Blaszczyk
  2026-08-21 19:08   ` Tejun Heo
  1 sibling, 1 reply; 5+ messages in thread
From: Michal Blaszczyk @ 2026-08-21 14:08 UTC (permalink / raw)
  To: Peter Zijlstra, Tejun Heo, David Vernet, Andrea Righi,
	Changwoo Min
  Cc: Michal Blaszczyk, Kuba Piecuch, sched-ext, linux-kernel

Concurrent writes to cgroup control files (such as cpu.shares or
cpu.weight) can lead to state divergence between CFS and SCX.

For instance, in cpu_shares_write_u64(), the CFS update is serialized
by shares_mutex (internal to fair.c), but this lock is dropped before
scx_group_set_weight() is called. The latter only acquires a read
semaphore (scx_cgroup_ops_rwsem), allowing multiple threads to evaluate
and act on the sched_ext update concurrently.

This serialization gap allows concurrent writes to interleave.
As a result, the recorded state in CFS, the SCX internal bookkeeping
(e.g., tg->scx.weight), and the BPF scheduler itself can end up operating
on completely distinct parameters (pairwise distinct values).

Similar races are present in tg_set_bandwidth(), cpu_idle_write_s64(),
cpu_weight_write_u64(), and cpu_weight_nice_write_s64().

Fix this by moving the CFS locking up into the core layer in
`kernel/sched/core.c`. By acquiring these locks directly in the core
write handlers, both the CFS and SCX callbacks are executed atomically
under the same lock.

Fixes: 819513666966 ("sched_ext: Add cgroup support")
Signed-off-by: Michal Blaszczyk <michalblk@google.com>
---
v2:
 - Lifted existing CFS locks up into the Core layer instead of introducing
   a new global mutex, ensuring both callbacks run atomically under
   the same lock.
 - Refactored internal locking callbacks to avoid double-locking scenarios.

 kernel/sched/core.c  | 35 +++++++++++++++++++++++------------
 kernel/sched/fair.c  | 23 ++++++++++++-----------
 kernel/sched/sched.h |  7 +++++++
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index f5f7ff8c680a..6dd21a701bb5 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9779,6 +9779,8 @@ static int cpu_uclamp_max_show(struct seq_file *sf, void *v)
 }
 #endif /* CONFIG_UCLAMP_TASK_GROUP */
 
+DEFINE_MUTEX(shares_mutex);
+
 #ifdef CONFIG_GROUP_SCHED_WEIGHT
 static unsigned long tg_weight(struct task_group *tg)
 {
@@ -9796,7 +9798,10 @@ static int cpu_shares_write_u64(struct cgroup_subsys_state *css,
 
 	if (shareval > scale_load_down(ULONG_MAX))
 		shareval = MAX_SHARES;
-	ret = sched_group_set_shares(css_tg(css), scale_load(shareval));
+
+	guard(mutex)(&shares_mutex);
+
+	ret = sched_group_set_shares_locked(css_tg(css), scale_load(shareval));
 	if (!ret)
 		scx_group_set_weight(css_tg(css),
 				     sched_weight_to_cgroup(shareval));
@@ -9811,8 +9816,6 @@ static u64 cpu_shares_read_u64(struct cgroup_subsys_state *css,
 #endif /* CONFIG_GROUP_SCHED_WEIGHT */
 
 #ifdef CONFIG_CFS_BANDWIDTH
-static DEFINE_MUTEX(cfs_constraints_mutex);
-
 static int __cfs_schedulable(struct task_group *tg, u64 period, u64 runtime);
 
 static int tg_set_cfs_bandwidth(struct task_group *tg,
@@ -9831,13 +9834,6 @@ static int tg_set_cfs_bandwidth(struct task_group *tg,
 
 	burst = (u64)burst_us * NSEC_PER_USEC;
 
-	/*
-	 * Prevent race between setting of cfs_rq->runtime_enabled and
-	 * unthrottle_offline_cfs_rqs().
-	 */
-	guard(cpus_read_lock)();
-	guard(mutex)(&cfs_constraints_mutex);
-
 	ret = __cfs_schedulable(tg, period, quota);
 	if (ret)
 		return ret;
@@ -10089,6 +10085,8 @@ static u64 cpu_period_read_u64(struct cgroup_subsys_state *css,
 	return period_us;
 }
 
+static DEFINE_MUTEX(cfs_constraints_mutex);
+
 static int tg_set_bandwidth(struct task_group *tg,
 			    u64 period_us, u64 quota_us, u64 burst_us)
 {
@@ -10131,6 +10129,13 @@ static int tg_set_bandwidth(struct task_group *tg,
 					burst_us + quota_us > max_bw_runtime_us))
 		return -EINVAL;
 
+	/*
+	 * Prevent race between setting of cfs_rq->runtime_enabled and
+	 * unthrottle_offline_cfs_rqs().
+	 */
+	guard(cpus_read_lock)();
+	guard(mutex)(&cfs_constraints_mutex);
+
 #ifdef CONFIG_CFS_BANDWIDTH
 	ret = tg_set_cfs_bandwidth(tg, period_us, quota_us, burst_us);
 #endif /* CONFIG_CFS_BANDWIDTH */
@@ -10229,6 +10234,8 @@ static int cpu_idle_write_s64(struct cgroup_subsys_state *css,
 {
 	int ret;
 
+	guard(mutex)(&shares_mutex);
+
 	ret = sched_group_set_idle(css_tg(css), idle);
 	if (!ret)
 		scx_group_set_idle(css_tg(css), idle);
@@ -10405,7 +10412,9 @@ 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(mutex)(&shares_mutex);
+
+	ret = sched_group_set_shares_locked(css_tg(css), scale_load(weight));
 	if (!ret)
 		scx_group_set_weight(css_tg(css), cgrp_weight);
 	return ret;
@@ -10442,7 +10451,9 @@ 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(mutex)(&shares_mutex);
+
+	ret = sched_group_set_shares_locked(css_tg(css), scale_load(weight));
 	if (!ret)
 		scx_group_set_weight(css_tg(css),
 				     sched_weight_to_cgroup(weight));
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 001140132a7d..9d69372dc0c1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -15392,8 +15392,6 @@ void init_tg_cfs_entry(struct task_group *tg, struct cfs_rq *cfs_rq,
 	se->parent = parent;
 }
 
-static DEFINE_MUTEX(shares_mutex);
-
 static int __sched_group_set_shares(struct task_group *tg, unsigned long shares)
 {
 	int i;
@@ -15430,36 +15428,40 @@ static int __sched_group_set_shares(struct task_group *tg, unsigned long shares)
 	return 0;
 }
 
-int sched_group_set_shares(struct task_group *tg, unsigned long shares)
+int sched_group_set_shares_locked(struct task_group *tg, unsigned long shares)
 {
 	int ret;
 
-	mutex_lock(&shares_mutex);
+	lockdep_assert_held(&shares_mutex);
+
 	if (tg_is_idle(tg))
 		ret = -EINVAL;
 	else
 		ret = __sched_group_set_shares(tg, shares);
-	mutex_unlock(&shares_mutex);
 
 	return ret;
 }
 
+int sched_group_set_shares(struct task_group *tg, unsigned long shares)
+{
+	guard(mutex)(&shares_mutex);
+	return sched_group_set_shares_locked(tg, shares);
+}
+
 int sched_group_set_idle(struct task_group *tg, long idle)
 {
 	int i;
 
+	lockdep_assert_held(&shares_mutex);
+
 	if (tg == &root_task_group)
 		return -EINVAL;
 
 	if (idle < 0 || idle > 1)
 		return -EINVAL;
 
-	mutex_lock(&shares_mutex);
-
-	if (tg->idle == idle) {
-		mutex_unlock(&shares_mutex);
+	if (tg->idle == idle)
 		return 0;
-	}
 
 	tg->idle = idle;
 
@@ -15505,7 +15507,6 @@ int sched_group_set_idle(struct task_group *tg, long idle)
 	else
 		__sched_group_set_shares(tg, NICE_0_LOAD);
 
-	mutex_unlock(&shares_mutex);
 	return 0;
 }
 
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 26ae13c86b69..9e14b07bddcf 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -599,7 +599,10 @@ extern void sched_release_group(struct task_group *tg);
 extern void sched_move_task(struct task_struct *tsk, bool for_autogroup);
 
 #ifdef CONFIG_FAIR_GROUP_SCHED
+extern struct mutex shares_mutex;
+
 extern int sched_group_set_shares(struct task_group *tg, unsigned long shares);
+extern int sched_group_set_shares_locked(struct task_group *tg, unsigned long shares);
 
 extern int sched_group_set_idle(struct task_group *tg, long idle);
 
@@ -607,6 +610,10 @@ extern void set_task_rq_fair(struct sched_entity *se,
 			     struct cfs_rq *prev, struct cfs_rq *next);
 #else /* !CONFIG_FAIR_GROUP_SCHED: */
 static inline int sched_group_set_shares(struct task_group *tg, unsigned long shares) { return 0; }
+static inline int sched_group_set_shares_locked(struct task_group *tg, unsigned long shares)
+{
+	return 0;
+}
 static inline int sched_group_set_idle(struct task_group *tg, long idle) { return 0; }
 #endif /* !CONFIG_FAIR_GROUP_SCHED */
 
-- 
2.55.0.860.g4b6b3295ed-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] sched: Lift cgroup update locking to core to prevent CFS/SCX divergence
  2026-08-21 14:08 ` [PATCH v2] sched: Lift cgroup update locking to core to prevent CFS/SCX divergence Michal Blaszczyk
@ 2026-08-21 19:08   ` Tejun Heo
  0 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2026-08-21 19:08 UTC (permalink / raw)
  To: Michal Blaszczyk
  Cc: Peter Zijlstra, David Vernet, Andrea Righi, Changwoo Min,
	Kuba Piecuch, sched-ext, linux-kernel

On Fri, Aug 21, 2026 at 02:08:18PM +0000, Michal Blaszczyk wrote:
> +DEFINE_MUTEX(shares_mutex);

Maybe cpu_weight_mutex?

> +static DEFINE_MUTEX(cfs_constraints_mutex);

and cpu_max_mutex?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-21 19:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 16:09 [PATCH] sched: Serialize cgroup updates to prevent CFS/SCX state divergence Michal Blaszczyk
2026-08-20 17:21 ` Tejun Heo
2026-08-21  7:26   ` Peter Zijlstra
2026-08-21 14:08 ` [PATCH v2] sched: Lift cgroup update locking to core to prevent CFS/SCX divergence Michal Blaszczyk
2026-08-21 19:08   ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.