Re: [PATCH] sched/fair: Let sync wakeups target the waker's core

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Vineeth,

On 8/4/2026 5:43 PM, Madadi Vineeth Reddy wrote:
> Hello Prateek,
> 
> On 04/08/26 10:19, K Prateek Nayak wrote:
>> Hello Vineeth,
>>
>> On 8/1/2026 9:25 AM, Madadi Vineeth Reddy wrote:
>>> -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) {
>>
>> If I'm not wrong, all you want to make is the sync_waker appear idle and
>> then see if you can then consider that core as idle core or not right?
>>
> 
> Correct.
> 
>> Why can't this be done in select_idle_sibling() extending that early
>> check for (!has_idle_core && cpus_share_cache(prev, target)) condition
>> and then initializing "idle_cpu" in select_idle_cpu() accordingly?
>>
>> Something along the lines of:
>>
>>   (Only build tested)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index df8c9c2c7918..dd62bceb3838 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);
>>  
>> @@ -8661,10 +8660,11 @@ 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 idle_cpu)
>>  {
>>  	struct cpumask *cpus = this_cpu_cpumask_var_ptr(select_rq_mask);
>> -	int i, cpu, idle_cpu = -1, nr = INT_MAX;
>> +	int i, cpu, nr = INT_MAX;
>>  
>>  	if (sched_feat(SIS_UTIL) && sd->shared) {
>>  		/*
>> @@ -8928,7 +8928,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;
>> @@ -9028,16 +9028,19 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>>  		return target;
>>  
>>  	if (sched_smt_active()) {
>> +		int cpu = ((unsigned)sync_cpu < nr_cpumask_bits) ? sync_cpu : prev;
>> +
>>  		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)
>> +		if (sync_cpu == target || (!has_idle_core && cpus_share_cache(prev, target))) {
>> +			i = select_idle_smt(p, sd, cpu);
>> +
>> +			if (!has_idle_core && ((unsigned int)i < nr_cpumask_bits))
>>  				return i;
>>  		}
>>  	}
>>  
>> -	i = select_idle_cpu(p, sd, has_idle_core, target);
>> +	i = select_idle_cpu(p, sd, has_idle_core, target, i);
> 
> `i` which is passed could be garbage value if we don't enter sched_smt_active block.
> 
>>  	if ((unsigned)i < nr_cpumask_bits)
>>  		return i;
>>  
> 
> This is different from what I wanted to achieve in a couple of ways.
> 
> - Calling `select_idle_smt()` in sync case, would only give an idle CPU in that core but doesn't
> test if that core is idle. That would be lost.
> - You return `i` only when `!has_idle_core`, but I wanted to return waker core given that rest of the siblings
> in that waker core are idle even though there are other idle cores present in the LLC.
> 
> I agree that this could be done in `select_idle_sibling` but with a helper function.
> Something like below (build tested)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index df8c9c2c7918..448af3c4b183 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, bool sync_core);

Is this forward declaration necessary? I think you can remove it
entirely.

>  static unsigned long task_h_load(struct task_struct *p);
>  static unsigned long capacity_of(int cpu);
>  
> @@ -8656,6 +8656,27 @@ static int select_idle_smt(struct task_struct *p, struct sched_domain *sd, int t
>  	return -1;
>  }
>  
> +static int select_idle_sync_core(struct task_struct *p, struct sched_domain *sd,
> +				 int target)
> +{
> +	int cpu, idle_sibling = -1;
> +
> +	for_each_cpu(cpu, cpu_smt_mask(target)) {
> +		if (cpu == target)
> +			continue;
> +
> +		if (!available_idle_cpu(cpu))
> +			return -1;

Reads a lot like select_idle_smt(). Perhaps you can pass "has_idle_core"
hint to the same and return idle CPU early if !has_idle_core or otherwise
return the first idle sibling if core is idle.

That way even sync_core case is handled by the same function. You can
gate the call with (sync_core || !has_idle_core)

> +
> +		if (idle_sibling == -1 &&
> +		    cpumask_test_cpu(cpu, sched_domain_span(sd)) &&
> +		    cpumask_test_cpu(cpu, p->cpus_ptr))
> +			idle_sibling = cpu;
> +	}
> +
> +	return idle_sibling;
> +}
> +
>  /*
>   * Scan the LLC domain for idle CPUs; this is dynamically regulated by
>   * comparing the average scan cost (tracked in sd->avg_scan_cost) against the
> @@ -8928,7 +8949,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, bool sync_core)
>  {
>  	bool has_idle_core = false;
>  	struct sched_domain *sd;
> @@ -9035,6 +9056,12 @@ static int select_idle_sibling(struct task_struct *p, int prev, int target)
>  			if ((unsigned int)i < nr_cpumask_bits)
>  				return i;
>  		}
> +
> +		if (sync_core) {
> +			i = select_idle_sync_core(p, sd, target);
> +			if ((unsigned int)i < nr_cpumask_bits)
> +				return i;
> +		}
>  	}
>  
>  	i = select_idle_cpu(p, sd, has_idle_core, target);
> @@ -9733,8 +9760,16 @@ 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) {
> +		bool sync_core = false;
> +		if (want_affine && sync && new_cpu == cpu) {
> +			struct rq *rq = cpu_rq(cpu);
> +
> +			sync_core = (rq->nr_running - cfs_h_nr_delayed(rq)) == 1;

Instead of computing this twice on wake_affine path, you can have a task
flag like sched_task_hot that you just set in wake_affine_idle() before
it returns early from the sync branch.

You can clear it up top in select_task_rq_fair() and check it
select_idle_sibling() to conditionally call select_idle_sync_core().

> +		}
> +
> +		return select_idle_sibling(p, prev_cpu, new_cpu, sync_core);
> +	}
>  
>  	return new_cpu;
>  }
> 
> This should also solve the issue raised by Zhan Xusheng and first target waker core given
> that it is idle by giving exception to waker cpu.
> 
> Thoughts?

Let me give it a spin. I'll report back if I see anything unexpected.

-- 
Thanks and Regards,
Prateek