Re: [PATCH v1] sched: fix nohz idle load balancer issues

[Suresh Siddha <suresh.b.siddha@intel.com>]

On Tue, 2011-09-27 at 21:15 -0700, Srivatsa Vaddagiri wrote:
> * Suresh Siddha <suresh.b.siddha@intel.com> [2011-09-27 16:49:36]:
> 
> > I will post this patch mostly tomorrow. That patch will not use the
> > idle_at_tick check in the nohz_idle_balance(). So that should address
> > your issue in some cases if not most.
> 
> Ok ..would be glad to test your change ..

Vatsa, Here is the promised patch. I haven't completely made up my mind
about this patch.  Specially the change that I am doing now in
scheduler_ipi().

Initially I was planning to use resched_cpu() to trigger the
nohz_idle_balance() as part of the idle_balance(). But preemption is
disabled  during idle_balance() and I don't want to do more work in
idle_balance. Thomas/PeterZ may not like that. And also the idle cpu
would have done tick_nohz_restart_sched_tick() unnecessarily etc.

So ended up with using kick_process() and the scheduler_ipi() context to
trigger the SCHED_SOFTIRQ instead of using smp call function vector
sequence (which has a deadlock scenario in the context of heavy
interrupts which I can explain in detail when I send the complete
changelog). And also I am explicitly requesting for idle balance to
address the stale idle_at_tick condition.

Anyways, this appended patch is just for your quick experiment. I will
think a bit more tomorrow morning before sending the patch with complete
changelog. Thanks.
---

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---
 kernel/sched.c      |   10 +++++++---
 kernel/sched_fair.c |   25 +++----------------------
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index c5cf15e..482b9d2 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -644,7 +644,7 @@ struct rq {
 
 	unsigned long cpu_power;
 
-	unsigned char idle_at_tick;
+	unsigned char idle_balance;
 	/* For active balancing */
 	int post_schedule;
 	int active_balance;
@@ -2733,6 +2733,11 @@ void scheduler_ipi(void)
 	struct rq *rq = this_rq();
 	struct task_struct *list = xchg(&rq->wake_list, NULL);
 
+	if (unlikely((rq->idle == current) && rq->nohz_balance_kick)) {
+		rq->idle_balance = 1;
+		raise_softirq_irqoff(SCHED_SOFTIRQ);
+	}
+
 	if (!list)
 		return;
 
@@ -4257,7 +4262,7 @@ void scheduler_tick(void)
 	perf_event_task_tick();
 
 #ifdef CONFIG_SMP
-	rq->idle_at_tick = idle_cpu(cpu);
+	rq->idle_balance = idle_cpu(cpu);
 	trigger_load_balance(rq, cpu);
 #endif
 }
@@ -8303,7 +8308,6 @@ void __init sched_init(void)
 		rq_attach_root(rq, &def_root_domain);
 #ifdef CONFIG_NO_HZ
 		rq->nohz_balance_kick = 0;
-		init_sched_softirq_csd(&per_cpu(remote_sched_softirq_cb, i));
 #endif
 #endif
 		init_rq_hrtick(rq);
diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index fef0bfd..bcefcb8 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -4269,22 +4269,6 @@ out_unlock:
 }
 
 #ifdef CONFIG_NO_HZ
-
-static DEFINE_PER_CPU(struct call_single_data, remote_sched_softirq_cb);
-
-static void trigger_sched_softirq(void *data)
-{
-	raise_softirq_irqoff(SCHED_SOFTIRQ);
-}
-
-static inline void init_sched_softirq_csd(struct call_single_data *csd)
-{
-	csd->func = trigger_sched_softirq;
-	csd->info = NULL;
-	csd->flags = 0;
-	csd->priv = 0;
-}
-
 /*
  * idle load balancing details
  * - One of the idle CPUs nominates itself as idle load_balancer, while
@@ -4450,11 +4434,8 @@ static void nohz_balancer_kick(int cpu)
 	}
 
 	if (!cpu_rq(ilb_cpu)->nohz_balance_kick) {
-		struct call_single_data *cp;
-
 		cpu_rq(ilb_cpu)->nohz_balance_kick = 1;
-		cp = &per_cpu(remote_sched_softirq_cb, cpu);
-		__smp_call_function_single(ilb_cpu, cp, 0);
+		kick_process(idle_task(ilb_cpu));
 	}
 	return;
 }
@@ -4687,7 +4668,7 @@ static inline int nohz_kick_needed(struct rq *rq, int cpu)
 	if (time_before(now, nohz.next_balance))
 		return 0;
 
-	if (rq->idle_at_tick)
+	if (idle_cpu(cpu))
 		return 0;
 
 	first_pick_cpu = atomic_read(&nohz.first_pick_cpu);
@@ -4723,7 +4704,7 @@ static void run_rebalance_domains(struct softirq_action *h)
 {
 	int this_cpu = smp_processor_id();
 	struct rq *this_rq = cpu_rq(this_cpu);
-	enum cpu_idle_type idle = this_rq->idle_at_tick ?
+	enum cpu_idle_type idle = this_rq->idle_balance ?
 						CPU_IDLE : CPU_NOT_IDLE;
 
 	rebalance_domains(this_cpu, idle);