[BUG] idle_balance() does not call load_balance_newidle()

[BUG] idle_balance() does not call load_balance_newidle()

[Vaidyanathan Srinivasan]

Hi,

load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is
set at higher level domain (3-CPU) and not in low level domain (2-MC).

pulled_task is initialised to -1 and checked for non-zero which is
always true if the lowest level sched_domain does not have
SD_BALANCE_NEWIDLE flag set.

Trivial fix to initialise pulled_task to zero.
Patch against 2.6.28-rc7

Thanks,
Vaidy

Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
---

 kernel/sched.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/kernel/sched.c b/kernel/sched.c
index f79c7c4..4abdce1 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -3801,7 +3801,7 @@ out_balanced:
 static void idle_balance(int this_cpu, struct rq *this_rq)
 {
 	struct sched_domain *sd;
-	int pulled_task = -1;
+	int pulled_task = 0;
 	unsigned long next_balance = jiffies + HZ;
 	cpumask_t tmpmask;
 

Re: [BUG] idle_balance() does not call load_balance_newidle()

[Peter Zijlstra]

On Mon, 2008-12-08 at 20:52 +0530, Vaidyanathan Srinivasan wrote:
> Hi,
> 
> load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is
> set at higher level domain (3-CPU) and not in low level domain (2-MC).
> 
> pulled_task is initialised to -1 and checked for non-zero which is
> always true if the lowest level sched_domain does not have
> SD_BALANCE_NEWIDLE flag set.
> 
> Trivial fix to initialise pulled_task to zero.
> Patch against 2.6.28-rc7
> 
> Thanks,
> Vaidy
> 
> Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>

Seems sane

> ---
> 
>  kernel/sched.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index f79c7c4..4abdce1 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -3801,7 +3801,7 @@ out_balanced:
>  static void idle_balance(int this_cpu, struct rq *this_rq)
>  {
>  	struct sched_domain *sd;
> -	int pulled_task = -1;
> +	int pulled_task = 0;
>  	unsigned long next_balance = jiffies + HZ;
>  	cpumask_t tmpmask;
>  

Re: [BUG] idle_balance() does not call load_balance_newidle()

[Ingo Molnar]

* Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:

> Hi,
> 
> load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is
> set at higher level domain (3-CPU) and not in low level domain (2-MC).
> 
> pulled_task is initialised to -1 and checked for non-zero which is
> always true if the lowest level sched_domain does not have
> SD_BALANCE_NEWIDLE flag set.
> 
> Trivial fix to initialise pulled_task to zero.
> Patch against 2.6.28-rc7

applied to tip/sched/core, thanks! (Not for v2.6.28 because this could 
affect performance.)

	Ingo

Re: [BUG] idle_balance() does not call load_balance_newidle()

[Vaidyanathan Srinivasan]

* Ingo Molnar <mingo@elte.hu> [2008-12-08 16:49:39]:

> 
> * Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:
> 
> > Hi,
> > 
> > load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is
> > set at higher level domain (3-CPU) and not in low level domain (2-MC).
> > 
> > pulled_task is initialised to -1 and checked for non-zero which is
> > always true if the lowest level sched_domain does not have
> > SD_BALANCE_NEWIDLE flag set.
> > 
> > Trivial fix to initialise pulled_task to zero.
> > Patch against 2.6.28-rc7
> 
> applied to tip/sched/core, thanks! (Not for v2.6.28 because this could 
> affect performance.)

Thanks Ingo.  This patch does not change any functionality in v2.6.28
and hence will not affect performance.  The SD flags are not touched.
I found this bug while setting different SD flags at MC level and CPU
level in my power saving balance patches.

--Vaidy

Re: [BUG] idle_balance() does not call load_balance_newidle()

[Ingo Molnar]

* Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:

> * Ingo Molnar <mingo@elte.hu> [2008-12-08 16:49:39]:
> 
> > 
> > * Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:
> > 
> > > Hi,
> > > 
> > > load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is
> > > set at higher level domain (3-CPU) and not in low level domain (2-MC).
> > > 
> > > pulled_task is initialised to -1 and checked for non-zero which is
> > > always true if the lowest level sched_domain does not have
> > > SD_BALANCE_NEWIDLE flag set.
> > > 
> > > Trivial fix to initialise pulled_task to zero.
> > > Patch against 2.6.28-rc7
> > 
> > applied to tip/sched/core, thanks! (Not for v2.6.28 because this could 
> > affect performance.)
> 
> Thanks Ingo.  This patch does not change any functionality in v2.6.28 
> and hence will not affect performance.  The SD flags are not touched. I 
> found this bug while setting different SD flags at MC level and CPU 
> level in my power saving balance patches.

if it does not change any functionality then we would not be doing the 
change, right?

It does change functionality, because:

> > > load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is 
> > > set at higher level domain (3-CPU) and not in low level domain 
> > > (2-MC).

even though it's a bug fix, it affects how the SD flags are interpreted 
and acted upon by the load balancer - i.e. the change can impact 
performance.

	Ingo

Re: [BUG] idle_balance() does not call load_balance_newidle()

[Vaidyanathan Srinivasan]

* Ingo Molnar <mingo@elte.hu> [2008-12-09 07:34:48]:

> 
> * Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:
> 
> > * Ingo Molnar <mingo@elte.hu> [2008-12-08 16:49:39]:
> > 
> > > 
> > > * Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> wrote:
> > > 
> > > > Hi,
> > > > 
> > > > load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is
> > > > set at higher level domain (3-CPU) and not in low level domain (2-MC).
> > > > 
> > > > pulled_task is initialised to -1 and checked for non-zero which is
> > > > always true if the lowest level sched_domain does not have
> > > > SD_BALANCE_NEWIDLE flag set.
> > > > 
> > > > Trivial fix to initialise pulled_task to zero.
> > > > Patch against 2.6.28-rc7
> > > 
> > > applied to tip/sched/core, thanks! (Not for v2.6.28 because this could 
> > > affect performance.)
> > 
> > Thanks Ingo.  This patch does not change any functionality in v2.6.28 
> > and hence will not affect performance.  The SD flags are not touched. I 
> > found this bug while setting different SD flags at MC level and CPU 
> > level in my power saving balance patches.
> 
> if it does not change any functionality then we would not be doing the 
> change, right?
 
 :) 
  
> It does change functionality, because:
> 
> > > > load_balance_newidle() does not get called if SD_BALANCE_NEWIDLE is 
> > > > set at higher level domain (3-CPU) and not in low level domain 
> > > > (2-MC).
> 
> even though it's a bug fix, it affects how the SD flags are interpreted 
> and acted upon by the load balancer - i.e. the change can impact 
> performance.

Agreed.  In my test setup with two levels of sched_domains,
SD_BALANCE_NEWIDLE has not been set in both CPU level and MC level
starting from 2.6.28-rc4.  In a setup with more levels of
sched_domains and if the higher levels have SD_BALANCE_NEWIDLE set,
then we will have a functional impact.  

You are right in queueing it for 2.6.29.

Thanks for the clarification.

--Vaidy