Re: [PATCH v3] sched/fair: Prefer waker CPU for non-SMT reciprocal sync wakeups

[Madadi Vineeth Reddy <vineethr@linux.ibm.com>]

Hi Prateek,

On 30/07/26 11:57, K Prateek Nayak wrote:
> Hello Shubhang,
> 
> On 7/28/2026 5:28 AM, Shubhang Kaushik (Ampere) wrote:
>> Pipe-style ping-pong workloads can be dominated by handoff cost. In
>> such cases, placing the wakee on an idle CPU can be slower than keeping
>> the pair on the same runqueue.
>>
>> Use the existing last_wakee and wake_wide() state to identify narrow
>> reciprocal WF_SYNC wakeups:
>>
>> A wakes B
>> B wakes A
>> A wakes B
>> ...
>>
>> When the wake-affine domain allows SD_WAKE_AFFINE, prefer the waker CPU
>> for these narrow reciprocal handoffs on non-SMT systems. Do so only when
>> the waker CPU has no other runnable fair task and the wakee fits there on
>> asymmetric-capacity systems.
>>
>> SMT systems, and wakeups that do not match this pattern, continue through
>> the existing wake_affine() and select_idle_sibling() path.
>>
>> Signed-off-by: Shubhang Kaushik (Ampere) <sh@gentwo.org>
>> ---
>> Tested on 80-core non-SMT Ampere Altra: perf bench sched pipe -l 1000000
>> improved by about 30%, averaged over 40 runs. Hackbench, schbench and
>> SPECjBB showed no material regression.
>>
>> Baseline: v7.2-rc5
>> ---
>> Changes in v3:
>>   - Limit the direct waker-CPU preference to !sched_smt_active(); SMT
>>     systems continue through the existing wake_affine() and
>>     select_idle_sibling() path.
> 
> Building on top of Chris' suggestion on v2 for systems with SMT, we can
> push that check further down into select_idle_sibling() and can take a
> call at the point where we know what test_idle_core() returns.
> 
> This is what I tried out on top of tip:sched/core:
> 
>   (Lightly tested on a SMT-2 system)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index df8c9c2c7918..5821cbd930ae 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1301,7 +1301,6 @@ 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 unsigned long task_h_load(struct task_struct *p);
>  static unsigned long capacity_of(int cpu);
>  
> @@ -8636,7 +8635,7 @@ static int select_idle_core(struct task_struct *p, int core, struct cpumask *cpu
>  /*
>   * Scan the local SMT mask for idle CPUs.
>   */
> -static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int target)
> +static int select_idle_smt(struct task_struct *p, struct root_domain *rd, int target)
>  {
>  	int cpu;
>  
> @@ -8644,10 +8643,13 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
>  		if (cpu == target)
>  			continue;
>  		/*
> -		 * Check if the CPU is in the LLC scheduling domain of @target.
> -		 * Due to isolcpus, there is no guarantee that all the siblings are in the domain.
> +		 * Check if the CPU is in the scheduling domain of @target.
> +		 * Due to isolcpus, there is no guarantee that all the
> +		 * siblings are in the domain.
>  		 */
> -		if (!cpumask_test_cpu(cpu, sched_domain_span(sd)))
> +		if (!cpumask_test_cpu(cpu, rd->span))
> +			continue;
> +		if (sched_asym_cpucap_active() && !task_fits_cpu(p, cpu))
>  			continue;
>  		if (choose_idle_cpu(cpu, p))
>  			return cpu;
> @@ -8928,12 +8930,12 @@ 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)
>  {
>  	bool has_idle_core = false;
>  	struct sched_domain *sd;
>  	unsigned long task_util, util_min, util_max;
> -	int i, recent_used_cpu, prev_aff = -1;
> +	int i, this_cpu, recent_used_cpu, prev_aff = -1;
>  
>  	/*
>  	 * On asymmetric system, update task utilization because we will check
> @@ -8977,9 +8979,10 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  	 * essentially a sync wakeup. An obvious example of this
>  	 * pattern is IO completions.
>  	 */
> +	this_cpu = smp_processor_id();
>  	if (is_per_cpu_kthread(current) &&
>  	    in_task() &&
> -	    prev == smp_processor_id() &&
> +	    prev == this_cpu &&
>  	    this_rq()->nr_running <= 1 &&
>  	    asym_fits_cpu(task_util, util_min, util_max, prev)) {
>  		return prev;
> @@ -9003,6 +9006,32 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  		recent_used_cpu = -1;
>  	}
>  
> +	has_idle_core = sched_smt_active() && test_idle_cores(target);
> +
> +	if (!has_idle_core) {
> +		struct rq *target_rq = cpu_rq(target);
> +
> +		/* Prefer an idle thread on same core where data is hot. */
> +		if (sched_smt_active() && cpus_share_cache(prev, target)) {
> +			i = select_idle_smt(p, target_rq->rd, prev);
> +			if ((unsigned int)i < nr_cpumask_bits)
> +				return i;
> +		}
> +
> +		/*
> +		 * Tasks are likely a sync wakeup pair that passed WA_IDLE.
> +		 * Prefer to temporarily stack them on the same CPU since the
> +		 * waker is likely to go away soon and there are no idle cores.
> +		 */
> +		if (sync &&
> +		    in_task() &&
> +		    target == this_cpu &&
> +		    p->last_wakee == current &&
> +		    (target_rq->nr_running - cfs_h_nr_delayed(target_rq)) <= 1 &&
> +		    asym_fits_cpu(task_util, util_min, util_max, target))
> +			return target;
> +	}
> +
>  	/*
>  	 * For asymmetric CPU capacity systems, our domain of interest is
>  	 * sd_asym_cpucapacity rather than sd_llc.
> @@ -9027,16 +9056,6 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  	if (!sd)
>  		return target;
>  
> -	if (sched_smt_active()) {
> -		has_idle_core = test_idle_cores(target);
> -
> -		if (!has_idle_core && cpus_share_cache(prev, target)) {
> -			i = select_idle_smt(p, sd, prev);
> -			if ((unsigned int)i < nr_cpumask_bits)
> -				return i;
> -		}
> -	}
> -
>  	i = select_idle_cpu(p, sd, has_idle_core, target);
>  	if ((unsigned)i < nr_cpumask_bits)
>  		return i;
> @@ -9734,7 +9753,7 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
>  
>  	/* Fast path */
>  	if (wake_flags & WF_TTWU)
> -		return select_idle_sibling(p, prev_cpu, new_cpu);
> +		return select_idle_sibling(p, prev_cpu, new_cpu, sync);
>  
>  	return new_cpu;
>  }

I have been looking at the same problem from the SMT side which I mentioned in v2
of this patch:
https://lore.kernel.org/all/60a584c5-25ac-4077-a725-a2f9ee74318d@linux.ibm.com/

Posted a patch for it today:
https://lore.kernel.org/lkml/20260801035532.260625-1-vineethr@linux.ibm.com/

It lets the waker's CPU count as idle inside select_idle_core(), so the
waker's core stays an idle-core candidate and the wakee lands on one of
its sibling threads. On a sync wakeup the waker's core already holds the
data, so this keeps the cache sharing.

Thanks,
Vineeth

> ---
> 
> I'm currently seeing a ~10% improvement for the workload you mentioned
> (perf bench sched pipe -l 1000000) on average. I haven't tried anything
> else yet but would love to know your thoughts.
> 
> I'm using rq->rd->span to know the CPUs covered by the cpuset instead of
> sched_domain_span(sd_llc) in select_idle_smt() to make it work for
> sched_asym_cpucap_active() + sched_smt_active() where some cores may
> have more than one CPUs and the LLC is defined at core boundary.
> 
> Basically I wanted to avoid this ugly:
> 
>     sd = rcu_dereference_all(per_cpu((sched_asym_cpucap_active()) ? sd_asym : sd_llc, target));
> 
>     if (!sd)
>         goto skip;
> 
> pattern and rq->rd->span seemed just fine since it doesn't need a null
> check and gives the desired boundary.
> 
> Could you please check if the improvements still persist on your system
> with the check pushed down into select_idle_sibling(). Thank you.
> 
> 
>>   - Drop the redundant affinity check; want_affine already verifies the
>>     waker CPU is allowed.
>>   - Use a plain p->last_wakee read instead of READ_ONCE().
>>   - Rebase and refresh testing on v7.2-rc5.
>>
>> Link to v2: https://lore.kernel.org/r/20260722-b4-sched-sync-wakeup-v2-1-f1164560b24b@gentwo.org
>>
>> Changes in v2:
>>   - Move the reciprocal handoff preference under the existing
>>     SD_WAKE_AFFINE domain check.
>>   - Drop futex from the changelog motivation.
>>   - Refresh perf bench sched pipe results after rebasing.
>>
>> Link to v1: https://lore.kernel.org/r/20260721-b4-sched-sync-wakeup-v1-1-dc94f184e27f@gentwo.org
>> ---
>>  kernel/sched/fair.c | 25 +++++++++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index d78467ec6ee1343050fcc2794dafb38ade3599e5..e61062d20da772d29da6f5f377a150b4b5128619 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -8794,6 +8794,26 @@ static inline bool asym_fits_cpu(unsigned long util,
>>  	return true;
>>  }
>>  
>> +/*
>> + * For reciprocal WF_SYNC handoffs, prefer the waker CPU when it has no
>> + * other runnable fair task.
>> + */
>> +static bool prefer_sync_pair_cpu(struct task_struct *p, int cpu)
>> +{
>> +	struct rq *rq = cpu_rq(cpu);
>> +
>> +	if ((rq->nr_running - cfs_h_nr_delayed(rq)) != 1)
>> +		return false;
>> +
>> +	if (sched_asym_cpucap_active()) {
>> +		sync_entity_load_avg(&p->se);
>> +		if (!task_fits_cpu(p, cpu))
>> +			return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>  /*
>>   * Try and locate an idle core/thread in the LLC cache domain.
>>   */
>> @@ -9579,6 +9599,11 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int wake_flags)
>>  		 */
>>  		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
>>  		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
>> +			if (sync && !sched_smt_active() &&
> 
> For the record, without !sched_smt_active(), the runtime for
> "perf bench sched pipe -l 1000000" almost doubles in my case but
> looks like that condition might overall be good with a bunch of
> defensive checks on SMT systems too.
> 
>> +			    p->last_wakee == current &&
>> +			    prefer_sync_pair_cpu(p, cpu))
>> +				return cpu;
>> +
>>  			if (cpu != prev_cpu)
>>  				new_cpu = wake_affine(tmp, p, cpu, prev_cpu, sync);
>>  
>>
>> ---
>> base-commit: f5098b6bae761e346ebcd9da7f95622c04733cff
>> change-id: 20260721-b4-sched-sync-wakeup-04d40cbeb1da
>>
>> Best regards,
>