Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-03 Thread Joonsoo Kim
Hello, Peter. On Tue, Apr 02, 2013 at 12:29:42PM +0200, Peter Zijlstra wrote: > On Tue, 2013-04-02 at 12:00 +0200, Peter Zijlstra wrote: > > On Tue, 2013-04-02 at 18:50 +0900, Joonsoo Kim wrote: > > > > > > It seems that there is some misunderstanding about this patch. > > > In this patch, we

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-03 Thread Joonsoo Kim
Hello, Peter. On Tue, Apr 02, 2013 at 12:29:42PM +0200, Peter Zijlstra wrote: On Tue, 2013-04-02 at 12:00 +0200, Peter Zijlstra wrote: On Tue, 2013-04-02 at 18:50 +0900, Joonsoo Kim wrote: It seems that there is some misunderstanding about this patch. In this patch, we don't iterate

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Peter Zijlstra
On Tue, 2013-04-02 at 12:00 +0200, Peter Zijlstra wrote: > On Tue, 2013-04-02 at 18:50 +0900, Joonsoo Kim wrote: > > > > It seems that there is some misunderstanding about this patch. > > In this patch, we don't iterate all groups. Instead, we iterate on > > cpus of local sched_group only. So

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Peter Zijlstra
On Tue, 2013-04-02 at 18:50 +0900, Joonsoo Kim wrote: > > It seems that there is some misunderstanding about this patch. > In this patch, we don't iterate all groups. Instead, we iterate on > cpus of local sched_group only. So there is no penalty you mentioned. OK, I'll go stare at it again..

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Joonsoo Kim
Hello, Peter. On Tue, Apr 02, 2013 at 10:10:06AM +0200, Peter Zijlstra wrote: > On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: > > Now checking that this cpu is appropriate to balance is embedded into > > update_sg_lb_stats() and this checking has no direct relationship to > > this > >

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Peter Zijlstra
On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: > Now checking that this cpu is appropriate to balance is embedded into > update_sg_lb_stats() and this checking has no direct relationship to > this > function. > > There is not enough reason to place this checking at > update_sg_lb_stats(),

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Peter Zijlstra
On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: Now checking that this cpu is appropriate to balance is embedded into update_sg_lb_stats() and this checking has no direct relationship to this function. There is not enough reason to place this checking at update_sg_lb_stats(), except

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Joonsoo Kim
Hello, Peter. On Tue, Apr 02, 2013 at 10:10:06AM +0200, Peter Zijlstra wrote: On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: Now checking that this cpu is appropriate to balance is embedded into update_sg_lb_stats() and this checking has no direct relationship to this function.

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Peter Zijlstra
On Tue, 2013-04-02 at 18:50 +0900, Joonsoo Kim wrote: It seems that there is some misunderstanding about this patch. In this patch, we don't iterate all groups. Instead, we iterate on cpus of local sched_group only. So there is no penalty you mentioned. OK, I'll go stare at it again.. -- To

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-04-02 Thread Peter Zijlstra
On Tue, 2013-04-02 at 12:00 +0200, Peter Zijlstra wrote: On Tue, 2013-04-02 at 18:50 +0900, Joonsoo Kim wrote: It seems that there is some misunderstanding about this patch. In this patch, we don't iterate all groups. Instead, we iterate on cpus of local sched_group only. So there is no

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-31 Thread Joonsoo Kim
On Fri, Mar 29, 2013 at 12:58:26PM +0100, Peter Zijlstra wrote: > On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: > > +static int should_we_balance(struct lb_env *env) > > +{ > > + struct sched_group *sg = env->sd->groups; > > + int cpu, balance_cpu = -1; > > + > > + /* > >

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-31 Thread Joonsoo Kim
Hello, Peter. On Fri, Mar 29, 2013 at 12:45:14PM +0100, Peter Zijlstra wrote: > On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: > > There is not enough reason to place this checking at > > update_sg_lb_stats(), > > except saving one iteration for sched_group_cpus. But with this > > change,

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-31 Thread Joonsoo Kim
Hello, Peter. On Fri, Mar 29, 2013 at 12:45:14PM +0100, Peter Zijlstra wrote: On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: There is not enough reason to place this checking at update_sg_lb_stats(), except saving one iteration for sched_group_cpus. But with this change, we can

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-31 Thread Joonsoo Kim
On Fri, Mar 29, 2013 at 12:58:26PM +0100, Peter Zijlstra wrote: On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: +static int should_we_balance(struct lb_env *env) +{ + struct sched_group *sg = env-sd-groups; + int cpu, balance_cpu = -1; + + /* +* In

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-29 Thread Peter Zijlstra
On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: > +static int should_we_balance(struct lb_env *env) > +{ > + struct sched_group *sg = env->sd->groups; > + int cpu, balance_cpu = -1; > + > + /* > +* In the newly idle case, we will allow all the cpu's > +* to

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-29 Thread Peter Zijlstra
On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: > There is not enough reason to place this checking at > update_sg_lb_stats(), > except saving one iteration for sched_group_cpus. But with this > change, > we can save two memset cost and can expect better compiler > optimization, > so

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-29 Thread Peter Zijlstra
On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: There is not enough reason to place this checking at update_sg_lb_stats(), except saving one iteration for sched_group_cpus. But with this change, we can save two memset cost and can expect better compiler optimization, so clean-up may

Re: [PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-29 Thread Peter Zijlstra
On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote: +static int should_we_balance(struct lb_env *env) +{ + struct sched_group *sg = env-sd-groups; + int cpu, balance_cpu = -1; + + /* +* In the newly idle case, we will allow all the cpu's +* to do the

[PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-28 Thread Joonsoo Kim
Now checking that this cpu is appropriate to balance is embedded into update_sg_lb_stats() and this checking has no direct relationship to this function. There is not enough reason to place this checking at update_sg_lb_stats(), except saving one iteration for sched_group_cpus. But with this

[PATCH 2/5] sched: factor out code to should_we_balance()

2013-03-28 Thread Joonsoo Kim
Now checking that this cpu is appropriate to balance is embedded into update_sg_lb_stats() and this checking has no direct relationship to this function. There is not enough reason to place this checking at update_sg_lb_stats(), except saving one iteration for sched_group_cpus. But with this