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@xxxxxxxxx>
> Sent: Thursday, November 5, 2026 11:04 AM
> To: Jianyong Wu <wujianyong@xxxxxxxx>
> Cc: mingo@xxxxxxxxxx; peterz@xxxxxxxxxxxxx; juri.lelli@xxxxxxxxxx;
> vincent.guittot@xxxxxxxxxx; tim.c.chen@xxxxxxxxxxxxxxx;
> dietmar.eggemann@xxxxxxx; rostedt@xxxxxxxxxxx; bsegall@xxxxxxxxxx;
> mgorman@xxxxxxx; vschneid@xxxxxxxxxx; kprateek.nayak@xxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; jianyong.wu@xxxxxxxxxxx; Yibin Liu
> <liuyibin@xxxxxxxx>; Yuan Zhong <zhongyuan@xxxxxxxx>
> 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