linux-kernel - Re: [PATCH v2 2/3] sched/fair: Change likelyhood of nohz.nr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ec789892-4ecb-4ae1-b7e1-98b65878fe16@amd.com>
Date: Mon, 5 Jan 2026 12:11:41 +0530
From: K Prateek Nayak <kprateek.nayak@....com>
To: Shrikanth Hegde <sshegde@...ux.ibm.com>
CC: <linux-kernel@...r.kernel.org>, <juri.lelli@...hat.com>,
	<vschneid@...hat.com>, <tglx@...utronix.de>, <dietmar.eggemann@....com>,
	<anna-maria@...utronix.de>, <frederic@...nel.org>, <wangyang.guo@...el.com>,
	<mingo@...nel.org>, <peterz@...radead.org>, <vincent.guittot@...aro.org>
Subject: Re: [PATCH v2 2/3] sched/fair: Change likelyhood of nohz.nr_cpus and
 do stats update if its due

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