[PATCH v2 0/3] sched/fair: improve nohz fields for large systems

[PATCH v2 0/3] sched/fair: improve nohz fields for large systems

[Shrikanth Hegde]

Running on large systems nohz.nr_cpus cacheline was seen as contended.
There is atomic inc/dec and read happening on many
CPUs at a time and it is possible for this line to bounce often.

Gist of the series is to get rid of nr_cpus. Instead, use the cpumask
which is always updated alongside with it. Functionally it should serve
the same purpose. Rest of the fields aren't updated that often. So this
line shouldn't bounce that often.

Contention issue with nohz.idle_cpus_mask still remains. Mostly it is in
separate cacheline than nohz. There are ongoing efforts to mitigate it. It
is not addressed by this series.

v1 -> v2:
- Dropped patch to check has_blocked based on time.
- Detailed changelog for removing nr_cpus (Thanks to Ingo Molnar)

v1: https://lore.kernel.org/all/20251201183146.74443-1-sshegde@linux.ibm.com/

Shrikanth Hegde (3):
  sched/fair: Move checking for nohz cpus after time check
  sched/fair: Change likelyhood of nohz.nr_cpus and do stats update if
    its due
  sched/fair: Remove nohz.nr_cpus and use weight of cpumask instead

 kernel/sched/fair.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

-- 
2.47.3

[PATCH v2 1/3] sched/fair: Move checking for nohz cpus after time check

[Shrikanth Hegde]

Idle load balancer is kicked off only after time check. So move
the atomic read after the time check to avoid the overhead.

If there are no nohz CPUs and next_blocked has passed, then there
will be one additional stats based load balancing which would set the
has_blocked_load to 0. It shouldn't make a difference.

Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> 
---
 kernel/sched/fair.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7377f9117501..cd1c78d2c272 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12447,13 +12447,6 @@ static void nohz_balancer_kick(struct rq *rq)
 	 */
 	nohz_balance_exit_idle(rq);
 
-	/*
-	 * None are in tickless mode and hence no need for NOHZ idle load
-	 * balancing:
-	 */
-	if (likely(!atomic_read(&nohz.nr_cpus)))
-		return;
-
 	if (READ_ONCE(nohz.has_blocked_load) &&
 	    time_after(now, READ_ONCE(nohz.next_blocked)))
 		flags = NOHZ_STATS_KICK;
@@ -12461,6 +12454,13 @@ static void nohz_balancer_kick(struct rq *rq)
 	if (time_before(now, nohz.next_balance))
 		goto out;
 
+	/*
+	 * None are in tickless mode and hence no need for NOHZ idle load
+	 * balancing:
+	 */
+	if (likely(!atomic_read(&nohz.nr_cpus)))
+		return;
+
 	if (rq->nr_running >= 2) {
 		flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
 		goto out;
-- 
2.47.3

Re: [PATCH v2 1/3] sched/fair: Move checking for nohz cpus after time check

[Peter Zijlstra]

On Fri, Jan 02, 2026 at 06:17:42PM +0530, Shrikanth Hegde wrote:
> Idle load balancer is kicked off only after time check. So move
> the atomic read after the time check to avoid the overhead.

Presumably this is because the nr_cpus field is more contended than the
next_balance field; but aren't they in the same cacheline? That is,
*what* !??!

> If there are no nohz CPUs and next_blocked has passed, then there
> will be one additional stats based load balancing which would set the
> has_blocked_load to 0. It shouldn't make a difference.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com> 
> ---
>  kernel/sched/fair.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 7377f9117501..cd1c78d2c272 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -12447,13 +12447,6 @@ static void nohz_balancer_kick(struct rq *rq)
>  	 */
>  	nohz_balance_exit_idle(rq);
>  
> -	/*
> -	 * None are in tickless mode and hence no need for NOHZ idle load
> -	 * balancing:
> -	 */
> -	if (likely(!atomic_read(&nohz.nr_cpus)))
> -		return;
> -
>  	if (READ_ONCE(nohz.has_blocked_load) &&
>  	    time_after(now, READ_ONCE(nohz.next_blocked)))
>  		flags = NOHZ_STATS_KICK;
> @@ -12461,6 +12454,13 @@ static void nohz_balancer_kick(struct rq *rq)
>  	if (time_before(now, nohz.next_balance))
>  		goto out;
>  
> +	/*
> +	 * None are in tickless mode and hence no need for NOHZ idle load
> +	 * balancing:
> +	 */
> +	if (likely(!atomic_read(&nohz.nr_cpus)))
> +		return;
> +
>  	if (rq->nr_running >= 2) {
>  		flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
>  		goto out;
> -- 
> 2.47.3
> 

Re: [PATCH v2 1/3] sched/fair: Move checking for nohz cpus after time check

[Shrikanth Hegde]

Hi Peter.

On 1/6/26 9:20 PM, Peter Zijlstra wrote:
> On Fri, Jan 02, 2026 at 06:17:42PM +0530, Shrikanth Hegde wrote:
>> Idle load balancer is kicked off only after time check. So move
>> the atomic read after the time check to avoid the overhead.
> 
> Presumably this is because the nr_cpus field is more contended than the
> next_balance field; but aren't they in the same cacheline? That is,
> *what* !??!

Main reason for cacheline contention is nr_cpus in nohz struct.
Other one is idle_cpus_mask, but that is in different
cacheline itself(most of the time)

This patch is not due to next_balance being contended. It is
simply to check the value when we are sure time has passed to
attempt NOHZ balance again.

> 
>> If there are no nohz CPUs and next_blocked has passed, then there
>> will be one additional stats based load balancing which would set the
>> has_blocked_load to 0. It shouldn't make a difference.
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>>   kernel/sched/fair.c | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 7377f9117501..cd1c78d2c272 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -12447,13 +12447,6 @@ static void nohz_balancer_kick(struct rq *rq)
>>   	 */
>>   	nohz_balance_exit_idle(rq);
>>   
>> -	/*
>> -	 * None are in tickless mode and hence no need for NOHZ idle load
>> -	 * balancing:
>> -	 */
>> -	if (likely(!atomic_read(&nohz.nr_cpus)))
>> -		return;
>> -
>>   	if (READ_ONCE(nohz.has_blocked_load) &&
>>   	    time_after(now, READ_ONCE(nohz.next_blocked)))
>>   		flags = NOHZ_STATS_KICK;
>> @@ -12461,6 +12454,13 @@ static void nohz_balancer_kick(struct rq *rq)
>>   	if (time_before(now, nohz.next_balance))
>>   		goto out;
>>   
>> +	/*
>> +	 * None are in tickless mode and hence no need for NOHZ idle load
>> +	 * balancing:
>> +	 */
>> +	if (likely(!atomic_read(&nohz.nr_cpus)))
>> +		return;
>> +
>>   	if (rq->nr_running >= 2) {
>>   		flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
>>   		goto out;
>> -- 
>> 2.47.3
>>

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

[Shrikanth Hegde]

These days most of the system have multi cores. The likelyhood of
at least one or more CPUs in nohz (idle state) is higher.
So move likely to unlikely.

Allow stats balancing to complete when there are no nr_cpus as the check
happens later. This may do an additional stats based load balancing
which would reset has_blocked_load. Code also looks saner by removing
that uncharactiristic return in between.

Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 kernel/sched/fair.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index cd1c78d2c272..5ceb9126d441 100644
--- 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;
 
 	if (rq->nr_running >= 2) {
 		flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;
-- 
2.47.3

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/2/2026 6:17 PM, Shrikanth Hegde wrote:
> These days most of the system have multi cores. The likelyhood of
> at least one or more CPUs in nohz (idle state) is higher.
> So move likely to unlikely.
> 
> Allow stats balancing to complete when there are no nr_cpus as the check
> happens later. This may do an additional stats based load balancing
> which would reset has_blocked_load. Code also looks saner by removing
> that uncharactiristic return in between.
> 
> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
> ---
>  kernel/sched/fair.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index cd1c78d2c272..5ceb9126d441 100644
> --- 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.nohz_balancer_kick)))
> +		goto out;

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.

>  
>  	if (rq->nr_running >= 2) {
>  		flags = NOHZ_STATS_KICK | NOHZ_BALANCE_KICK;

-- 
Thanks and Regards,
Prateek

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

[Shrikanth Hegde]

On 1/5/26 9:22 AM, K Prateek Nayak wrote:
> Hello Shrikanth,
> 
> On 1/2/2026 6:17 PM, Shrikanth Hegde wrote:
>> These days most of the system have multi cores. The likelyhood of
>> at least one or more CPUs in nohz (idle state) is higher.
>> So move likely to unlikely.
>>
>> Allow stats balancing to complete when there are no nr_cpus as the check
>> happens later. This may do an additional stats based load balancing
>> which would reset has_blocked_load. Code also looks saner by removing
>> that uncharactiristic return in between.
>>
>> Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
>> ---
>>   kernel/sched/fair.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index cd1c78d2c272..5ceb9126d441 100644
>> --- 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.nohz_balancer_kick)))
>> +		goto out;

Did something got edited here?

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

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

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

[Shrikanth Hegde]

Hi Prateek,

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

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);
+
         return -1;
  }

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

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

[Shrikanth Hegde]

On 1/6/26 8:33 AM, K Prateek Nayak wrote:
> 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
>> +        */

cpumask_empty check.

>> +       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.
> 

Yes. I even thought to put cpumask_empty check here and then do these updates.
But since there might be a window where a remote CPU may be going idle,
might race with it and possibly lose a chance for idle balance.

Since it is extreme case of 100% busy, all these checks may not be necessary.
Will put a return instead :)

Thanks for pointing that out.

> 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

That reminds me, need to upgrade this to smp_mb now, given atomic is gone.

> 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;
>>   }
>>
>>
> 

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/6/2026 9:53 AM, Shrikanth Hegde wrote:
>> There are also nuances like the smp_mb__after_atomic() in
> 
> That reminds me, need to upgrade this to smp_mb now, given atomic is gone.

Isn't cpumask_set_cpu() also an atomic op?

set_bit() has a comment stating it is a "relaxed atomic operation" in
asm-generic/bitops/instrumented-atomic.h which makes it similar to a
atomic_inc().
-- 
Thanks and Regards,
Prateek

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

[Shrikanth Hegde]

On 1/6/26 11:20 AM, K Prateek Nayak wrote:
> Hello Shrikanth,
> 
> On 1/6/2026 9:53 AM, Shrikanth Hegde wrote:
>>> There are also nuances like the smp_mb__after_atomic() in
>>
>> That reminds me, need to upgrade this to smp_mb now, given atomic is gone.
> 
> Isn't cpumask_set_cpu() also an atomic op?
> 
> set_bit() has a comment stating it is a "relaxed atomic operation" in
> asm-generic/bitops/instrumented-atomic.h which makes it similar to a
> atomic_inc().

"
  * set_bit - Atomically set a bit in memory
  * @nr: the bit to set
  * @addr: the address to start counting from
  *
  * This is a relaxed atomic operation (no implied memory barriers).
"

I believed the comment. If it was relaxed atomic operation then
one would need a smp barrier.

But it calls arch_set_bit.
Looking at x86 implementation in arch/x86/include/asm/bitops.h it
looks like it does full smp_mb. Whereas on powerpc it doesn't.
So smp_mb__after_atomic can remain as is.


Effective change to v3 is return instead of out.

[PATCH v2 3/3] sched/fair: Remove nohz.nr_cpus and use weight of cpumask instead

[Shrikanth Hegde]

nohz.nr_cpus was observed as contended cacheline when running
enterprise workload on large systems.

Fundamental scalability challenge with nohz.idle_cpus_mask
and nohz.nr_cpus is the following:

 (1) nohz_balancer_kick() observes (reads) nohz.nr_cpus 
     (or nohz.idle_cpu_mask) and nohz.has_blocked to  see whether there's
     any nohz balancing work to do, in every scheduler tick.

 (2) nohz_balance_enter_idle() and nohz_balance_exit_idle()
     (through nohz_balancer_kick() via sched_tick()) modify (write)
     nohz.nr_cpus (and/or nohz.idle_cpu_mask) and nohz.has_blocked.

The characteristic frequencies are the following:

 (1) nohz_balancer_kick() happens at scheduler (busy)tick frequency
     on CPU(which has not gone idle). This is a relatively constant
     frequency  in the ~1 kHz range or lower.

 (2) happens at idle enter/exit frequency on every CPU that goes to idle.
     This is workload dependent, but can easily be hundreds of kHz for
     IO-bound loads and high CPU counts. Ie. can be orders of magnitude
     higher than (1), in which case a cachemiss at every invocation of (1)
     is almost inevitable. idle exit will trigger (1) on the CPU
     which is coming out of idle.

There's two types of costs from these functions:

 (A) scheduler tick cost via (1): this happens on busy CPUs too, and is
     thus a primary scalability cost. But the rate here is constant and
     typically much lower than (B), hence the absolute benefit to workload
     scalability will be lower as well.

 (B) idle cost via (2): going-to-idle and coming-from-idle costs are
     secondary concerns, because they impact power efficiency more than
     they impact scalability. But in terms of absolute cost this scales
     up with nr_cpus as well, and a much faster rate, and thus may also
     approach and negatively impact system limits like
     memory bus/fabric bandwidth.

Above mentioned fundamental scalability challenge remains true for
nohz.idle_cpus_mask even after this patch. But nr_cpus can be derived from
the mask itself. Its usage doesn't warrant a functionally correct value.
It can race, at worst an additional load balance may be attempted.

So, derive the value from the idle_cpus_mask. This helps to
save some bus bandwidth w.r.t to that nohz cacheline(approx 50%).
This in turn helps to improve enterprise workload throughput.

This theory holds true for CPUMASK_OFFSTACK=y and mostly true for
CPUMASK_OFFSTACK=n (last few bits based on NR_CPUs could be in same 
cacheline as nr_cpus)

On system with 480 CPUs, running hackbench 40 process 10000 loops
(Avg of 3 runs)
baseline:
     0.81%  hackbench          [k] nohz_balance_exit_idle
     0.21%  hackbench          [k] nohz_balancer_kick
     0.09%  swapper            [k] nohz_run_idle_balance

With patch:
     0.35%  hackbench          [k] nohz_balance_exit_idle
     0.09%  hackbench          [k] nohz_balancer_kick
     0.07%  swapper            [k] nohz_run_idle_balance

[Ingo Molnar: scalability analysis changlog]
Signed-off-by: Shrikanth Hegde <sshegde@linux.ibm.com>
---
 kernel/sched/fair.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 5ceb9126d441..805b53d9709e 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7141,7 +7141,6 @@ static DEFINE_PER_CPU(cpumask_var_t, should_we_balance_tmpmask);
 
 static struct {
 	cpumask_var_t idle_cpus_mask;
-	atomic_t nr_cpus;
 	int has_blocked_load;		/* Idle CPUS has blocked load */
 	int needs_update;		/* Newly idle CPUs need their next_balance collated */
 	unsigned long next_balance;     /* in jiffy units */
@@ -12458,7 +12457,7 @@ static void nohz_balancer_kick(struct rq *rq)
 	 * None are in tickless mode and hence no need for NOHZ idle load
 	 * balancing, do stats update if its due
 	 */
-	if (unlikely(!atomic_read(&nohz.nr_cpus)))
+	if (unlikely(cpumask_empty(nohz.idle_cpus_mask)))
 		goto out;
 
 	if (rq->nr_running >= 2) {
@@ -12571,7 +12570,6 @@ void nohz_balance_exit_idle(struct rq *rq)
 
 	rq->nohz_tick_stopped = 0;
 	cpumask_clear_cpu(rq->cpu, nohz.idle_cpus_mask);
-	atomic_dec(&nohz.nr_cpus);
 
 	set_cpu_sd_state_busy(rq->cpu);
 }
@@ -12629,7 +12627,6 @@ void nohz_balance_enter_idle(int cpu)
 	rq->nohz_tick_stopped = 1;
 
 	cpumask_set_cpu(cpu, nohz.idle_cpus_mask);
-	atomic_inc(&nohz.nr_cpus);
 
 	/*
 	 * Ensures that if nohz_idle_balance() fails to observe our
-- 
2.47.3