Re: [PATCH v4] sched/fair: Prefer fully idle cores for NOHZ balancing

[Shrikanth Hegde <sshegde@linux.ibm.com>]

Hi Andrea,

On 8/4/26 8:43 PM, Andrea Righi wrote:
> find_new_ilb() selects the first idle housekeeping CPU without
> considering whether another thread is running on the same physical core.
> On an SMT system, the idle load balancer can therefore activate both
> siblings even when another housekeeping CPU has an entirely idle core.
> 
> On most SMT systems, this is not problematic because the idle load
> balancer is a short-lived activity and the transient wakeup of a sibling
> has negligible performance impact.
> 
> However, this can be particularly costly on NVIDIA Olympus cores used in
> Vera. Briefly activating an otherwise idle sibling can reduce the
> performance available to the other sibling and this effect does not
> necessarily end once the activated sibling becomes idle: after the ILB
> finishes and its CPU enters WFI, full single-thread performance is
> restored only after the sibling has remained idle for a qualification
> interval (10 Ki cycles on the tested Vera system). Repeated short
> sibling wakeups can therefore sustain the interference even with little
> actual overlap.

Is this hardware cost has been accounted in things like cpufreq or cpuidle?
or have you set it already to max performance.

> 
> Prevent this by preferring an idle housekeeping CPU whose entire SMT
> core is idle. Retain the first idle CPU as a fallback when no fully idle
> core is available, so NOHZ balancing continues to make forward progress.
> Once a partially busy core has been examined, skip its remaining SMT
> siblings to avoid repeating the core-idle check on wide SMT systems.
> 

Just Curious, making nohz_full=<except first core> yeilds similar numbers?

> Tests performed using an ad hoc GEMM benchmark running one CPU-intensive
> task per SMT core within its CPU affinity mask improved from
> approximately 6.2 TFLOP/s to 9.4 TFLOP/s.
> 
> Note that this preference may wake a fully idle physical core instead of
> using an idle sibling of an active core, potentially increasing ILB
> wakeup latency or energy consumption on some architectures. It may also
> scan additional CPUs before selecting the one to run the ILB. The
> selection falls back to the first idle CPU when no fully idle SMT core
> is available. Non-SMT systems continue to select the first idle
> housekeeping CPU.
> 
> Cc: Vincent Guittot <vincent.guittot@linaro.org>
> Cc: K Prateek Nayak <kprateek.nayak@amd.com>
> Cc: Shrikanth Hegde <sshegde@linux.ibm.com>
> Signed-off-by: Andrea Righi <arighi@nvidia.com>
> Reviewed-by: Mete Durlu <meted@linux.ibm.com>
> ---
> Changes in v4:
>   - Remove redundant this_cpu check (Prateek Nayak, Vincent Guittot)
>   - Link to v3: https://lore.kernel.org/all/20260731191957.3199642-1-arighi@nvidia.com/
> 
> Changes in v3:
>   - After finding an idle fallback, skip all siblings when a busy CPU is
>     encountered, avoiding per-CPU traversal of known-busy cores (Mete Durlu)
>   - Link to v2: https://lore.kernel.org/all/20260729163225.1987068-1-arighi@nvidia.com/
> 
> Changes in v2:
>   - Avoid repeated is_core_idle() checks on wide SMT systems by pruning
>     the remaining siblings of a partially busy core (Prateek Nayak)
>   - Link to v1: https://lore.kernel.org/r/20260728214442.1648483-1-arighi@nvidia.com/
> 
>   kernel/sched/fair.c | 55 ++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 44 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 37001c63452e5..9a975a684b487 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13964,29 +13964,62 @@ static inline int on_null_domain(struct rq *rq)
>    */
>   static inline int find_new_ilb(void)
>   {
> -	int this_cpu = smp_processor_id();
> -	const struct cpumask *hk_mask;
> -	int ilb_cpu;
> +	struct cpumask *ilb_cpus;
> +	int ilb_cpu, fallback = -1;
> +
> +	lockdep_assert_irqs_disabled();
> +
> +	/*
> +	 * Reuse the per-CPU select_rq_mask, which is protected from concurrent
> +	 * use on this CPU by having interrupts disabled.
> +	 */
> +	ilb_cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
> +	cpumask_and(ilb_cpus, nohz.idle_cpus_mask,
> +		    housekeeping_cpumask(HK_TYPE_KERNEL_NOISE));
> +
> +	for_each_cpu(ilb_cpu, ilb_cpus) {
> +		if (!idle_cpu(ilb_cpu)) {
> +			/*
> +			 * Once an idle fallback exists, a busy CPU proves that
> +			 * this core cannot be fully idle. Skip its siblings.
> +			 */
> +			if (sched_smt_active() && fallback >= 0)
> +				cpumask_andnot(ilb_cpus, ilb_cpus, cpu_smt_mask(ilb_cpu));
> +			continue;
> +		}
>   
> -	hk_mask = housekeeping_cpumask(HK_TYPE_KERNEL_NOISE);
> +		/*
> +		 * Running the idle load balancer on an idle sibling of a busy
> +		 * SMT core can reduce the capacity available to its sibling. Prefer
> +		 * a CPU whose entire core is idle, but retain the first idle CPU as
> +		 * a fallback so idle balancing can still make progress when no fully
> +		 * idle core exists.
> +		 */
> +		if (sched_smt_active() && !is_core_idle(ilb_cpu)) {
> +			if (fallback < 0)
> +				fallback = ilb_cpu;
>   
> -	for_each_cpu_and(ilb_cpu, nohz.idle_cpus_mask, hk_mask) {
> -		if (ilb_cpu == this_cpu)
> +			/*
> +			 * The core is not idle, so there is no need to check
> +			 * any of its other SMT siblings.
> +			 */
> +			cpumask_andnot(ilb_cpus, ilb_cpus,
> +				       cpu_smt_mask(ilb_cpu));
>   			continue;
> +		}
>   
> -		if (idle_cpu(ilb_cpu))
> -			return ilb_cpu;
> +		return ilb_cpu;
>   	}
>   
> -	return -1;
> +	return fallback;
>   }
>   
>   /*
>    * Kick a CPU to do the NOHZ balancing, if it is time for it, via a cross-CPU
>    * SMP function call (IPI).
>    *
> - * We pick the first idle CPU in the HK_TYPE_KERNEL_NOISE housekeeping set
> - * (if there is one).
> + * Prefer a CPU on a fully idle core in the HK_TYPE_KERNEL_NOISE housekeeping
> + * set. Fall back to the first idle CPU when no fully idle core exists.
>    */
>   static void kick_ilb(unsigned int flags)
>   {