All of lore.kernel.org
 help / color / mirror / Atom feed
From: K Prateek Nayak <kprateek.nayak@amd.com>
To: Andrea Righi <arighi@nvidia.com>, Ingo Molnar <mingo@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	"Vincent Guittot" <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Christian Loehle <christian.loehle@arm.com>,
	Shrikanth Hegde <sshegde@linux.ibm.com>,
	"Phil Auld" <pauld@redhat.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] sched/fair: Prefer fully idle cores for NOHZ balancing
Date: Wed, 29 Jul 2026 13:48:14 +0530	[thread overview]
Message-ID: <bec8a289-b26b-4251-89b6-2fdf4f934b76@amd.com> (raw)
In-Reply-To: <20260728214442.1648483-1-arighi@nvidia.com>

Hello Andrea,

On 7/29/2026 3:14 AM, Andrea Righi wrote:
> @@ -13974,19 +13974,32 @@ static inline int find_new_ilb(void)
>  		if (ilb_cpu == this_cpu)
>  			continue;
>  
> -		if (idle_cpu(ilb_cpu))
> +		if (!idle_cpu(ilb_cpu))
> +			continue;
> +
> +		/*
> +		 * 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))

nit.

is_core_idle() here would iterate all siblings and on systems with SMT-4
and SMT-8, that overhead is apparently visible when one thread per core
is occupied based on past optimizations like f8858d96061f ("sched/fair:
Optimize should_we_balance() for large SMT systems").

Copying the same approach from that optimization, can we do:

  (Only build tested)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d78467ec6ee1..814bce21ccf1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13849,21 +13849,35 @@ static inline int on_null_domain(struct rq *rq)
  */
 static inline int find_new_ilb(void)
 {
+	struct cpumask *ilb_cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
 	int this_cpu = smp_processor_id();
-	const struct cpumask *hk_mask;
-	int ilb_cpu;
+	int ilb_cpu, fallback = -1;
 
-	hk_mask = housekeeping_cpumask(HK_TYPE_KERNEL_NOISE);
+	cpumask_and(ilb_cpus, nohz.idle_cpus_mask, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE));
 
-	for_each_cpu_and(ilb_cpu, nohz.idle_cpus_mask, hk_mask) {
+	for_each_cpu(ilb_cpu, ilb_cpus) {
 		if (ilb_cpu == this_cpu)
 			continue;
 
-		if (idle_cpu(ilb_cpu))
-			return ilb_cpu;
+		if (!idle_cpu(ilb_cpu))
+			continue;
+
+		if (sched_smt_active() && !is_core_idle(ilb_cpu)) {
+			if (fallback == -1)
+				fallback = ilb_cpu;
+			/*
+			 * If the core is not idle, and first SMT sibling which is
+			 * idle has been found, then its not needed to check other
+			 * SMT siblings for idleness:
+			 */
+			cpumask_andnot(ilb_cpus, ilb_cpus, cpu_smt_mask(ilb_cpu));
+			continue;
+		}
+
+		return ilb_cpu;
 	}
 
-	return -1;
+	return fallback;
 }
 
 /*
---

It is safe to use "select_rq_mask" here since this is the tick handler
trying to find an ilb_cpu and "select_rq_mask" is only used in contexts
with IRQs disabled. It can probably be renamed to suggest that it is
safe to be used in any IRQ disabled context as a temporary mask.

Thoughts?

-- 
Thanks and Regards,
Prateek


  reply	other threads:[~2026-07-29  8:18 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 21:44 [PATCH] sched/fair: Prefer fully idle cores for NOHZ balancing Andrea Righi
2026-07-29  8:18 ` K Prateek Nayak [this message]
2026-07-29  9:35   ` Andrea Righi
2026-07-29 10:39     ` K Prateek Nayak
2026-07-29 10:56       ` Shrikanth Hegde
2026-07-29 14:25       ` Andrea Righi
2026-07-29 14:51         ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bec8a289-b26b-4251-89b6-2fdf4f934b76@amd.com \
    --to=kprateek.nayak@amd.com \
    --cc=arighi@nvidia.com \
    --cc=bsegall@google.com \
    --cc=christian.loehle@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sshegde@linux.ibm.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.