lkml.org 
[lkml]   [2019]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 8/8] sched/fair: use utilization to select misfit task
On Thu, 1 Aug 2019 at 18:27, Valentin Schneider
<valentin.schneider@arm.com> wrote:
>
> On 01/08/2019 15:40, Vincent Guittot wrote:
> > @@ -8261,7 +8261,7 @@ static bool update_sd_pick_busiest(struct lb_env *env,
> > * If we have more than one misfit sg go with the
> > * biggest misfit.
> > */
> > - if (sgs->group_misfit_task_load < busiest->group_misfit_task_load)
> > + if (sgs->group_misfit_task_util < busiest->group_misfit_task_util)
> > return false;
>
> I previously said this change would render the maximization useless, but I
> had forgotten one thing: with PELT time scaling, task utilization can go
> above its CPU's capacity.
>
> So if you have two LITTLE CPUs running a busy loop (misfit task) each, the
> one that's been running the longest would have the highest utilization
> (assuming they haven't reached 1024 yet). In other words "utilizations
> above the capacity_margin can't be compared" doesn't really stand.
>
> Still, maximizing load would lead us there. Furthermore, if we have to pick
> between two rqs with misfit tasks, I still believe we should pick the one
> with the highest load, not the highest utilization.
>
> We could keep load and fix the issue of detaching the wrong task with
> something like:
>
> -----8<-----
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 53e64a7b2ae0..bfc2b624ee98 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -7489,12 +7489,8 @@ static int detach_tasks(struct lb_env *env)
> case migrate_misfit:
> load = task_h_load(p);
>
> - /*
> - * utilization of misfit task might decrease a bit
> - * since it has been recorded. Be conservative in the
> - * condition.
> - */
> - if (load < env->imbalance)
> + /* This is not a misfit task */
> + if (task_fits_capacity(p, capacity_of(env->src_cpu)))
> goto next;

This could be a solution for make sure to pull only misfit task and
keep using load

>
> env->imbalance = 0;
> ----->8-----
>
> However what would be *even* better IMO would be:
>
> -----8<-----
> @@ -8853,6 +8853,7 @@ voluntary_active_balance(struct lb_env *env)
> return 1;
> }
>
> + /* XXX: make sure current is still a misfit task? */
> if (env->balance_type == migrate_misfit)
> return 1;
>
> @@ -8966,6 +8967,20 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> env.src_rq = busiest;
>
> ld_moved = 0;
> +
> + /*
> + * Misfit tasks are only misfit if they are currently running, see
> + * update_misfit_status().
> + *
> + * - If they're not running, we'll get an opportunity at wakeup to
> + * migrate them to a bigger CPU.
> + * - If they're running, we need to active balance them to a bigger CPU.
> + *
> + * Do the smart thing and go straight for active balance.
> + */
> + if (env->balance_type == migrate_misfit)
> + goto active_balance;
> +

This looks ugly and add a new bypass which this patchset tries to remove
This doesn't work if your misfit task has been preempted by another
one during the load balance and waiting for the runqueue


> if (busiest->nr_running > 1) {
> /*
> * Attempt to move tasks. If find_busiest_group has found
> @@ -9074,7 +9089,7 @@ static int load_balance(int this_cpu, struct rq *this_rq,
> goto out_all_pinned;
> }
> }
> -
> +active_balance:
> if (!ld_moved) {
> schedstat_inc(sd->lb_failed[idle]);
> /*
> ----->8-----

\
 
 \ /
  Last update: 2019-08-02 10:29    [W:0.134 / U:1.636 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site