| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CABk29NuXmH1nUo9K+uM6eW-Ya_uM3ei93_Ug3eC=GLVMrBxjWg@mail.gmail.com>
Date: Thu, 15 Jan 2026 16:48:19 -0800
From: Josh Don <joshdon@...gle.com>
To: Zecheng Li <zli94@...u.edu>
Cc: Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
Juri Lelli <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
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>, Nilay Vaish <nilayvaish@...gle.com>,
linux-kernel@...r.kernel.org, Zecheng Li <zecheng@...gle.com>
Subject: Re: [PATCH v6 1/3] sched/fair: Co-locate cfs_rq and sched_entity
Hi Zecheng,
Thanks for this, looks generally reasonable.
On Mon, Jan 12, 2026 at 10:51 AM Zecheng Li <zli94@...u.edu> wrote:
>
> From: Zecheng Li <zecheng@...gle.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@...gle.com>
> Signed-off-by: Zecheng Li <zli94@...u.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
>
Powered by blists - more mailing lists