On 07/05/2018 03:24 PM, Matt Fleming wrote:
On Thu, 05 Jul, at 11:52:21AM, Dietmar Eggemann wrote:
Moving the code from _nohz_idle_balance to nohz_idle_balance let it disappear:
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 02be51c9dcc1..070924f07c68 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9596,16 +9596,6 @@ static bool _nohz_idle_balance(struct rq *this_rq,
unsigned int flags,
*/
smp_mb();
- /*
- * Ensure this_rq's clock and load are up-to-date before we
- * rebalance since it's possible that they haven't been
- * updated for multiple schedule periods, i.e. many seconds.
- */
- raw_spin_lock_irq(&this_rq->lock);
- update_rq_clock(this_rq);
- cpu_load_update_idle(this_rq);
- raw_spin_unlock_irq(&this_rq->lock);
-
for_each_cpu(balance_cpu, nohz.idle_cpus_mask) {
if (balance_cpu == this_cpu || !idle_cpu(balance_cpu))
continue;
@@ -9701,6 +9691,16 @@ static bool nohz_idle_balance(struct rq *this_rq, enum
cpu_idle_type idle)
if (!(flags & NOHZ_KICK_MASK))
return false;
+ /*
+ * Ensure this_rq's clock and load are up-to-date before we
+ * rebalance since it's possible that they haven't been
+ * updated for multiple schedule periods, i.e. many seconds.
+ */
+ raw_spin_lock_irq(&this_rq->lock);
+ update_rq_clock(this_rq);
+ cpu_load_update_idle(this_rq);
+ raw_spin_unlock_irq(&this_rq->lock);
+
_nohz_idle_balance(this_rq, flags, idle);
return true;
Hmm.. it still looks to me like we should be saving and restoring IRQs
since this can be called from IRQ context, no?
You mean in nohz_idle_balance()? Yes.
I just wanted to hint that since you need this cpu_load_update_idle() in
the nohz idle balance case and not in idle balance, you could just
update it in nohz_idle_balance() rather than in _nohz_idle_balance()
(which is also called in nohz_newidle_balance()) avoiding this
inconsistent lock state warning.
[...]
