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

[K Prateek Nayak]

Hello Shrikanth,

On 1/5/2026 10:37 AM, Shrikanth Hegde wrote:
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -12456,10 +12456,10 @@ static void nohz_balancer_kick(struct rq *rq)
>>>         /*
>>>        * None are in tickless mode and hence no need for NOHZ idle load
>>> -     * balancing:
>>> +     * balancing, do stats update if its due
>>>        */
>>> -    if (likely(!atomic_read(&nohz.nr_cpus)))
>>> -        return;
>>> +    if (unlikely(!atomic_read(&nohz.nr_cpus)))
>>> +        goto out;
> 
> Did something got edited here?

Welp! Stray edit. My bad. Reverted back to original diff.

> 
>> Since we are sure that "nohz.nr_cpus" is 0, there is a good chance
>> find_new_ilb() in kick_ilb() will not find any CPU to run balance on, so
>> why not just retain that return?
>>
>> The "flags" can only be set to (NOHZ_NEXT_KICK | NOHZ_STATS_KICK) on
>> this path and kick_ilb() will simply return early without updating
>> "nohz.next_balance" when it doesn't see NOHZ_BALANCE_KICK and fails to
>> find any CPU. Might as well keep the early return.
>>
> 
> The only reason why flags would be set is, if nohz.has_blocked_load
> is set and time is after next_blocked. In that case, doing a stats
> based balance will make nohz.has_blocked_load=0 and subsequent invocations
> flags =0 and no load balance will happen if nr_cpus stays 0.
> 
> However, if we just, has_blocked_load might remains stale value.
> 
> Isn't that the case?

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)?

-- 
Thanks and Regards,
Prateek