Re: [PATCH 01/10] sched: Remove one division operation in find_busiest_queue()

2013-08-22 Thread Peter Zijlstra 
On Thu, Aug 22, 2013 at 01:58:28AM -0700, Paul Turner wrote:

> > wl_i / power_i > wl_j / power_j :=
> > wl_i * power_j > wl_j * power_i
> >
> > struct rq *busiest = NULL, *rq;
> > -   unsigned long max_load = 0;
> > +   unsigned long busiest_load = 0, busiest_power = SCHED_POWER_SCALE;
> 
> Initializing this to SCHED_POWER_SCALE assigns a meaning that isn't
> really there.  How about just 1?

Right, 1 works, all we really need is for wl to be > 0.

> > int i;
> >
> > for_each_cpu(i, sched_group_cpus(group)) {
> > @@ -5049,10 +5049,9 @@ static struct rq *find_busiest_queue(str
> >  * the load can be moved away from the cpu that is 
> > potentially
> >  * running at a lower capacity.
> >  */
> > -   wl = (wl * SCHED_POWER_SCALE) / power;
> > -
> > -   if (wl > max_load) {
> > -   max_load = wl;
> 
> A comment wouldn't hurt here.

Agreed, something like so?

/*
 * Since we're looking for max(wl_i / power_i) crosswise multiplication
 * to rid ourselves of the division works out to:
 * wl_i * power_j > wl_j * power_i;  where j is our previous maximum.
 */

> > +   if (wl * busiest_power > busiest_load * power) {
> > +   busiest_load = wl;
> > +   busiest_power = power;
> > busiest = rq;
> > }
> > }
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/10] sched: Remove one division operation in find_busiest_queue()

2013-08-22 Thread Paul Turner 
On Mon, Aug 19, 2013 at 9:00 AM, Peter Zijlstra  wrote:
> From: Joonsoo Kim 
>
> Remove one division operation in find_busiest_queue() by using
> crosswise multiplication:
>
> wl_i / power_i > wl_j / power_j :=
> wl_i * power_j > wl_j * power_i
>
> Signed-off-by: Joonsoo Kim 
> [peterz: expanded changelog]
> Signed-off-by: Peter Zijlstra 
> Link: 
> http://lkml.kernel.org/r/1375778203-31343-2-git-send-email-iamjoonsoo@lge.com
> ---
>  kernel/sched/fair.c |9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -5018,7 +5018,7 @@ static struct rq *find_busiest_queue(str
>  struct sched_group *group)
>  {
> struct rq *busiest = NULL, *rq;
> -   unsigned long max_load = 0;
> +   unsigned long busiest_load = 0, busiest_power = SCHED_POWER_SCALE;

Initializing this to SCHED_POWER_SCALE assigns a meaning that isn't
really there.  How about just 1?

> int i;
>
> for_each_cpu(i, sched_group_cpus(group)) {
> @@ -5049,10 +5049,9 @@ static struct rq *find_busiest_queue(str
>  * the load can be moved away from the cpu that is potentially
>  * running at a lower capacity.
>  */
> -   wl = (wl * SCHED_POWER_SCALE) / power;
> -
> -   if (wl > max_load) {
> -   max_load = wl;

A comment wouldn't hurt here.

> +   if (wl * busiest_power > busiest_load * power) {
> +   busiest_load = wl;
> +   busiest_power = power;
> busiest = rq;
> }
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/10] sched: Remove one division operation in find_busiest_queue()

2013-08-22 Thread Paul Turner 
On Mon, Aug 19, 2013 at 9:00 AM, Peter Zijlstra pet...@infradead.org wrote:
 From: Joonsoo Kim iamjoonsoo@lge.com

 Remove one division operation in find_busiest_queue() by using
 crosswise multiplication:

 wl_i / power_i  wl_j / power_j :=
 wl_i * power_j  wl_j * power_i

 Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
 [peterz: expanded changelog]
 Signed-off-by: Peter Zijlstra pet...@infradead.org
 Link: 
 http://lkml.kernel.org/r/1375778203-31343-2-git-send-email-iamjoonsoo@lge.com
 ---
  kernel/sched/fair.c |9 -
  1 file changed, 4 insertions(+), 5 deletions(-)

 --- a/kernel/sched/fair.c
 +++ b/kernel/sched/fair.c
 @@ -5018,7 +5018,7 @@ static struct rq *find_busiest_queue(str
  struct sched_group *group)
  {
 struct rq *busiest = NULL, *rq;
 -   unsigned long max_load = 0;
 +   unsigned long busiest_load = 0, busiest_power = SCHED_POWER_SCALE;

Initializing this to SCHED_POWER_SCALE assigns a meaning that isn't
really there.  How about just 1?

 int i;

 for_each_cpu(i, sched_group_cpus(group)) {
 @@ -5049,10 +5049,9 @@ static struct rq *find_busiest_queue(str
  * the load can be moved away from the cpu that is potentially
  * running at a lower capacity.
  */
 -   wl = (wl * SCHED_POWER_SCALE) / power;
 -
 -   if (wl  max_load) {
 -   max_load = wl;

A comment wouldn't hurt here.

 +   if (wl * busiest_power  busiest_load * power) {
 +   busiest_load = wl;
 +   busiest_power = power;
 busiest = rq;
 }
 }


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

Re: [PATCH 01/10] sched: Remove one division operation in find_busiest_queue()

2013-08-22 Thread Peter Zijlstra 
On Thu, Aug 22, 2013 at 01:58:28AM -0700, Paul Turner wrote:

  wl_i / power_i  wl_j / power_j :=
  wl_i * power_j  wl_j * power_i
 
  struct rq *busiest = NULL, *rq;
  -   unsigned long max_load = 0;
  +   unsigned long busiest_load = 0, busiest_power = SCHED_POWER_SCALE;
 
 Initializing this to SCHED_POWER_SCALE assigns a meaning that isn't
 really there.  How about just 1?

Right, 1 works, all we really need is for wl to be  0.

  int i;
 
  for_each_cpu(i, sched_group_cpus(group)) {
  @@ -5049,10 +5049,9 @@ static struct rq *find_busiest_queue(str
   * the load can be moved away from the cpu that is 
  potentially
   * running at a lower capacity.
   */
  -   wl = (wl * SCHED_POWER_SCALE) / power;
  -
  -   if (wl  max_load) {
  -   max_load = wl;
 
 A comment wouldn't hurt here.

Agreed, something like so?

/*
 * Since we're looking for max(wl_i / power_i) crosswise multiplication
 * to rid ourselves of the division works out to:
 * wl_i * power_j  wl_j * power_i;  where j is our previous maximum.
 */

  +   if (wl * busiest_power  busiest_load * power) {
  +   busiest_load = wl;
  +   busiest_power = power;
  busiest = rq;
  }
  }
 
 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 01/10] sched: Remove one division operation in find_busiest_queue()

2013-08-19 Thread Peter Zijlstra 
From: Joonsoo Kim 

Remove one division operation in find_busiest_queue() by using
crosswise multiplication:

wl_i / power_i > wl_j / power_j := 
wl_i * power_j > wl_j * power_i

Signed-off-by: Joonsoo Kim 
[peterz: expanded changelog]
Signed-off-by: Peter Zijlstra 
Link: 
http://lkml.kernel.org/r/1375778203-31343-2-git-send-email-iamjoonsoo@lge.com
---
 kernel/sched/fair.c |9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5018,7 +5018,7 @@ static struct rq *find_busiest_queue(str
 struct sched_group *group)
 {
struct rq *busiest = NULL, *rq;
-   unsigned long max_load = 0;
+   unsigned long busiest_load = 0, busiest_power = SCHED_POWER_SCALE;
int i;
 
for_each_cpu(i, sched_group_cpus(group)) {
@@ -5049,10 +5049,9 @@ static struct rq *find_busiest_queue(str
 * the load can be moved away from the cpu that is potentially
 * running at a lower capacity.
 */
-   wl = (wl * SCHED_POWER_SCALE) / power;
-
-   if (wl > max_load) {
-   max_load = wl;
+   if (wl * busiest_power > busiest_load * power) {
+   busiest_load = wl;
+   busiest_power = power;
busiest = rq;
}
}


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

[PATCH 01/10] sched: Remove one division operation in find_busiest_queue()

2013-08-19 Thread Peter Zijlstra 
From: Joonsoo Kim iamjoonsoo@lge.com

Remove one division operation in find_busiest_queue() by using
crosswise multiplication:

wl_i / power_i  wl_j / power_j := 
wl_i * power_j  wl_j * power_i

Signed-off-by: Joonsoo Kim iamjoonsoo@lge.com
[peterz: expanded changelog]
Signed-off-by: Peter Zijlstra pet...@infradead.org
Link: 
http://lkml.kernel.org/r/1375778203-31343-2-git-send-email-iamjoonsoo@lge.com
---
 kernel/sched/fair.c |9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5018,7 +5018,7 @@ static struct rq *find_busiest_queue(str
 struct sched_group *group)
 {
struct rq *busiest = NULL, *rq;
-   unsigned long max_load = 0;
+   unsigned long busiest_load = 0, busiest_power = SCHED_POWER_SCALE;
int i;
 
for_each_cpu(i, sched_group_cpus(group)) {
@@ -5049,10 +5049,9 @@ static struct rq *find_busiest_queue(str
 * the load can be moved away from the cpu that is potentially
 * running at a lower capacity.
 */
-   wl = (wl * SCHED_POWER_SCALE) / power;
-
-   if (wl  max_load) {
-   max_load = wl;
+   if (wl * busiest_power  busiest_load * power) {
+   busiest_load = wl;
+   busiest_power = power;
busiest = rq;
}
}


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