mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Zecheng Li <zli94@ncsu.edu>, Ingo Molnar <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Rik van Riel <riel@surriel.com>, Chris Mason <clm@fb.com>,
	Madadi Vineeth Reddy <vineethr@linux.ibm.com>,
	Xu Liu <xliuprof@google.com>, Blake Jones <blakejones@google.com>,
	Josh Don <joshdon@google.com>,
	Nilay Vaish <nilayvaish@google.com>,
	<linux-kernel@vger.kernel.org>, Zecheng Li <zecheng@google.com>
Subject: Re: [PATCH v7 3/3] sched/fair: Allocate both cfs_tg_state with percpu allocator
Date: Mon, 19 Jan 2026 12:47:36 +0530	[thread overview]
Message-ID: <47e98557-e898-4302-b89d-c2f62b84ce38@amd.com> (raw)
In-Reply-To: <20260118033431.158464-4-zli94@ncsu.edu>

Hello Zecheng,

On 1/18/2026 9:04 AM, Zecheng Li wrote:
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -8549,7 +8549,7 @@ static struct kmem_cache *task_group_cache __ro_after_init;
>  
>  void __init sched_init(void)
>  {
> -	unsigned long ptr = 0;
> +	unsigned long __maybe_unused ptr = 0;

Since "ptr" is now only used for CONFIG_RT_GROUP_SCHED ...

>  	int i;
>  
>  	/* Make sure the linker didn't screw up */
> @@ -8565,33 +8565,24 @@ void __init sched_init(void)
>  	wait_bit_init();
>  
>  #ifdef CONFIG_FAIR_GROUP_SCHED
> -	ptr += nr_cpu_ids * sizeof(void **);
> -#endif
> -#ifdef CONFIG_RT_GROUP_SCHED
> -	ptr += 2 * nr_cpu_ids * sizeof(void **);
> -#endif
> -	if (ptr) {
> -		ptr = (unsigned long)kzalloc(ptr, GFP_NOWAIT);
> +	root_task_group.cfs_rq = &runqueues.cfs;
>  
> -#ifdef CONFIG_FAIR_GROUP_SCHED
> -		root_task_group.cfs_rq = (struct cfs_rq **)ptr;
> -		ptr += nr_cpu_ids * sizeof(void **);
> -
> -		root_task_group.shares = ROOT_TASK_GROUP_LOAD;
> -		init_cfs_bandwidth(&root_task_group.cfs_bandwidth, NULL);
> +	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
> -		scx_tg_init(&root_task_group);
> +	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;
> -		ptr += nr_cpu_ids * sizeof(void **);
> +	ptr += 2 * nr_cpu_ids * sizeof(void **);
> +	ptr = (unsigned long)kzalloc(ptr, GFP_NOWAIT);
> +	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 **);
> +	root_task_group.rt_rq = (struct rt_rq **)ptr;
> +	ptr += nr_cpu_ids * sizeof(void **);

Can we also optimize the CONFIG_RT_GROUP_SCHED stuff in the same way
although I'm assuming the benefit is far less since they aren't
accessed as frequently as the cfs bits. Something like:

(Only build and boot tested on top of your series)

diff --git a/kernel/sched/autogroup.c b/kernel/sched/autogroup.c
index 954137775f38..7135f19d5fa2 100644
--- a/kernel/sched/autogroup.c
+++ b/kernel/sched/autogroup.c
@@ -52,7 +52,6 @@ static inline void autogroup_destroy(struct kref *kref)
 
 #ifdef CONFIG_RT_GROUP_SCHED
 	/* We've redirected RT tasks to the root task group... */
-	ag->tg->rt_se = NULL;
 	ag->tg->rt_rq = NULL;
 #endif
 	sched_release_group(ag->tg);
@@ -109,7 +108,6 @@ static inline struct autogroup *autogroup_create(void)
 	 * the policy change to proceed.
 	 */
 	free_rt_sched_group(tg);
-	tg->rt_se = root_task_group.rt_se;
 	tg->rt_rq = root_task_group.rt_rq;
 #endif /* CONFIG_RT_GROUP_SCHED */
 	tg->autogroup = ag;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 80e8f4eb3f87..cad8e8a1f519 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -8549,7 +8549,6 @@ static struct kmem_cache *task_group_cache __ro_after_init;
 
 void __init sched_init(void)
 {
-	unsigned long __maybe_unused ptr = 0;
 	int i;
 
 	/* Make sure the linker didn't screw up */
@@ -8574,14 +8573,7 @@ void __init sched_init(void)
 	scx_tg_init(&root_task_group);
 #endif /* CONFIG_EXT_GROUP_SCHED */
 #ifdef CONFIG_RT_GROUP_SCHED
-	ptr += 2 * nr_cpu_ids * sizeof(void **);
-	ptr = (unsigned long)kzalloc(ptr, GFP_NOWAIT);
-	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 **);
-
+	root_task_group.rt_rq = &runqueues.rt;
 #endif /* CONFIG_RT_GROUP_SCHED */
 
 	init_defrootdomain();
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 0a9b2cd6da72..963004905a7d 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -201,26 +201,16 @@ void unregister_rt_sched_group(struct task_group *tg)
 	if (!rt_group_sched_enabled())
 		return;
 
-	if (tg->rt_se)
+	if (!is_root_task_group(tg))
 		destroy_rt_bandwidth(&tg->rt_bandwidth);
 }
 
 void free_rt_sched_group(struct task_group *tg)
 {
-	int i;
-
 	if (!rt_group_sched_enabled())
 		return;
 
-	for_each_possible_cpu(i) {
-		if (tg->rt_rq)
-			kfree(tg->rt_rq[i]);
-		if (tg->rt_se)
-			kfree(tg->rt_se[i]);
-	}
-
-	kfree(tg->rt_rq);
-	kfree(tg->rt_se);
+	free_percpu(tg->rt_rq);
 }
 
 void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,
@@ -234,9 +224,6 @@ void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,
 	rt_rq->rq = rq;
 	rt_rq->tg = tg;
 
-	tg->rt_rq[cpu] = rt_rq;
-	tg->rt_se[cpu] = rt_se;
-
 	if (!rt_se)
 		return;
 
@@ -252,42 +239,32 @@ void init_tg_rt_entry(struct task_group *tg, struct rt_rq *rt_rq,
 
 int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
 {
-	struct rt_rq *rt_rq;
+	struct rt_tg_state __percpu *state;
 	struct sched_rt_entity *rt_se;
+	struct rt_rq *rt_rq;
 	int i;
 
 	if (!rt_group_sched_enabled())
 		return 1;
 
-	tg->rt_rq = kcalloc(nr_cpu_ids, sizeof(rt_rq), GFP_KERNEL);
-	if (!tg->rt_rq)
-		goto err;
-	tg->rt_se = kcalloc(nr_cpu_ids, sizeof(rt_se), GFP_KERNEL);
-	if (!tg->rt_se)
+	state = alloc_percpu_gfp(struct rt_tg_state, GFP_KERNEL);
+	if (!state)
 		goto err;
 
+	tg->rt_rq = &state->rt_rq;
 	init_rt_bandwidth(&tg->rt_bandwidth, ktime_to_ns(global_rt_period()), 0);
 
 	for_each_possible_cpu(i) {
-		rt_rq = kzalloc_node(sizeof(struct rt_rq),
-				     GFP_KERNEL, cpu_to_node(i));
-		if (!rt_rq)
-			goto err;
-
-		rt_se = kzalloc_node(sizeof(struct sched_rt_entity),
-				     GFP_KERNEL, cpu_to_node(i));
-		if (!rt_se)
-			goto err_free_rq;
+		rt_rq = tg_rt_rq(tg, i);
+		rt_se = rt_rq_se(rt_rq);
 
 		init_rt_rq(rt_rq);
 		rt_rq->rt_runtime = tg->rt_bandwidth.rt_runtime;
-		init_tg_rt_entry(tg, rt_rq, rt_se, i, parent->rt_se[i]);
+		init_tg_rt_entry(tg, rt_rq, rt_se, i, tg_rt_se(parent, i));
 	}
 
 	return 1;
 
-err_free_rq:
-	kfree(rt_rq);
 err:
 	return 0;
 }
@@ -510,7 +487,7 @@ static inline struct task_group *next_task_group(struct task_group *tg)
 
 #define for_each_rt_rq(rt_rq, iter, rq)					\
 	for (iter = &root_task_group;					\
-		iter && (rt_rq = iter->rt_rq[cpu_of(rq)]);		\
+		iter && (rt_rq = tg_rt_rq(iter, cpu_of(rq)));		\
 		iter = next_task_group(iter))
 
 #define for_each_sched_rt_entity(rt_se) \
@@ -528,11 +505,7 @@ static void sched_rt_rq_enqueue(struct rt_rq *rt_rq)
 {
 	struct task_struct *donor = rq_of_rt_rq(rt_rq)->donor;
 	struct rq *rq = rq_of_rt_rq(rt_rq);
-	struct sched_rt_entity *rt_se;
-
-	int cpu = cpu_of(rq);
-
-	rt_se = rt_rq->tg->rt_se[cpu];
+	struct sched_rt_entity *rt_se = rt_rq_se(rt_rq);
 
 	if (rt_rq->rt_nr_running) {
 		if (!rt_se)
@@ -547,10 +520,7 @@ static void sched_rt_rq_enqueue(struct rt_rq *rt_rq)
 
 static void sched_rt_rq_dequeue(struct rt_rq *rt_rq)
 {
-	struct sched_rt_entity *rt_se;
-	int cpu = cpu_of(rq_of_rt_rq(rt_rq));
-
-	rt_se = rt_rq->tg->rt_se[cpu];
+	struct sched_rt_entity *rt_se = rt_rq_se(rt_rq);
 
 	if (!rt_se) {
 		dequeue_top_rt_rq(rt_rq, rt_rq->rt_nr_running);
@@ -586,7 +556,7 @@ static inline const struct cpumask *sched_rt_period_mask(void)
 static inline
 struct rt_rq *sched_rt_period_rt_rq(struct rt_bandwidth *rt_b, int cpu)
 {
-	return container_of(rt_b, struct task_group, rt_bandwidth)->rt_rq[cpu];
+	return tg_rt_rq(container_of(rt_b, struct task_group, rt_bandwidth), cpu);
 }
 
 static inline struct rt_bandwidth *sched_rt_bandwidth(struct rt_rq *rt_rq)
@@ -2563,7 +2533,7 @@ static int task_is_throttled_rt(struct task_struct *p, int cpu)
 	struct rt_rq *rt_rq;
 
 #ifdef CONFIG_RT_GROUP_SCHED // XXX maybe add task_rt_rq(), see also sched_rt_period_rt_rq
-	rt_rq = task_group(p)->rt_rq[cpu];
+	rt_rq = tg_rt_rq(task_group(p), cpu);
 	WARN_ON(!rt_group_sched_enabled() && rt_rq->tg != &root_task_group);
 #else
 	rt_rq = &cpu_rq(cpu)->rt;
@@ -2752,7 +2722,7 @@ static int tg_set_rt_bandwidth(struct task_group *tg,
 	tg->rt_bandwidth.rt_runtime = rt_runtime;
 
 	for_each_possible_cpu(i) {
-		struct rt_rq *rt_rq = tg->rt_rq[i];
+		struct rt_rq *rt_rq = tg_rt_rq(tg, i);
 
 		raw_spin_lock(&rt_rq->rt_runtime_lock);
 		rt_rq->rt_runtime = rt_runtime;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 8ad1780edfe8..62d7c89c74d8 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -488,8 +488,7 @@ struct task_group {
 #endif /* CONFIG_FAIR_GROUP_SCHED */
 
 #ifdef CONFIG_RT_GROUP_SCHED
-	struct sched_rt_entity	**rt_se;
-	struct rt_rq		**rt_rq;
+	struct rt_rq __percpu	*rt_rq;
 
 	struct rt_bandwidth	rt_bandwidth;
 #endif
@@ -2228,6 +2227,41 @@ static inline struct sched_entity *cfs_rq_se(struct cfs_rq *cfs_rq)
 }
 #endif
 
+#ifdef CONFIG_RT_GROUP_SCHED
+struct rt_tg_state {
+	struct rt_rq		rt_rq;
+	struct sched_rt_entity	rt_se;
+} __no_randomize_layout;
+
+/* Access a specific CPU's rt_rq from a task group */
+static inline struct rt_rq *tg_rt_rq(struct task_group *tg, int cpu)
+{
+	return per_cpu_ptr(tg->rt_rq, cpu);
+}
+
+static inline struct sched_rt_entity *tg_rt_se(struct task_group *tg, int cpu)
+{
+	struct rt_tg_state *state;
+
+	if (is_root_task_group(tg))
+		return NULL;
+
+	state = container_of(tg_rt_rq(tg, cpu), struct rt_tg_state, rt_rq);
+	return &state->rt_se;
+}
+
+static inline struct sched_rt_entity *rt_rq_se(struct rt_rq *rt_rq)
+{
+	struct rt_tg_state *state;
+
+	if (is_root_task_group(rt_rq->tg))
+		return NULL;
+
+	state = container_of(rt_rq, struct rt_tg_state, rt_rq);
+	return &state->rt_se;
+}
+#endif /* CONFIG_RT_GROUP_SCHED */
+
 /* Change a task's cfs_rq and parent entity if it moves across CPUs/groups */
 static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
 {
@@ -2250,8 +2284,8 @@ static inline void set_task_rq(struct task_struct *p, unsigned int cpu)
 	 */
 	if (!rt_group_sched_enabled())
 		tg = &root_task_group;
-	p->rt.rt_rq  = tg->rt_rq[cpu];
-	p->rt.parent = tg->rt_se[cpu];
+	p->rt.rt_rq  = tg_rt_rq(tg, cpu);
+	p->rt.parent = tg_rt_se(tg, cpu);
 #endif /* CONFIG_RT_GROUP_SCHED */
 }
 
-- 
Thanks and Regards,
Prateek


  reply	other threads:[~2026-01-19  7:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-18  3:34 [PATCH v7 0/3] sched/fair: Optimize cfs_rq and sched_entity allocation for better data locality Zecheng Li
2026-01-18  3:34 ` [PATCH v7 1/3] sched/fair: Co-locate cfs_rq and sched_entity in cfs_tg_state Zecheng Li
2026-01-19  6:40   ` K Prateek Nayak
2026-01-18  3:34 ` [PATCH v7 2/3] sched/fair: Remove task_group->se pointer array Zecheng Li
2026-01-19  6:43   ` K Prateek Nayak
2026-01-18  3:34 ` [PATCH v7 3/3] sched/fair: Allocate both cfs_tg_state with percpu allocator Zecheng Li
2026-01-19  7:17   ` K Prateek Nayak [this message]
2026-01-20 19:11     ` Zecheng Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=47e98557-e898-4302-b89d-c2f62b84ce38@amd.com \
    --to=kprateek.nayak@amd.com \
    --cc=blakejones@google.com \
    --cc=bsegall@google.com \
    --cc=clm@fb.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=joshdon@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=nilayvaish@google.com \
    --cc=peterz@infradead.org \
    --cc=riel@surriel.com \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vineethr@linux.ibm.com \
    --cc=vschneid@redhat.com \
    --cc=xliuprof@google.com \
    --cc=zecheng@google.com \
    --cc=zli94@ncsu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome