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 <[email protected]>
---
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;
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 <[email protected]>
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;
>
* Vaidyanathan Srinivasan <[email protected]> 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
* Ingo Molnar <[email protected]> [2008-12-08 16:49:39]:
>
> * Vaidyanathan Srinivasan <[email protected]> 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
* Vaidyanathan Srinivasan <[email protected]> wrote:
> * Ingo Molnar <[email protected]> [2008-12-08 16:49:39]:
>
> >
> > * Vaidyanathan Srinivasan <[email protected]> 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
* Ingo Molnar <[email protected]> [2008-12-09 07:34:48]:
>
> * Vaidyanathan Srinivasan <[email protected]> wrote:
>
> > * Ingo Molnar <[email protected]> [2008-12-08 16:49:39]:
> >
> > >
> > > * Vaidyanathan Srinivasan <[email protected]> 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