[PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Alex Shi]

Since power saving code was removed from sched now, the implement
code is out of service in this function, and even pollute other logical.
like, 'want_sd' never has chance to be set '0', that remove the effect
of SD_WAKE_AFFINE here.

So, clean up the obsolete code and some other unnecessary code.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 kernel/sched/fair.c |   32 +++-----------------------------
 1 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 22321db..8a1db69 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2686,7 +2686,6 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 	int prev_cpu = task_cpu(p);
 	int new_cpu = cpu;
 	int want_affine = 0;
-	int want_sd = 1;
 	int sync = wake_flags & WF_SYNC;
 
 	if (p->nr_cpus_allowed == 1)
@@ -2704,48 +2703,23 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 			continue;
 
 		/*
-		 * If power savings logic is enabled for a domain, see if we
-		 * are not overloaded, if so, don't balance wider.
-		 */
-		if (tmp->flags & (SD_PREFER_LOCAL)) {
-			unsigned long power = 0;
-			unsigned long nr_running = 0;
-			unsigned long capacity;
-			int i;
-
-			for_each_cpu(i, sched_domain_span(tmp)) {
-				power += power_of(i);
-				nr_running += cpu_rq(i)->cfs.nr_running;
-			}
-
-			capacity = DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE);
-
-			if (nr_running < capacity)
-				want_sd = 0;
-		}
-
-		/*
 		 * If both cpu and prev_cpu are part of this domain,
 		 * cpu is a valid SD_WAKE_AFFINE target.
 		 */
 		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
 		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
 			affine_sd = tmp;
-			want_affine = 0;
-		}
-
-		if (!want_sd && !want_affine)
 			break;
+		}
 
 		if (!(tmp->flags & sd_flag))
 			continue;
 
-		if (want_sd)
-			sd = tmp;
+		sd = tmp;
 	}
 
 	if (affine_sd) {
-		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
+		if (wake_affine(affine_sd, p, sync))
 			prev_cpu = cpu;
 
 		new_cpu = select_idle_sibling(p, prev_cpu);
-- 
1.7.5.4

[PATCH 2/2] sched: fix a logical error in select_task_rq_fair

[Alex Shi]

If find_idlest_cpu() return '-1', and sd->child is NULL. The function
select_task_rq_fair will return -1. That is not the function's purpose.

The patch introduced a latest_cpu as temporay varible to store
find_idlest_cpu() return value, and let new_cpu to store the latest
workable cpu. If find_idlest_cpu() doesn't find idlest cpu, we still
have a latest workable cpu.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 kernel/sched/fair.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 8a1db69..7e4bab48 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2730,6 +2730,7 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 		int load_idx = sd->forkexec_idx;
 		struct sched_group *group;
 		int weight;
+		int latest_cpu;
 
 		if (!(sd->flags & sd_flag)) {
 			sd = sd->child;
@@ -2745,8 +2746,12 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 			continue;
 		}
 
-		new_cpu = find_idlest_cpu(group, p, cpu);
-		if (new_cpu == -1 || new_cpu == cpu) {
+		latest_cpu = find_idlest_cpu(group, p, cpu);
+
+		if (latest_cpu != -1)
+			new_cpu = latest_cpu;
+
+		if (latest_cpu == -1 || latest_cpu == cpu) {
 			/* Now try balancing at a lower domain level of cpu */
 			sd = sd->child;
 			continue;
-- 
1.7.5.4

Re: [PATCH 2/2] sched: fix a logical error in select_task_rq_fair

[Peter Zijlstra]

On Thu, 2012-07-26 at 13:27 +0800, Alex Shi wrote:
> If find_idlest_cpu() return '-1', and sd->child is NULL. The function
> select_task_rq_fair will return -1. That is not the function's purpose.

But find_idlest_cpu() will only return -1 if the group mask is fully
excluded by the cpus_allowed mask, right?

In that case aren't we covering up a bug in find_idlest_group(), it
appears to have returned a group that isn't eligible to be idlest.

Re: [PATCH 2/2] sched: fix a logical error in select_task_rq_fair

[Alex Shi]

On 07/26/2012 04:17 PM, Peter Zijlstra wrote:

> On Thu, 2012-07-26 at 13:27 +0800, Alex Shi wrote:
>> If find_idlest_cpu() return '-1', and sd->child is NULL. The function
>> select_task_rq_fair will return -1. That is not the function's purpose.
> 
> But find_idlest_cpu() will only return -1 if the group mask is fully
> excluded by the cpus_allowed mask, right?


Yes.

> 
> In that case aren't we covering up a bug in find_idlest_group(), it
> appears to have returned a group that isn't eligible to be idlest.


If it is possible happening in sched_domain rebuilding?
I didn't meet this bug. Just guess.

> 
> 

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Mike Galbraith]

On Thu, 2012-07-26 at 13:27 +0800, Alex Shi wrote:

> 	if (affine_sd) {
> -		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
> +		if (wake_affine(affine_sd, p, sync))
>  			prev_cpu = cpu;
>  
>  		new_cpu = select_idle_sibling(p, prev_cpu);

Hm, if cpu == prev_cpu, asking wake_affine() if it's ok to put wakee
back where it came from is wasted cycles.. that's where the task is
headed regardless of reply.

-Mike

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Alex Shi]

On 07/26/2012 05:37 PM, Mike Galbraith wrote:

> On Thu, 2012-07-26 at 13:27 +0800, Alex Shi wrote:
> 
>> 	if (affine_sd) {
>> -		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
>> +		if (wake_affine(affine_sd, p, sync))
>>  			prev_cpu = cpu;
>>  
>>  		new_cpu = select_idle_sibling(p, prev_cpu);
> 
> Hm, if cpu == prev_cpu, asking wake_affine() if it's ok to put wakee
> back where it came from is wasted cycles.. that's where the task is
> headed regardless of reply.


Sure. You'r right.

> 
> -Mike
> 

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Alex Shi]

On 07/26/2012 05:37 PM, Mike Galbraith wrote:

> On Thu, 2012-07-26 at 13:27 +0800, Alex Shi wrote:
> 
>> 	if (affine_sd) {
>> -		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
>> +		if (wake_affine(affine_sd, p, sync))
>>  			prev_cpu = cpu;
>>  
>>  		new_cpu = select_idle_sibling(p, prev_cpu);
> 
> Hm, if cpu == prev_cpu, asking wake_affine() if it's ok to put wakee
> back where it came from is wasted cycles.. that's where the task is
> headed regardless of reply.
> 
> -Mike
> 




Sure. I modified the patch as below:

===
>From 610515185d8a98c14c7c339c25381bc96cd99d93 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Thu, 26 Jul 2012 08:55:34 +0800
Subject: [PATCH 1/3] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
 code clean up

Since power saving code was removed from sched now, the implement
code is out of service in this function, and even pollute other logical.
like, 'want_sd' never has chance to be set '0', that remove the effect
of SD_WAKE_AFFINE here.

So, clean up the obsolete code and some other unnecessary code.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 kernel/sched/fair.c |   32 +++-----------------------------
 1 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 22321db..53fd8db 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2686,7 +2686,6 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 	int prev_cpu = task_cpu(p);
 	int new_cpu = cpu;
 	int want_affine = 0;
-	int want_sd = 1;
 	int sync = wake_flags & WF_SYNC;
 
 	if (p->nr_cpus_allowed == 1)
@@ -2704,48 +2703,23 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 			continue;
 
 		/*
-		 * If power savings logic is enabled for a domain, see if we
-		 * are not overloaded, if so, don't balance wider.
-		 */
-		if (tmp->flags & (SD_PREFER_LOCAL)) {
-			unsigned long power = 0;
-			unsigned long nr_running = 0;
-			unsigned long capacity;
-			int i;
-
-			for_each_cpu(i, sched_domain_span(tmp)) {
-				power += power_of(i);
-				nr_running += cpu_rq(i)->cfs.nr_running;
-			}
-
-			capacity = DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE);
-
-			if (nr_running < capacity)
-				want_sd = 0;
-		}
-
-		/*
 		 * If both cpu and prev_cpu are part of this domain,
 		 * cpu is a valid SD_WAKE_AFFINE target.
 		 */
 		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
 		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
 			affine_sd = tmp;
-			want_affine = 0;
-		}
-
-		if (!want_sd && !want_affine)
 			break;
+		}
 
 		if (!(tmp->flags & sd_flag))
 			continue;
 
-		if (want_sd)
-			sd = tmp;
+		sd = tmp;
 	}
 
 	if (affine_sd) {
-		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
+		if (cpu != prev_cpu && wake_affine(affine_sd, p, sync))
 			prev_cpu = cpu;
 
 		new_cpu = select_idle_sibling(p, prev_cpu);
-- 
1.7.5.4

Re: [PATCH 2/2] sched: fix a logical error in select_task_rq_fair

[Alex Shi]

>> In that case aren't we covering up a bug in find_idlest_group(), it
>> appears to have returned a group that isn't eligible to be idlest.
> 
> 
> If it is possible happening in sched_domain rebuilding?
> I didn't meet this bug. Just guess.


Even so, it is not related with this patch.
So, Thanks for clarify!  and forget this patch.

> 
>>
>>
> 
> 

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Mike Galbraith]

On Fri, 2012-07-27 at 09:47 +0800, Alex Shi wrote: 
> On 07/26/2012 05:37 PM, Mike Galbraith wrote:
> 
> > On Thu, 2012-07-26 at 13:27 +0800, Alex Shi wrote:
> > 
> >> 	if (affine_sd) {
> >> -		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
> >> +		if (wake_affine(affine_sd, p, sync))
> >>  			prev_cpu = cpu;
> >>  
> >>  		new_cpu = select_idle_sibling(p, prev_cpu);
> > 
> > Hm, if cpu == prev_cpu, asking wake_affine() if it's ok to put wakee
> > back where it came from is wasted cycles.. that's where the task is
> > headed regardless of reply.
> > 
> > -Mike
> > 
> 
> 
> 
> 
> Sure. I modified the patch as below:

(dang, plain text can't make upside down ack;)


> ===
> From 610515185d8a98c14c7c339c25381bc96cd99d93 Mon Sep 17 00:00:00 2001
> From: Alex Shi <alex.shi@intel.com>
> Date: Thu, 26 Jul 2012 08:55:34 +0800
> Subject: [PATCH 1/3] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
>  code clean up
> 
> Since power saving code was removed from sched now, the implement
> code is out of service in this function, and even pollute other logical.
> like, 'want_sd' never has chance to be set '0', that remove the effect
> of SD_WAKE_AFFINE here.
> 
> So, clean up the obsolete code and some other unnecessary code.
> 
> Signed-off-by: Alex Shi <alex.shi@intel.com>
> ---
>  kernel/sched/fair.c |   32 +++-----------------------------
>  1 files changed, 3 insertions(+), 29 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 22321db..53fd8db 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2686,7 +2686,6 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
>  	int prev_cpu = task_cpu(p);
>  	int new_cpu = cpu;
>  	int want_affine = 0;
> -	int want_sd = 1;
>  	int sync = wake_flags & WF_SYNC;
>  
>  	if (p->nr_cpus_allowed == 1)
> @@ -2704,48 +2703,23 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
>  			continue;
>  
>  		/*
> -		 * If power savings logic is enabled for a domain, see if we
> -		 * are not overloaded, if so, don't balance wider.
> -		 */
> -		if (tmp->flags & (SD_PREFER_LOCAL)) {
> -			unsigned long power = 0;
> -			unsigned long nr_running = 0;
> -			unsigned long capacity;
> -			int i;
> -
> -			for_each_cpu(i, sched_domain_span(tmp)) {
> -				power += power_of(i);
> -				nr_running += cpu_rq(i)->cfs.nr_running;
> -			}
> -
> -			capacity = DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE);
> -
> -			if (nr_running < capacity)
> -				want_sd = 0;
> -		}
> -
> -		/*
>  		 * If both cpu and prev_cpu are part of this domain,
>  		 * cpu is a valid SD_WAKE_AFFINE target.
>  		 */
>  		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
>  		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
>  			affine_sd = tmp;
> -			want_affine = 0;
> -		}
> -
> -		if (!want_sd && !want_affine)
>  			break;
> +		}
>  
>  		if (!(tmp->flags & sd_flag))
>  			continue;
>  
> -		if (want_sd)
> -			sd = tmp;
> +		sd = tmp;
>  	}
>  
>  	if (affine_sd) {
> -		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
> +		if (cpu != prev_cpu && wake_affine(affine_sd, p, sync))
>  			prev_cpu = cpu;
>  
>  		new_cpu = select_idle_sibling(p, prev_cpu);

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Peter Zijlstra]

On Fri, 2012-07-27 at 09:47 +0800, Alex Shi wrote:

> From 610515185d8a98c14c7c339c25381bc96cd99d93 Mon Sep 17 00:00:00 2001
> From: Alex Shi <alex.shi@intel.com>
> Date: Thu, 26 Jul 2012 08:55:34 +0800
> Subject: [PATCH 1/3] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
>  code clean up
> 
> Since power saving code was removed from sched now, the implement
> code is out of service in this function, and even pollute other logical.
> like, 'want_sd' never has chance to be set '0', that remove the effect
> of SD_WAKE_AFFINE here.
> 
> So, clean up the obsolete code and some other unnecessary code.
> 
> Signed-off-by: Alex Shi <alex.shi@intel.com>

I think your code leaves an unused definition of SD_PREFER_LOCAL around.

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Alex Shi]

On 07/27/2012 04:32 PM, Peter Zijlstra wrote:

> On Fri, 2012-07-27 at 09:47 +0800, Alex Shi wrote:
> 
>> From 610515185d8a98c14c7c339c25381bc96cd99d93 Mon Sep 17 00:00:00 2001
>> From: Alex Shi <alex.shi@intel.com>
>> Date: Thu, 26 Jul 2012 08:55:34 +0800
>> Subject: [PATCH 1/3] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
>>  code clean up
>>
>> Since power saving code was removed from sched now, the implement
>> code is out of service in this function, and even pollute other logical.
>> like, 'want_sd' never has chance to be set '0', that remove the effect
>> of SD_WAKE_AFFINE here.
>>
>> So, clean up the obsolete code and some other unnecessary code.
>>
>> Signed-off-by: Alex Shi <alex.shi@intel.com>
> 
> I think your code leaves an unused definition of SD_PREFER_LOCAL around.




I had thought it maybe useful in power saving recovery. But you are right. 
It is better to remove them and alignment code now. 

===

>From 5eba8f31207e54ca6cbd481cfc23f149a0554b2a Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Thu, 26 Jul 2012 08:55:34 +0800
Subject: [PATCH 1/3] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
 code clean up

Since power saving code was removed from sched now, the implement
code is out of service in this function, and even pollute other logical.
like, 'want_sd' never has chance to be set '0', that remove the effect
of SD_WAKE_AFFINE here.

So, clean up the obsolete code, like SD_PREFER_LOCAL.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/sched.h    |    1 -
 include/linux/topology.h |    2 --
 kernel/sched/core.c      |    1 -
 kernel/sched/fair.c      |   32 +++-----------------------------
 4 files changed, 3 insertions(+), 33 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index d77877d..1a1e3e45 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -855,7 +855,6 @@ enum cpu_idle_type {
 #define SD_BALANCE_FORK		0x0008	/* Balance on fork, clone */
 #define SD_BALANCE_WAKE		0x0010  /* Balance on wakeup */
 #define SD_WAKE_AFFINE		0x0020	/* Wake task to waking CPU */
-#define SD_PREFER_LOCAL		0x0040  /* Prefer to keep tasks local to this domain */
 #define SD_SHARE_CPUPOWER	0x0080	/* Domain members share cpu power */
 #define SD_SHARE_PKG_RESOURCES	0x0200	/* Domain members share cpu pkg resources */
 #define SD_SERIALIZE		0x0400	/* Only a single load balancing instance */
diff --git a/include/linux/topology.h b/include/linux/topology.h
index fec12d6..d3cf0d6 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -129,7 +129,6 @@ int arch_update_cpu_topology(void);
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
 				| 1*SD_WAKE_AFFINE			\
-				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 1*SD_SHARE_PKG_RESOURCES		\
 				| 0*SD_SERIALIZE			\
@@ -160,7 +159,6 @@ int arch_update_cpu_topology(void);
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
 				| 1*SD_WAKE_AFFINE			\
-				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 0*SD_SHARE_PKG_RESOURCES		\
 				| 0*SD_SERIALIZE			\
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 82ad284..7dbb6c1 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6582,7 +6582,6 @@ sd_numa_init(struct sched_domain_topology_level *tl, int cpu)
 					| 0*SD_BALANCE_FORK
 					| 0*SD_BALANCE_WAKE
 					| 0*SD_WAKE_AFFINE
-					| 0*SD_PREFER_LOCAL
 					| 0*SD_SHARE_CPUPOWER
 					| 0*SD_SHARE_PKG_RESOURCES
 					| 1*SD_SERIALIZE
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 22321db..53fd8db 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2686,7 +2686,6 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 	int prev_cpu = task_cpu(p);
 	int new_cpu = cpu;
 	int want_affine = 0;
-	int want_sd = 1;
 	int sync = wake_flags & WF_SYNC;
 
 	if (p->nr_cpus_allowed == 1)
@@ -2704,48 +2703,23 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 			continue;
 
 		/*
-		 * If power savings logic is enabled for a domain, see if we
-		 * are not overloaded, if so, don't balance wider.
-		 */
-		if (tmp->flags & (SD_PREFER_LOCAL)) {
-			unsigned long power = 0;
-			unsigned long nr_running = 0;
-			unsigned long capacity;
-			int i;
-
-			for_each_cpu(i, sched_domain_span(tmp)) {
-				power += power_of(i);
-				nr_running += cpu_rq(i)->cfs.nr_running;
-			}
-
-			capacity = DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE);
-
-			if (nr_running < capacity)
-				want_sd = 0;
-		}
-
-		/*
 		 * If both cpu and prev_cpu are part of this domain,
 		 * cpu is a valid SD_WAKE_AFFINE target.
 		 */
 		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
 		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
 			affine_sd = tmp;
-			want_affine = 0;
-		}
-
-		if (!want_sd && !want_affine)
 			break;
+		}
 
 		if (!(tmp->flags & sd_flag))
 			continue;
 
-		if (want_sd)
-			sd = tmp;
+		sd = tmp;
 	}
 
 	if (affine_sd) {
-		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
+		if (cpu != prev_cpu && wake_affine(affine_sd, p, sync))
 			prev_cpu = cpu;
 
 		new_cpu = select_idle_sibling(p, prev_cpu);
-- 
1.7.5.4

Re: [PATCH 1/2] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[Alex Shi]

On 07/27/2012 04:32 PM, Peter Zijlstra wrote:

> On Fri, 2012-07-27 at 09:47 +0800, Alex Shi wrote:
> 
>> From 610515185d8a98c14c7c339c25381bc96cd99d93 Mon Sep 17 00:00:00 2001
>> From: Alex Shi <alex.shi@intel.com>
>> Date: Thu, 26 Jul 2012 08:55:34 +0800
>> Subject: [PATCH 1/3] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
>>  code clean up
>>
>> Since power saving code was removed from sched now, the implement
>> code is out of service in this function, and even pollute other logical.
>> like, 'want_sd' never has chance to be set '0', that remove the effect
>> of SD_WAKE_AFFINE here.
>>
>> So, clean up the obsolete code and some other unnecessary code.
>>
>> Signed-off-by: Alex Shi <alex.shi@intel.com>
> 
> I think your code leaves an unused definition of SD_PREFER_LOCAL around.
> 




I removed the SD_PREFER_LOCAL. and resend. :) 

>From be2e235ab4626246766b220908a3fc714809d8e4 Mon Sep 17 00:00:00 2001
From: Alex Shi <alex.shi@intel.com>
Date: Thu, 26 Jul 2012 08:55:34 +0800
Subject: [PATCH 3/7] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and
 code clean up

Since power saving code was removed from sched now, the implement
code is out of service in this function, and even pollute other logical.
like, 'want_sd' never has chance to be set '0', that remove the effect
of SD_WAKE_AFFINE here.

So, clean up the obsolete code, includes SD_PREFER_LOCAL.

Signed-off-by: Alex Shi <alex.shi@intel.com>
---
 include/linux/sched.h    |    1 -
 include/linux/topology.h |    2 --
 kernel/sched/core.c      |    1 -
 kernel/sched/fair.c      |   34 +++-------------------------------
 4 files changed, 3 insertions(+), 35 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index c147e70..96e026c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -868,7 +868,6 @@ enum cpu_idle_type {
 #define SD_BALANCE_FORK		0x0008	/* Balance on fork, clone */
 #define SD_BALANCE_WAKE		0x0010  /* Balance on wakeup */
 #define SD_WAKE_AFFINE		0x0020	/* Wake task to waking CPU */
-#define SD_PREFER_LOCAL		0x0040  /* Prefer to keep tasks local to this domain */
 #define SD_SHARE_CPUPOWER	0x0080	/* Domain members share cpu power */
 #define SD_SHARE_PKG_RESOURCES	0x0200	/* Domain members share cpu pkg resources */
 #define SD_SERIALIZE		0x0400	/* Only a single load balancing instance */
diff --git a/include/linux/topology.h b/include/linux/topology.h
index fec12d6..d3cf0d6 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -129,7 +129,6 @@ int arch_update_cpu_topology(void);
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
 				| 1*SD_WAKE_AFFINE			\
-				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 1*SD_SHARE_PKG_RESOURCES		\
 				| 0*SD_SERIALIZE			\
@@ -160,7 +159,6 @@ int arch_update_cpu_topology(void);
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
 				| 1*SD_WAKE_AFFINE			\
-				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 0*SD_SHARE_PKG_RESOURCES		\
 				| 0*SD_SERIALIZE			\
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 82ad284..7dbb6c1 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6582,7 +6582,6 @@ sd_numa_init(struct sched_domain_topology_level *tl, int cpu)
 					| 0*SD_BALANCE_FORK
 					| 0*SD_BALANCE_WAKE
 					| 0*SD_WAKE_AFFINE
-					| 0*SD_PREFER_LOCAL
 					| 0*SD_SHARE_CPUPOWER
 					| 0*SD_SHARE_PKG_RESOURCES
 					| 1*SD_SERIALIZE
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 84fa9c5..61e399c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2686,7 +2686,6 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 	int prev_cpu = task_cpu(p);
 	int new_cpu = cpu;
 	int want_affine = 0;
-	int want_sd = 1;
 	int sync = wake_flags & WF_SYNC;
 
 	if (p->nr_cpus_allowed == 1)
@@ -2704,48 +2703,21 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 			continue;
 
 		/*
-		 * If power savings logic is enabled for a domain, see if we
-		 * are not overloaded, if so, don't balance wider.
-		 */
-		if (tmp->flags & (SD_PREFER_LOCAL)) {
-			unsigned long power = 0;
-			unsigned long nr_running = 0;
-			unsigned long capacity;
-			int i;
-
-			for_each_cpu(i, sched_domain_span(tmp)) {
-				power += power_of(i);
-				nr_running += cpu_rq(i)->cfs.nr_running;
-			}
-
-			capacity = DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE);
-
-			if (nr_running < capacity)
-				want_sd = 0;
-		}
-
-		/*
 		 * If both cpu and prev_cpu are part of this domain,
 		 * cpu is a valid SD_WAKE_AFFINE target.
 		 */
 		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
 		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
 			affine_sd = tmp;
-			want_affine = 0;
-		}
-
-		if (!want_sd && !want_affine)
 			break;
+		}
 
-		if (!(tmp->flags & sd_flag))
-			continue;
-
-		if (want_sd)
+		if (tmp->flags & sd_flag)
 			sd = tmp;
 	}
 
 	if (affine_sd) {
-		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
+		if (cpu != prev_cpu && wake_affine(affine_sd, p, sync))
 			prev_cpu = cpu;
 
 		new_cpu = select_idle_sibling(p, prev_cpu);
-- 
1.7.5.4

[tip:sched/core] sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

[tip-bot for Alex Shi]

Commit-ID:  f03542a7019c600163ac4441d8a826c92c1bd510
Gitweb:     http://git.kernel.org/tip/f03542a7019c600163ac4441d8a826c92c1bd510
Author:     Alex Shi <alex.shi@intel.com>
AuthorDate: Thu, 26 Jul 2012 08:55:34 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 13 Aug 2012 19:02:05 +0200

sched: recover SD_WAKE_AFFINE in select_task_rq_fair and code clean up

Since power saving code was removed from sched now, the implement
code is out of service in this function, and even pollute other logical.
like, 'want_sd' never has chance to be set '0', that remove the effect
of SD_WAKE_AFFINE here.

So, clean up the obsolete code, includes SD_PREFER_LOCAL.

Signed-off-by: Alex Shi <alex.shi@intel.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/5028F431.6000306@intel.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/sched.h    |    1 -
 include/linux/topology.h |    2 --
 kernel/sched/core.c      |    1 -
 kernel/sched/fair.c      |   34 +++-------------------------------
 4 files changed, 3 insertions(+), 35 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index b8c8664..f3eebc1 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -860,7 +860,6 @@ enum cpu_idle_type {
 #define SD_BALANCE_FORK		0x0008	/* Balance on fork, clone */
 #define SD_BALANCE_WAKE		0x0010  /* Balance on wakeup */
 #define SD_WAKE_AFFINE		0x0020	/* Wake task to waking CPU */
-#define SD_PREFER_LOCAL		0x0040  /* Prefer to keep tasks local to this domain */
 #define SD_SHARE_CPUPOWER	0x0080	/* Domain members share cpu power */
 #define SD_SHARE_PKG_RESOURCES	0x0200	/* Domain members share cpu pkg resources */
 #define SD_SERIALIZE		0x0400	/* Only a single load balancing instance */
diff --git a/include/linux/topology.h b/include/linux/topology.h
index fec12d6..d3cf0d6 100644
--- a/include/linux/topology.h
+++ b/include/linux/topology.h
@@ -129,7 +129,6 @@ int arch_update_cpu_topology(void);
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
 				| 1*SD_WAKE_AFFINE			\
-				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 1*SD_SHARE_PKG_RESOURCES		\
 				| 0*SD_SERIALIZE			\
@@ -160,7 +159,6 @@ int arch_update_cpu_topology(void);
 				| 1*SD_BALANCE_FORK			\
 				| 0*SD_BALANCE_WAKE			\
 				| 1*SD_WAKE_AFFINE			\
-				| 0*SD_PREFER_LOCAL			\
 				| 0*SD_SHARE_CPUPOWER			\
 				| 0*SD_SHARE_PKG_RESOURCES		\
 				| 0*SD_SERIALIZE			\
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c9a3655..4376c9f 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6622,7 +6622,6 @@ sd_numa_init(struct sched_domain_topology_level *tl, int cpu)
 					| 0*SD_BALANCE_FORK
 					| 0*SD_BALANCE_WAKE
 					| 0*SD_WAKE_AFFINE
-					| 0*SD_PREFER_LOCAL
 					| 0*SD_SHARE_CPUPOWER
 					| 0*SD_SHARE_PKG_RESOURCES
 					| 1*SD_SERIALIZE
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 287bfac..01d3eda 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2686,7 +2686,6 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 	int prev_cpu = task_cpu(p);
 	int new_cpu = cpu;
 	int want_affine = 0;
-	int want_sd = 1;
 	int sync = wake_flags & WF_SYNC;
 
 	if (p->nr_cpus_allowed == 1)
@@ -2704,48 +2703,21 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
 			continue;
 
 		/*
-		 * If power savings logic is enabled for a domain, see if we
-		 * are not overloaded, if so, don't balance wider.
-		 */
-		if (tmp->flags & (SD_PREFER_LOCAL)) {
-			unsigned long power = 0;
-			unsigned long nr_running = 0;
-			unsigned long capacity;
-			int i;
-
-			for_each_cpu(i, sched_domain_span(tmp)) {
-				power += power_of(i);
-				nr_running += cpu_rq(i)->cfs.nr_running;
-			}
-
-			capacity = DIV_ROUND_CLOSEST(power, SCHED_POWER_SCALE);
-
-			if (nr_running < capacity)
-				want_sd = 0;
-		}
-
-		/*
 		 * If both cpu and prev_cpu are part of this domain,
 		 * cpu is a valid SD_WAKE_AFFINE target.
 		 */
 		if (want_affine && (tmp->flags & SD_WAKE_AFFINE) &&
 		    cpumask_test_cpu(prev_cpu, sched_domain_span(tmp))) {
 			affine_sd = tmp;
-			want_affine = 0;
-		}
-
-		if (!want_sd && !want_affine)
 			break;
+		}
 
-		if (!(tmp->flags & sd_flag))
-			continue;
-
-		if (want_sd)
+		if (tmp->flags & sd_flag)
 			sd = tmp;
 	}
 
 	if (affine_sd) {
-		if (cpu == prev_cpu || wake_affine(affine_sd, p, sync))
+		if (cpu != prev_cpu && wake_affine(affine_sd, p, sync))
 			prev_cpu = cpu;
 
 		new_cpu = select_idle_sibling(p, prev_cpu);