Re: [PATCH v5 06/24] sched/core: allow only preferred CPUs in is_cpu_allowed

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

Hi Yury.

On 6/29/26 9:39 AM, Yury Norov wrote:
> On Sat, Jun 27, 2026 at 12:13:49AM +0530, Shrikanth Hegde wrote:
>> Hi Yury.
>>
>> On 6/26/26 6:55 PM, Shrikanth Hegde wrote:
>>> Hi Yury. Thanks for going through the patches.
>>>
>>
>> [...]
>>
>>>> So, you've got 3 options to declare the status: self-explaining enum,
>>>> self-explaining #defines, and this random numbers explained in
>>>> comment. The latter option is the worst to me.
>>>
>>> ok. I will define the enums.
>>>
>>>>
>>>> And you didn't provide any benchmark advocating this caching
>>>> optimization.
>>
>>
>> I did below to see. Made interval as 100ms.
>> Ran ./hackbench 30 process 30000 loops in both the VM at the same time.
>> Values are average of 5 runs.
>>
>> With optimization:
>> 13.6 seconds
>>
>> Without optimization:
>> 13.8 seconds
> 
> And what's the p-value for them?
> 
> ...
>   
>>> If we move to local variable then this won;t be necessary,
>>> just enum's would be enough (I think). Let me go stare at it.
>>
>> I have made it use the local variable instead. There maybe better names
>> for variable, put something quickly to check the idea.
>> Effectively this PATCH 6 becomes:
>>
>> Does this seems better?
>> Please let me know your comments.
> 
> I think, the below is too massive change for optimization of an
> optimization for a particular, not too common config. And the whole
> improvement is ~1%, assuming it's statistically important...
> 
> Just as said on previous round. Please order your series such that the
> core logic goes first, and all sorts of complications, like this
> optimization, are appended at the end.

Ok. I will split it up into two patches.

One without any optimization but with comment explaining the rare case of N**2.
Second one at the end of the series, with a patch do the optimization.

> 
>> ---
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 9e16946c9d62..fafedd52611f 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -2498,8 +2498,10 @@ static inline bool rq_has_pinned_tasks(struct rq *rq)
>>    * Per-CPU kthreads are allowed to run on !active && online CPUs, see
>>    * __set_cpus_allowed_ptr() and select_fallback_rq().
>>    */
>> -static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
>> +static inline bool is_cpu_allowed(struct task_struct *p, int cpu, int cached)
>>   {
>> +       bool task_check_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,24 @@ 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.
>> +        */
>> +       task_check_preferred_cpu = !cpu_preferred(cpu) &&
>> +                                  task_has_preferred_cpus(p, cached);
>> +
>>          /* Non kernel threads are not allowed during either online or offline. */
>> -       if (!(p->flags & PF_KTHREAD))
>> +       if (!(p->flags & PF_KTHREAD)) {
>> +               if (task_check_preferred_cpu)
>> +                       return false;
>>                  return cpu_active(cpu);
>> +       }
>>          /* KTHREAD_IS_PER_CPU is always allowed. */
>>          if (kthread_is_per_cpu(p))
>> @@ -2520,6 +2537,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_check_preferred_cpu)
>> +               return false;
>> +
>>          /* But are allowed during online. */
>>          return cpu_online(cpu);
>>   }
>> @@ -2595,7 +2616,7 @@ static struct rq *__migrate_task(struct rq *rq, struct rq_flags *rf,
>>          __must_hold(__rq_lockp(rq))
>>   {
>>          /* Affinity changed (again). */
>> -       if (!is_cpu_allowed(p, dest_cpu))
>> +       if (!is_cpu_allowed(p, dest_cpu, NO_CACHED_VAL))
>>                  return rq;
> 
> This thing I really dislike. The unrelated code should not be
> affected. You can make it less visually invasive with:
>          
>          #define is_cpu_allowed(p, cpu) __is_cpu_allowed(p, cpu, NO_CACHED_VAL)
> 
> Please reconsider your code to have the changes better localized.
> 
> Thanks,
> Yury
> 

That was typed out too fast. I did refactor something like that later.
But i will split this into twp patches as said above.


diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9e16946c9d62..a1b21c21aa9c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2498,8 +2498,11 @@ static inline bool rq_has_pinned_tasks(struct rq *rq)
   * Per-CPU kthreads are allowed to run on !active && online CPUs, see
   * __set_cpus_allowed_ptr() and select_fallback_rq().
   */
-static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
+static inline bool __is_cpu_allowed(struct task_struct *p, int cpu,
+                                   int pref_state)
  {
+       bool task_check_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 +2511,24 @@ 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.
+        */
+       task_check_preferred_cpu = !cpu_preferred(cpu) &&
+                                  task_has_preferred_cpus(p, pref_state);
+
         /* Non kernel threads are not allowed during either online or offline. */
-       if (!(p->flags & PF_KTHREAD))
+       if (!(p->flags & PF_KTHREAD)) {
+               if (task_check_preferred_cpu)
+                       return false;
                 return cpu_active(cpu);
+       }
  
         /* KTHREAD_IS_PER_CPU is always allowed. */
         if (kthread_is_per_cpu(p))
@@ -2520,10 +2538,19 @@ 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_check_preferred_cpu)
+               return false;
+
         /* But are allowed during online. */
         return cpu_online(cpu);
  }
  
+static inline bool is_cpu_allowed(struct task_struct *p, int cpu)
+{
+       return __is_cpu_allowed(p, cpu, PREFERRED_CPU_UNKNOWN);
+}
+
  /*
   * This is how migration works:
   *
@@ -3547,7 +3574,15 @@ static int select_fallback_rq(int cpu, struct task_struct *p)
         int nid = cpu_to_node(cpu);
         const struct cpumask *nodemask = NULL;
         enum { cpuset, possible, fail } state = cpuset;
-       int dest_cpu;
+       int dest_cpu, pref_state;
+
+       /*
+        * Cache the value whether task's affinity spans preferred CPUs.
+        * This helps to avoid repeating the same for each CPU
+        * later in the loop.
+        */
+       pref_state = task_has_preferred_cpus(p, PREFERRED_CPU_UNKNOWN) ?
+                       PREFERRED_CPU_EXISTS : PREFERRED_CPU_NONE;
  
         /*
          * If the node that the CPU is on has been offlined, cpu_to_node()
@@ -3559,7 +3594,7 @@ static int select_fallback_rq(int cpu, struct task_struct *p)
  
                 /* Look for allowed, online CPU in same node. */
                 for_each_cpu(dest_cpu, nodemask) {
-                       if (is_cpu_allowed(p, dest_cpu))
+                       if (__is_cpu_allowed(p, dest_cpu, pref_state))
                                 return dest_cpu;
                 }
         }
@@ -3567,7 +3602,7 @@ static int select_fallback_rq(int cpu, struct task_struct *p)
         for (;;) {
                 /* Any allowed, online CPU? */
                 for_each_cpu(dest_cpu, p->cpus_ptr) {
-                       if (!is_cpu_allowed(p, dest_cpu))
+                       if (!__is_cpu_allowed(p, dest_cpu, pref_state))
                                 continue;
  
                         goto out;
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index c7c2dea65edd..6a352d235503 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -4213,4 +4213,33 @@ DEFINE_CLASS_IS_UNCONDITIONAL(sched_change)
  
  #include "ext.h"
  
+/*
+ * PREFERRED_CPU_UNKNOWN: Unknown and need to evaluate.
+ * PREFERRED_CPU_NONE  : Known and No preferred CPUs exists in task's affinity.
+ * PREFERRED_CPU_EXISTS: Known and preferred CPU exists in task's affinity.
+ */
+
+enum task_preferred_cached {
+       PREFERRED_CPU_UNKNOWN,
+       PREFERRED_CPU_NONE,
+       PREFERRED_CPU_EXISTS,
+};
+
+/*
+ * Value is known when called via select_fallback_rq(). This helps to
+ * avoid calling cpumask_intersects repeatedly in the loop.
+ *
+ * Only affects FAIR task.
+ */
+static inline bool task_has_preferred_cpus(struct task_struct *p, int pref_state)
+{
+       /* Only FAIR tasks honor preferred CPU state */
+       if (unlikely(p->sched_class != &fair_sched_class))
+               return false;
+
+       if (pref_state != PREFERRED_CPU_UNKNOWN)
+               return pref_state == PREFERRED_CPU_EXISTS;
+
+       return cpumask_intersects(p->cpus_ptr, cpu_preferred_mask);
+}
  #endif /* _KERNEL_SCHED_SCHED_H */