linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2 sched_ext/for-6.16-fixes] sched_ext: Make scx_group_set_weight() always update tg->scx.weight
@ 2025-06-13 23:23 Tejun Heo
  2025-06-13 23:25 ` [PATCH 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group() Tejun Heo
  0 siblings, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2025-06-13 23:23 UTC (permalink / raw)
  To: David Vernet, Andrea Righi, Changwoo Min; +Cc: linux-kernel

Otherwise, tg->scx.weight can go out of sync while scx_cgroup is not enabled
and ops.cgroup_init() may be called with a stale weight value.

Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: 819513666966 ("sched_ext: Add cgroup support")
Cc: stable@vger.kernel.org # v6.12+
---
 kernel/sched/ext.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -4241,12 +4241,12 @@ void scx_group_set_weight(struct task_gr
 
 	percpu_down_read(&scx_cgroup_rwsem);
 
-	if (scx_cgroup_enabled && tg->scx_weight != weight) {
-		if (SCX_HAS_OP(sch, cgroup_set_weight))
-			SCX_CALL_OP(sch, SCX_KF_UNLOCKED, cgroup_set_weight, NULL,
-				    tg_cgrp(tg), weight);
-		tg->scx_weight = weight;
-	}
+	if (scx_cgroup_enabled && SCX_HAS_OP(sch, cgroup_set_weight) &&
+	    tg->scx_weight != weight)
+		SCX_CALL_OP(sch, SCX_KF_UNLOCKED, cgroup_set_weight, NULL,
+			    tg_cgrp(tg), weight);
+
+	tg->scx_weight = weight;
 
 	percpu_up_read(&scx_cgroup_rwsem);
 }


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

* [PATCH 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group()
  2025-06-13 23:23 [PATCH 1/2 sched_ext/for-6.16-fixes] sched_ext: Make scx_group_set_weight() always update tg->scx.weight Tejun Heo
@ 2025-06-13 23:25 ` Tejun Heo
  2025-06-16 20:13   ` [PATCH v2 " Tejun Heo
  2025-06-17  8:30   ` [PATCH " Peter Zijlstra
  0 siblings, 2 replies; 5+ messages in thread
From: Tejun Heo @ 2025-06-13 23:25 UTC (permalink / raw)
  To: David Vernet, Andrea Righi, Changwoo Min
  Cc: linux-kernel, Ingo Molnar, Peter Zijlstra

During task_group creation, sched_create_group() calls
scx_group_set_weight() with CGROUP_WEIGHT_DFL to initialize the sched_ext
portion. This is premature and ends up calling ops.cgroup_set_weight() with
an incorrect @cgrp before ops.cgroup_init() is called.

sched_create_group() should just initialize SCX related fields in the new
task_group. Fix it by factoring out scx_tg_init() from sched_init() and
making sched_create_group() call that function instead of
scx_group_set_weight().

Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: 819513666966 ("sched_ext: Add cgroup support")
Cc: stable@vger.kernel.org # v6.12+
---
Ingo, Peter, while this touches kernel/sched/core.c, the changes are trivial
and only affect sched_ext. I'm planning to route it through
sched_ext/for-6.14-fixes. Please holler if you want it routed through tip.

Thanks.

 kernel/sched/core.c |    9 ++++-----
 kernel/sched/ext.c  |    5 +++++
 kernel/sched/ext.h  |    2 ++
 3 files changed, 11 insertions(+), 5 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8544,17 +8544,16 @@ void __init sched_init(void)
 		root_task_group.shares = ROOT_TASK_GROUP_LOAD;
 		init_cfs_bandwidth(&root_task_group.cfs_bandwidth, NULL);
 #endif /* CONFIG_FAIR_GROUP_SCHED */
-#ifdef CONFIG_EXT_GROUP_SCHED
-		root_task_group.scx_weight = CGROUP_WEIGHT_DFL;
-#endif /* CONFIG_EXT_GROUP_SCHED */
+
 #ifdef CONFIG_RT_GROUP_SCHED
 		root_task_group.rt_se = (struct sched_rt_entity **)ptr;
 		ptr += nr_cpu_ids * sizeof(void **);
 
 		root_task_group.rt_rq = (struct rt_rq **)ptr;
 		ptr += nr_cpu_ids * sizeof(void **);
-
 #endif /* CONFIG_RT_GROUP_SCHED */
+
+		scx_tg_init(&root_task_group);
 	}
 
 #ifdef CONFIG_SMP
@@ -8985,7 +8984,7 @@ struct task_group *sched_create_group(st
 	if (!alloc_rt_sched_group(tg, parent))
 		goto err;
 
-	scx_group_set_weight(tg, CGROUP_WEIGHT_DFL);
+	scx_tg_init(tg);
 	alloc_uclamp_sched_group(tg, parent);
 
 	return tg;
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -4092,6 +4092,11 @@ bool scx_can_stop_tick(struct rq *rq)
 DEFINE_STATIC_PERCPU_RWSEM(scx_cgroup_rwsem);
 static bool scx_cgroup_enabled;
 
+void scx_tg_init(struct task_group *tg)
+{
+	tg->scx_weight = CGROUP_WEIGHT_DFL;
+}
+
 int scx_tg_online(struct task_group *tg)
 {
 	struct scx_sched *sch = scx_root;
--- a/kernel/sched/ext.h
+++ b/kernel/sched/ext.h
@@ -79,6 +79,7 @@ static inline void scx_update_idle(struc
 
 #ifdef CONFIG_CGROUP_SCHED
 #ifdef CONFIG_EXT_GROUP_SCHED
+void scx_tg_init(struct task_group *tg);
 int scx_tg_online(struct task_group *tg);
 void scx_tg_offline(struct task_group *tg);
 int scx_cgroup_can_attach(struct cgroup_taskset *tset);
@@ -88,6 +89,7 @@ void scx_cgroup_cancel_attach(struct cgr
 void scx_group_set_weight(struct task_group *tg, unsigned long cgrp_weight);
 void scx_group_set_idle(struct task_group *tg, bool idle);
 #else	/* CONFIG_EXT_GROUP_SCHED */
+static inline void scx_tg_init(struct task_group *tg) {}
 static inline int scx_tg_online(struct task_group *tg) { return 0; }
 static inline void scx_tg_offline(struct task_group *tg) {}
 static inline int scx_cgroup_can_attach(struct cgroup_taskset *tset) { return 0; }

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

* [PATCH v2 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group()
  2025-06-13 23:25 ` [PATCH 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group() Tejun Heo
@ 2025-06-16 20:13   ` Tejun Heo
  2025-06-17 18:20     ` Tejun Heo
  2025-06-17  8:30   ` [PATCH " Peter Zijlstra
  1 sibling, 1 reply; 5+ messages in thread
From: Tejun Heo @ 2025-06-16 20:13 UTC (permalink / raw)
  To: David Vernet, Andrea Righi, Changwoo Min
  Cc: linux-kernel, Ingo Molnar, Peter Zijlstra

During task_group creation, sched_create_group() calls
scx_group_set_weight() with CGROUP_WEIGHT_DFL to initialize the sched_ext
portion. This is premature and ends up calling ops.cgroup_set_weight() with
an incorrect @cgrp before ops.cgroup_init() is called.

sched_create_group() should just initialize SCX related fields in the new
task_group. Fix it by factoring out scx_tg_init() from sched_init() and
making sched_create_group() call that function instead of
scx_group_set_weight().

v2: Retain CONFIG_EXT_GROUP_SCHED ifdef in sched_init() as removing it leads
    to build failures on !CONFIG_GROUP_SCHED configs.

Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: 819513666966 ("sched_ext: Add cgroup support")
Cc: stable@vger.kernel.org # v6.12+
---
 kernel/sched/core.c |    4 ++--
 kernel/sched/ext.c  |    5 +++++
 kernel/sched/ext.h  |    2 ++
 3 files changed, 9 insertions(+), 2 deletions(-)

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8437,7 +8437,7 @@ void __init sched_init(void)
 		init_cfs_bandwidth(&root_task_group.cfs_bandwidth, NULL);
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 #ifdef CONFIG_EXT_GROUP_SCHED
-		root_task_group.scx_weight = CGROUP_WEIGHT_DFL;
+		scx_tg_init(&root_task_group);
 #endif /* CONFIG_EXT_GROUP_SCHED */
 #ifdef CONFIG_RT_GROUP_SCHED
 		root_task_group.rt_se = (struct sched_rt_entity **)ptr;
@@ -8872,7 +8872,7 @@ struct task_group *sched_create_group(st
 	if (!alloc_rt_sched_group(tg, parent))
 		goto err;
 
-	scx_group_set_weight(tg, CGROUP_WEIGHT_DFL);
+	scx_tg_init(tg);
 	alloc_uclamp_sched_group(tg, parent);
 
 	return tg;
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -4056,6 +4056,11 @@ bool scx_can_stop_tick(struct rq *rq)
 DEFINE_STATIC_PERCPU_RWSEM(scx_cgroup_rwsem);
 static bool scx_cgroup_enabled;
 
+void scx_tg_init(struct task_group *tg)
+{
+	tg->scx_weight = CGROUP_WEIGHT_DFL;
+}
+
 int scx_tg_online(struct task_group *tg)
 {
 	struct scx_sched *sch = scx_root;
--- a/kernel/sched/ext.h
+++ b/kernel/sched/ext.h
@@ -95,6 +95,7 @@ static inline void scx_update_idle(struc
 
 #ifdef CONFIG_CGROUP_SCHED
 #ifdef CONFIG_EXT_GROUP_SCHED
+void scx_tg_init(struct task_group *tg);
 int scx_tg_online(struct task_group *tg);
 void scx_tg_offline(struct task_group *tg);
 int scx_cgroup_can_attach(struct cgroup_taskset *tset);
@@ -104,6 +105,7 @@ void scx_cgroup_cancel_attach(struct cgr
 void scx_group_set_weight(struct task_group *tg, unsigned long cgrp_weight);
 void scx_group_set_idle(struct task_group *tg, bool idle);
 #else	/* CONFIG_EXT_GROUP_SCHED */
+static inline void scx_tg_init(struct task_group *tg) {}
 static inline int scx_tg_online(struct task_group *tg) { return 0; }
 static inline void scx_tg_offline(struct task_group *tg) {}
 static inline int scx_cgroup_can_attach(struct cgroup_taskset *tset) { return 0; }

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

* Re: [PATCH 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group()
  2025-06-13 23:25 ` [PATCH 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group() Tejun Heo
  2025-06-16 20:13   ` [PATCH v2 " Tejun Heo
@ 2025-06-17  8:30   ` Peter Zijlstra
  1 sibling, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2025-06-17  8:30 UTC (permalink / raw)
  To: Tejun Heo
  Cc: David Vernet, Andrea Righi, Changwoo Min, linux-kernel,
	Ingo Molnar

On Fri, Jun 13, 2025 at 01:25:56PM -1000, Tejun Heo wrote:
> During task_group creation, sched_create_group() calls
> scx_group_set_weight() with CGROUP_WEIGHT_DFL to initialize the sched_ext
> portion. This is premature and ends up calling ops.cgroup_set_weight() with
> an incorrect @cgrp before ops.cgroup_init() is called.
> 
> sched_create_group() should just initialize SCX related fields in the new
> task_group. Fix it by factoring out scx_tg_init() from sched_init() and
> making sched_create_group() call that function instead of
> scx_group_set_weight().
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Fixes: 819513666966 ("sched_ext: Add cgroup support")
> Cc: stable@vger.kernel.org # v6.12+
> ---
> Ingo, Peter, while this touches kernel/sched/core.c, the changes are trivial
> and only affect sched_ext. I'm planning to route it through
> sched_ext/for-6.14-fixes. Please holler if you want it routed through tip.

Seems fine, go ahead and take it.

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

* Re: [PATCH v2 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group()
  2025-06-16 20:13   ` [PATCH v2 " Tejun Heo
@ 2025-06-17 18:20     ` Tejun Heo
  0 siblings, 0 replies; 5+ messages in thread
From: Tejun Heo @ 2025-06-17 18:20 UTC (permalink / raw)
  To: David Vernet, Andrea Righi, Changwoo Min
  Cc: linux-kernel, Ingo Molnar, Peter Zijlstra

On Mon, Jun 16, 2025 at 10:13:25AM -1000, Tejun Heo wrote:
> During task_group creation, sched_create_group() calls
> scx_group_set_weight() with CGROUP_WEIGHT_DFL to initialize the sched_ext
> portion. This is premature and ends up calling ops.cgroup_set_weight() with
> an incorrect @cgrp before ops.cgroup_init() is called.
> 
> sched_create_group() should just initialize SCX related fields in the new
> task_group. Fix it by factoring out scx_tg_init() from sched_init() and
> making sched_create_group() call that function instead of
> scx_group_set_weight().
> 
> v2: Retain CONFIG_EXT_GROUP_SCHED ifdef in sched_init() as removing it leads
>     to build failures on !CONFIG_GROUP_SCHED configs.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Fixes: 819513666966 ("sched_ext: Add cgroup support")
> Cc: stable@vger.kernel.org # v6.12+

Applied 1-2 to sched_ext/for-6.16-fixes.

Thanks.

-- 
tejun

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

end of thread, other threads:[~2025-06-17 18:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-13 23:23 [PATCH 1/2 sched_ext/for-6.16-fixes] sched_ext: Make scx_group_set_weight() always update tg->scx.weight Tejun Heo
2025-06-13 23:25 ` [PATCH 2/2 sched_ext/for-6.16-fixes] sched_ext, sched/core: Don't call scx_group_set_weight() prematurely from sched_create_group() Tejun Heo
2025-06-16 20:13   ` [PATCH v2 " Tejun Heo
2025-06-17 18:20     ` Tejun Heo
2025-06-17  8:30   ` [PATCH " Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).