Re: [PATCH v2 2/3] sched/fair: Change likelyhood of nohz.nr_cpus and do stats update if its due

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

Hello Shrikanth,

On 1/5/2026 4:09 PM, Shrikanth Hegde wrote:
>> So cumulatively, including Patch 3, we do:
>>
>>      flags = 0;
>>
>>      if (READ_ONCE(nohz.has_blocked_load) && ...)
>>          flags = NOHZ_STATS_KICK;
>>
>>      if (time_before(now, nohz.next_balance))
>>          goto out; /* Checks nohz.idle_cpus_mask in find_new_ilb() ... (1) */
>>
>>      if (unlikely(cpumask_empty(nohz.idle_cpus_mask)))
>>          goto out; /* Still goes to kick_ilb()                     ... (2) */
>>
>>      ...
>>
>> out:
>>      if (READ_ONCE(nohz.needs_update))
>>          flags |= NOHZ_NEXT_KICK;
>>
>>      /* assume either NOHZ_STATS_KICK or NOHZ_NEXT_KICK is set */
>>      kick_ilb()
>>      {
>>           if (flags & NOHZ_BALANCE_KICK) /* Not possible */
>>               ...
>>
>>           ilb_cpu = find_new_ilb(); /* Find CPU in nohz.idle_cpus_mask */
>>
>>
>> If we arrive here from (2), we know "nohz.idle_cpus_mask" was empty a
>> while back and we've not updated any global "nohz" state. If we don't
>> find an ilb_cpu, we just do:
>>
>>          if (ilb_cpu < 0)
>>              return;
>>
>> So why not simply return from (2)?
>>
> 
> I see, kick_ilb though called will not do a balance since ilb_cpu was not found.
> 
> I don't want to have that return in between the two out's.
> 
> How about we do below? When there are no idle CPUs left, both has_blocked_load
> and needs_update should be reset. no?
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 805b53d9709e..fa0e6065bc9c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -12377,6 +12377,15 @@ static inline int find_new_ilb(void)
>                         return ilb_cpu;
>         }
>  
> +       /* There is no idle CPU left.
> +        * reset has_blocked_load and needs_update, such that unless
> +        * some CPU enters idle state, it will not trigger kick_ilb
> +        */
> +       if (READ_ONCE(nohz.has_blocked_load))
> +               WRITE_ONCE(nohz.has_blocked_load, 0);
> +       if (READ_ONCE(nohz.needs_update))
> +               WRITE_ONCE(nohz.needs_update, 0);
> +

I'm slightly skeptical - find_new_ilb() will also fail to find any CPU
if idle_cpu() returns false momentarily.

Those CPUs can again go back to idle without updating
"nohz.has_blocked_load" - tick_nohz_idle_stop_tick() will skip
nohz_balance_enter_idle() if the CPU already had TS_FLAG_STOPPED set.

For those cases, we'll need to retain the "nohz" state as is until a
suitable ILB CPU can be found - this is the reason why we delay
clearing the "nohz" state until _nohz_idle_balance() and reconstruct it
once the CPU is done with idle balancing.

There are also nuances like the smp_mb__after_atomic() in
nohz_balance_enter_idle() which requires us to check the
"nohz.idle_cpus_mask" after we are done clearing "nohz.needs_update"
and "nohz.has_blocked_load".

>         return -1;
>  }
> 
> 

-- 
Thanks and Regards,
Prateek