Re: [PATCH v6 05/23] sched/core: Try to use a preferred CPU in is_cpu_allowed

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

Hi Yury,

On 7/1/26 9:39 PM, Yury Norov wrote:
> On Wed, Jul 01, 2026 at 07:46:36PM +0530, Shrikanth Hegde wrote:
>> When possible, choose a preferred CPUs to pick.
>>
>> Push task mechanism uses stopper thread which going to call
>> select_fallback_rq and use this mechanism to pick only a preferred CPU.
>>
>> When task is affined only to non-preferred CPUs it should continue to
>> run there. Detect that by checking if cpus_ptr and cpu_preferred_mask
>> intersect or not.
>>
>> This takes care of wakeup path optimization for FAIR tasks.
>> is_cpu_allowed is called to ensure wakeup happens on preferred CPUs.
>> With that, additional checks in available_idle_cpu is not necessary.
>>
>> Add a comment on rare case of O(N**2) in select_fallback_rq.
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>> v5->v6:
>> - Drop optimization for select_fallback_rq
>> - Keep comment on N**2
>>
>>   kernel/sched/core.c  | 29 ++++++++++++++++++++++++++++-
>>   kernel/sched/sched.h |  9 +++++++++
>>   2 files changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index a45f7c308329..1fb1c17e8387 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -2500,6 +2500,8 @@ static inline bool rq_has_pinned_tasks(struct rq *rq)
>>    */
>>   static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
>>   {
>> +	bool task_has_preferred_cpu;
>> +
>>   	/* When not in the task's cpumask, no point in looking further. */
>>   	if (!task_allowed_on_cpu(p, cpu))
>>   		return false;
>> @@ -2508,9 +2510,30 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
>>   	if (is_migration_disabled(p))
>>   		return cpu_online(cpu);
>>   
>> +	/*
>> +	 * This is essential to maintain user affinities when preferred
>> +	 * CPUs change. A task pinned on non-preferred CPU should continue
>> +	 * to run there, since this is non-user triggered.
>> +	 *
>> +	 * If CPU is non-preferred and task can run on other CPUs which are
>> +	 * currently preferred, then choose those other CPUs instead.
>> +	 * Overhead is minimal when CPU is preferred.
>> +	 *
>> +	 * For majority of the cases this would still keep select_fallback_rq
>> +	 * as O(N). task_has_preferred_cpus which is O(N) is called only if
>> +	 * !cpu_preferred. Then task running there is expected to move out.
>> +	 * So subsequent it should run on preferred CPU. This becomes O(N**2)
>> +	 * only for tasks pinned only non preferred CPUs. That is rare case.
>> +	 */
> 
> The is_cpu_allowed() is ~20 lines now, and your patch doubles that count.
> Can you keep this type of thoughts in commit message? 90% of setups
> will disable preferred CPUs, and I guess 99% of developers don't care.
> 
> This is the code, not a scientific paper, after all.
> 

Ok. I will update the comments and share updated one soon
as reply to this.

>> +	task_has_preferred_cpu = !cpu_preferred(cpu) &&
>> +				 task_has_preferred_cpus(p);
> 
> Maybe it's just me, but the name looks illogical. Because if
> 'cpu' is preferred, the task indeed has some preferred CPUs.
> 
> Maybe 'can_sched_on_preferred' or something like that?
> 

ok.

>> +
>>   	/* Non kernel threads are not allowed during either online or offline. */
>> -	if (!(p->flags & PF_KTHREAD))
>> +	if (!(p->flags & PF_KTHREAD)) {
>> +		if (task_has_preferred_cpu)
>> +			return false;
>>   		return cpu_active(cpu);
>> +	}
> 
> The comment on top of the block seems to be applicable to the 2nd
> return only, right?
> 

First return is for !kthread and second return is kthread.
It is applicable for both. (since kthreads are FAIR class)

But on that thought, do unbound kthreads run too often or
are they usually bound to a CPU? If it is later, we can even drop
that second return.

>>   
>>   	/* KTHREAD_IS_PER_CPU is always allowed. */
>>   	if (kthread_is_per_cpu(p))
>> @@ -2520,6 +2543,10 @@ static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
>>   	if (cpu_dying(cpu))
>>   		return false;
>>   
>> +	/* Try on preferred CPU first if possible*/
>> +	if (task_has_preferred_cpu)
>> +		return false;
> 
> Would it look better if you drop the comment and:
>          
>          if (need_sched_on_preferred)
>                  return false;
> 
>> +
>>   	/* But are allowed during online. */
> 
> This comment is the continuation of the cpu_dying() case. With your
> change it's not anymore, and it needs to be reworded.
> 
>>   	return cpu_online(cpu);
>>   }
>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>> index 26ae13c86b69..36ae20310891 100644
>> --- a/kernel/sched/sched.h
>> +++ b/kernel/sched/sched.h
>> @@ -4230,4 +4230,13 @@ DEFINE_CLASS_IS_UNCONDITIONAL(sched_change)
>>   
>>   #include "ext/ext.h"
>>   
>> +static inline bool task_has_preferred_cpus(struct task_struct *p)
>> +{
>> +	/* Only FAIR tasks honor preferred CPU state */
>> +	if (unlikely(p->sched_class != &fair_sched_class))
>> +		return false;
>> +
>> +	return cpumask_intersects(p->cpus_ptr, cpu_preferred_mask);
>> +}
>> +
>>   #endif /* _KERNEL_SCHED_SCHED_H */
>> -- 
>> 2.47.3