Date:   Sun, 3 May 2020 16:34:07 +0800
From:   Peng Liu <iwtbavbm@...il.com>
To:     linux-kernel@...r.kernel.org
Cc:     mingo@...hat.com, peterz@...radead.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
        valentin.schneider@....com, iwtbavbm@...il.com
Subject: [PATCH] sched/fair: Fix nohz.next_balance update

commit c5afb6a87f23 ("sched/fair: Fix nohz.next_balance update")
During idle load balance, this_cpu(ilb) do load balance for the other
idle CPUs, also gather the earliest (nohz.)next_balance.

Since commit:
  'b7031a02ec75 ("sched/fair: Add NOHZ_STATS_KICK")'

We update nohz.next_balance like this:

  _nohz_idle_balance() {
      for_each_cpu(nohz.idle_cpus_mask) {
    	  rebalance_domains() {
      	      update nohz.next_balance <-- compare and update
    	  }
      }
      rebalance_domains(this_cpu) {
	  update nohz.next_balance <-- compare and update
      }
      update nohz.next_balance <-- unconditionally update
  }

For instance, nohz.idle_cpus_mask spans {cpu2,3,5,8}, and this_cpu is
cpu5. After the above loop we could gather the earliest *next_balance*
among {cpu2,3,8}, then rebalance_domains(this_cpu) update
nohz.next_balance with this_rq->next_balance, but finally overwrite
nohz.next_balance with the earliest *next_balance* among {cpu2,3,8},
we may end up with not getting the earliest next_balance.

Since we can gather all the updated rq->next_balance, including this_cpu,
in _nohz_idle_balance(), it's safe to remove the extra lines in
rebalance_domains() which are originally intended for this_cpu. And
finally the updating only happen in _nohz_idle_balance().

Signed-off-by: Peng Liu <iwtbavbm@...il.com>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Juri Lelli <juri.lelli@...hat.com>
Cc: Vincent Guittot <vincent.guittot@...aro.org>
Cc: Dietmar Eggemann <dietmar.eggemann@....com>
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Ben Segall <bsegall@...gle.com>
Cc: Mel Gorman <mgorman@...e.de>
Cc: Valentin Schneider <valentin.schneider@....com>
---
 kernel/sched/fair.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 02f323b85b6d..1d0cf33fefad 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9943,22 +9943,8 @@ static void rebalance_domains(struct rq *rq, enum cpu_idle_type idle)
 	 * When the cpu is attached to null domain for ex, it will not be
 	 * updated.
 	 */
-	if (likely(update_next_balance)) {
+	if (likely(update_next_balance))
 		rq->next_balance = next_balance;
-
-#ifdef CONFIG_NO_HZ_COMMON
-		/*
-		 * If this CPU has been elected to perform the nohz idle
-		 * balance. Other idle CPUs have already rebalanced with
-		 * nohz_idle_balance() and nohz.next_balance has been
-		 * updated accordingly. This CPU is now running the idle load
-		 * balance for itself and we need to update the
-		 * nohz.next_balance accordingly.
-		 */
-		if ((idle == CPU_IDLE) && time_after(nohz.next_balance, rq->next_balance))
-			nohz.next_balance = rq->next_balance;
-#endif
-	}
 }
 
 static inline int on_null_domain(struct rq *rq)
@@ -10321,9 +10307,15 @@ static bool _nohz_idle_balance(struct rq *this_rq, unsigned int flags,
 		has_blocked_load |= this_rq->has_blocked_load;
 	}
 
-	if (flags & NOHZ_BALANCE_KICK)
+	if (flags & NOHZ_BALANCE_KICK) {
 		rebalance_domains(this_rq, CPU_IDLE);
 
+		if (time_after(next_balance, this_rq->next_balance)) {
+			next_balance = this_rq->next_balance;
+			update_next_balance = 1;
+		}
+	}
+
 	WRITE_ONCE(nohz.next_blocked,
 		now + msecs_to_jiffies(LOAD_AVG_PERIOD));
 
-- 
2.17.1

Powered by blists - more mailing lists