Re: [PATCH 1/3] sched/fair: Fix asym packing to select correct cpu

[Michael Neuling]

On Wed, 2016-03-23 at 17:04 +0530, Srikar Dronamraju wrote:
> If asymmetric packing is used when target cpu is busy,
> update_sd_pick_busiest(), can select a lightly loaded cpu.
> find_busiest_group() has checks to ensure asym packing is only used
> when target cpu is not busy.  However it may not be able to avoid a
> lightly loaded cpu selected by update_sd_pick_busiest from being
> selected as source cpu for eventual load balancing.
> 
> Also when using asymmetric scheduling, always select higher cpu as
> source cpu for load balancing.
> 
> While doing this change, move the check to see if target cpu is busy
> into check_asym_packing().
> 
> 1. Record per second ebizzy (32 threads) on a 64 cpu power 7 box. (5 iterations)
> 4.5.0-master/ebizzy_32.out
>     N           Min           Max        Median           Avg        Stddev
> x   5       5205896      17260530      12141204      10759008       4765419
> 
> 4.5.0-master-asym-changes/ebizzy_32.out
>     N           Min           Max        Median           Avg        Stddev
> x   5       7044349      19112876      17440718      14947658       5263970
> 
> 2. Record per second ebizzy (32 threads) on a 64 cpu power 7 box. (5 iterations)
> 4.5.0-master/ebizzy_64.out
>     N           Min           Max        Median           Avg        Stddev
> x   5       5400083      14091418       8673907     8872662.4     3389746.8
> 
> 4.5.0-master-asym-changes/ebizzy_64.out
>     N           Min           Max        Median           Avg        Stddev
> x   5       7533907      17232623      15083583      13364894     3776877.9
> 
> 3. Record per second ebizzy (32 threads) on a 64 cpu power 7 box. (5 iterations)
> 4.5.0-master/ebizzy_128.out
>     N           Min           Max        Median           Avg        Stddev
> x   5      35328039      41642699      37564951      38378409       2671280
> 
> 4.5.0-master-asym-changes/ebizzy_128.out
>     N           Min           Max        Median           Avg        Stddev
> x   5      37102220      42736809      38442478      39529626     2298389.4

I'm not sure how to interpret these.  Any chance you can give a summary of
what these results mean?

> Signed-off-by: Srikar Dronamraju <srikar@xxxxxxxxxxxxxxxxxx>

FWIW, this still passes my scheduler tests on POWER7, but they weren't 
failing before anyway.

Mikey

> ---
>  kernel/sched/fair.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 56b7d4b..9abfb16 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6517,6 +6517,8 @@ static bool update_sd_pick_busiest(struct lb_env *env,
>  	if (!(env->sd->flags & SD_ASYM_PACKING))
>  		return true;
>  
> +	if (env->idle == CPU_NOT_IDLE)
> +		return true;
>  	/*
>  	 * ASYM_PACKING needs to move all the work to the lowest
>  	 * numbered CPUs in the group, therefore mark all groups
> @@ -6526,7 +6528,7 @@ static bool update_sd_pick_busiest(struct lb_env *env,
>  		if (!sds->busiest)
>  			return true;
>  
> -		if (group_first_cpu(sds->busiest) > group_first_cpu(sg))
> +		if (group_first_cpu(sds->busiest) < group_first_cpu(sg))
>  			return true;
>  	}
>  
> @@ -6672,6 +6674,9 @@ static int check_asym_packing(struct lb_env *env, struct sd_lb_stats *sds)
>  	if (!(env->sd->flags & SD_ASYM_PACKING))
>  		return 0;
>  
> +	if (env->idle == CPU_NOT_IDLE)
> +		return 0;
> +
>  	if (!sds->busiest)
>  		return 0;
>  
> @@ -6864,8 +6869,7 @@ static struct sched_group *find_busiest_group(struct lb_env *env)
>  	busiest = &sds.busiest_stat;
>  
>  	/* ASYM feature bypasses nice load balance check */
> -	if ((env->idle == CPU_IDLE || env->idle == CPU_NEWLY_IDLE) &&
> -	    check_asym_packing(env, &sds))
> +	if (check_asym_packing(env, &sds))
>  		return sds.busiest;
>  
>  	/* There is no busy sibling group to pull tasks from */