[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <47e98557-e898-4302-b89d-c2f62b84ce38@amd.com>
Date: Mon, 19 Jan 2026 12:47:36 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Zecheng Li <zli94@...u.edu>, Ingo Molnar <mingo@...hat.com>, "Peter
Zijlstra" <peterz@...radead.org>, Juri Lelli <juri.lelli@...hat.com>,
"Vincent Guittot" <vincent.guittot@...aro.org>
CC: Dietmar Eggemann <dietmar.eggemann@....com>, Steven Rostedt
<rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>, Mel Gorman
<mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>, Rik van Riel
<riel@...riel.com>, Chris Mason <clm@...com>, Madadi Vineeth Reddy
<vineethr@...ux.ibm.com>, Xu Liu <xliuprof@...gle.com>, Blake Jones
<blakejones@...gle.com>, Josh Don <joshdon@...gle.com>, Nilay Vaish
<nilayvaish@...gle.com>, <linux-kernel@...r.kernel.org>, Zecheng Li
<zecheng@...gle.com>
Subject: Re: [PATCH v7 3/3] sched/fair: Allocate both cfs_tg_state with percpu
allocator
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
Powered by blists - more mailing lists