Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1502877 > unrolled thread

Re: [PATCH] sched/fair: Do not decay new task load on first enqueue

Started byMatt Fleming <matt@codeblueprint.co.uk>
First post2016-10-18 12:30 +0200
Last post2016-10-19 17:30 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] sched/fair: Do not decay new task load on first enqueue Matt Fleming <matt@codeblueprint.co.uk> - 2016-10-18 12:30 +0200
    Re: [PATCH] sched/fair: Do not decay new task load on first enqueue Peter Zijlstra <peterz@infradead.org> - 2016-10-18 13:20 +0200
      Re: [PATCH] sched/fair: Do not decay new task load on first enqueue Matt Fleming <matt@codeblueprint.co.uk> - 2016-10-18 13:40 +0200
        Re: [PATCH] sched/fair: Do not decay new task load on first enqueue Peter Zijlstra <peterz@infradead.org> - 2016-10-18 14:20 +0200
          Re: [PATCH] sched/fair: Do not decay new task load on first enqueue Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-19 08:40 +0200
            Re: [PATCH] sched/fair: Do not decay new task load on first enqueue Peter Zijlstra <peterz@infradead.org> - 2016-10-19 17:30 +0200

#1502877 — Re: [PATCH] sched/fair: Do not decay new task load on first enqueue

FromMatt Fleming <matt@codeblueprint.co.uk>
Date2016-10-18 12:30 +0200
SubjectRe: [PATCH] sched/fair: Do not decay new task load on first enqueue
Message-ID<stzOG-lH-17@gated-at.bofh.it>
On Tue, 11 Oct, at 11:24:53AM, Matt Fleming wrote:
> 
> That's a lot more costly cross-DIE migrations. I think this patch is
> along the right lines, but there's something fishy happening on this
> box.

I wasn't really able to track down why this machine regressed, and the
patch shows enough improvement for other boxes that I think it's worth
sticking in tip/sched/core.

I saw a number of double digit gains for hackbench with this patch.
Here's a 4-cpu box that shows improvements in arithmetic mean and some
nice improvements to the stddev,

hackbench-process-pipes
                            4.8.0                 4.8.0
                          vanilla  find-idle-group-v1r1
Amean    1       3.2580 (  0.00%)      3.0607 (  6.06%)
Amean    3       8.0387 (  0.00%)      8.2027 ( -2.04%)
Amean    5      12.0507 (  0.00%)     12.1703 ( -0.99%)
Amean    7      16.0027 (  0.00%)     16.4567 ( -2.84%)
Amean    12     26.9030 (  0.00%)     21.7767 ( 19.05%)
Amean    16     33.4127 (  0.00%)     28.9877 ( 13.24%)
Stddev   1       0.1383 (  0.00%)      0.1787 (-29.22%)
Stddev   3       0.1297 (  0.00%)      0.1677 (-29.29%)
Stddev   5       0.5495 (  0.00%)      0.4549 ( 17.22%)
Stddev   7       0.7401 (  0.00%)      0.6161 ( 16.75%)
Stddev   12      2.3187 (  0.00%)      0.7102 ( 69.37%)
Stddev   16      4.0734 (  0.00%)      2.9364 ( 27.91%)

And a 60-cpu box with similar improvements,

hackbench-process-sockets
                             4.8.0                 4.8.0
                           vanilla  find-idle-group-v1r1
Amean    1       32.8810 (  0.00%)      2.0590 ( 93.74%)
Amean    4       34.8663 (  0.00%)      3.3177 ( 90.48%)
Amean    7       34.1243 (  0.00%)      4.3323 ( 87.30%)
Amean    12      36.8373 (  0.00%)      5.3650 ( 85.44%)
Amean    21      29.0333 (  0.00%)      6.0280 ( 79.24%)
Amean    30      27.2807 (  0.00%)      7.2653 ( 73.37%)
Amean    48      23.2453 (  0.00%)      9.9810 ( 57.06%)
Amean    79      29.6660 (  0.00%)     16.6680 ( 43.81%)
Amean    110     41.3680 (  0.00%)     23.1787 ( 43.97%)
Amean    141     45.3720 (  0.00%)     29.4130 ( 35.17%)
Amean    172     45.6403 (  0.00%)     34.3783 ( 24.68%)
Amean    203     47.6120 (  0.00%)     40.0063 ( 15.97%)
Amean    234     50.1703 (  0.00%)     43.4503 ( 13.39%)
Amean    240     50.1110 (  0.00%)     44.4660 ( 11.26%)
Stddev   1        0.9714 (  0.00%)      0.6617 ( 31.88%)
Stddev   4        0.4459 (  0.00%)      0.2943 ( 34.01%)
Stddev   7        0.3091 (  0.00%)      0.2722 ( 11.94%)
Stddev   12       0.4603 (  0.00%)      0.1424 ( 69.05%)
Stddev   21       1.3647 (  0.00%)      0.1566 ( 88.52%)
Stddev   30       4.1277 (  0.00%)      0.0481 ( 98.84%)
Stddev   48       1.3689 (  0.00%)      0.4968 ( 63.71%)
Stddev   79       2.0772 (  0.00%)      0.5309 ( 74.44%)
Stddev   110      4.5655 (  0.00%)      0.1846 ( 95.96%)
Stddev   141      4.2637 (  0.00%)      0.6675 ( 84.34%)
Stddev   172      1.5741 (  0.00%)      1.0242 ( 34.93%)
Stddev   203      1.0973 (  0.00%)      1.4043 (-27.98%)
Stddev   234      2.2048 (  0.00%)      0.8709 ( 60.50%)
Stddev   240      1.2849 (  0.00%)      0.2688 ( 79.08%)

[toc] | [next] | [standalone]


#1502900

FromPeter Zijlstra <peterz@infradead.org>
Date2016-10-18 13:20 +0200
Message-ID<stAB3-T8-7@gated-at.bofh.it>
In reply to#1502877
On Tue, Oct 18, 2016 at 11:29:37AM +0100, Matt Fleming wrote:
> On Tue, 11 Oct, at 11:24:53AM, Matt Fleming wrote:
> > 
> > That's a lot more costly cross-DIE migrations. I think this patch is
> > along the right lines, but there's something fishy happening on this
> > box.
> 
> I wasn't really able to track down why this machine regressed, and the
> patch shows enough improvement for other boxes that I think it's worth
> sticking in tip/sched/core.
> 

I'm entirely lost as to which patch we're talking about by now ;-)

[toc] | [prev] | [next] | [standalone]


#1502923

FromMatt Fleming <matt@codeblueprint.co.uk>
Date2016-10-18 13:40 +0200
Message-ID<stAUp-108-19@gated-at.bofh.it>
In reply to#1502900
On Tue, 18 Oct, at 01:10:17PM, Peter Zijlstra wrote:
> 
> I'm entirely lost as to which patch we're talking about by now ;-)

Heh, this one from Vincent,

  https://lkml.kernel.org/r/20161010173440.GA28945@linaro.org

[toc] | [prev] | [next] | [standalone]


#1502944

FromPeter Zijlstra <peterz@infradead.org>
Date2016-10-18 14:20 +0200
Message-ID<stBx7-1xw-9@gated-at.bofh.it>
In reply to#1502923
On Tue, Oct 18, 2016 at 12:29:57PM +0100, Matt Fleming wrote:
> On Tue, 18 Oct, at 01:10:17PM, Peter Zijlstra wrote:
> > 
> > I'm entirely lost as to which patch we're talking about by now ;-)
> 
> Heh, this one from Vincent,
> 
>   https://lkml.kernel.org/r/20161010173440.GA28945@linaro.org

Ah, right.

Seems like a sensible thing to do, and I suppose I should go finish my
(and yours) update_rq_clock() patches that supersede the patch referred
to in that thing and is depended upon.


It might make sense to have helper functions to evaluate those
conditions, because currently there's two instances of each, once in the
branch selection and then again (but inverted, we miss the == case fwiw)
in the return NULL case.

[toc] | [prev] | [next] | [standalone]


#1503580

FromVincent Guittot <vincent.guittot@linaro.org>
Date2016-10-19 08:40 +0200
Message-ID<stSHD-5Ix-5@gated-at.bofh.it>
In reply to#1502944
On 18 October 2016 at 14:15, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Oct 18, 2016 at 12:29:57PM +0100, Matt Fleming wrote:
>> On Tue, 18 Oct, at 01:10:17PM, Peter Zijlstra wrote:
>> >
>> > I'm entirely lost as to which patch we're talking about by now ;-)
>>
>> Heh, this one from Vincent,
>>
>>   https://lkml.kernel.org/r/20161010173440.GA28945@linaro.org
>
> Ah, right.
>
> Seems like a sensible thing to do, and I suppose I should go finish my
> (and yours) update_rq_clock() patches that supersede the patch referred
> to in that thing and is depended upon.
>
>
> It might make sense to have helper functions to evaluate those

The main issue is the number of parameters used in these conditions
that makes helper function not really more readable.

> conditions, because currently there's two instances of each, once in the
> branch selection and then again (but inverted, we miss the == case fwiw)

not sure to catch the comment about inverted and miss the == case
The test splits runnable_load_avg is 3 ranges:
[0 .. (min_runnable_load - imbalance)] : use the
runnable_loab_avg/this_runnable_load which is significantly smaller
] (min_runnable_load - imbalance) .. (min_runnable_load + imbalance) [
: min_runnable_load and runnable_loab_avg/this_runnable_load are close
so we compare min_load_avg with avg_load/this_avg_load to choose
[(min_runnable_load + imbalance) .. ULONG_MAX] : use min_runnable_load

The condition is used when we look for the best other group in the
sched_domain and  to compare the local group with this best other
group

> in the return NULL case.
>
>
>

[toc] | [prev] | [next] | [standalone]


#1503885

FromPeter Zijlstra <peterz@infradead.org>
Date2016-10-19 17:30 +0200
Message-ID<su0Yx-33U-7@gated-at.bofh.it>
In reply to#1503580
On Wed, Oct 19, 2016 at 08:38:12AM +0200, Vincent Guittot wrote:
> > It might make sense to have helper functions to evaluate those
> 
> The main issue is the number of parameters used in these conditions
> that makes helper function not really more readable.

Fair enough I suppose..

> > conditions, because currently there's two instances of each, once in the
> > branch selection and then again (but inverted, we miss the == case fwiw)
> 
> not sure to catch the comment about inverted and miss the == case

Oh, you're right. Which proves my point on this not being entirely
readable :/

Initially I thought the things were of the form:

	else if (ineq) {
	}

	...

	if (... || !ineq || ...)


Which would get you things like: a<b, !a<b := a>b, and leave a==b
undefined. But if I put them along side one another like:


+		} else if (min_runnable_load > (runnable_load + imbalance)) {

+	                  (min_runnable_load > (this_runnable_load + imbalance)) ||


+		} else if ((runnable_load < (min_runnable_load + imbalance)) &&
+				(100*min_avg_load > sd->imbalance_pct*avg_load)) {

+	             ((this_runnable_load < (min_runnable_load + imbalance)) &&
+			        (100*min_avg_load > sd->imbalance_pct*this_avg_load)))

We can see this is not in fact the case.

Blergh, I also cannot see a pretty way to increase readability here,
because while they have the same general shape, there's this small
variation with this_*.

A well..

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web