[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7050ce6c-64d6-4359-9a4c-9fd4362e5290@amd.com>
Date: Tue, 6 Jan 2026 08:33:16 +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 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
Powered by blists - more mailing lists