diff mbox

[v2,v3.18+,regression,fix] sched: Further improve spurious CPU_IDLE active migrations

Message ID 20160906130127.GA21960@vingu-laptop
State New
Headers show

Commit Message

Vincent Guittot Sept. 6, 2016, 1:01 p.m. UTC 
Le Monday 05 Sep 2016 à 18:26:53 (+0200), Mike Galbraith a écrit :
> Coming back to this, how about this instead, only increase the group

> imbalance threshold when sd_llc_size == 2.  Newer L3 equipped

> processors then aren't affected.

>


Not sure that all systems with sd_llc_size == 2 wants this behavior.

Why not adding a sched_feature for changing the 2nd half of the test for some systems ?

something like below

---
 kernel/sched/fair.c     | 11 ++++++++---
 kernel/sched/features.h |  7 +++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

--

> 

> 

> 43f4d666 partially cured uprious migrations, but when there are

> completely idle groups on a lightly loaded processor, and there is

> a buddy pair occupying the busiest group, we will not attempt to

> migrate due to select_idle_sibling() buddy placement, leaving the

> busiest queue with one task.  We skip balancing, but increment

> nr_balance_failed until we kick active balancing, and bounce a

> buddy pair endlessly, demolishing throughput.

> 

> Increase group imbalance threshold to two when sd_llc_size == 2 to

> allow buddies to share L2 without affecting larger L3 processors.

> 

> Regression detected on X5472 box, which has 4 MC groups of 2 cores.

> 

> netperf -l 60 -H 127.0.0.1 -t UDP_STREAM -i5,1 -I 95,5

> pre:

> !!! WARNING

> !!! Desired confidence was not achieved within the specified iterations.

> !!! This implies that there was variability in the test environment that

> !!! must be investigated before going further.

> !!! Confidence intervals: Throughput      : 66.421%

> !!!                       Local CPU util  : 0.000%

> !!!                       Remote CPU util : 0.000%

> 

> Socket  Message  Elapsed      Messages                

> Size    Size     Time         Okay Errors   Throughput

> bytes   bytes    secs            #      #   10^6bits/sec

> 

> 212992   65507   60.00     1779143      0    15539.49

> 212992           60.00     1773551           15490.65

> 

> post:

> Socket  Message  Elapsed      Messages                

> Size    Size     Time         Okay Errors   Throughput

> bytes   bytes    secs            #      #   10^6bits/sec

> 

> 212992   65507   60.00     3719377      0    32486.01

> 212992           60.00     3717492           32469.54

> 

> Signed-off-by: Mike Galbraith <mgalbraith@suse.de>

> Fixes: caeb178c sched/fair: Make update_sd_pick_busiest() return 'true' on a busier sd

> Cc: <stable@vger.kernel.org> # v3.18+

> ---

>  kernel/sched/fair.c |   17 ++++++++++++-----

>  1 file changed, 12 insertions(+), 5 deletions(-)

> 

> --- a/kernel/sched/fair.c

> +++ b/kernel/sched/fair.c

> @@ -7249,12 +7249,19 @@ static struct sched_group *find_busiest_

>  		 * This cpu is idle. If the busiest group is not overloaded

>  		 * and there is no imbalance between this and busiest group

>  		 * wrt idle cpus, it is balanced. The imbalance becomes

> -		 * significant if the diff is greater than 1 otherwise we

> -		 * might end up to just move the imbalance on another group

> +		 * significant if the diff is greater than 1 for most CPUs,

> +		 * or 2 for older CPUs having multiple groups of 2 cores

> +		 * sharing an L2, otherwise we may end up uselessly moving

> +		 * the imbalance to another group, or starting a tug of war

> +		 * with idle L2 groups constantly ripping communicating

> +		 * tasks apart, and no L3 to mitigate the cache miss pain.

>  		 */

> -		if ((busiest->group_type != group_overloaded) &&

> -				(local->idle_cpus <= (busiest->idle_cpus + 1)))

> -			goto out_balanced;

> +		if (busiest->group_type != group_overloaded) {

> +			int imbalance = __this_cpu_read(sd_llc_size) == 2 ? 2 : 1;

> +

> +			if (local->idle_cpus <= busiest->idle_cpus + imbalance)

> +				goto out_balanced;

> +		}

>  	} else {

>  		/*

>  		 * In the CPU_NEWLY_IDLE, CPU_NOT_IDLE cases, use

Comments

Vincent Guittot Sept. 6, 2016, 1:42 p.m. UTC | #1 
On 6 September 2016 at 15:07, Mike Galbraith <umgwanakikbuti@gmail.com> wrote:
> On Tue, 2016-09-06 at 15:01 +0200, Vincent Guittot wrote:

>> Le Monday 05 Sep 2016 à 18:26:53 (+0200), Mike Galbraith a écrit :

>> > Coming back to this, how about this instead, only increase the group

>> > imbalance threshold when sd_llc_size == 2.  Newer L3 equipped

>> > processors then aren't affected.

>> >

>>

>> Not sure that all systems with sd_llc_size == 2 wants this behavior.

>>

>> Why not adding a sched_feature for changing the 2nd half of the test

>> for some systems ?

>

> Because users won't know, and shouldn't need to know that they need to

> flip that switch.


fair enough

so how can we detect this specific system configuration ?

sd_llc_size == 2 is not enough IMHO

>

>         -Mike
diff mbox

Patch

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4185e0a..65c9363 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -7395,9 +7395,14 @@  static struct sched_group *find_busiest_group(struct lb_env *env)
 		 * significant if the diff is greater than 1 otherwise we
 		 * might end up to just move the imbalance on another group
 		 */
-		if ((busiest->group_type != group_overloaded) &&
-				(local->idle_cpus <= (busiest->idle_cpus + 1)))
-			goto out_balanced;
+		if (busiest->group_type != group_overloaded) {
+			int imbalance = 1;
+			if (!sched_feat(BALANCE_IDLE_CPUS))
+				imbalance = __this_cpu_read(sd_llc_size);
+
+			if (local->idle_cpus <= busiest->idle_cpus + imbalance)
+				goto out_balanced;
+		}
 	} else {
 		/*
 		 * In the CPU_NEWLY_IDLE, CPU_NOT_IDLE cases, use
diff --git a/kernel/sched/features.h b/kernel/sched/features.h
index 69631fa..16c34ec 100644
--- a/kernel/sched/features.h
+++ b/kernel/sched/features.h
@@ -69,3 +69,10 @@  SCHED_FEAT(RT_RUNTIME_SHARE, true)
 SCHED_FEAT(LB_MIN, false)
 SCHED_FEAT(ATTACH_AGE_LOAD, true)
 
+/*
+ * Try to balance the number of idle CPUs in each group to minimize contention
+ * on shared ressources. Nevertheless, some older systems without L3 seems to
+ * prefer to share resource for minimizing the migration between groups
+ */
+SCHED_FEAT(BALANCE_IDLE_CPUS, true)
+