[PATCH] sched/cache: Fix group_llc_balance being unreachable above the first NUMA level

[PATCH] sched/cache: Fix group_llc_balance being unreachable above the first NUMA level

[Jianyong Wu]

sched_balance_find_src_group() only considers group_llc_balance when
sds.prefer_sibling is set:

	if (sds.prefer_sibling && local->group_type == group_has_spare &&
	    (busiest->group_type == group_llc_balance ||
	     sibling_imbalance(env, &sds, busiest, local) > 1))
		goto force_balance;

sds.prefer_sibling mirrors SD_PREFER_SIBLING of the busiest group's child
sched domain, and sd_init() clears that flag on every SD_NUMA domain.
So on any domain whose child is itself a NUMA domain, i.e. on systems
with more than one NUMA level such as multiple nodes per socket, groups
tagged with group_llc_balance by llc_balance() can never take the
force_balance path, and the group_llc_balance test there is dead code.

Such a group falls into the !group_overloaded branch below and in the
common case ends up in out_balanced, so a task that was placed beyond
the first NUMA level is never pulled into its preferred LLC.

On a 2-socket Hygon system (8 nodes, 32 LLCs, 4 cores per LLC), with
"spin 3"/"spin 4" (3/4 busy threads of one process), all threads were
in one LLC in only 41/140 and 0/142 samples; every failing case had a
straggler on the other socket. With this patch: 195/196 and 180/196.

Only require sds.prefer_sibling for the sibling_imbalance() case, which
is what it was originally introduced for.

Fixes: f38cc2f0d8a3 ("sched/cache: Prioritize tasks preferring destination LLC during balancing")
Reported-by: Yibin Liu <liuyibin@hygon.cn>
Signed-off-by: Jianyong Wu <wujianyong@hygon.cn>
---
 kernel/sched/fair.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 57360f5cdde4..205a0891b0a0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13294,9 +13294,10 @@ static struct sched_group *sched_balance_find_src_group(struct lb_env *env)
 	 * Try to move all excess tasks to a sibling domain of the busiest
 	 * group's child domain.
 	 */
-	if (sds.prefer_sibling && local->group_type == group_has_spare &&
+	if (local->group_type == group_has_spare &&
 	    (busiest->group_type == group_llc_balance ||
-	    sibling_imbalance(env, &sds, busiest, local) > 1))
+	     (sds.prefer_sibling &&
+	      sibling_imbalance(env, &sds, busiest, local) > 1)))
 		goto force_balance;
 
 	if (busiest->group_type != group_overloaded) {
-- 
2.43.0

Re: [PATCH] sched/cache: Fix group_llc_balance being unreachable above the first NUMA level

[Tim Chen]

On Wed, 2026-11-04 at 22:13 +0800, Jianyong Wu wrote:
> sched_balance_find_src_group() only considers group_llc_balance when
> sds.prefer_sibling is set:
> 
> 	if (sds.prefer_sibling && local->group_type == group_has_spare &&
> 	    (busiest->group_type == group_llc_balance ||
> 	     sibling_imbalance(env, &sds, busiest, local) > 1))
> 		goto force_balance;
> 
> sds.prefer_sibling mirrors SD_PREFER_SIBLING of the busiest group's child
> sched domain, and sd_init() clears that flag on every SD_NUMA domain.
> So on any domain whose child is itself a NUMA domain, i.e. on systems
> with more than one NUMA level such as multiple nodes per socket, groups
> tagged with group_llc_balance by llc_balance() can never take the
> force_balance path, and the group_llc_balance test there is dead code.
> 
> Such a group falls into the !group_overloaded branch below and in the
> common case ends up in out_balanced, so a task that was placed beyond
> the first NUMA level is never pulled into its preferred LLC.
> 
> On a 2-socket Hygon system (8 nodes, 32 LLCs, 4 cores per LLC), with
> "spin 3"/"spin 4" (3/4 busy threads of one process), all threads were
> in one LLC in only 41/140 and 0/142 samples; every failing case had a
> straggler on the other socket. With this patch: 195/196 and 180/196.
> 
> Only require sds.prefer_sibling for the sibling_imbalance() case, which
> is what it was originally introduced for.


Hi Jianyong,

Here's a suggested reorg of the patch to make the code logic clearer.  Also
incorporate some suggestions from Peter on your previous proposal
https://lore.kernel.org/all/20260827122816.756234-15-wujianyong@hygon.cn/ .
So we do not do the llc_balance check for non cache aware scheduling case.

Tim

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index cfc982205b98..8a7442189a3b 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12192,6 +12192,16 @@ static bool update_llc_busiest(struct lb_env *env,
         */
        return sgs->nr_pref_dst_llc > busiest->nr_pref_dst_llc;
 }
+
+/*
+ * The busiest group has tasks that prefer the destination LLC and
+ * cache aware scheduling is enabled.
+ */
+static inline bool busiest_llc_balance(struct sg_lb_stats *busiest)
+{
+       return sched_cache_enabled() &&
+              busiest->group_type == group_llc_balance;
+}
 #else
 static inline void record_sg_llc_stats(struct lb_env *env, struct sg_lb_stats *sgs,
                                       struct sched_group *group)
@@ -12210,6 +12220,11 @@ static bool update_llc_busiest(struct lb_env *env,
 {
        return false;
 }
+
+static inline bool busiest_llc_balance(struct sg_lb_stats *busiest)
+{
+       return false;
+}
 #endif

 /**
@@ -13303,14 +13318,25 @@ static struct sched_group *sched_balance_find_src_group(struct lb_env *env)
                        goto out_balanced;
        }

-       /*
-        * Try to move all excess tasks to a sibling domain of the busiest
-        * group's child domain.
-        */
-       if (sds.prefer_sibling && local->group_type == group_has_spare &&
-           (busiest->group_type == group_llc_balance ||
-           sibling_imbalance(env, &sds, busiest, local) > 1))
-               goto force_balance;
+       if (local->group_type == group_has_spare) {
+
+               if (busiest_llc_balance(busiest))
+                       goto force_balance;
+
+               /*
+                * Try to move all excess tasks to a sibling domain of the
+                * busiest group's child domain.
+                */
+               if (sds.prefer_sibling &&
+                   sibling_imbalance(env, &sds, busiest, local) > 1)
+                       goto force_balance;
+       }


> 
> Fixes: f38cc2f0d8a3 ("sched/cache: Prioritize tasks preferring destination LLC during balancing")
> Reported-by: Yibin Liu <liuyibin@hygon.cn>
> Signed-off-by: Jianyong Wu <wujianyong@hygon.cn>
> ---
>  kernel/sched/fair.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 57360f5cdde4..205a0891b0a0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13294,9 +13294,10 @@ static struct sched_group *sched_balance_find_src_group(struct lb_env *env)
>  	 * Try to move all excess tasks to a sibling domain of the busiest
>  	 * group's child domain.
>  	 */
> -	if (sds.prefer_sibling && local->group_type == group_has_spare &&
> +	if (local->group_type == group_has_spare &&
>  	    (busiest->group_type == group_llc_balance ||
> -	    sibling_imbalance(env, &sds, busiest, local) > 1))
> +	     (sds.prefer_sibling &&
> +	      sibling_imbalance(env, &sds, busiest, local) > 1)))
>  		goto force_balance;
>  
>  	if (busiest->group_type != group_overloaded) {

RE: [PATCH] sched/cache: Fix group_llc_balance being unreachable above the first NUMA level

[Jianyong Wu]

Hi Tim,


> So we do not do the llc_balance check for non cache aware scheduling
> case.
>

Thanks, this is clearer than my version, and cheaper too: when cache aware
scheduling is disabled the group type check is either compiled out or skipped
by the static branch.

This patch comes from my earlier series that scales cache aware scheduling at
LLC granularity.  Yibin found that without it mainline does not pull a task
back into its preferred LLC once the task sits beyond the first NUMA level, so
I picked it out and sent it as a standalone fix.

I'll include your version in v2.

Thanks
Jianyong


> Tim
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index cfc982205b98..8a7442189a3b 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -12192,6 +12192,16 @@ static bool update_llc_busiest(struct lb_env
> *env,
>          */
>         return sgs->nr_pref_dst_llc > busiest->nr_pref_dst_llc;
>  }
> +
> +/*
> + * The busiest group has tasks that prefer the destination LLC and
> + * cache aware scheduling is enabled.
> + */
> +static inline bool busiest_llc_balance(struct sg_lb_stats *busiest)
> +{
> +       return sched_cache_enabled() &&
> +              busiest->group_type == group_llc_balance;
> +}
>  #else
>  static inline void record_sg_llc_stats(struct lb_env *env, struct sg_lb_stats
> *sgs,
>                                        struct sched_group *group)
> @@ -12210,6 +12220,11 @@ static bool update_llc_busiest(struct lb_env
> *env,
>  {
>         return false;
>  }
> +
> +static inline bool busiest_llc_balance(struct sg_lb_stats *busiest)
> +{
> +       return false;
> +}
>  #endif
> 
>  /**
> @@ -13303,14 +13318,25 @@ static struct sched_group
> *sched_balance_find_src_group(struct lb_env *env)
>                         goto out_balanced;
>         }
> 
> -       /*
> -        * Try to move all excess tasks to a sibling domain of the busiest
> -        * group's child domain.
> -        */
> -       if (sds.prefer_sibling && local->group_type == group_has_spare
> &&
> -           (busiest->group_type == group_llc_balance ||
> -           sibling_imbalance(env, &sds, busiest, local) > 1))
> -               goto force_balance;
> +       if (local->group_type == group_has_spare) {
> +
> +               if (busiest_llc_balance(busiest))
> +                       goto force_balance;
> +
> +               /*
> +                * Try to move all excess tasks to a sibling domain of
> the
> +                * busiest group's child domain.
> +                */
> +               if (sds.prefer_sibling &&
> +                   sibling_imbalance(env, &sds, busiest, local) > 1)
> +                       goto force_balance;
> +       }
> 
> 
> >
> > Fixes: f38cc2f0d8a3 ("sched/cache: Prioritize tasks preferring destination
> LLC during balancing")
> > Reported-by: Yibin Liu <liuyibin@hygon.cn>
> > Signed-off-by: Jianyong Wu <wujianyong@hygon.cn>
> > ---
> >  kernel/sched/fair.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 57360f5cdde4..205a0891b0a0 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -13294,9 +13294,10 @@ static struct sched_group
> *sched_balance_find_src_group(struct lb_env *env)
> >  	 * Try to move all excess tasks to a sibling domain of the busiest
> >  	 * group's child domain.
> >  	 */
> > -	if (sds.prefer_sibling && local->group_type == group_has_spare &&
> > +	if (local->group_type == group_has_spare &&
> >  	    (busiest->group_type == group_llc_balance ||
> > -	    sibling_imbalance(env, &sds, busiest, local) > 1))
> > +	     (sds.prefer_sibling &&
> > +	      sibling_imbalance(env, &sds, busiest, local) > 1)))
> >  		goto force_balance;
> >
> >  	if (busiest->group_type != group_overloaded) {

Re: [PATCH] sched/cache: Fix group_llc_balance being unreachable above the first NUMA level

[Chen Yu]

Hi Jianyong,

On Wed, Nov 04, 2026 at 10:13:13PM +0800, Jianyong Wu wrote:
> sched_balance_find_src_group() only considers group_llc_balance when
> sds.prefer_sibling is set:
> 
> 	if (sds.prefer_sibling && local->group_type == group_has_spare &&
> 	    (busiest->group_type == group_llc_balance ||
> 	     sibling_imbalance(env, &sds, busiest, local) > 1))
> 		goto force_balance;
> 
> sds.prefer_sibling mirrors SD_PREFER_SIBLING of the busiest group's child
> sched domain, and sd_init() clears that flag on every SD_NUMA domain.
> So on any domain whose child is itself a NUMA domain, i.e. on systems
> with more than one NUMA level such as multiple nodes per socket, groups
> tagged with group_llc_balance by llc_balance() can never take the
> force_balance path, and the group_llc_balance test there is dead code.
> 
> Such a group falls into the !group_overloaded branch below and in the
> common case ends up in out_balanced, so a task that was placed beyond
> the first NUMA level is never pulled into its preferred LLC.
> 
> On a 2-socket Hygon system (8 nodes, 32 LLCs, 4 cores per LLC), with
> "spin 3"/"spin 4" (3/4 busy threads of one process), all threads were
> in one LLC in only 41/140 and 0/142 samples; every failing case had a
> straggler on the other socket. With this patch: 195/196 and 180/196.
> 
> Only require sds.prefer_sibling for the sibling_imbalance() case, which
> is what it was originally introduced for.
> 
> Fixes: f38cc2f0d8a3 ("sched/cache: Prioritize tasks preferring destination LLC during balancing")

This patch looks reasonable. Is this an enhancement rather than a fix? Before this patch,
NUMA node task aggregation is disabled and task aggregation across SNC nodes (Intel) or
level1 Nodes (AMD, Hygon with NPS2/3...) is enabled. After this patch, both the NUMA level and the
SNC level enable task aggregation. Maybe some benchmark results in the next version would
be helpful to indicate that no regression is found while there is benefit on multi-node
domain systems (NUMA balancing enabled/disabled, etc.).

thanks,
Chenyu

> Reported-by: Yibin Liu <liuyibin@hygon.cn>
> Signed-off-by: Jianyong Wu <wujianyong@hygon.cn>
> ---
>  kernel/sched/fair.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 57360f5cdde4..205a0891b0a0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13294,9 +13294,10 @@ static struct sched_group *sched_balance_find_src_group(struct lb_env *env)
>  	 * Try to move all excess tasks to a sibling domain of the busiest
>  	 * group's child domain.
>  	 */
> -	if (sds.prefer_sibling && local->group_type == group_has_spare &&
> +	if (local->group_type == group_has_spare &&
>  	    (busiest->group_type == group_llc_balance ||
> -	    sibling_imbalance(env, &sds, busiest, local) > 1))
> +	     (sds.prefer_sibling &&
> +	      sibling_imbalance(env, &sds, busiest, local) > 1)))
>  		goto force_balance;
>  
>  	if (busiest->group_type != group_overloaded) {
> -- 
> 2.43.0
> 
> 

RE: [PATCH] sched/cache: Fix group_llc_balance being unreachable above the first NUMA level

[Jianyong Wu]

Hi Chenyu,

> -----Original Message-----
> From: Chen Yu <yu.c.chen@intel.com>
> Sent: Thursday, November 5, 2026 11:04 AM
> To: Jianyong Wu <wujianyong@hygon.cn>
> Cc: mingo@redhat.com; peterz@infradead.org; juri.lelli@redhat.com;
> vincent.guittot@linaro.org; tim.c.chen@linux.intel.com;
> dietmar.eggemann@arm.com; rostedt@goodmis.org; bsegall@google.com;
> mgorman@suse.de; vschneid@redhat.com; kprateek.nayak@amd.com;
> linux-kernel@vger.kernel.org; jianyong.wu@outlook.com; Yibin Liu
> <liuyibin@hygon.cn>; Yuan Zhong <zhongyuan@hygon.cn>
> Subject: Re: [PATCH] sched/cache: Fix group_llc_balance being unreachable
> above the first NUMA level
> 
> Hi Jianyong,
> 
> On Wed, Nov 04, 2026 at 10:13:13PM +0800, Jianyong Wu wrote:
> > sched_balance_find_src_group() only considers group_llc_balance when
> > sds.prefer_sibling is set:
> >
> > 	if (sds.prefer_sibling && local->group_type == group_has_spare &&
> > 	    (busiest->group_type == group_llc_balance ||
> > 	     sibling_imbalance(env, &sds, busiest, local) > 1))
> > 		goto force_balance;
> >
> > sds.prefer_sibling mirrors SD_PREFER_SIBLING of the busiest group's
> child
> > sched domain, and sd_init() clears that flag on every SD_NUMA domain.
> > So on any domain whose child is itself a NUMA domain, i.e. on systems
> > with more than one NUMA level such as multiple nodes per socket,
> groups
> > tagged with group_llc_balance by llc_balance() can never take the
> > force_balance path, and the group_llc_balance test there is dead code.
> >
> > Such a group falls into the !group_overloaded branch below and in the
> > common case ends up in out_balanced, so a task that was placed
> beyond
> > the first NUMA level is never pulled into its preferred LLC.
> >
> > On a 2-socket Hygon system (8 nodes, 32 LLCs, 4 cores per LLC), with
> > "spin 3"/"spin 4" (3/4 busy threads of one process), all threads were
> > in one LLC in only 41/140 and 0/142 samples; every failing case had a
> > straggler on the other socket. With this patch: 195/196 and 180/196.
> >
> > Only require sds.prefer_sibling for the sibling_imbalance() case, which
> > is what it was originally introduced for.
> >
> > Fixes: f38cc2f0d8a3 ("sched/cache: Prioritize tasks preferring destination
> LLC during balancing")
> 
> This patch looks reasonable. Is this an enhancement rather than a fix?
> Before this patch,
> NUMA node task aggregation is disabled and task aggregation across SNC
> nodes (Intel) or
> level1 Nodes (AMD, Hygon with NPS2/3...) is enabled. After this patch,
> both the NUMA level and the
> SNC level enable task aggregation.

Right, it enables aggregation at the NUMA level too, so it can be seen as an
enhancement. I'll drop the "Fixes:" tag.

> Maybe some benchmark results in the
> next version would
> be helpful to indicate that no regression is found while there is benefit on
> multi-node
> domain systems (NUMA balancing enabled/disabled, etc.).
> 

Sure, I'll run benchmarks with NUMA balancing on and off and include the
numbers in v2.

Thanks
Jianyong