[PATCH 0/2] tick/nohz: CPU cannot enter NOHZ idle balance state

[PATCH 0/2] tick/nohz: CPU cannot enter NOHZ idle balance state

[Adam Li]

A few CPUs stay in idle while others are 100% busy when running llama
on an arm64 server. CONFIG_NO_HZ_FULL is set and all CPUs are in
the nohz_full list.

We can see that the idle CPUs are not in nohz.idle_cpus_mask. The NOHZ
idle load balancing only considers CPUs in nohz.idle_cpus_mask. The ticks
on the idle CPUs are stopped and therefore period load balancing
is not triggered. Therefore the CPUs are not used and the
imbalance persists.

A CPU is added to nohz.idle_cpus_mask in:
do_idle()
   -> tick_nohz_idle_stop_tick()
      -> nohz_balance_enter_idle()

nohz_balance_enter_idle() depends on '!was_stopped' condition.
It looks 'was_stopped' is used to avoid duplicated calling
nohz_balance_enter_idle() and duplicated setting 'ts->idle_jiffies'.

When the CPU is in nohz_full mode, 'was_stopped' may alwasy be true.
The call path might be:

tick_nohz_full_stop_tick() /* stop tick and set TS_FLAG_STOPPED */
... ...
do_idle()
    -> tick_nohz_idle_stop_tick() /* was_stoppped == 1 */

The first patch "Fix wrong NOHZ idle CPU state" makes
nohz_balance_enter_idle() independent of '!was_stopped'. It is safe
since in nohz_balance_enter_idle(), there exists a condition check
'rq->nohz_tick_stopped' to avoid duplicated nohz.idle_cpus_mask setting.

The second patch "Trigger warning when CPU in wrong NOHZ idle state"
is for debug only. It is not intended to be merged. The patch can help
to reproduce the bug.

Warning is triggerred when CPU is in this 'wrong' state:
1) tick was already stopped before tick_nohz_idle_stop_tick()
   stops the tick
2) and CPU is not in nohz.idle_cpus_mask
3) and CPU is idle
4) and tick is stopped

When kernel booting on my system there is warning:
[   15.536604] WARNING: CPU: 1 PID: 0 at kernel/time/tick-sched.c:1230 tick_nohz_idle_stop_tick+0x148/0x160
[   15.550687] Modules linked in:
[   15.553731] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 6.17.0-rc1-cls-00002-g39cde4c0206e-dirty #109 VOLUNTARY
[   15.580390] pstate: 614000c9 (nZCv daIF +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
<snip>
[   15.703028] Call trace:
[   15.705462]  tick_nohz_idle_stop_tick+0x148/0x160 (P)
[   15.710502]  cpuidle_idle_call+0x118/0x1d0
[   15.714588]  do_idle+0xf4/0x100
[   15.717717]  cpu_startup_entry+0x40/0x50
[   15.721627]  secondary_start_kernel+0xe4/0x128
[   15.732745]  __secondary_switched+0xc0/0xc8

After the first patch, CPU is added to nohz.idle_cpus_mask.
NOHZ idle balancing can move task to this CPU.

Adam Li (2):
  tick/nohz: Fix wrong NOHZ idle CPU state
  tick/nohz: Trigger warning when CPU in wrong NOHZ idle state

 include/linux/sched/nohz.h | 2 ++
 kernel/sched/fair.c        | 5 +++++
 kernel/time/tick-sched.c   | 8 +++++---
 3 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/2] tick/nohz: Fix wrong NOHZ idle CPU state

[Adam Li]

NOHZ idle load balance is done among CPUs in nohz.idle_cpus_mask.
A CPU is added to nohz.idle_cpus_mask in:
do_idle()
  -> tick_nohz_idle_stop_tick()
     -> nohz_balance_enter_idle()

nohz_balance_enter_idle() is called if:
1) tick is stopped (TS_FLAG_STOPPED is set)
2) and tick was not already stopped before tick_nohz_idle_stop_tick()
   stops the tick (!was_stopped)

When CONFIG_NO_HZ_FULL is set and the CPU is in the nohz_full list
then 'was_stopped' may always be true.
The flag 'TS_FLAG_STOPPED' may be already set in
tick_nohz_full_stop_tick(). So nohz_balance_enter_idle() has no chance
to be called.

As a result, CPU will stay in a 'wrong' state:
1) tick is stopped (TS_FLAG_STOPPED is set)
2) and CPU is not in nohz.idle_cpus_mask
3) and CPU stays idle

Neither the periodic nor the NOHZ idle load balancing can move task
to this CPU. Some CPUs keep idle while others busy.

In nohz_balance_enter_idle(), 'rq->nohz_tick_stopped' is checked to avoid
duplicated nohz.idle_cpus_mask setting. So for nohz_balance_enter_idle()
there is no need to check the '!was_stopped' condition.

This patch will add the CPU to nohz.idle_cpus_mask as expected.

Signed-off-by: Adam Li <adamli@os.amperecomputing.com>
Reviewed-by: Christoph Lameter (Ampere) <cl@gentwo.org>
---
 kernel/time/tick-sched.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index c527b421c865..b900a120ab54 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1229,8 +1229,9 @@ void tick_nohz_idle_stop_tick(void)
 		ts->idle_sleeps++;
 		ts->idle_expires = expires;
 
-		if (!was_stopped && tick_sched_flag_test(ts, TS_FLAG_STOPPED)) {
-			ts->idle_jiffies = ts->last_jiffies;
+		if (tick_sched_flag_test(ts, TS_FLAG_STOPPED)) {
+			if (!was_stopped)
+				ts->idle_jiffies = ts->last_jiffies;
 			nohz_balance_enter_idle(cpu);
 		}
 	} else {
-- 
2.34.1

[PATCH 2/2] tick/nohz: Trigger warning when CPU in wrong NOHZ idle state

[Adam Li]

This patch is for debug only.

Warning is triggerred when CPU is in this state:
1) tick was already stopped before tick_nohz_idle_stop_tick()
   stops the tick
2) and CPU is not in nohz.idle_cpus_mask
3) and CPU is idle
4) and tick is stopped

CPU will stay idle in this state, since neither the periodic nor
the NOHZ idle load balancing can move task to this CPU.

Signed-off-by: Adam Li <adamli@os.amperecomputing.com>
---
 include/linux/sched/nohz.h | 2 ++
 kernel/sched/fair.c        | 5 +++++
 kernel/time/tick-sched.c   | 3 ++-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched/nohz.h b/include/linux/sched/nohz.h
index 0db7f67935fe..ea6e07777395 100644
--- a/include/linux/sched/nohz.h
+++ b/include/linux/sched/nohz.h
@@ -9,8 +9,10 @@
 #ifdef CONFIG_NO_HZ_COMMON
 extern void nohz_balance_enter_idle(int cpu);
 extern int get_nohz_timer_target(void);
+extern bool nohz_balance_idle_cpu(int cpu);
 #else
 static inline void nohz_balance_enter_idle(int cpu) { }
+static inline bool nohz_balance_idle_cpu(int cpu) { return false; }
 #endif
 
 #ifdef CONFIG_NO_HZ_COMMON
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b173a059315c..cd1c17368e05 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7109,6 +7109,11 @@ static struct {
 	unsigned long next_blocked;	/* Next update of blocked load in jiffies */
 } nohz ____cacheline_aligned;
 
+inline bool nohz_balance_idle_cpu(int cpu)
+{
+	return cpumask_test_cpu(cpu, nohz.idle_cpus_mask);
+}
+
 #endif /* CONFIG_NO_HZ_COMMON */
 
 static unsigned long cpu_load(struct rq *rq)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index b900a120ab54..8241b14842f3 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -1228,7 +1228,8 @@ void tick_nohz_idle_stop_tick(void)
 
 		ts->idle_sleeps++;
 		ts->idle_expires = expires;
-
+		WARN_ON_ONCE(was_stopped && !nohz_balance_idle_cpu(cpu) &&
+			idle_cpu(cpu) && tick_nohz_tick_stopped_cpu(cpu));
 		if (tick_sched_flag_test(ts, TS_FLAG_STOPPED)) {
 			if (!was_stopped)
 				ts->idle_jiffies = ts->last_jiffies;
-- 
2.34.1