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

From: Michael Neuling
Date: Wed Mar 23 2016 - 19:40:07 EST


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 */