lkml.org 
[lkml]   [2019]   [Oct]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 04/10] sched/fair: rework load_balance
From
Date
On 02/10/2019 10:23, Vincent Guittot wrote:
> On Tue, 1 Oct 2019 at 18:53, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>>
>> On 01/10/2019 10:14, Vincent Guittot wrote:
>>> On Mon, 30 Sep 2019 at 18:24, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>>>>
>>>> Hi Vincent,
>>>>
>>>> On 19/09/2019 09:33, Vincent Guittot wrote:
>>
> [...]
>
>>
>>>>> + if (busiest->group_weight == 1 || sds->prefer_sibling) {
>>>>> + /*
>>>>> + * When prefer sibling, evenly spread running tasks on
>>>>> + * groups.
>>>>> + */
>>>>> + env->balance_type = migrate_task;
>>>>> + env->imbalance = (busiest->sum_h_nr_running - local->sum_h_nr_running) >> 1;
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * If there is no overload, we just want to even the number of
>>>>> + * idle cpus.
>>>>> + */
>>>>> + env->balance_type = migrate_task;
>>>>> + env->imbalance = max_t(long, 0, (local->idle_cpus - busiest->idle_cpus) >> 1);
>>>>
>>>> Why do we need a max_t(long, 0, ...) here and not for the 'if
>>>> (busiest->group_weight == 1 || sds->prefer_sibling)' case?
>>>
>>> For env->imbalance = (busiest->sum_h_nr_running - local->sum_h_nr_running) >> 1;
>>>
>>> either we have sds->prefer_sibling && busiest->sum_nr_running >
>>> local->sum_nr_running + 1
>>
>> I see, this corresponds to
>>
>> /* Try to move all excess tasks to child's sibling domain */
>> if (sds.prefer_sibling && local->group_type == group_has_spare &&
>> busiest->sum_h_nr_running > local->sum_h_nr_running + 1)
>> goto force_balance;
>>
>> in find_busiest_group, I assume.
>
> yes. But it seems that I missed a case:
>
> prefer_sibling is set
> busiest->sum_h_nr_running <= local->sum_h_nr_running + 1 so we skip
> goto force_balance above
> But env->idle != CPU_NOT_IDLE and local->idle_cpus >
> (busiest->idle_cpus + 1) so we also skip goto out_balance and finally
> call calculate_imbalance()
>
> in calculate_imbalance with prefer_sibling set, imbalance =
> (busiest->sum_h_nr_running - local->sum_h_nr_running) >> 1;
>
> so we probably want something similar to max_t(long, 0,
> (busiest->sum_h_nr_running - local->sum_h_nr_running) >> 1)

Makes sense.

Caught a couple of

[ 369.310464] 0-3->4-7 2->5 env->imbalance = 2147483646
[ 369.310796] 0-3->4-7 2->4 env->imbalance = 2147483647

in this if condition on h620 running hackbench.

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