All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sched/fair: Let sync wakeups target the waker's core
@ 2026-08-01  3:55 Madadi Vineeth Reddy
  2026-08-01  6:43 ` Zhan Xusheng
  0 siblings, 1 reply; 2+ messages in thread
From: Madadi Vineeth Reddy @ 2026-08-01  3:55 UTC (permalink / raw)
  To: Ingo Molnar, Peter Zijlstra, Juri Lelli, Vincent Guittot,
	Dietmar Eggemann, Steven Rostedt, Ben Segall, Mel Gorman,
	Valentin Schneider, K Prateek Nayak
  Cc: linux-kernel, sh, Madadi Vineeth Reddy

WF_SYNC tells the scheduler the waker is about to block, and
wake_affine_idle() already acts on it: when the waker's runqueue holds a
single runnable task it returns the waker's CPU. select_idle_sibling()
then discards that decision, because available_idle_cpu() is false for a
CPU that is still running the waker, and the scan continues elsewhere in
the LLC.

When the wakee's previous CPU is idle and shares cache with the target,
SIS returns it early and nothing is lost. Once prev_cpu is busy, though,
the scan has no way to distinguish "this core is busy" from "this core is
running only the task that is about to sleep", so the wakee is placed on
a cold CPU even though the waker's core is about to have capacity and
already holds the data in cache.

Pass the waker's CPU down to select_idle_core() in that case and let it
count as idle. The waker's core then remains an idle-core candidate and
the wakee is placed on one of its sibling threads.

The change is a no-op when the waker's core has no idle sibling, when the
waker's runqueue holds more than one runnable task, when the wakee's
previous CPU is already a valid target, and on non-SMT systems.

Tested on POWER11, SMT8, 160 CPUs / 20 cores. 5 runs per case, all
figures normalised to baseline.

POWER11:
[producer_consumer] (time/access, median, lower is better)
==================================================
case             load       base   base+this patch
time/access      -l 5       1.00   0.89 (+11.11%)
time/access      -l 10      1.00   0.92 ( +7.69%)
time/access      -l 20      1.00   1.00 ( +0.00%)
time/access      -l 100     1.00   1.00 ( +0.00%)

A strict two-task handoff. As the per-iteration work grows, the
wakeup path becomes a smaller fraction of each iteration and
placement matters less: 11% at -l 5, nothing from -l 20 upwards.

[hackbench] (mean completion time, lower is better; 150000 loops)
=================================================================
case             load       baseline   base+this patch   sd%
process-pipe     1-group    1.00       0.94 ( +6.39%)     6.2
thread-pipe      1-group    1.00       0.92 ( +7.83%)     8.4
process-pipe     2-group    1.00       0.94 ( +6.18%)     7.1
thread-pipe      2-group    1.00       0.97 ( +3.12%)     7.9
process-pipe     4-group    1.00       0.98 ( +1.69%)     7.2
thread-pipe      4-group    1.00       1.09 ( -8.80%)    11.9
process-pipe     8-group    1.00       1.02 ( -2.32%)     3.8
thread-pipe      8-group    1.00       0.99 ( +0.56%)     2.6
process-socket   2-group    1.00       0.95 ( +5.20%)     6.1
thread-socket    2-group    1.00       1.02 ( -2.13%)     5.8

Each group is 40 tasks, so 1 group is 25% of the 160 CPUs and 8 groups is
200%. The patch needs an idle core in the LLC, so its impact shrinks as
the utilization increase. Mostly numbers are positive and within
run to run variation.

[schbench] (mean p99 wakeup latency, lower is better)
=====================================================
case             load       baseline   base+this patch   sd%
p99-latency        8-wkr    1.00       1.06 ( -6.06%)     8.3
p99-latency       40-wkr    1.00       0.95 ( +5.26%)     7.8
p99-latency       80-wkr    1.00       1.08 ( -7.58%)     9.9
p99-latency      240-wkr    1.00       1.00 ( -0.05%)     0.6

[schbench] (mean current rps, higher is better)
===============================================
case             load       baseline   base+this patch   sd%
rps                8-wkr    1.00       1.00 ( -0.17%)     0.4
rps               40-wkr    1.00       1.01 ( +0.87%)     4.9
rps               80-wkr    1.00       0.94 ( -5.55%)     7.9
rps              240-wkr    1.00       1.00 ( -0.28%)     0.3

schbench numbers are within run to run variation and hence not
much impacted with this patch.

Signed-off-by: Madadi Vineeth Reddy <vineethr@linux.ibm.com>

---

Shubhang Kaushik returns the waker CPU directly from the wake-affine
branch in select_task_rq_fair(); v3 restricts that to
!sched_smt_active(), since on SMT it stacks the pair onto one hardware
thread while siblings sit idle:
https://lore.kernel.org/lkml/20260727-b4-sched-sync-wakeup-v3-1-90cf481dbd85@gentwo.org/

Prateek proposed moving the decision into select_idle_sibling()
under if (!has_idle_core), preferring an idle sibling of prev and then
stacking on the waker:
https://lore.kernel.org/lkml/f3d5530f-3811-42af-8c34-c40cf314deed@amd.com/

This patch covers the disjoint case: on a sync wakeup the waker's core
already holds the data, so where an idle core exists in the LLC this
redirects the wakee onto a sibling of the waker's core rather than a cold
one, and where none exists it is a no-op. I mentioned this approach on
the v2 thread of Shubhang's patch:
https://lore.kernel.org/all/60a584c5-25ac-4077-a725-a2f9ee74318d@linux.ibm.com/
---
 kernel/sched/fair.c | 49 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index df8c9c2c7918..bb75b228c817 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1301,7 +1301,7 @@ static bool update_deadline(struct cfs_rq *cfs_rq, struct sched_entity *se)
 
 #include "pelt.h"
 
-static int select_idle_sibling(struct task_struct *p, int prev_cpu, int cpu);
+static int select_idle_sibling(struct task_struct *p, int prev_cpu, int cpu, int sync_cpu);
 static unsigned long task_h_load(struct task_struct *p);
 static unsigned long capacity_of(int cpu);
 
@@ -8604,13 +8604,24 @@ void __update_idle_core(struct rq *rq)
  * sd_balance_shared->has_idle_cores and enabled through update_idle_core()
  * above.
  */
-static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpus, int *idle_cpu)
+static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpus,
+				int *idle_cpu, int sync_cpu)
 {
 	bool idle = true;
 	int cpu;
 
 	for_each_cpu(cpu, cpu_smt_mask(core)) {
-		if (!available_idle_cpu(cpu)) {
+		bool sync_waker = (cpu == sync_cpu);
+
+		/*
+		 * @sync_cpu, if set, is running a waker that is about to
+		 * block with nothing else runnable behind it. Treat it as
+		 * idle so this core stays an idle-core candidate: placing
+		 * the wakee on a sibling keeps the cache sharing that
+		 * stacking on the waker's rq would get, without serialising
+		 * the wakee behind the waker's remaining work.
+		 */
+		if (!available_idle_cpu(cpu) && !sync_waker) {
 			idle = false;
 			if (*idle_cpu == -1) {
 				if (choose_sched_idle_rq(cpu_rq(cpu), p) &&
@@ -8622,7 +8633,12 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
 			}
 			break;
 		}
-		if (*idle_cpu == -1 && cpumask_test_cpu(cpu, cpus))
+
+		/*
+		 * The waker is not idle yet, so it must not be offered as
+		 * the fallback target if this core turns out to be busy.
+		 */
+		if (!sync_waker && *idle_cpu == -1 && cpumask_test_cpu(cpu, cpus))
 			*idle_cpu = cpu;
 	}
 
@@ -8661,7 +8677,8 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
  * comparing the average scan cost (tracked in sd->avg_scan_cost) against the
  * average idle time for this rq (as found in rq->avg_idle).
  */
-static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool has_idle_core, int target)
+static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool has_idle_core,
+				int target, int sync_cpu)
 {
 	struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
 	int i, cpu, idle_cpu = -1, nr = INT_MAX;
@@ -8694,7 +8711,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
 					continue;
 
 				if (has_idle_core) {
-					i = select_idle_core(p, cpu, cpus, &idle_cpu);
+					i = select_idle_core(p, cpu, cpus, &idle_cpu, sync_cpu);
 					if ((unsigned int)i < nr_cpumask_bits)
 						return i;
 				} else {
@@ -8711,7 +8728,7 @@ static int select_idle_cpu(struct task_struct *p, struct sched_domain *sd, bool
 
 	for_each_cpu_wrap(cpu, cpus, target + 1) {
 		if (has_idle_core) {
-			i = select_idle_core(p, cpu, cpus, &idle_cpu);
+			i = select_idle_core(p, cpu, cpus, &idle_cpu, sync_cpu);
 			if ((unsigned int)i < nr_cpumask_bits)
 				return i;
 
@@ -8928,7 +8945,7 @@ static inline bool asym_fits_cpu(unsigned long util,
 /*
  * Try and locate an idle core/thread in the LLC cache domain.
  */
-static int select_idle_sibling(struct task_struct *p, int prev, int target)
+static int select_idle_sibling(struct task_struct *p, int prev, int target, int sync_cpu)
 {
 	bool has_idle_core = false;
 	struct sched_domain *sd;
@@ -9037,7 +9054,7 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
 		}
 	}
 
-	i = select_idle_cpu(p, sd, has_idle_core, target);
+	i = select_idle_cpu(p, sd, has_idle_core, target, sync_cpu);
 	if ((unsigned)i < nr_cpumask_bits)
 		return i;
 
@@ -9733,8 +9750,18 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
 		return sched_balance_find_dst_cpu(sd, p, cpu, prev_cpu, sd_flag);
 
 	/* Fast path */
-	if (wake_flags & WF_TTWU)
-		return select_idle_sibling(p, prev_cpu, new_cpu);
+	if (wake_flags & WF_TTWU) {
+		int sync_cpu = -1;
+
+		if (want_affine && sync && new_cpu == cpu) {
+			struct rq *rq = cpu_rq(cpu);
+
+			if ((rq->nr_running - cfs_h_nr_delayed(rq)) == 1)
+				sync_cpu = cpu;
+		}
+
+		return select_idle_sibling(p, prev_cpu, new_cpu, sync_cpu);
+	}
 
 	return new_cpu;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] sched/fair: Let sync wakeups target the waker's core
  2026-08-01  3:55 [PATCH] sched/fair: Let sync wakeups target the waker's core Madadi Vineeth Reddy
@ 2026-08-01  6:43 ` Zhan Xusheng
  0 siblings, 0 replies; 2+ messages in thread
From: Zhan Xusheng @ 2026-08-01  6:43 UTC (permalink / raw)
  To: vineethr, mingo, peterz, juri.lelli, vincent.guittot,
	dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	kprateek.nayak
  Cc: linux-kernel, sh, zhanxusheng, Zhan Xusheng

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

On Sat, Aug 01, 2026 at 09:25:32AM +0530, Madadi Vineeth Reddy wrote:
> Pass the waker's CPU down to select_idle_core() in that case and let it
> count as idle. The waker's core then remains an idle-core candidate and
> the wakee is placed on one of its sibling threads.

Nice, and thanks for laying out the other approaches in the changelog.

If I read select_idle_cpu() right, which sibling the wakee actually lands
on depends on the wrap scan order: it iterates
for_each_cpu_wrap(cpu, cpus, target + 1) and select_idle_core() returns the
first fully-idle core it hits.

On POWER SMT8 the SMT siblings are numbered contiguously, so the waker's
sibling sits right at target + 1 and is reached first - exactly the intent.

On layouts where the siblings are not adjacent to the waker - e.g. the
common x86 enumeration where the sibling is at cpu + nr_cores rather than
cpu + 1 - the scan visits other cores first, and if any of them is fully
idle it returns that (cold) core before ever reaching the waker's sibling.
There the cache-sharing win would not materialise; the wakee lands where
plain SIS would have put it. It should not regress, but the benefit looks
like it could be largely specific to contiguously numbered SMT.

Would you be able to share x86 / arm64 SMT2 numbers? That would help show
whether the waker's sibling actually gets picked on those layouts, or
whether the effect is mostly seen on contiguously numbered SMT.

Thanks,
Zhan Xusheng

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-01  6:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-01  3:55 [PATCH] sched/fair: Let sync wakeups target the waker's core Madadi Vineeth Reddy
2026-08-01  6:43 ` Zhan Xusheng

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.