From: Zecheng Li <zecheng@google.com>
Improve data locality and reduce pointer chasing by allocating struct
cfs_rq and struct sched_entity together for non-root task groups. This
is achieved by introducing a new combined struct cfs_rq_with_se that
holds both objects in a single allocation.
This patch:
- Moves struct sched_entity_stats from stats.h to sched.h to allow
cfs_rq_with_se to use it while avoiding include order issues.
- Defines the new struct cfs_rq_with_se containing cfs_rq and
sched_entity_stats for type safety.
- Modifies alloc_fair_sched_group() and free_fair_sched_group() to
allocate and free the new struct as a single unit.
- Modifies the per-CPU pointers in task_group->se and task_group->cfs_rq
to point to the members in the new combined structure.
Signed-off-by: Zecheng Li <zecheng@google.com>
Signed-off-by: Zecheng Li <zli94@ncsu.edu>
---
kernel/sched/fair.c | 25 +++++++++++--------------
kernel/sched/sched.h | 17 +++++++++++++++++
kernel/sched/stats.h | 7 -------
3 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7377f9117501..eef10f2ef2a9 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13617,10 +13617,11 @@ void free_fair_sched_group(struct task_group *tg)
int i;
for_each_possible_cpu(i) {
- if (tg->cfs_rq)
- kfree(tg->cfs_rq[i]);
- if (tg->se)
- kfree(tg->se[i]);
+ if (tg->cfs_rq && tg->cfs_rq[i]) {
+ struct cfs_rq_with_se *combined =
+ container_of(tg->cfs_rq[i], struct cfs_rq_with_se, cfs_rq);
+ kfree(combined);
+ }
}
kfree(tg->cfs_rq);
@@ -13629,6 +13630,7 @@ void free_fair_sched_group(struct task_group *tg)
int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
{
+ struct cfs_rq_with_se *combined;
struct sched_entity *se;
struct cfs_rq *cfs_rq;
int i;
@@ -13645,16 +13647,13 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
init_cfs_bandwidth(tg_cfs_bandwidth(tg), tg_cfs_bandwidth(parent));
for_each_possible_cpu(i) {
- cfs_rq = kzalloc_node(sizeof(struct cfs_rq),
- GFP_KERNEL, cpu_to_node(i));
- if (!cfs_rq)
+ combined = kzalloc_node(sizeof(*combined),
+ GFP_KERNEL, cpu_to_node(i));
+ if (!combined)
goto err;
- se = kzalloc_node(sizeof(struct sched_entity_stats),
- GFP_KERNEL, cpu_to_node(i));
- if (!se)
- goto err_free_rq;
-
+ cfs_rq = &combined->cfs_rq;
+ se = &combined->ses.se;
init_cfs_rq(cfs_rq);
init_tg_cfs_entry(tg, cfs_rq, se, i, parent->se[i]);
init_entity_runnable_average(se);
@@ -13662,8 +13661,6 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
return 1;
-err_free_rq:
- kfree(cfs_rq);
err:
return 0;
}
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 3ceaa9dc9a9e..be32810f7475 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -2172,6 +2172,23 @@ static inline struct task_group *task_group(struct task_struct *p)
return p->sched_task_group;
}
+#ifdef CONFIG_FAIR_GROUP_SCHED
+/*
+ * Defined here to be available before stats.h is included, since
+ * cfs_rq_with_se needs it but stats.h has dependencies on things
+ * defined later in this file.
+ */
+struct sched_entity_stats {
+ struct sched_entity se;
+ struct sched_statistics stats;
+} __no_randomize_layout;
+
+struct cfs_rq_with_se {
+ struct cfs_rq cfs_rq;
+ struct sched_entity_stats ses;
+};
+#endif
+
/* 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)
{
diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index c903f1a42891..6d80d5c7a202 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -89,13 +89,6 @@ static inline void rq_sched_info_depart (struct rq *rq, unsigned long long delt
#endif /* CONFIG_SCHEDSTATS */
-#ifdef CONFIG_FAIR_GROUP_SCHED
-struct sched_entity_stats {
- struct sched_entity se;
- struct sched_statistics stats;
-} __no_randomize_layout;
-#endif
-
static inline struct sched_statistics *
__schedstats_from_se(struct sched_entity *se)
{
--
2.52.0Hi Zecheng,
Thanks for this, looks generally reasonable.
On Mon, Jan 12, 2026 at 10:51 AM Zecheng Li <zli94@ncsu.edu> wrote:
>
> From: Zecheng Li <zecheng@google.com>
>
> Improve data locality and reduce pointer chasing by allocating struct
> cfs_rq and struct sched_entity together for non-root task groups. This
> is achieved by introducing a new combined struct cfs_rq_with_se that
> holds both objects in a single allocation.
>
> This patch:
>
> - Moves struct sched_entity_stats from stats.h to sched.h to allow
> cfs_rq_with_se to use it while avoiding include order issues.
>
> - Defines the new struct cfs_rq_with_se containing cfs_rq and
> sched_entity_stats for type safety.
>
> - Modifies alloc_fair_sched_group() and free_fair_sched_group() to
> allocate and free the new struct as a single unit.
>
> - Modifies the per-CPU pointers in task_group->se and task_group->cfs_rq
> to point to the members in the new combined structure.
>
> Signed-off-by: Zecheng Li <zecheng@google.com>
> Signed-off-by: Zecheng Li <zli94@ncsu.edu>
> ---
> kernel/sched/fair.c | 25 +++++++++++--------------
> kernel/sched/sched.h | 17 +++++++++++++++++
> kernel/sched/stats.h | 7 -------
> 3 files changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 7377f9117501..eef10f2ef2a9 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13617,10 +13617,11 @@ void free_fair_sched_group(struct task_group *tg)
> int i;
>
> for_each_possible_cpu(i) {
> - if (tg->cfs_rq)
> - kfree(tg->cfs_rq[i]);
> - if (tg->se)
> - kfree(tg->se[i]);
> + if (tg->cfs_rq && tg->cfs_rq[i]) {
> + struct cfs_rq_with_se *combined =
> + container_of(tg->cfs_rq[i], struct cfs_rq_with_se, cfs_rq);
> + kfree(combined);
> + }
> }
>
> kfree(tg->cfs_rq);
> @@ -13629,6 +13630,7 @@ void free_fair_sched_group(struct task_group *tg)
>
> int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
> {
> + struct cfs_rq_with_se *combined;
> struct sched_entity *se;
> struct cfs_rq *cfs_rq;
> int i;
> @@ -13645,16 +13647,13 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
> init_cfs_bandwidth(tg_cfs_bandwidth(tg), tg_cfs_bandwidth(parent));
>
> for_each_possible_cpu(i) {
> - cfs_rq = kzalloc_node(sizeof(struct cfs_rq),
> - GFP_KERNEL, cpu_to_node(i));
> - if (!cfs_rq)
> + combined = kzalloc_node(sizeof(*combined),
> + GFP_KERNEL, cpu_to_node(i));
> + if (!combined)
> goto err;
>
> - se = kzalloc_node(sizeof(struct sched_entity_stats),
> - GFP_KERNEL, cpu_to_node(i));
> - if (!se)
> - goto err_free_rq;
> -
> + cfs_rq = &combined->cfs_rq;
> + se = &combined->ses.se;
> init_cfs_rq(cfs_rq);
> init_tg_cfs_entry(tg, cfs_rq, se, i, parent->se[i]);
> init_entity_runnable_average(se);
> @@ -13662,8 +13661,6 @@ int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
>
> return 1;
>
> -err_free_rq:
> - kfree(cfs_rq);
> err:
> return 0;
> }
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 3ceaa9dc9a9e..be32810f7475 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -2172,6 +2172,23 @@ static inline struct task_group *task_group(struct task_struct *p)
> return p->sched_task_group;
> }
>
> +#ifdef CONFIG_FAIR_GROUP_SCHED
> +/*
> + * Defined here to be available before stats.h is included, since
> + * cfs_rq_with_se needs it but stats.h has dependencies on things
> + * defined later in this file.
> + */
> +struct sched_entity_stats {
> + struct sched_entity se;
> + struct sched_statistics stats;
> +} __no_randomize_layout;
Now that we have a general purpose container struct (cfs_rq_with_se),
I wonder if it makes more sense to get rid of sched_entity_stats and
directly embed se and stats in cfs_rq_with_se.
> +
> +struct cfs_rq_with_se {
> + struct cfs_rq cfs_rq;
> + struct sched_entity_stats ses;
> +};
> +#endif
nit: The struct name feels a bit awkward, perhaps something like tg_cfs_state?
> +
> /* 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)
> {
> diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
> index c903f1a42891..6d80d5c7a202 100644
> --- a/kernel/sched/stats.h
> +++ b/kernel/sched/stats.h
> @@ -89,13 +89,6 @@ static inline void rq_sched_info_depart (struct rq *rq, unsigned long long delt
>
> #endif /* CONFIG_SCHEDSTATS */
>
> -#ifdef CONFIG_FAIR_GROUP_SCHED
> -struct sched_entity_stats {
> - struct sched_entity se;
> - struct sched_statistics stats;
> -} __no_randomize_layout;
> -#endif
> -
> static inline struct sched_statistics *
> __schedstats_from_se(struct sched_entity *se)
> {
> --
> 2.52.0
>
Hi Josh,
Thanks for the review.
On Thu, Jan 15, 2026 at 7:48 PM Josh Don <joshdon@google.com> wrote:
>
> Now that we have a general purpose container struct (cfs_rq_with_se),
> I wonder if it makes more sense to get rid of sched_entity_stats and
> directly embed se and stats in cfs_rq_with_se.
Yes, that makes sense. I'll remove sched_entity_stats and embed
cfs_rq, sched_entity and stats in the container struct for v7.
> > +
> > +struct cfs_rq_with_se {
> > + struct cfs_rq cfs_rq;
> > + struct sched_entity_stats ses;
> > +};
> > +#endif
>
> nit: The struct name feels a bit awkward, perhaps something like tg_cfs_state?
>
Sure. I'm thinking cfs_tg_state to keep consistent with the cfs_
prefix used elsewhere in the fair scheduler.
© 2016 - 2026 Red Hat, Inc.