All of lore.kernel.org
 help / color / mirror / Atom feed
* [performance bug] volanomark regression on 37-rc1
@ 2010-11-16  9:34 Alex,Shi
  2010-11-16 14:38 ` Rakib Mullick
  2010-11-16 16:21 ` Nikhil Rao
  0 siblings, 2 replies; 13+ messages in thread
From: Alex,Shi @ 2010-11-16  9:34 UTC (permalink / raw)
  To: ncrao, a.p.zijlstra, mingo; +Cc: linux-kernel, Chen, Tim C, zheng.z.yan

When do performance testing on 37-rc1 kernel on Core2 machines, we find
the volanomark loopback performance drop about 30%, that due to
commit:fab476228ba37907ad7 

Volanomark link: http://www.volano.com/benchmarks.html 
Our volanomark testing parameters as following:
"-count 25000 -rooms 10 "
JVM is jrockit-R27.3.1-jre1.5.0_11
java_options is "-Xmx1500m -Xms1500m -Xns750m -XXaggressive -Xlargepages
-XXlazyUnlocking -Xgc:genpar -XXtlasize:min=16k,preferred=64k"
and we set /proc/sys/kernel/sched_compat_yield as "1". 

We find if with the following patch, the regression can be recovered. 

Signed-off-by:Alex Shi <alex.shi@intel.com>

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index f4f6a83..5dca678 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -1766,7 +1766,6 @@ static void pull_task(struct rq *src_rq, struct task_struct *p,
 	check_preempt_curr(this_rq, p, 0);
 
 	/* re-arm NEWIDLE balancing when moving tasks */
-	src_rq->avg_idle = this_rq->avg_idle = 2*sysctl_sched_migration_cost;
 	this_rq->idle_stamp = 0;
 }
 

It seems some of load_balance() is not necessary that caused by avg_idle
setting. But do not know more details of the volano running. Anyone like
to give a comments for this issue? 

Ncrao, I have no idea of your benchmarks, but just guess removing the
avg_idle setting won't bring much wakeup delay for tasks. Could you like
to show some data of this?

The vmstat output for .36 and .37-rc1 kernel as below: 
2.6.36 
procs -----------memory---------- ---swap-- -----io---- --system-- -----cpu------
 r  b   swpd   free   buff  cache   si   so    bi    bo   in   cs us sy id wa st
394  0      0 4680728   3168 118972    0    0     0     3 5314 1348711 38 62  0  0  0
396  0      0 4680500   3184 118976    0    0     0     8 5345 1303237 38 62  0  0  0
413  0      0 4680252   3200 118976    0    0     0     3 5082 1326851 38 61  0  0  0

2.6.37-rc1
procs -----------memory---------- ---swap-- -----io---- --system-- -----cpu------
 r  b   swpd   free   buff  cache   si   so    bi    bo   in   cs us sy id wa st
329  0      0 4653748   4600 134032    0    0     0     7 11649 918370 35 64  0  0  0
210  0      0 4653492   4608 134032    0    0     0     6 11957 898011 36 64  0  0  0
373  0      0 4653496   4624 134032    0    0     0     4 11736 912468 36 64  0  0  0

Regards
Alex 


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16  9:34 [performance bug] volanomark regression on 37-rc1 Alex,Shi
@ 2010-11-16 14:38 ` Rakib Mullick
  2010-11-16 15:26   ` Mike Galbraith
  2010-11-17  0:23   ` [performance bug] volanomark regression on 37-rc1 Alex,Shi
  2010-11-16 16:21 ` Nikhil Rao
  1 sibling, 2 replies; 13+ messages in thread
From: Rakib Mullick @ 2010-11-16 14:38 UTC (permalink / raw)
  To: Alex,Shi
  Cc: ncrao, a.p.zijlstra, mingo, linux-kernel, Chen, Tim C, zheng.z.yan

On 11/16/10, Alex,Shi <alex.shi@intel.com> wrote:
> When do performance testing on 37-rc1 kernel on Core2 machines, we find
> the volanomark loopback performance drop about 30%, that due to
> commit:fab476228ba37907ad7
>
Was that test was made before and after applying above commit? Would
love to know, how did you find that commit (I mean was it a git
bisection)?

> Volanomark link: http://www.volano.com/benchmarks.html
> Our volanomark testing parameters as following:
> "-count 25000 -rooms 10 "
> JVM is jrockit-R27.3.1-jre1.5.0_11
> java_options is "-Xmx1500m -Xms1500m -Xns750m -XXaggressive -Xlargepages
> -XXlazyUnlocking -Xgc:genpar -XXtlasize:min=16k,preferred=64k"
> and we set /proc/sys/kernel/sched_compat_yield as "1".
>
> We find if with the following patch, the regression can be recovered.
>

What are the VolanoMark test results, after and before applying this patch?

>
> It seems some of load_balance() is not necessary that caused by avg_idle
> setting. But do not know more details of the volano running. Anyone like
> to give a comments for this issue?
>
Does VolanoMark is used for scheduler benchmarking? If I'm not wrong,
I don't think it directly relates to scheduler benchmarking.

> Ncrao, I have no idea of your benchmarks, but just guess removing the
> avg_idle setting won't bring much wakeup delay for tasks. Could you like
> to show some data of this?
>
> The vmstat output for .36 and .37-rc1 kernel as below:

You are showing the output of .36 and .37-rc1. If Ncrao's commit is
guilty for this performance regression, then what are the results of
before and after applied Ncrao's commit. Then, what are the result
after applying your patch. You are showing vmstat output of .36 and
.37-rc1, which really doesn't prove the point of your patch. It needs
to be more clearer.


thanks,
rakib


> Regards
> Alex
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 14:38 ` Rakib Mullick
@ 2010-11-16 15:26   ` Mike Galbraith
  2010-11-16 16:31     ` Nikhil Rao
  2010-11-17  0:23   ` [performance bug] volanomark regression on 37-rc1 Alex,Shi
  1 sibling, 1 reply; 13+ messages in thread
From: Mike Galbraith @ 2010-11-16 15:26 UTC (permalink / raw)
  To: Rakib Mullick
  Cc: Alex,Shi, ncrao, a.p.zijlstra, mingo, linux-kernel, Chen, Tim C,
	zheng.z.yan

On Tue, 2010-11-16 at 20:38 +0600, Rakib Mullick wrote:

> Does VolanoMark is used for scheduler benchmarking? If I'm not wrong,
> I don't think it directly relates to scheduler benchmarking.

It's not generally considered to be a wonderful benchmark, but it is a
good indicator, and worth keeping an eye on IMHO.

I don't recall whether that patch works with the idle testcase without
resetting the throttle, or if it's only a bit less effective.  If it's
only a little less effective, I'd be inclined to just whack the reset as
Alex did.  Whatever is done has to prevent high frequency balancing.

	-Mike


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16  9:34 [performance bug] volanomark regression on 37-rc1 Alex,Shi
  2010-11-16 14:38 ` Rakib Mullick
@ 2010-11-16 16:21 ` Nikhil Rao
  1 sibling, 0 replies; 13+ messages in thread
From: Nikhil Rao @ 2010-11-16 16:21 UTC (permalink / raw)
  To: Alex,Shi; +Cc: a.p.zijlstra, mingo, linux-kernel, Chen, Tim C, zheng.z.yan

On Tue, Nov 16, 2010 at 1:34 AM, Alex,Shi <alex.shi@intel.com> wrote:
> When do performance testing on 37-rc1 kernel on Core2 machines, we find
> the volanomark loopback performance drop about 30%, that due to
> commit:fab476228ba37907ad7
>
> Volanomark link: http://www.volano.com/benchmarks.html
> Our volanomark testing parameters as following:
> "-count 25000 -rooms 10 "
> JVM is jrockit-R27.3.1-jre1.5.0_11
> java_options is "-Xmx1500m -Xms1500m -Xns750m -XXaggressive -Xlargepages
> -XXlazyUnlocking -Xgc:genpar -XXtlasize:min=16k,preferred=64k"
> and we set /proc/sys/kernel/sched_compat_yield as "1".
>
> We find if with the following patch, the regression can be recovered.
>
> Signed-off-by:Alex Shi <alex.shi@intel.com>
>
> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> index f4f6a83..5dca678 100644
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -1766,7 +1766,6 @@ static void pull_task(struct rq *src_rq, struct task_struct *p,
>        check_preempt_curr(this_rq, p, 0);
>
>        /* re-arm NEWIDLE balancing when moving tasks */
> -       src_rq->avg_idle = this_rq->avg_idle = 2*sysctl_sched_migration_cost;
>        this_rq->idle_stamp = 0;
>  }
>
>
> It seems some of load_balance() is not necessary that caused by avg_idle
> setting. But do not know more details of the volano running. Anyone like
> to give a comments for this issue?
>

I'm not very familiar with the Volanomark benchmark, but from the
patch you posted it looks like resetting the idle throttle (i.e.
making newidle balance more likely) hurts this benchmark. This is also
evident in the sharp increase in ctx rate in 2.6.37-rc1. Resetting
throttling was a heuristic added to induce more frequent idle
balancing after a migration, and it isn't strictly required to make
the other parts of that patch work. In your patch above, can you
please also remove the comment and idle_stamp reset.


> Ncrao, I have no idea of your benchmarks, but just guess removing the
> avg_idle setting won't bring much wakeup delay for tasks. Could you like
> to show some data of this?
>

The benchmark was pretty simple; run nr_cpu while-1 loops, one of them
niced to -15. I also experimented with some payloads with random
sleep/wakeup times, but nothing with high frequency balancing. I think
removing the reset should be OK for the idle test cases.

-Thanks,
Nikhil

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 15:26   ` Mike Galbraith
@ 2010-11-16 16:31     ` Nikhil Rao
  2010-11-16 17:32       ` Mike Galbraith
  0 siblings, 1 reply; 13+ messages in thread
From: Nikhil Rao @ 2010-11-16 16:31 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Rakib Mullick, Alex,Shi, a.p.zijlstra, mingo, linux-kernel, Chen,
	Tim C, zheng.z.yan

On Tue, Nov 16, 2010 at 7:26 AM, Mike Galbraith <efault@gmx.de> wrote:
> On Tue, 2010-11-16 at 20:38 +0600, Rakib Mullick wrote:
>
>> Does VolanoMark is used for scheduler benchmarking? If I'm not wrong,
>> I don't think it directly relates to scheduler benchmarking.
>
> It's not generally considered to be a wonderful benchmark, but it is a
> good indicator, and worth keeping an eye on IMHO.
>
> I don't recall whether that patch works with the idle testcase without
> resetting the throttle, or if it's only a bit less effective.  If it's
> only a little less effective, I'd be inclined to just whack the reset as
> Alex did.  Whatever is done has to prevent high frequency balancing.
>

>From what I recall, I think removing the reset makes the original
patch a little less effective. I agree that we can remove the reset if
it hurts high frequency balancing.

-Thanks,
Nikhil

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 16:31     ` Nikhil Rao
@ 2010-11-16 17:32       ` Mike Galbraith
  2010-11-16 19:27         ` Nikhil Rao
                           ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Mike Galbraith @ 2010-11-16 17:32 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: Rakib Mullick, Alex,Shi, a.p.zijlstra, mingo, linux-kernel, Chen,
	Tim C, zheng.z.yan

On Tue, 2010-11-16 at 08:31 -0800, Nikhil Rao wrote:
> On Tue, Nov 16, 2010 at 7:26 AM, Mike Galbraith <efault@gmx.de> wrote:
> > On Tue, 2010-11-16 at 20:38 +0600, Rakib Mullick wrote:
> >
> >> Does VolanoMark is used for scheduler benchmarking? If I'm not wrong,
> >> I don't think it directly relates to scheduler benchmarking.
> >
> > It's not generally considered to be a wonderful benchmark, but it is a
> > good indicator, and worth keeping an eye on IMHO.
> >
> > I don't recall whether that patch works with the idle testcase without
> > resetting the throttle, or if it's only a bit less effective.  If it's
> > only a little less effective, I'd be inclined to just whack the reset as
> > Alex did.  Whatever is done has to prevent high frequency balancing.
> >
> 
> >From what I recall, I think removing the reset makes the original
> patch a little less effective. I agree that we can remove the reset if
> it hurts high frequency balancing.

Ok, let's do that.  I added your ack, OK?

From: Alex Shi <alex.shi@intel.com>
Date: Tue, 16 Nov 2010 17:34:02 +0800

    sched: volanomark regression fix

    Commit fab4762 triggers excessive idle balancing, causing a ~30% loss in
    volanomark throughput.  Remove idle balancing throttle reset.

    Signed-off-by: Mike Galbraith <efault@gmx.de>
    Acked-by: Nikhil Rao <ncrao@google.com>
    Reported-by: Alex Shi <alex.shi@intel.com>
    Cc: Ingo Molnar <mingo@elte.hu>
    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    LKML-Reference: <new-submission>

---
 kernel/sched_fair.c |    4 ----
 1 file changed, 4 deletions(-)

Index: linux-2.6/kernel/sched_fair.c
===================================================================
--- linux-2.6.orig/kernel/sched_fair.c
+++ linux-2.6/kernel/sched_fair.c
@@ -1758,10 +1758,6 @@ static void pull_task(struct rq *src_rq,
 	set_task_cpu(p, this_cpu);
 	activate_task(this_rq, p, 0);
 	check_preempt_curr(this_rq, p, 0);
-
-	/* re-arm NEWIDLE balancing when moving tasks */
-	src_rq->avg_idle = this_rq->avg_idle = 2*sysctl_sched_migration_cost;
-	this_rq->idle_stamp = 0;
 }
 
 /*



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 17:32       ` Mike Galbraith
@ 2010-11-16 19:27         ` Nikhil Rao
  2010-11-16 19:37           ` Peter Zijlstra
  2010-11-17  1:17         ` Alex,Shi
  2010-11-18 14:08         ` [tip:sched/urgent] sched: Fix volanomark performance regression tip-bot for Alex Shi
  2 siblings, 1 reply; 13+ messages in thread
From: Nikhil Rao @ 2010-11-16 19:27 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Rakib Mullick, Alex,Shi, a.p.zijlstra, mingo, linux-kernel, Chen,
	Tim C, zheng.z.yan

On Tue, Nov 16, 2010 at 9:32 AM, Mike Galbraith <efault@gmx.de> wrote:
> On Tue, 2010-11-16 at 08:31 -0800, Nikhil Rao wrote:
>> On Tue, Nov 16, 2010 at 7:26 AM, Mike Galbraith <efault@gmx.de> wrote:
>> > On Tue, 2010-11-16 at 20:38 +0600, Rakib Mullick wrote:
>> >
>> >> Does VolanoMark is used for scheduler benchmarking? If I'm not wrong,
>> >> I don't think it directly relates to scheduler benchmarking.
>> >
>> > It's not generally considered to be a wonderful benchmark, but it is a
>> > good indicator, and worth keeping an eye on IMHO.
>> >
>> > I don't recall whether that patch works with the idle testcase without
>> > resetting the throttle, or if it's only a bit less effective.  If it's
>> > only a little less effective, I'd be inclined to just whack the reset as
>> > Alex did.  Whatever is done has to prevent high frequency balancing.
>> >
>>
>> >From what I recall, I think removing the reset makes the original
>> patch a little less effective. I agree that we can remove the reset if
>> it hurts high frequency balancing.
>
> Ok, let's do that.  I added your ack, OK?
>

Yes, thanks for the patch.

> From: Alex Shi <alex.shi@intel.com>
> Date: Tue, 16 Nov 2010 17:34:02 +0800
>
>    sched: volanomark regression fix
>
>    Commit fab4762 triggers excessive idle balancing, causing a ~30% loss in
>    volanomark throughput.  Remove idle balancing throttle reset.
>
>    Signed-off-by: Mike Galbraith <efault@gmx.de>
>    Acked-by: Nikhil Rao <ncrao@google.com>
>    Reported-by: Alex Shi <alex.shi@intel.com>
>    Cc: Ingo Molnar <mingo@elte.hu>
>    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>    LKML-Reference: <new-submission>
>
> ---
>  kernel/sched_fair.c |    4 ----
>  1 file changed, 4 deletions(-)
>
> Index: linux-2.6/kernel/sched_fair.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_fair.c
> +++ linux-2.6/kernel/sched_fair.c
> @@ -1758,10 +1758,6 @@ static void pull_task(struct rq *src_rq,
>        set_task_cpu(p, this_cpu);
>        activate_task(this_rq, p, 0);
>        check_preempt_curr(this_rq, p, 0);
> -
> -       /* re-arm NEWIDLE balancing when moving tasks */
> -       src_rq->avg_idle = this_rq->avg_idle = 2*sysctl_sched_migration_cost;
> -       this_rq->idle_stamp = 0;
>  }
>
>  /*
>
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 19:27         ` Nikhil Rao
@ 2010-11-16 19:37           ` Peter Zijlstra
  0 siblings, 0 replies; 13+ messages in thread
From: Peter Zijlstra @ 2010-11-16 19:37 UTC (permalink / raw)
  To: Nikhil Rao
  Cc: Mike Galbraith, Rakib Mullick, Alex,Shi, mingo, linux-kernel,
	Chen, Tim C, zheng.z.yan

On Tue, 2010-11-16 at 11:27 -0800, Nikhil Rao wrote:
> 
> > Ok, let's do that.  I added your ack, OK?
> >
> 
> Yes, thanks for the patch. 

Applied, thanks!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 14:38 ` Rakib Mullick
  2010-11-16 15:26   ` Mike Galbraith
@ 2010-11-17  0:23   ` Alex,Shi
  1 sibling, 0 replies; 13+ messages in thread
From: Alex,Shi @ 2010-11-17  0:23 UTC (permalink / raw)
  To: Rakib Mullick
  Cc: ncrao, a.p.zijlstra, mingo, linux-kernel, Chen, Tim C, Yan, Zheng Z

On Tue, 2010-11-16 at 22:38 +0800, Rakib Mullick wrote:
> On 11/16/10, Alex,Shi <alex.shi@intel.com> wrote:
> > When do performance testing on 37-rc1 kernel on Core2 machines, we find
> > the volanomark loopback performance drop about 30%, that due to
> > commit:fab476228ba37907ad7
> >
> Was that test was made before and after applying above commit? Would
> love to know, how did you find that commit (I mean was it a git
> bisection)?
Yes, git bisect found this commit. 

> >
> > It seems some of load_balance() is not necessary that caused by avg_idle
> > setting. But do not know more details of the volano running. Anyone like
> > to give a comments for this issue?
> >
> Does VolanoMark is used for scheduler benchmarking? If I'm not wrong,
> I don't think it directly relates to scheduler benchmarking.
Yes, but lots of benchmarks often find other part kernel issues. like
hackbench/netperf often find VM performance issues. And an our cache
testing tool often find scheduler problem. 
> 
> > Ncrao, I have no idea of your benchmarks, but just guess removing the
> > avg_idle setting won't bring much wakeup delay for tasks. Could you like
> > to show some data of this?
> >
> > The vmstat output for .36 and .37-rc1 kernel as below:
> 
> You are showing the output of .36 and .37-rc1. If Ncrao's commit is
> guilty for this performance regression, then what are the results of
> before and after applied Ncrao's commit. Then, what are the result
> after applying your patch. You are showing vmstat output of .36 and
> .37-rc1, which really doesn't prove the point of your patch. It needs
> to be more clearer.
The vmstat for .36 represents original kernel, .37-rc1 represent with
the Ncrao's patch.
> 
> 
> thanks,
> rakib
> 
> 
> > Regards
> > Alex
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
  2010-11-16 17:32       ` Mike Galbraith
  2010-11-16 19:27         ` Nikhil Rao
@ 2010-11-17  1:17         ` Alex,Shi
       [not found]           ` <1290022924-3548-1-git-send-email-ncrao@google.com>
  2010-11-18 14:08         ` [tip:sched/urgent] sched: Fix volanomark performance regression tip-bot for Alex Shi
  2 siblings, 1 reply; 13+ messages in thread
From: Alex,Shi @ 2010-11-17  1:17 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Nikhil Rao, Rakib Mullick, a.p.zijlstra, mingo, linux-kernel,
	Chen, Tim C, Yan, Zheng Z



> ---
>  kernel/sched_fair.c |    4 ----
>  1 file changed, 4 deletions(-)
> 
> Index: linux-2.6/kernel/sched_fair.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched_fair.c
> +++ linux-2.6/kernel/sched_fair.c
> @@ -1758,10 +1758,6 @@ static void pull_task(struct rq *src_rq,
>  	set_task_cpu(p, this_cpu);
>  	activate_task(this_rq, p, 0);
>  	check_preempt_curr(this_rq, p, 0);
> -
> -	/* re-arm NEWIDLE balancing when moving tasks */
> -	src_rq->avg_idle = this_rq->avg_idle = 2*sysctl_sched_migration_cost;
> -	this_rq->idle_stamp = 0;
>  }
>  
>  /*
> 
> 
In the original source (.36 kernel) the rq->idle_stamp is set as zero
after task was pulled to this cpu in load_balance(). Nikhil move this
setting to pull_task(), that has same effect.
I don't know what the details effect of removing idle_stamp setting
instead of recovered it on idle_balance(). :) 

My machines are doing rc2 performance testing. I may try this patch
after testing finish. 

The following is part of Nikhil's old patch. 
===
@@ -3162,10 +3186,8 @@ static void idle_balance(int this_cpu, struct rq
*this_rq)
                interval = msecs_to_jiffies(sd->balance_interval);
                if (time_after(next_balance, sd->last_balance +
interval))
                        next_balance = sd->last_balance + interval;
-               if (pulled_task) {
-                       this_rq->idle_stamp = 0;
+               if (pulled_task)
                        break;
-               }
        }

Regards
Alex




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [performance bug] volanomark regression on 37-rc1
       [not found]           ` <1290022924-3548-1-git-send-email-ncrao@google.com>
@ 2010-11-17 19:45             ` Nikhil Rao
  2010-11-18 14:08             ` [tip:sched/urgent] sched: Fix idle balancing tip-bot for Nikhil Rao
  1 sibling, 0 replies; 13+ messages in thread
From: Nikhil Rao @ 2010-11-17 19:45 UTC (permalink / raw)
  To: Peter Zijlstra, Mike Galbraith, Ingo Molnar
  Cc: Alex Shi, Nikhil Rao, Rakib Mullick, linux-kernel, Chen, Tim C,
	Yan, Zheng Z

forgot to add cc-list in previous mail.

On Wed, Nov 17, 2010 at 11:42 AM, Nikhil Rao <ncrao@google.com> wrote:
> On Tue, Nov 16, 2010 at 5:17 PM, Alex,Shi <alex.shi@intel.com> wrote:
>>
>> In the original source (.36 kernel) the rq->idle_stamp is set as zero
>> after task was pulled to this cpu in load_balance(). Nikhil move this
>> setting to pull_task(), that has same effect.
>> I don't know what the details effect of removing idle_stamp setting
>> instead of recovered it on idle_balance(). :)
>>
>> My machines are doing rc2 performance testing. I may try this patch
>> after testing finish.
>>
>> The following is part of Nikhil's old patch.
>> ===
>> @@ -3162,10 +3186,8 @@ static void idle_balance(int this_cpu, struct rq
>> *this_rq)
>>                interval = msecs_to_jiffies(sd->balance_interval);
>>                if (time_after(next_balance, sd->last_balance +
>> interval))
>>                        next_balance = sd->last_balance + interval;
>> -               if (pulled_task) {
>> -                       this_rq->idle_stamp = 0;
>> +               if (pulled_task)
>>                        break;
>> -               }
>>        }
>>
>> Regards
>> Alex
>>
>
> Ah, should have caught this when reviewing Mike's patch. :-(
>
> Thanks for catching that. We need to reset idle_stamp to 0 or else the avg_idle
> calculations are incorrect. I've attached a patch below that resets idle_stamp
> in the newidle path when we pull.
>
> ---
> sched: volanomark regression fix (part 2)
>
> An earlier commit reverts idle balancing throttling reset to fix a 30%
> regression in volanomark throughput. We still need to reset idle_stamp when we
> pull a task in newidle balance.
>
> Reported-by: Alex Shi <alex.shi@intel.com>
> Signed-off-by: Nikhil Rao <ncrao@google.com>
> ---
>  kernel/sched_fair.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> index 83f65dd..e6e7d4b 100644
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -3193,8 +3193,10 @@ static void idle_balance(int this_cpu, struct rq *this_rq)
>                interval = msecs_to_jiffies(sd->balance_interval);
>                if (time_after(next_balance, sd->last_balance + interval))
>                        next_balance = sd->last_balance + interval;
> -               if (pulled_task)
> +               if (pulled_task) {
> +                       this_rq->idle_stamp = 0;
>                        break;
> +               }
>        }
>
>        raw_spin_lock(&this_rq->lock);
> --
> 1.7.3.1
>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [tip:sched/urgent] sched: Fix volanomark performance regression
  2010-11-16 17:32       ` Mike Galbraith
  2010-11-16 19:27         ` Nikhil Rao
  2010-11-17  1:17         ` Alex,Shi
@ 2010-11-18 14:08         ` tip-bot for Alex Shi
  2 siblings, 0 replies; 13+ messages in thread
From: tip-bot for Alex Shi @ 2010-11-18 14:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, ncrao, efault, alex.shi,
	tglx, mingo

Commit-ID:  b5482cfa1c95a188b3054fa33274806add91bbe5
Gitweb:     http://git.kernel.org/tip/b5482cfa1c95a188b3054fa33274806add91bbe5
Author:     Alex Shi <alex.shi@intel.com>
AuthorDate: Tue, 16 Nov 2010 17:34:02 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 18 Nov 2010 13:11:43 +0100

sched: Fix volanomark performance regression

Commit fab4762 triggers excessive idle balancing, causing a ~30% loss in
volanomark throughput. Remove idle balancing throttle reset.

Originally-by: Alex Shi <alex.shi@intel.com>
Signed-off-by: Mike Galbraith <efault@gmx.de>
Acked-by: Nikhil Rao <ncrao@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1289928732.5169.211.camel@maggy.simson.net>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_fair.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 52ab113..ba0556d 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -1758,10 +1758,6 @@ static void pull_task(struct rq *src_rq, struct task_struct *p,
 	set_task_cpu(p, this_cpu);
 	activate_task(this_rq, p, 0);
 	check_preempt_curr(this_rq, p, 0);
-
-	/* re-arm NEWIDLE balancing when moving tasks */
-	src_rq->avg_idle = this_rq->avg_idle = 2*sysctl_sched_migration_cost;
-	this_rq->idle_stamp = 0;
 }
 
 /*

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [tip:sched/urgent] sched: Fix idle balancing
       [not found]           ` <1290022924-3548-1-git-send-email-ncrao@google.com>
  2010-11-17 19:45             ` Nikhil Rao
@ 2010-11-18 14:08             ` tip-bot for Nikhil Rao
  1 sibling, 0 replies; 13+ messages in thread
From: tip-bot for Nikhil Rao @ 2010-11-18 14:08 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, ncrao, alex.shi, tglx, mingo

Commit-ID:  d5ad140bc1505a98c0f040937125bfcbb508078f
Gitweb:     http://git.kernel.org/tip/d5ad140bc1505a98c0f040937125bfcbb508078f
Author:     Nikhil Rao <ncrao@google.com>
AuthorDate: Wed, 17 Nov 2010 11:42:04 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Thu, 18 Nov 2010 13:12:33 +0100

sched: Fix idle balancing

An earlier commit reverts idle balancing throttling reset to fix a 30%
regression in volanomark throughput. We still need to reset idle_stamp
when we pull a task in newidle balance.

Reported-by: Alex Shi <alex.shi@intel.com>
Signed-off-by: Nikhil Rao <ncrao@google.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1290022924-3548-1-git-send-email-ncrao@google.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_fair.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index ba0556d..00ebd76 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -3215,8 +3215,10 @@ static void idle_balance(int this_cpu, struct rq *this_rq)
 		interval = msecs_to_jiffies(sd->balance_interval);
 		if (time_after(next_balance, sd->last_balance + interval))
 			next_balance = sd->last_balance + interval;
-		if (pulled_task)
+		if (pulled_task) {
+			this_rq->idle_stamp = 0;
 			break;
+		}
 	}
 
 	raw_spin_lock(&this_rq->lock);

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2010-11-18 14:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16  9:34 [performance bug] volanomark regression on 37-rc1 Alex,Shi
2010-11-16 14:38 ` Rakib Mullick
2010-11-16 15:26   ` Mike Galbraith
2010-11-16 16:31     ` Nikhil Rao
2010-11-16 17:32       ` Mike Galbraith
2010-11-16 19:27         ` Nikhil Rao
2010-11-16 19:37           ` Peter Zijlstra
2010-11-17  1:17         ` Alex,Shi
     [not found]           ` <1290022924-3548-1-git-send-email-ncrao@google.com>
2010-11-17 19:45             ` Nikhil Rao
2010-11-18 14:08             ` [tip:sched/urgent] sched: Fix idle balancing tip-bot for Nikhil Rao
2010-11-18 14:08         ` [tip:sched/urgent] sched: Fix volanomark performance regression tip-bot for Alex Shi
2010-11-17  0:23   ` [performance bug] volanomark regression on 37-rc1 Alex,Shi
2010-11-16 16:21 ` Nikhil Rao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.