Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group

From	Vincent Guittot <vincent.guittot@linaro.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group
Date	2016-12-08 15:40 +0100
Message-ID	<sM81z-7fI-5@gated-at.bofh.it> (permalink)
References	(2 earlier) <sK9l8-4hE-1@gated-at.bofh.it> <sKqvE-6H7-9@gated-at.bofh.it> <sKXKV-2Ja-9@gated-at.bofh.it> <sL1ER-58Y-15@gated-at.bofh.it> <sM7Id-795-3@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On 8 December 2016 at 15:09, Matt Fleming <matt@codeblueprint.co.uk> wrote:
> On Mon, 05 Dec, at 01:35:46PM, Matt Fleming wrote:
>> On Mon, 05 Dec, at 10:27:36AM, Vincent Guittot wrote:
>> >
>> > Hi Matt,
>> >
>> > Thanks for the results.
>> >
>> > During the review, it has been pointed out by Morten that the test condition
>> > (100*this_avg_load < imbalance_scale*min_avg_load) makes more sense than
>> > (100*min_avg_load > imbalance_scale*this_avg_load). But i see lower
>> > performances with this change. Coud you run tests with the change below on
>> > top of the patchset ?
>> >
>> > ---
>> >  kernel/sched/fair.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> > index e8d1ae7..0129fbb 100644
>> > --- a/kernel/sched/fair.c
>> > +++ b/kernel/sched/fair.c
>> > @@ -5514,7 +5514,7 @@ find_idlest_group(struct sched_domain *sd, struct task_struct *p,
>> >     if (!idlest ||
>> >         (min_runnable_load > (this_runnable_load + imbalance)) ||
>> >         ((this_runnable_load < (min_runnable_load + imbalance)) &&
>> > -                   (100*min_avg_load > imbalance_scale*this_avg_load)))
>> > +                   (100*this_avg_load < imbalance_scale*min_avg_load)))
>> >             return NULL;
>> >     return idlest;
>> >  }
>>
>> Queued for testing.
>
> Most of the machines didn't notice the difference with this new patch.
> However, I did come across one test that showed a negative change,

OK so you don't see perf impact between the 2 test condition except
for the machine below
So IMHO, we should use the new test condition which tries to keep task
local if there is no other CPU that is obviously less loaded.

I'm going to prepare a new version of the patchset and apply all comments

Thanks
Vincent

>
>
> hackbench-thread-pipes
>                         4.9.0-rc6             4.9.0-rc6             4.9.0-rc6             4.9.0-rc6
>                         tip-sched      fix-fig-for-fork               fix-sig   fix-fig-for-fork-v2
> Amean    1       0.1266 (  0.00%)      0.1269 ( -0.23%)      0.1287 ( -1.69%)      0.1357 ( -7.22%)
> Amean    4       0.4989 (  0.00%)      0.5174 ( -3.72%)      0.5251 ( -5.27%)      0.5117 ( -2.58%)
> Amean    7       0.8510 (  0.00%)      0.8517 ( -0.08%)      0.8964 ( -5.34%)      0.8801 ( -3.42%)
> Amean    12      1.0699 (  0.00%)      1.0484 (  2.00%)      1.0147 (  5.15%)      1.0759 ( -0.56%)
> Amean    21      1.2816 (  0.00%)      1.2140 (  5.27%)      1.1879 (  7.31%)      1.2414 (  3.13%)
> Amean    30      1.4440 (  0.00%)      1.4003 (  3.03%)      1.3969 (  3.26%)      1.4057 (  2.65%)
> Amean    48      1.5773 (  0.00%)      1.5983 ( -1.33%)      1.3984 ( 11.34%)      1.5624 (  0.94%)
> Amean    79      2.2343 (  0.00%)      2.3066 ( -3.24%)      2.0053 ( 10.25%)      2.2630 ( -1.29%)
> Amean    96      2.6736 (  0.00%)      2.4313 (  9.06%)      2.4181 (  9.55%)      2.4717 (  7.55%)
>
>            4.9.0-rc6   4.9.0-rc6   4.9.0-rc6   4.9.0-rc6
>            tip-schedfix-fig-for-fork     fix-sigfix-fig-for-fork-v2
> User          129.53      128.64      127.70      131.00
> System       1784.54     1744.21     1654.08     1744.00
> Elapsed        92.07       90.44       86.95       91.00
>
> Looking at the 48 and 79 groups rows for mean there's a noticeable
> drop off in performance of ~10%, which should be outside of the noise
> for this test. This is a 2 socket, 4 NUMA node (yes, really), 24 cpus
> AMD opteron circa 2010.
>
> Given the age of this machine, I don't think it's worth holding up the
> patch.

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH 2/2 v2] sched: use load_avg for selecting idlest group Vincent Guittot <vincent.guittot@linaro.org> - 2016-11-25 16:40 +0100
  Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Morten Rasmussen <morten.rasmussen@arm.com> - 2016-11-30 13:50 +0100
    Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Vincent Guittot <vincent.guittot@linaro.org> - 2016-11-30 15:00 +0100
      Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Vincent Guittot <vincent.guittot@linaro.org> - 2016-11-30 15:00 +0100
        Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Morten Rasmussen <morten.rasmussen@arm.com> - 2016-11-30 15:30 +0100
          Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Vincent Guittot <vincent.guittot@linaro.org> - 2016-12-02 16:30 +0100
            Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Matt Fleming <matt@codeblueprint.co.uk> - 2016-12-02 23:30 +0100
      Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Morten Rasmussen <morten.rasmussen@arm.com> - 2016-11-30 15:30 +0100
  Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Brendan Gregg <brendan.d.gregg@gmail.com> - 2016-12-03 04:40 +0100
    Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Matt Fleming <matt@codeblueprint.co.uk> - 2016-12-03 23:00 +0100
      Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Vincent Guittot <vincent.guittot@linaro.org> - 2016-12-05 10:30 +0100
        Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Matt Fleming <matt@codeblueprint.co.uk> - 2016-12-05 14:40 +0100
          Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Matt Fleming <matt@codeblueprint.co.uk> - 2016-12-08 15:20 +0100
            Re: [PATCH 2/2 v2] sched: use load_avg for selecting idlest group Vincent Guittot <vincent.guittot@linaro.org> - 2016-12-08 15:40 +0100

csiph-web