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

Groups > linux.kernel > #1430041 > unrolled thread

Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and SPECjvm2008 benchmarks against 4.6 kernel

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: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-23 20:40 +0200
    Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-23 20:50 +0200
      Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Jirka Hladky <jhladky@redhat.com> - 2016-06-24 09:50 +0200
        Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 10:10 +0200
          Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Jirka Hladky <jhladky@redhat.com> - 2016-06-24 10:30 +0200
        Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Jirka Hladky <jhladky@redhat.com> - 2016-06-24 14:10 +0200
          Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Jirka Hladky <jhladky@redhat.com> - 2016-06-24 14:40 +0200
          Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 14:40 +0200
        Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 14:10 +0200
          Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Vincent Guittot <vincent.guittot@linaro.org> - 2016-06-24 14:50 +0200
            Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 15:10 +0200
              Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Vincent Guittot <vincent.guittot@linaro.org> - 2016-06-24 15:30 +0200
                Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 15:40 +0200
                Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 15:50 +0200
            Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Jirka Hladky <jhladky@redhat.com> - 2016-06-24 15:10 +0200
            Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 15:50 +0200
              Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Peter Zijlstra <peterz@infradead.org> - 2016-06-24 18:00 +0200
                Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and  SPECjvm2008 benchmarks against 4.6 kernel Jirka Hladky <jhladky@redhat.com> - 2016-06-25 00:20 +0200

#1430041 — Re: Kernel 4.7rc3 - Performance drop 30-40% for SPECjbb2005 and SPECjvm2008 benchmarks against 4.6 kernel

On Fri, Jun 17, 2016 at 01:04:23AM +0200, Jirka Hladky wrote:

> > What kind of config and userspace setup? Do you run this cruft in a
> > cgroup of sorts?
> 
>  No, we don't do any special setup except to control the number of threads.

OK, so I'm fairly certain you _do_ run in a cgroup, because its made
almost impossible not to these days.

Run:

	grep java /proc/sched_debug

while the thing is running. That'll show you the actual cgroup the stuff
is running in.

This modern Linux stuff stinks loads. And even Debian seems infected to
the point of almost being useless :-(

The _only_ reason I could reproduce was because I recently did an
upgrade of Debian Testing and I hadn't noticed just how messed up things
had become.

When I run it in the root cgroup (I had to kill cgmanager and reboot)
the numbers are just fine.

In any case, now I gotta go look at the cgroup code...

[toc] | [next] | [standalone]

#1430044

On Thu, Jun 23, 2016 at 08:33:18PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 17, 2016 at 01:04:23AM +0200, Jirka Hladky wrote:
> 
> > > What kind of config and userspace setup? Do you run this cruft in a
> > > cgroup of sorts?
> > 
> >  No, we don't do any special setup except to control the number of threads.
> 
> OK, so I'm fairly certain you _do_ run in a cgroup, because its made
> almost impossible not to these days.
> 
> Run:
> 
> 	grep java /proc/sched_debug
> 
> while the thing is running. That'll show you the actual cgroup the stuff
> is running in.

That'll end up looking something like:

root@ivb-ep:/usr/src/linux-2.6# grep java /proc/sched_debug
            java  2714     18270.634925        89   120         0.000000         1.490023         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
            java  2666     18643.629673         2   120         0.000000         0.063129         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
            java  2676     18655.652878         3   120         0.000000         0.077127         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
            java  2680     18655.683384         3   120         0.000000         0.082993         0.000000 0 0 /user.slice/user-0.slice/session-2.scope

which shows a 3 deep hierarchy. Clearly these people haven't the
faintest clue about the cost of what they're doing. This stuff ain't
free.

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

#1430429

Hi Peter,

thanks a lot for looking into it!

I have tried to disable autogroups

sysctl -w kernel.sched_autogroup_enabled=0

and I can confirm that performance is then back at level as in 4.6 kernel.

I have double checked default settings and

kernel.sched_autogroup_enabled

is by default ON both in 4.6 and 4.7 kernel.

Jirka

On Thu, Jun 23, 2016 at 8:43 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Jun 23, 2016 at 08:33:18PM +0200, Peter Zijlstra wrote:
>> On Fri, Jun 17, 2016 at 01:04:23AM +0200, Jirka Hladky wrote:
>>
>> > > What kind of config and userspace setup? Do you run this cruft in a
>> > > cgroup of sorts?
>> >
>> >  No, we don't do any special setup except to control the number of threads.
>>
>> OK, so I'm fairly certain you _do_ run in a cgroup, because its made
>> almost impossible not to these days.
>>
>> Run:
>>
>>       grep java /proc/sched_debug
>>
>> while the thing is running. That'll show you the actual cgroup the stuff
>> is running in.
>
> That'll end up looking something like:
>
> root@ivb-ep:/usr/src/linux-2.6# grep java /proc/sched_debug
>             java  2714     18270.634925        89   120         0.000000         1.490023         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
>             java  2666     18643.629673         2   120         0.000000         0.063129         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
>             java  2676     18655.652878         3   120         0.000000         0.077127         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
>             java  2680     18655.683384         3   120         0.000000         0.082993         0.000000 0 0 /user.slice/user-0.slice/session-2.scope
>
> which shows a 3 deep hierarchy. Clearly these people haven't the
> faintest clue about the cost of what they're doing. This stuff ain't
> free.
>
>

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

#1430442

On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
> I have double checked default settings and
> 
> kernel.sched_autogroup_enabled
> 
> is by default ON both in 4.6 and 4.7 kernel.

Yeah, if you enable that CONFIG its default enabled. In any case, I'll
go trawl through the cgroup code now. I spend yesterday looking at the
'wrong' part things.

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

#1430456

I had a look and

CONFIG_SCHED_AUTOGROUP=y

is used both in RHEL6 and RHEL7.  We compile the upstream kernels with
config derived from RHEL7 config file.

Jirka

On Fri, Jun 24, 2016 at 10:08 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
>> I have double checked default settings and
>>
>> kernel.sched_autogroup_enabled
>>
>> is by default ON both in 4.6 and 4.7 kernel.
>
> Yeah, if you enable that CONFIG its default enabled. In any case, I'll
> go trawl through the cgroup code now. I spend yesterday looking at the
> 'wrong' part things.
>
>

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

#1430622

Thank you Peter!

Should I apply it to v4.7-rc4 ?

Jirka

On Fri, Jun 24, 2016 at 2:02 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
>> Hi Peter,
>>
>> thanks a lot for looking into it!
>>
>> I have tried to disable autogroups
>>
>> sysctl -w kernel.sched_autogroup_enabled=0
>>
>> and I can confirm that performance is then back at level as in 4.6 kernel.
>
> So unless the heat has made me do really silly things, the below seems
> to cure things. Could you please verify?
>
>
> ---
>  kernel/sched/fair.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 22d64b3f5876..d4f6fb2f3057 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>          */
>         tg_weight = atomic_long_read(&tg->load_avg);
>         tg_weight -= cfs_rq->tg_load_avg_contrib;
> -       tg_weight += cfs_rq->load.weight;
> +       tg_weight += cfs_rq->avg.load_avg;
>
>         return tg_weight;
>  }
> @@ -2494,7 +2494,7 @@ static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
>         long tg_weight, load, shares;
>
>         tg_weight = calc_tg_weight(tg, cfs_rq);
> -       load = cfs_rq->load.weight;
> +       load = cfs_rq->avg.load_avg;
>
>         shares = (tg->shares * load);
>         if (tg_weight)

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

#1430631

OK, I have applied to v4.7-rc4 via git am

Compiling kernel, should have the results soon.

Jirka

On Fri, Jun 24, 2016 at 2:09 PM, Jirka Hladky <jhladky@redhat.com> wrote:
> Thank you Peter!
>
> Should I apply it to v4.7-rc4 ?
>
> Jirka
>
> On Fri, Jun 24, 2016 at 2:02 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
>>> Hi Peter,
>>>
>>> thanks a lot for looking into it!
>>>
>>> I have tried to disable autogroups
>>>
>>> sysctl -w kernel.sched_autogroup_enabled=0
>>>
>>> and I can confirm that performance is then back at level as in 4.6 kernel.
>>
>> So unless the heat has made me do really silly things, the below seems
>> to cure things. Could you please verify?
>>
>>
>> ---
>>  kernel/sched/fair.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 22d64b3f5876..d4f6fb2f3057 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>>          */
>>         tg_weight = atomic_long_read(&tg->load_avg);
>>         tg_weight -= cfs_rq->tg_load_avg_contrib;
>> -       tg_weight += cfs_rq->load.weight;
>> +       tg_weight += cfs_rq->avg.load_avg;
>>
>>         return tg_weight;
>>  }
>> @@ -2494,7 +2494,7 @@ static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
>>         long tg_weight, load, shares;
>>
>>         tg_weight = calc_tg_weight(tg, cfs_rq);
>> -       load = cfs_rq->load.weight;
>> +       load = cfs_rq->avg.load_avg;
>>
>>         shares = (tg->shares * load);
>>         if (tg_weight)

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

#1430632

On Fri, Jun 24, 2016 at 02:09:30PM +0200, Jirka Hladky wrote:
> Thank you Peter!
> 
> Should I apply it to v4.7-rc4 ?

It does indeed apply to v4.7-rc4, although I only tested it against
tip/master.

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

#1430623

On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
> Hi Peter,
> 
> thanks a lot for looking into it!
> 
> I have tried to disable autogroups
> 
> sysctl -w kernel.sched_autogroup_enabled=0
> 
> and I can confirm that performance is then back at level as in 4.6 kernel.

So unless the heat has made me do really silly things, the below seems
to cure things. Could you please verify?


---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 22d64b3f5876..d4f6fb2f3057 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
 	 */
 	tg_weight = atomic_long_read(&tg->load_avg);
 	tg_weight -= cfs_rq->tg_load_avg_contrib;
-	tg_weight += cfs_rq->load.weight;
+	tg_weight += cfs_rq->avg.load_avg;
 
 	return tg_weight;
 }
@@ -2494,7 +2494,7 @@ static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
 	long tg_weight, load, shares;
 
 	tg_weight = calc_tg_weight(tg, cfs_rq);
-	load = cfs_rq->load.weight;
+	load = cfs_rq->avg.load_avg;
 
 	shares = (tg->shares * load);
 	if (tg_weight)

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

#1430636

Hi Peter,

On 24 June 2016 at 14:02, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
>> Hi Peter,
>>
>> thanks a lot for looking into it!
>>
>> I have tried to disable autogroups
>>
>> sysctl -w kernel.sched_autogroup_enabled=0
>>
>> and I can confirm that performance is then back at level as in 4.6 kernel.
>
> So unless the heat has made me do really silly things, the below seems
> to cure things. Could you please verify?
>
>
> ---
>  kernel/sched/fair.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 22d64b3f5876..d4f6fb2f3057 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>          */
>         tg_weight = atomic_long_read(&tg->load_avg);
>         tg_weight -= cfs_rq->tg_load_avg_contrib;
> -       tg_weight += cfs_rq->load.weight;
> +       tg_weight += cfs_rq->avg.load_avg;

IIUC, you are reverting
commit  fde7d22e01aa (sched/fair: Fix overly small weight for
interactive group entities)

I have one question regarding the use of cfs_rq->avg.load_avg
cfs_rq->tg_load_avg_contrib is the sampling of cfs_rq->avg.load_avg so
I'm curious to understand why you use cfs_rq->avg.load_avg instead of
keeping cfs_rq->tg_load_avg_contrib. Do you think that the sampling is
not accurate enough to prevent any significant difference between both
when we use tg->load_avg ?


>
>         return tg_weight;
>  }
> @@ -2494,7 +2494,7 @@ static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
>         long tg_weight, load, shares;
>
>         tg_weight = calc_tg_weight(tg, cfs_rq);
> -       load = cfs_rq->load.weight;
> +       load = cfs_rq->avg.load_avg;
>
>         shares = (tg->shares * load);
>         if (tg_weight)

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

#1430649

On Fri, Jun 24, 2016 at 02:44:07PM +0200, Vincent Guittot wrote:

> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 22d64b3f5876..d4f6fb2f3057 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
> >          */
> >         tg_weight = atomic_long_read(&tg->load_avg);
> >         tg_weight -= cfs_rq->tg_load_avg_contrib;
> > -       tg_weight += cfs_rq->load.weight;
> > +       tg_weight += cfs_rq->avg.load_avg;
> 
> IIUC, you are reverting
> commit  fde7d22e01aa (sched/fair: Fix overly small weight for
> interactive group entities)

Ah!, I hadn't yet done a git-blame on this. Right you are, we should
have put a comment there.

So the problem here is that since commit:

  2159197d6677 ("sched/core: Enable increased load resolution on 64-bit kernels")

load.weight and avg.load_avg are in different metrics. Which completely
wrecked things.

The obvious alternative is using:

	scale_load_down(cfs_rq->load.weight);

Let me go run that through the benchmark.

> I have one question regarding the use of cfs_rq->avg.load_avg
> cfs_rq->tg_load_avg_contrib is the sampling of cfs_rq->avg.load_avg so
> I'm curious to understand why you use cfs_rq->avg.load_avg instead of
> keeping cfs_rq->tg_load_avg_contrib. Do you think that the sampling is
> not accurate enough to prevent any significant difference between both
> when we use tg->load_avg ?

I'm not entirely sure I understand your question; is it to the existence
of calc_tg_weight()? That is, why use calc_tg_weight() and not use
tg->load_avg as is?

It seemed like a simple and cheap way to increase accuracy, nothing more
behind it until the commit you referred to.

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

#1430658

On 24 June 2016 at 15:09, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 24, 2016 at 02:44:07PM +0200, Vincent Guittot wrote:
>
>> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> > index 22d64b3f5876..d4f6fb2f3057 100644
>> > --- a/kernel/sched/fair.c
>> > +++ b/kernel/sched/fair.c
>> > @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>> >          */
>> >         tg_weight = atomic_long_read(&tg->load_avg);
>> >         tg_weight -= cfs_rq->tg_load_avg_contrib;
>> > -       tg_weight += cfs_rq->load.weight;
>> > +       tg_weight += cfs_rq->avg.load_avg;
>>
>> IIUC, you are reverting
>> commit  fde7d22e01aa (sched/fair: Fix overly small weight for
>> interactive group entities)
>
> Ah!, I hadn't yet done a git-blame on this. Right you are, we should
> have put a comment there.
>
> So the problem here is that since commit:
>
>   2159197d6677 ("sched/core: Enable increased load resolution on 64-bit kernels")
>
> load.weight and avg.load_avg are in different metrics. Which completely
> wrecked things.
>
> The obvious alternative is using:
>
>         scale_load_down(cfs_rq->load.weight);
>
> Let me go run that through the benchmark.

Yes, looks to be good alternative

>
>> I have one question regarding the use of cfs_rq->avg.load_avg
>> cfs_rq->tg_load_avg_contrib is the sampling of cfs_rq->avg.load_avg so
>> I'm curious to understand why you use cfs_rq->avg.load_avg instead of
>> keeping cfs_rq->tg_load_avg_contrib. Do you think that the sampling is
>> not accurate enough to prevent any significant difference between both
>> when we use tg->load_avg ?
>
> I'm not entirely sure I understand your question; is it to the existence
> of calc_tg_weight()? That is, why use calc_tg_weight() and not use
> tg->load_avg as is?

Yes

>
> It seemed like a simple and cheap way to increase accuracy, nothing more
> behind it until the commit you referred to.

Thanks for the clarification.
I thought that the difference should always be smaller than 1/64th of
the cfs_rq->avg.load_avg thanks to update_tg_load_avg

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

#1430662

On Fri, Jun 24, 2016 at 03:23:37PM +0200, Vincent Guittot wrote:
> On 24 June 2016 at 15:09, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, Jun 24, 2016 at 02:44:07PM +0200, Vincent Guittot wrote:
> >
> >> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> > index 22d64b3f5876..d4f6fb2f3057 100644
> >> > --- a/kernel/sched/fair.c
> >> > +++ b/kernel/sched/fair.c
> >> > @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
> >> >          */
> >> >         tg_weight = atomic_long_read(&tg->load_avg);
> >> >         tg_weight -= cfs_rq->tg_load_avg_contrib;
> >> > -       tg_weight += cfs_rq->load.weight;
> >> > +       tg_weight += cfs_rq->avg.load_avg;
> >>
> >> IIUC, you are reverting
> >> commit  fde7d22e01aa (sched/fair: Fix overly small weight for
> >> interactive group entities)
> >
> > Ah!, I hadn't yet done a git-blame on this. Right you are, we should
> > have put a comment there.
> >
> > So the problem here is that since commit:
> >
> >   2159197d6677 ("sched/core: Enable increased load resolution on 64-bit kernels")
> >
> > load.weight and avg.load_avg are in different metrics. Which completely
> > wrecked things.
> >
> > The obvious alternative is using:
> >
> >         scale_load_down(cfs_rq->load.weight);
> >
> > Let me go run that through the benchmark.
> 
> Yes, looks to be good alternative

Does indeed also work. Let me go write a Changelog and try and magic it
into sched/urgent.

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

#1430681

On Fri, Jun 24, 2016 at 03:23:37PM +0200, Vincent Guittot wrote:
> > It seemed like a simple and cheap way to increase accuracy, nothing more
> > behind it until the commit you referred to.
> 
> Thanks for the clarification.
> I thought that the difference should always be smaller than 1/64th of
> the cfs_rq->avg.load_avg thanks to update_tg_load_avg

Right, another reason I just remembered is that it ensures:

	tg_weight >= cfs_rq_weight

Because if this is the only task in the entire group and cfs_rq
increased (but did not exceed the 1/64th threshold) you get the group
weight being smaller than the entity weight, which would be weird.

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

#1430650

Hi Peter,

the proposed patch has fixed the performance issue. I have applied the
patch to v4.7-rc4

Jirka

On Fri, Jun 24, 2016 at 2:44 PM, Vincent Guittot
<vincent.guittot@linaro.org> wrote:
> Hi Peter,
>
> On 24 June 2016 at 14:02, Peter Zijlstra <peterz@infradead.org> wrote:
>> On Fri, Jun 24, 2016 at 09:44:41AM +0200, Jirka Hladky wrote:
>>> Hi Peter,
>>>
>>> thanks a lot for looking into it!
>>>
>>> I have tried to disable autogroups
>>>
>>> sysctl -w kernel.sched_autogroup_enabled=0
>>>
>>> and I can confirm that performance is then back at level as in 4.6 kernel.
>>
>> So unless the heat has made me do really silly things, the below seems
>> to cure things. Could you please verify?
>>
>>
>> ---
>>  kernel/sched/fair.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 22d64b3f5876..d4f6fb2f3057 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>>          */
>>         tg_weight = atomic_long_read(&tg->load_avg);
>>         tg_weight -= cfs_rq->tg_load_avg_contrib;
>> -       tg_weight += cfs_rq->load.weight;
>> +       tg_weight += cfs_rq->avg.load_avg;
>
> IIUC, you are reverting
> commit  fde7d22e01aa (sched/fair: Fix overly small weight for
> interactive group entities)
>
> I have one question regarding the use of cfs_rq->avg.load_avg
> cfs_rq->tg_load_avg_contrib is the sampling of cfs_rq->avg.load_avg so
> I'm curious to understand why you use cfs_rq->avg.load_avg instead of
> keeping cfs_rq->tg_load_avg_contrib. Do you think that the sampling is
> not accurate enough to prevent any significant difference between both
> when we use tg->load_avg ?
>
>
>>
>>         return tg_weight;
>>  }
>> @@ -2494,7 +2494,7 @@ static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg)
>>         long tg_weight, load, shares;
>>
>>         tg_weight = calc_tg_weight(tg, cfs_rq);
>> -       load = cfs_rq->load.weight;
>> +       load = cfs_rq->avg.load_avg;
>>
>>         shares = (tg->shares * load);
>>         if (tg_weight)

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

#1430675

On Fri, Jun 24, 2016 at 02:44:07PM +0200, Vincent Guittot wrote:
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
> >          */
> >         tg_weight = atomic_long_read(&tg->load_avg);
> >         tg_weight -= cfs_rq->tg_load_avg_contrib;
> > -       tg_weight += cfs_rq->load.weight;
> > +       tg_weight += cfs_rq->avg.load_avg;
> 
> IIUC, you are reverting
> commit  fde7d22e01aa (sched/fair: Fix overly small weight for
> interactive group entities)

Hurm.. looking at that commit again, that seems to wreck
effective_load(), since that doesn't compensate.

Maybe I'll remove calc_tg_weight and open code its slightly different
usages in the two sites.

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

#1430782

On Fri, Jun 24, 2016 at 03:42:26PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 24, 2016 at 02:44:07PM +0200, Vincent Guittot wrote:
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
> > >          */
> > >         tg_weight = atomic_long_read(&tg->load_avg);
> > >         tg_weight -= cfs_rq->tg_load_avg_contrib;
> > > -       tg_weight += cfs_rq->load.weight;
> > > +       tg_weight += cfs_rq->avg.load_avg;
> > 
> > IIUC, you are reverting
> > commit  fde7d22e01aa (sched/fair: Fix overly small weight for
> > interactive group entities)
> 
> Hurm.. looking at that commit again, that seems to wreck
> effective_load(), since that doesn't compensate.
> 
> Maybe I'll remove calc_tg_weight and open code its slightly different
> usages in the two sites.

OK, sorry for not actually posting, but I need to run. Please find the
two patches in:

  git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git sched/urgent

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

#1430984

Hi Peter,

I have compiled your version of linux kernel and run the SPECjvm2008
tests. Results are fine, performance is at the level of 4.6 kernel.

$ git rev-parse HEAD
02548776ded1185e6e16ad0a475481e982741ee9

Jirka




On Fri, Jun 24, 2016 at 5:54 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jun 24, 2016 at 03:42:26PM +0200, Peter Zijlstra wrote:
>> On Fri, Jun 24, 2016 at 02:44:07PM +0200, Vincent Guittot wrote:
>> > > --- a/kernel/sched/fair.c
>> > > +++ b/kernel/sched/fair.c
>> > > @@ -2484,7 +2484,7 @@ static inline long calc_tg_weight(struct task_group *tg, struct cfs_rq *cfs_rq)
>> > >          */
>> > >         tg_weight = atomic_long_read(&tg->load_avg);
>> > >         tg_weight -= cfs_rq->tg_load_avg_contrib;
>> > > -       tg_weight += cfs_rq->load.weight;
>> > > +       tg_weight += cfs_rq->avg.load_avg;
>> >
>> > IIUC, you are reverting
>> > commit  fde7d22e01aa (sched/fair: Fix overly small weight for
>> > interactive group entities)
>>
>> Hurm.. looking at that commit again, that seems to wreck
>> effective_load(), since that doesn't compensate.
>>
>> Maybe I'll remove calc_tg_weight and open code its slightly different
>> usages in the two sites.
>
> OK, sorry for not actually posting, but I need to run. Please find the
> two patches in:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/peterz/queue.git sched/urgent

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web