mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Phil Auld <pauld@redhat.com>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	Quentin Perret <quentin.perret@arm.com>,
	Morten Rasmussen <Morten.Rasmussen@arm.com>,
	Hillf Danton <hdanton@sina.com>
Subject: Re: [PATCH v3 04/10] sched/fair: rework load_balance
Date: Wed, 2 Oct 2019 11:24:51 +0200	[thread overview]
Message-ID: <1aea9422-a164-ceb6-5b8f-da728718bab9@arm.com> (raw)
In-Reply-To: <CAKfTPtAOErKG3XJDbLQEGgqs485ad4R7xc3k1UA9h5092GnkqQ@mail.gmail.com>

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.

  reply	other threads:[~2019-10-02  9:25 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-19  7:33 [PATCH v3 0/8] sched/fair: rework the CFS load balance Vincent Guittot
2019-09-19  7:33 ` [PATCH v3 01/10] sched/fair: clean up asym packing Vincent Guittot
2019-09-27 23:57   ` Rik van Riel
2019-09-19  7:33 ` [PATCH v3 02/10] sched/fair: rename sum_nr_running to sum_h_nr_running Vincent Guittot
2019-09-27 23:59   ` Rik van Riel
2019-10-01 17:11   ` Valentin Schneider
2019-09-19  7:33 ` [PATCH v3 03/10] sched/fair: remove meaningless imbalance calculation Vincent Guittot
2019-09-28  0:05   ` Rik van Riel
2019-10-01 17:12   ` Valentin Schneider
2019-10-02  6:28     ` Vincent Guittot
2019-09-19  7:33 ` [PATCH v3 04/10] sched/fair: rework load_balance Vincent Guittot
2019-09-30  1:12   ` Rik van Riel
2019-09-30  7:44     ` Vincent Guittot
2019-09-30 16:24   ` Dietmar Eggemann
2019-10-01  8:14     ` Vincent Guittot
2019-10-01 16:52       ` Dietmar Eggemann
2019-10-02  6:44         ` Vincent Guittot
2019-10-02  9:21           ` Dietmar Eggemann
2019-10-08 13:02             ` Peter Zijlstra
2019-10-02  8:23         ` Vincent Guittot
2019-10-02  9:24           ` Dietmar Eggemann [this message]
2019-10-01  8:15   ` Dietmar Eggemann
2019-10-01  9:14     ` Vincent Guittot
2019-10-01 16:57       ` Dietmar Eggemann
2019-10-01 17:47   ` Valentin Schneider
2019-10-02  8:30     ` Vincent Guittot
2019-10-02 10:47       ` Valentin Schneider
2019-10-08 14:16         ` Peter Zijlstra
2019-10-08 14:34           ` Valentin Schneider
2019-10-08 15:30             ` Vincent Guittot
2019-10-08 15:48               ` Valentin Schneider
2019-10-08 17:39               ` Peter Zijlstra
2019-10-08 18:45                 ` Vincent Guittot
2019-10-08 16:33             ` Peter Zijlstra
2019-10-08 16:39               ` Valentin Schneider
2019-10-08 17:36                 ` Valentin Schneider
2019-10-08 17:55   ` Peter Zijlstra
2019-10-08 18:47     ` Vincent Guittot
2019-10-16  7:21   ` Parth Shah
2019-10-16 11:56     ` Vincent Guittot
2019-10-18  5:34       ` Parth Shah
2019-09-19  7:33 ` [PATCH v3 05/10] sched/fair: use rq->nr_running when balancing load Vincent Guittot
2019-09-19  7:33 ` [PATCH v3 06/10] sched/fair: use load instead of runnable load in load_balance Vincent Guittot
2019-09-19  7:33 ` [PATCH v3 07/10] sched/fair: evenly spread tasks when not overloaded Vincent Guittot
2019-09-19  7:33 ` [PATCH v3 08/10] sched/fair: use utilization to select misfit task Vincent Guittot
2019-10-01 17:12   ` Valentin Schneider
2019-09-19  7:33 ` [PATCH v3 09/10] sched/fair: use load instead of runnable load in wakeup path Vincent Guittot
2019-10-07 15:14   ` Rik van Riel
2019-10-07 15:27     ` Vincent Guittot
2019-10-07 18:06       ` Rik van Riel
2019-09-19  7:33 ` [PATCH v3 10/10] sched/fair: optimize find_idlest_group Vincent Guittot
2019-10-08 14:32 ` [PATCH v3 0/8] sched/fair: rework the CFS load balance Phil Auld
2019-10-08 15:53   ` Vincent Guittot
2019-10-09 19:33     ` Phil Auld
2019-10-10  8:20       ` Vincent Guittot
2019-10-16  7:21 ` Parth Shah
2019-10-16 11:51   ` Vincent Guittot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1aea9422-a164-ceb6-5b8f-da728718bab9@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=Morten.Rasmussen@arm.com \
    --cc=hdanton@sina.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=quentin.perret@arm.com \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®