'autogroup' sched code KILLING responsiveness

'autogroup' sched code KILLING responsiveness

[Michael Witten]

Bisecting shows that this commit:

  5091faa449ee0b7d73bc296a93bca9540fc51d0a
  sched: Add 'autogroup' scheduling feature: automated per session task groups
  Date:   Tue Nov 30 14:18:03 2010 +0100

is the reason that my computer has become unusable.

With that code in place, a resource-intensive activity (such as
compiling the Linux kernel) causes my computer to become
unresponsive for many seconds at a time; the entire screen
does not refresh, typed keys are dropped or are handled very
late, etc (even in Linux's plain virtual consoles).

I'm using a uniprocessor (UP) machine, and I've noticed that such
codepaths often get clobbered by changes that only make it to SMP
configurations (I guess kernel hackers have better equipment than
I do); maybe that has something to do with it.

I'm a laymen, so all I can do at this time is report my experience;
if this has already been discussed, I would appeciate a link to
the right thread.

Sincerely,
Michael Witten

Re: 'autogroup' sched code KILLING responsiveness

[Mike Galbraith]

On Fri, 2011-01-21 at 10:20 -0800, Michael Witten wrote:
> Bisecting shows that this commit:
> 
>   5091faa449ee0b7d73bc296a93bca9540fc51d0a
>   sched: Add 'autogroup' scheduling feature: automated per session task groups
>   Date:   Tue Nov 30 14:18:03 2010 +0100
> 
> is the reason that my computer has become unusable.
> 
> With that code in place, a resource-intensive activity (such as
> compiling the Linux kernel) causes my computer to become
> unresponsive for many seconds at a time; the entire screen
> does not refresh, typed keys are dropped or are handled very
> late, etc (even in Linux's plain virtual consoles).

That's not what I'm experiencing with a UP kernel...

marge:~> time perf stat -a sh -c 'true'

 Performance counter stats for 'sh -c true':

             49580 cache-misses             #      5.170 M/sec  (scaled from 37.28%)
            336112 cache-references         #     35.048 M/sec  (scaled from 78.81%)
            185857 branch-misses            #     21.019 %      (scaled from 62.83%)
            884249 branches                 #     92.204 M/sec  (scaled from 21.25%)
          12672552 instructions             #      0.553 IPC    (scaled from 58.38%)
          22896301 cycles                   #   2387.490 M/sec  (scaled from 58.38%)
               410 page-faults              #      0.043 M/sec
                 0 CPU-migrations           #      0.000 M/sec
                13 context-switches         #      0.001 M/sec
          9.590115 task-clock-msecs         #      1.005 CPUs 

        0.009540836  seconds time elapsed


real    0m0.020s
user    0m0.000s
sys     0m0.000s
marge:~>

It took 20 ms to do the above and get it to my screen while the below
was running (among others).  I've got a make -j 100 running as I write
this, and don't even notice that it's running.

Can you provide some information such as hardware description,
configuration, and perhaps measurements demonstrating the problem?

top - 22:34:38 up 33 min, 22 users,  load average: 104.07, 101.73, 78.13
Tasks: 675 total, 101 running, 574 sleeping,   0 stopped,   0 zombie
Cpu(s): 95.2%us,  4.8%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  P COMMAND                                                              
 6770 root      20   0  401m  79m  29m R 46.9  1.0   3:22.66 0 mplayer                                                              
 6100 root      20   0  449m  94m  18m S  2.9  1.2   1:33.82 0 Xorg                                                                 
 7486 root      20   0  374m  46m  17m S  2.9  0.6   0:54.02 0 konsole                                                              
 7752 root      20   0  104m  66m 8072 R  1.0  0.8   0:01.76 0 cc1                                                                  
 9796 root      20   0 89764  49m 8028 R  1.0  0.6   0:01.21 0 cc1                                                                  
 9928 root      20   0 98916  59m 8164 R  1.0  0.7   0:01.18 0 cc1                                                                  
10321 root      20   0 88788  49m 8116 R  1.0  0.6   0:01.06 0 cc1                                                                  
10619 root      20   0 89736  50m 8068 R  1.0  0.6   0:01.00 0 cc1                                                                  
11356 root      20   0 19764 8152 2300 S  1.0  0.1   0:00.01 0 as                                                                   
11597 root      20   0 97212  56m 7924 R  1.0  0.7   0:00.65 0 cc1                                                                  
12203 root      20   0 79332  40m 7588 R  1.0  0.5   0:00.48 0 cc1                                                                  
12209 root      20   0 80760  40m 7536 R  1.0  0.5   0:00.47 0 cc1                                                                  
12247 root      20   0 85252  46m 7752 R  1.0  0.6   0:00.46 0 cc1                                                                  
12510 root      20   0 82192  40m 5184 R  1.0  0.5   0:00.36 0 cc1                                                                  
12612 root      20   0 81172  39m 5192 R  1.0  0.5   0:00.33 0 cc1                                                                  
12615 root      20   0 81160  38m 4360 R  1.0  0.5   0:00.33 0 cc1                                                                  

Re: 'autogroup' sched code KILLING responsiveness

[Michael Witten]

On Fri, Jan 21, 2011 at 16:27, Mike Galbraith <efault@gmx.de> wrote:
> On Fri, 2011-01-21 at 10:20 -0800, Michael Witten wrote:
>> Bisecting shows that this commit:
>>
>>   5091faa449ee0b7d73bc296a93bca9540fc51d0a
>>   sched: Add 'autogroup' scheduling feature: automated per session task groups
>>   Date:   Tue Nov 30 14:18:03 2010 +0100
>>
>> is the reason that my computer has become unusable.
>>
>> With that code in place, a resource-intensive activity (such as
>> compiling the Linux kernel) causes my computer to become
>> unresponsive for many seconds at a time; the entire screen
>> does not refresh, typed keys are dropped or are handled very
>> late, etc (even in Linux's plain virtual consoles).
>
> That's not what I'm experiencing with a UP kernel...

Before I try to gather any data, I'd like to point out that the
problem disappears when I disable CONFIG_SCHED_AUTOGROUP (General
setup -> Automatic process group scheduling).

Do you have it disabled?

Re: 'autogroup' sched code KILLING responsiveness

[Mike Galbraith]

On Fri, 2011-01-21 at 16:39 -0600, Michael Witten wrote:
> On Fri, Jan 21, 2011 at 16:27, Mike Galbraith <efault@gmx.de> wrote:
> > On Fri, 2011-01-21 at 10:20 -0800, Michael Witten wrote:
> >> Bisecting shows that this commit:
> >>
> >>   5091faa449ee0b7d73bc296a93bca9540fc51d0a
> >>   sched: Add 'autogroup' scheduling feature: automated per session task groups
> >>   Date:   Tue Nov 30 14:18:03 2010 +0100
> >>
> >> is the reason that my computer has become unusable.
> >>
> >> With that code in place, a resource-intensive activity (such as
> >> compiling the Linux kernel) causes my computer to become
> >> unresponsive for many seconds at a time; the entire screen
> >> does not refresh, typed keys are dropped or are handled very
> >> late, etc (even in Linux's plain virtual consoles).
> >
> > That's not what I'm experiencing with a UP kernel...
> 
> Before I try to gather any data, I'd like to point out that the
> problem disappears when I disable CONFIG_SCHED_AUTOGROUP (General
> setup -> Automatic process group scheduling).
> 
> Do you have it disabled?

No, ff I did, mplayer wouldn't be able to get more than 1% cpu.

top - 22:34:38 up 33 min, 22 users,  load average: 104.07, 101.73, 78.13
Tasks: 675 total, 101 running, 574 sleeping,   0 stopped,   0 zombie
Cpu(s): 95.2%us,  4.8%sy,  0.0%ni,  0.0%id,  0.0%wa,  0.0%hi,  0.0%si,  0.0%st

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  P COMMAND                                                              
 6770 root      20   0  401m  79m  29m R 46.9  1.0   3:22.66 0 mplayer                                                              
 6100 root      20   0  449m  94m  18m S  2.9  1.2   1:33.82 0 Xorg                                                                 
 7486 root      20   0  374m  46m  17m S  2.9  0.6   0:54.02 0 konsole

	-Mike

Re: 'autogroup' sched code KILLING responsiveness

[Michael Witten]

On Fri, 21 Jan 2011 23:27:50 +0100, Mike Galbraith wrote:
>On Fri, 2011-01-21 at 10:20 -0800, Michael Witten wrote:
>> Bisecting shows that this commit:
>> 
>>   5091faa449ee0b7d73bc296a93bca9540fc51d0a
>>   sched: Add 'autogroup' scheduling feature: automated per session task groups
>>   Date:   Tue Nov 30 14:18:03 2010 +0100
>> 
>> is the reason that my computer has become unusable.
>> 
>> With that code in place, a resource-intensive activity (such as
>> compiling the Linux kernel) causes my computer to become
>> unresponsive for many seconds at a time; the entire screen
>> does not refresh, typed keys are dropped or are handled very
>> late, etc (even in Linux's plain virtual consoles).
>
> That's not what I'm experiencing with a UP kernel...

Firstly, I apologize for the rather unhelpful email that
follows.

On my machine (x86, Dell Latitude D810), the problem is so
incredibly debilitating that it is incomprehensible others would
not notice if they were subject to the same problem; basically,
I don't know what to do at this point (I've played around with
`oprofile' and `tools/perf', but nothing looks very odd to me
either).

I'd appreciate it if you could give me some direction; I wish
I could give you more with which to work. Should I send you a
copy of my kernel configuration? (I was going to inline it,
as well as the results of `lspci' and `cat /proc/cpuinfo', but
I thought that might be unhelpful if not rude).

Invariably, running:

    yes
    
in a Linux virtual console completely locks me out of my system
(it's not even possible to login remotely via ssh); however,
running it within a terminal emulator in X doesn't seem to cause
the problems I've been seeing.

Things get stranger.

When I run the following in a Linux VC:

  sleep 10; yes | head -2500000

and then switch to X before `yes' begins to run, I can move my
mouse cursor around without trouble for the entire time that
`yes' is allowed to run. HOWEVER, if I ever press (and release if
you like) a key on my keyboard while `yes' is running, then the
entire screen freezes, including the mouse cursor; I only regain
control when `yes' finishes.

Similarly, if I build the Linux kernel:

  make

(this time even within a terminal emulator in X), then I can
move my mouse cursor around without trouble UNTIL I also press
keyboard keys (actually, I'm just holding one key down), at which
point the mouse cursor periodically freezes; the whole screen
freezes with each new status line of the build and then briefly
unfreezes before the next status line can appear (that is, before
the next file can be compiled); the indications are that nothing
else is getting the CPU, as keypresses are sometimes dropped.

Of course, I have none of these problems when I disable:

  CONFIG_AUTOGROUP_SCHED

Sincerely,
Michael Witten

Re: 'autogroup' sched code KILLING responsiveness

[Michael Witten]

On Sat, Jan 22, 2011 at 15:23, Michael Witten <mfwitten@gmail.com> wrote:
> Of course, I have none of these problems when I disable:
>
>  CONFIG_AUTOGROUP_SCHED

Of course, if CONFIG_AUTOGROUP_SCHED is enabled, then the problem also
goes away if the kernel is booted using the `noautogroup' parameter.

Re: 'autogroup' sched code KILLING responsiveness

[Mike Galbraith]

On Sat, 2011-01-22 at 21:32 -0600, Michael Witten wrote:
> On Sat, Jan 22, 2011 at 15:23, Michael Witten <mfwitten@gmail.com> wrote:
> > Of course, I have none of these problems when I disable:
> >
> >  CONFIG_AUTOGROUP_SCHED
> 
> Of course, if CONFIG_AUTOGROUP_SCHED is enabled, then the problem also
> goes away if the kernel is booted using the `noautogroup' parameter.

I built a UP kernel for my old P4 box, and fair group scheduling doesn't
appear to be working at all, whereas SMP does.  It was usable, but
pretty horrible.

I have other (hot) things to do atm, but will investigate this.  Thanks.

	-Mike

Re: 'autogroup' sched code KILLING responsiveness

[Christian Kujau]

On Fri, 21 Jan 2011 at 10:20, Michael Witten wrote:
> With that code in place, a resource-intensive activity (such as
> compiling the Linux kernel) causes my computer to become
> unresponsive for many seconds at a time; the entire screen
> does not refresh, typed keys are dropped or are handled very
> late, etc (even in Linux's plain virtual consoles).

Unfortunately, I'd like to add a "me too". 2.6.38-rc1 behaves fine, but 
with CONFIG_SCHED_AUTOGROUP=y and doing I/O and CPU intensive work (git 
prune/git repack on a Linux git tree), system load goes up to ~13 and 
becomes unresponse for some time too. This even happens when I start the 
jobs with nice -n10.

Without CONFIG_SCHED_AUTOGROUP enabled and doing the same work,
systemload goes up to 1 or maybe 2.

I'm on UP as well (PowerPC G4), disabling CONFIG_SCHED_AUTOGROUP helps 
here too.

Thanks,
Christian.
-- 
BOFH excuse #274:

It was OK before you touched it.

Re: 'autogroup' sched code KILLING responsiveness

[Christian Kujau]

On Sun, 23 Jan 2011 at 02:50, Christian Kujau wrote:
> On Fri, 21 Jan 2011 at 10:20, Michael Witten wrote:
> > With that code in place, a resource-intensive activity (such as
> > compiling the Linux kernel) causes my computer to become
> > unresponsive for many seconds at a time; the entire screen
> > does not refresh, typed keys are dropped or are handled very
> > late, etc (even in Linux's plain virtual consoles).
> 
> Unfortunately, I'd like to add a "me too". 2.6.38-rc1 behaves fine, but 
> with CONFIG_SCHED_AUTOGROUP=y and doing I/O and CPU intensive work (git 
> prune/git repack on a Linux git tree), system load goes up to ~13 and 
> becomes unresponse for some time too. This even happens when I start the 
> jobs with nice -n10.

"unresponsive for some time" turned out to be "system unusable, unless 
rebooted". When I started the "git repack" with the autogroup feature 
turned on, load would go to 4 very quickly (as compared to 1 or 2 w/o 
autogroup), after a while (seconds, minutes) it would go to 10 or 14 and I 
cannot use the system any more, after a while it goes back to 4 but then 
it goes up again, the last load number I see is 11.56 and now the system 
is stuck or so busy that I cannot login any more, not even locally. This 
happened before with 2.6.38-rc1 & CONFIG_SCHED_AUTOGROUP=y, so it's quite 
reproducible for me.

Christian.

> 
> Without CONFIG_SCHED_AUTOGROUP enabled and doing the same work,
> systemload goes up to 1 or maybe 2.
> 
> I'm on UP as well (PowerPC G4), disabling CONFIG_SCHED_AUTOGROUP helps 
> here too.
> 
> Thanks,
> Christian.

-- 
BOFH excuse #356:

the daemons! the daemons! the terrible daemons!

Re: 'autogroup' sched code KILLING responsiveness

[Yong Zhang]

On Sun, Jan 23, 2011 at 02:50:08AM -0800, Christian Kujau wrote:
> On Fri, 21 Jan 2011 at 10:20, Michael Witten wrote:
> > With that code in place, a resource-intensive activity (such as
> > compiling the Linux kernel) causes my computer to become
> > unresponsive for many seconds at a time; the entire screen
> > does not refresh, typed keys are dropped or are handled very
> > late, etc (even in Linux's plain virtual consoles).
> 
> Unfortunately, I'd like to add a "me too". 2.6.38-rc1 behaves fine, but 
> with CONFIG_SCHED_AUTOGROUP=y and doing I/O and CPU intensive work (git 
> prune/git repack on a Linux git tree), system load goes up to ~13 and 
> becomes unresponse for some time too. This even happens when I start the 
> jobs with nice -n10.
> 
> Without CONFIG_SCHED_AUTOGROUP enabled and doing the same work,
> systemload goes up to 1 or maybe 2.
> 
> I'm on UP as well (PowerPC G4), disabling CONFIG_SCHED_AUTOGROUP helps 
> here too.

I think below patch will fix it.

---
From: Yong Zhang <yong.zhang0@gmail.com>
Subject: [PATCH] sched: tg->se->load should be initialised to tg->shares

Michael reported that when enable autogroup on UP, system
responsiveness becomes very bad.
Because in init_tg_cfs_entry() we initialise se->load
to 0 instead of tg->shares, in the end we have 0-weight
sched entity on rq, then lead to misbehavior.

Reported-by: Michael Witten <mfwitten@gmail.com>
Reported-by: Christian Kujau <christian@nerdbynature.de>
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
---
 kernel/sched.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 18d38e4..3ec2c6c 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -7857,7 +7857,7 @@ static void init_tg_cfs_entry(struct task_group *tg, struct cfs_rq *cfs_rq,
 		se->cfs_rq = parent->my_q;
 
 	se->my_q = cfs_rq;
-	update_load_set(&se->load, 0);
+	update_load_set(&se->load, tg->shares);
 	se->parent = parent;
 }
 #endif
-- 
1.7.1

[PATCH] sched: fix autogroup nice tune on UP

[Yong Zhang]

And there is another issue on UP when FAIR_GROUP_SCHED
enabled.

---
From: Yong Zhang <yong.zhang0@gmail.com>
Subject: [PATCH] sched: fix autogroup nice tune on UP

When on UP with FAIR_GROUP_SCHED enabled, tune shares
only affect tg->shares, but is not reflected on
tg->se->load, the reason is update_cfs_shares()
do nothing on SMP.
So let sched_group_set_shares() update tg->se->load
directly for UP && FAIR_GROUP_SCHED.

This issue is found when enable autogroup, but also
exists on cgroup.cpu on UP.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
 kernel/sched.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 3ec2c6c..b8ed459 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -8483,8 +8483,8 @@ static DEFINE_MUTEX(shares_mutex);
 
 int sched_group_set_shares(struct task_group *tg, unsigned long shares)
 {
-	int i;
-	unsigned long flags;
+	int uninitialized_var(i);
+	unsigned long uninitialized_var(flags);
 
 	/*
 	 * We can't change the weight of the root cgroup.
@@ -8502,6 +8502,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
 		goto done;
 
 	tg->shares = shares;
+#ifdef CONFIG_SMP
 	for_each_possible_cpu(i) {
 		struct rq *rq = cpu_rq(i);
 		struct sched_entity *se;
@@ -8513,6 +8514,9 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
 			update_cfs_shares(group_cfs_rq(se), 0);
 		raw_spin_unlock_irqrestore(&rq->lock, flags);
 	}
+#else
+	update_load_set(&tg->se[0]->load, shares);
+#endif
 
 done:
 	mutex_unlock(&shares_mutex);
-- 
1.7.1

Re: 'autogroup' sched code KILLING responsiveness

[Ingo Molnar]

* Yong Zhang <yong.zhang0@gmail.com> wrote:

> On Sun, Jan 23, 2011 at 02:50:08AM -0800, Christian Kujau wrote:
> > On Fri, 21 Jan 2011 at 10:20, Michael Witten wrote:
> > > With that code in place, a resource-intensive activity (such as
> > > compiling the Linux kernel) causes my computer to become
> > > unresponsive for many seconds at a time; the entire screen
> > > does not refresh, typed keys are dropped or are handled very
> > > late, etc (even in Linux's plain virtual consoles).
> > 
> > Unfortunately, I'd like to add a "me too". 2.6.38-rc1 behaves fine, but 
> > with CONFIG_SCHED_AUTOGROUP=y and doing I/O and CPU intensive work (git 
> > prune/git repack on a Linux git tree), system load goes up to ~13 and 
> > becomes unresponse for some time too. This even happens when I start the 
> > jobs with nice -n10.
> > 
> > Without CONFIG_SCHED_AUTOGROUP enabled and doing the same work,
> > systemload goes up to 1 or maybe 2.
> > 
> > I'm on UP as well (PowerPC G4), disabling CONFIG_SCHED_AUTOGROUP helps 
> > here too.
> 
> I think below patch will fix it.

Christian, Michael, can you confirm that this and the second patch fixes the 
interactivity bug for you? If yes then i'd like to apply and send this fix to Linus 
ASAP.

Thanks,

	Ingo

Re: [PATCH] sched: fix autogroup nice tune on UP

[Pekka Enberg]

On Sun, Jan 23, 2011 at 5:03 PM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> And there is another issue on UP when FAIR_GROUP_SCHED
> enabled.
>
> ---
> From: Yong Zhang <yong.zhang0@gmail.com>
> Subject: [PATCH] sched: fix autogroup nice tune on UP
>
> When on UP with FAIR_GROUP_SCHED enabled, tune shares
> only affect tg->shares, but is not reflected on
> tg->se->load, the reason is update_cfs_shares()
> do nothing on SMP.

Typo here? You mean UP, right?

> So let sched_group_set_shares() update tg->se->load
> directly for UP && FAIR_GROUP_SCHED.
>
> This issue is found when enable autogroup, but also
> exists on cgroup.cpu on UP.
>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Mike Galbraith <efault@gmx.de>
> ---
>  kernel/sched.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 3ec2c6c..b8ed459 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -8483,8 +8483,8 @@ static DEFINE_MUTEX(shares_mutex);
>
>  int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>  {
> -       int i;
> -       unsigned long flags;
> +       int uninitialized_var(i);
> +       unsigned long uninitialized_var(flags);
>
>        /*
>         * We can't change the weight of the root cgroup.
> @@ -8502,6 +8502,7 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>                goto done;
>
>        tg->shares = shares;
> +#ifdef CONFIG_SMP
>        for_each_possible_cpu(i) {
>                struct rq *rq = cpu_rq(i);
>                struct sched_entity *se;
> @@ -8513,6 +8514,9 @@ int sched_group_set_shares(struct task_group *tg, unsigned long shares)
>                        update_cfs_shares(group_cfs_rq(se), 0);
>                raw_spin_unlock_irqrestore(&rq->lock, flags);
>        }
> +#else
> +       update_load_set(&tg->se[0]->load, shares);
> +#endif

I'm not an expert on the scheduler but this looks like the wrong thing
to do. Shouldn't you fix update_cfs_shares() to work on UP properly?

Re: 'autogroup' sched code KILLING responsiveness

[Michael Witten]

On Sun, Jan 23, 2011 at 09:15, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Yong Zhang <yong.zhang0@gmail.com> wrote:
>>
>> I think below patch will fix it.
>
> Christian, Michael, can you confirm...

YES!

As I write this, I'm:

  * Running yes in a VC
  * Running a kernel build
  * Running a 720p HD movie in mplayer

Fantastic!

Thanks,
Michael Witten

Re: 'autogroup' sched code KILLING responsiveness

[Andreas Mohr]

> Christian, Michael, can you confirm that this and the second patch fixes the 
> interactivity bug for you? If yes then i'd like to apply and send this fix to Linus 
> ASAP.
> 
> Thanks,
> 
> 	Ingo

I'm currently running with autogroup enabled for the first time,
on Athlon XP -rc2 with _first patch only_ applied (saw second part too late),
and it appears to be working fine with some stress tests added
(multi-app benchmark, while... true, yes etc.).
Interestingly I had chosen to DISABLE the cgroup part of the scheduler
several months ago since it was slower than non-cgroup by quite some margin.


One should perhaps consider this issue to be evidence of a notable lack
of (possibly automated) test/usage coverage of UP configs
(together with the recent build error on UP that was discovered
once hitting -rc1 only, and not earlier!).


For this particular fix, perhaps the 0 vs. tg->shares change could be
followed up by a WARN_ON_ONCE() in some more internal non-hotpath function
if such a check is feasible in the current case.

Thanks,

Andreas Mohr

Re: 'autogroup' sched code KILLING responsiveness

[Christian Kujau]

On Sun, 23 Jan 2011 at 16:15, Ingo Molnar wrote:
> Christian, Michael, can you confirm that this and the second patch fixes the 
> interactivity bug for you? If yes then i'd like to apply and send this fix to Linus 
> ASAP.

Running with both patches applied for some time now and the same (or 
slightly bigger workload) than before, system load is fine and 
responsiveness is much better, just like w/o the autogroup feature.

Feel free to add:

Tested-by: Christian Kujau <lists@nerdbynature.de>

Thanks to all involved,
Christian.
-- 
BOFH excuse #431:

Borg implants are failing

Re: [PATCH] sched: fix autogroup nice tune on UP

[Yong Zhang]

On Sun, Jan 23, 2011 at 11:16 PM, Pekka Enberg <penberg@kernel.org> wrote:
>> tg->se->load, the reason is update_cfs_shares()
>> do nothing on SMP.
>
> Typo here? You mean UP, right?

Ah, Yeah. Thanks for pointing it.



>
> I'm not an expert on the scheduler but this looks like the wrong thing
> to do. Shouldn't you fix update_cfs_shares() to work on UP properly?
>

I have through about that. No-poking on update_cfs_shares() is because
update_cfs_shares() is called at several hot path, like enqueue_entity()/
dequeue_entity(), but it doesn't make sense on UP.

But I guess you are right. We still need to update the load of cfs_rq
after update_load_set() is called even on UP, so
update_cfs_shares() is the suitable place to do that.

Will take another look at update_cfs_shares().

Thanks,
Yong

-- 
Only stand for myself

[PATCH V2] sched: fix autogroup nice tune on UP

[Yong Zhang]

When on UP with FAIR_GROUP_SCHED enabled, tune shares
only affect tg->shares, but is not reflected on
tg->se->load, the reason is update_cfs_shares()
do nothing on UP.
So introduce update_cfs_shares() for UP && FAIR_GROUP_SCHED.

This issue is found when enable autogroup, but also
exists on cgroup.cpu on UP.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---
 kernel/sched_fair.c |   61 ++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 77e9166..166892f 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -699,7 +699,24 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	cfs_rq->nr_running--;
 }
 
-#if defined CONFIG_SMP && defined CONFIG_FAIR_GROUP_SCHED
+#ifdef CONFIG_FAIR_GROUP_SCHED
+static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
+			    unsigned long weight)
+{
+	if (se->on_rq) {
+		/* commit outstanding execution time */
+		if (cfs_rq->curr == se)
+			update_curr(cfs_rq);
+		account_entity_dequeue(cfs_rq, se);
+	}
+
+	update_load_set(&se->load, weight);
+
+	if (se->on_rq)
+		account_entity_enqueue(cfs_rq, se);
+}
+
+# ifdef CONFIG_SMP
 static void update_cfs_rq_load_contribution(struct cfs_rq *cfs_rq,
 					    int global_update)
 {
@@ -762,22 +779,6 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 		list_del_leaf_cfs_rq(cfs_rq);
 }
 
-static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
-			    unsigned long weight)
-{
-	if (se->on_rq) {
-		/* commit outstanding execution time */
-		if (cfs_rq->curr == se)
-			update_curr(cfs_rq);
-		account_entity_dequeue(cfs_rq, se);
-	}
-
-	update_load_set(&se->load, weight);
-
-	if (se->on_rq)
-		account_entity_enqueue(cfs_rq, se);
-}
-
 static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 {
 	struct task_group *tg;
@@ -817,6 +818,32 @@ static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
 		update_cfs_shares(cfs_rq, 0);
 	}
 }
+# else /* CONFIG_SMP */
+static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
+{
+}
+
+static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
+{
+	struct task_group *tg;
+	struct sched_entity *se;
+
+	if (!cfs_rq)
+		return;
+
+	tg = cfs_rq->tg;
+	se = tg->se[0];
+	if (!se)
+		return;
+	if (likely(se->load.weight == tg->shares))
+		return;
+	reweight_entity(cfs_rq_of(se), se, tg->shares);
+}
+
+static inline void update_entity_shares_tick(struct cfs_rq *cfs_rq)
+{
+}
+# endif /* CONFIG_SMP */
 #else /* CONFIG_FAIR_GROUP_SCHED */
 static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 {
-- 
1.7.0.4

Re: [PATCH V2] sched: fix autogroup nice tune on UP

[Pekka Enberg]

Hi!

On Mon, Jan 24, 2011 at 7:40 AM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> +static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
> +{
> +       struct task_group *tg;
> +       struct sched_entity *se;
> +
> +       if (!cfs_rq)
> +               return;
> +
> +       tg = cfs_rq->tg;
> +       se = tg->se[0];
> +       if (!se)
> +               return;
> +       if (likely(se->load.weight == tg->shares))
> +               return;
> +       reweight_entity(cfs_rq_of(se), se, tg->shares);
> +}

Wouldn't it be cleaner if we'd separate the shares calculation in a
separate helper function that's just

  return tg->shares;

for UP and extract the current logic for the SMP version?

Re: [PATCH V2] sched: fix autogroup nice tune on UP

[Yong Zhang]

On Mon, Jan 24, 2011 at 1:54 PM, Pekka Enberg <penberg@kernel.org> wrote:
> On Mon, Jan 24, 2011 at 7:40 AM, Yong Zhang <yong.zhang0@gmail.com> wrote:
>> +static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
>> +{
>> +       struct task_group *tg;
>> +       struct sched_entity *se;
>> +
>> +       if (!cfs_rq)
>> +               return;
>> +
>> +       tg = cfs_rq->tg;
>> +       se = tg->se[0];
>> +       if (!se)
>> +               return;
>> +       if (likely(se->load.weight == tg->shares))
>> +               return;
>> +       reweight_entity(cfs_rq_of(se), se, tg->shares);
>> +}
>
> Wouldn't it be cleaner if we'd separate the shares calculation in a
> separate helper function that's just
>
>  return tg->shares;

I'm not sure I get your point correctly.
You mean the two tg->shares above, right?

If so, yeah, we can declare a variable for that.

>
> for UP and extract the current logic for the SMP version?
>

This is the UP specific version, I don't touch SMP version.
On SMP, update_cfs_shares() is more complex.

Thanks,
Yong


-- 
Only stand for myself

Re: [PATCH V2] sched: fix autogroup nice tune on UP

[Pekka Enberg]

Hi,

On Mon, Jan 24, 2011 at 8:11 AM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> On Mon, Jan 24, 2011 at 1:54 PM, Pekka Enberg <penberg@kernel.org> wrote:
>> On Mon, Jan 24, 2011 at 7:40 AM, Yong Zhang <yong.zhang0@gmail.com> wrote:
>>> +static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
>>> +{
>>> +       struct task_group *tg;
>>> +       struct sched_entity *se;
>>> +
>>> +       if (!cfs_rq)
>>> +               return;
>>> +
>>> +       tg = cfs_rq->tg;
>>> +       se = tg->se[0];
>>> +       if (!se)
>>> +               return;
>>> +       if (likely(se->load.weight == tg->shares))
>>> +               return;
>>> +       reweight_entity(cfs_rq_of(se), se, tg->shares);
>>> +}
>>
>> Wouldn't it be cleaner if we'd separate the shares calculation in a
>> separate helper function that's just
>>
>>  return tg->shares;
>
> I'm not sure I get your point correctly.
> You mean the two tg->shares above, right?
>
> If so, yeah, we can declare a variable for that.
>
>>
>> for UP and extract the current logic for the SMP version?
>>
>
> This is the UP specific version, I don't touch SMP version.
> On SMP, update_cfs_shares() is more complex.

I proposed extracting the shares calculation logic in
update_cfs_shares() to reduce code duplication for the SMP and UP
patch. So something like this:

#ifdef CONFIG_SMP
static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group
*tg, long weight_delta)
{
        long load_weight, load, shares;

        load = cfs_rq->load.weight + weight_delta;

        load_weight = atomic_read(&tg->load_weight);
        load_weight -= cfs_rq->load_contribution;
        load_weight += load;

        shares = (tg->shares * load);
        if (load_weight)
                shares /= load_weight;

        if (shares < MIN_SHARES)
                shares = MIN_SHARES;
        if (shares > tg->shares)
                shares = tg->shares;

        return shares;
}
#else
static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group
*tg, long weight_delta)
{
        return tg->shares;
}
#endif

static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
{
        struct task_group *tg;
        struct sched_entity *se;
        long shares;

        if (!cfs_rq)
                return;

        tg = cfs_rq->tg;
        se = tg->se[cpu_of(rq_of(cfs_rq))];
        if (!se)
                return;

#ifndef CONFIG_SMP
        if (likely(se->load.weight == tg->shares))
                return;
#else

        shares = calc_cfs_shares(cfs_rq, tg, weight_delta);

        reweight_entity(cfs_rq_of(se), se, shares);
}

[PATCH V3] sched: fix autogroup nice tune on UP

[Yong Zhang]

On Mon, Jan 24, 2011 at 08:18:11AM +0200, Pekka Enberg wrote:
> I proposed extracting the shares calculation logic in
> update_cfs_shares() to reduce code duplication for the SMP and UP
> patch. So something like this:

Thanks for your example.
So something like this?

---
From: Yong Zhang <yong.zhang0@gmail.com>
Subject: [PATCH V3] sched: fix autogroup nice tune on UP

When on UP with FAIR_GROUP_SCHED enabled, tune shares
only affect tg->shares, but is not reflected on
tg->se->load, the reason is update_cfs_shares()
do nothing on UP.
So introduce update_cfs_shares() for UP && FAIR_GROUP_SCHED.

This issue is found when enable autogroup, but also
exists on cgroup.cpu on UP.

Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
---

Changelog from V2:
  Move share caculation to calc_cfs_shares(), thus save
  some duplicated code for update_cfs_shares().
  Idea from Pekka Enberg.

 kernel/sched_fair.c |   78 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 77e9166..3547699 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -699,7 +699,8 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	cfs_rq->nr_running--;
 }
 
-#if defined CONFIG_SMP && defined CONFIG_FAIR_GROUP_SCHED
+#ifdef CONFIG_FAIR_GROUP_SCHED
+# ifdef CONFIG_SMP
 static void update_cfs_rq_load_contribution(struct cfs_rq *cfs_rq,
 					    int global_update)
 {
@@ -762,6 +763,51 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 		list_del_leaf_cfs_rq(cfs_rq);
 }
 
+static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
+				long weight_delta)
+{
+	long load_weight, load, shares;
+
+	load = cfs_rq->load.weight + weight_delta;
+
+	load_weight = atomic_read(&tg->load_weight);
+	load_weight -= cfs_rq->load_contribution;
+	load_weight += load;
+
+	shares = (tg->shares * load);
+	if (load_weight)
+		shares /= load_weight;
+
+	if (shares < MIN_SHARES)
+		shares = MIN_SHARES;
+	if (shares > tg->shares)
+		shares = tg->shares;
+
+	return shares;
+}
+
+static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
+{
+	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
+		update_cfs_load(cfs_rq, 0);
+		update_cfs_shares(cfs_rq, 0);
+	}
+}
+# else /* CONFIG_SMP */
+static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
+{
+}
+
+static inline long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
+				long weight_delta)
+{
+	return tg->shares;
+}
+
+static inline void update_entity_shares_tick(struct cfs_rq *cfs_rq)
+{
+}
+# endif /* CONFIG_SMP */
 static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 			    unsigned long weight)
 {
@@ -782,7 +828,7 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 {
 	struct task_group *tg;
 	struct sched_entity *se;
-	long load_weight, load, shares;
+	long shares;
 
 	if (!cfs_rq)
 		return;
@@ -791,32 +837,14 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 	se = tg->se[cpu_of(rq_of(cfs_rq))];
 	if (!se)
 		return;
-
-	load = cfs_rq->load.weight + weight_delta;
-
-	load_weight = atomic_read(&tg->load_weight);
-	load_weight -= cfs_rq->load_contribution;
-	load_weight += load;
-
-	shares = (tg->shares * load);
-	if (load_weight)
-		shares /= load_weight;
-
-	if (shares < MIN_SHARES)
-		shares = MIN_SHARES;
-	if (shares > tg->shares)
-		shares = tg->shares;
+#ifndef CONFIG_SMP
+	if (likely(se->load.weight == tg->shares))
+		return;
+#endif
+	shares = calc_cfs_shares(cfs_rq, tg, weight_delta);
 
 	reweight_entity(cfs_rq_of(se), se, shares);
 }
-
-static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
-{
-	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
-		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
-	}
-}
 #else /* CONFIG_FAIR_GROUP_SCHED */
 static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 {
-- 
1.7.0.4

Re: [PATCH V3] sched: fix autogroup nice tune on UP

[Pekka Enberg]

Hi!

On Mon, Jan 24, 2011 at 08:18:11AM +0200, Pekka Enberg wrote:
>> I proposed extracting the shares calculation logic in
>> update_cfs_shares() to reduce code duplication for the SMP and UP
>> patch. So something like this:

On Mon, Jan 24, 2011 at 9:33 AM, Yong Zhang <yong.zhang0@gmail.com> wrote:
> Thanks for your example.
> So something like this?
>
> ---
> From: Yong Zhang <yong.zhang0@gmail.com>
> Subject: [PATCH V3] sched: fix autogroup nice tune on UP
>
> When on UP with FAIR_GROUP_SCHED enabled, tune shares
> only affect tg->shares, but is not reflected on
> tg->se->load, the reason is update_cfs_shares()
> do nothing on UP.
> So introduce update_cfs_shares() for UP && FAIR_GROUP_SCHED.
>
> This issue is found when enable autogroup, but also
> exists on cgroup.cpu on UP.
>
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Mike Galbraith <efault@gmx.de>

Looks good to me! FWIW,

Acked-by: Pekka Enberg <penberg@kernel.org>

Re: [PATCH V3] sched: fix autogroup nice tune on UP

[Mike Galbraith]

On Mon, 2011-01-24 at 15:33 +0800, Yong Zhang wrote:
> On Mon, Jan 24, 2011 at 08:18:11AM +0200, Pekka Enberg wrote:
> > I proposed extracting the shares calculation logic in
> > update_cfs_shares() to reduce code duplication for the SMP and UP
> > patch. So something like this:
> 
> Thanks for your example.
> So something like this?

I plugged this and your first into P4 box, and took it for a brief
test-drive... seems all better now.

	-Mike


> ---
> From: Yong Zhang <yong.zhang0@gmail.com>
> Subject: [PATCH V3] sched: fix autogroup nice tune on UP
> 
> When on UP with FAIR_GROUP_SCHED enabled, tune shares
> only affect tg->shares, but is not reflected on
> tg->se->load, the reason is update_cfs_shares()
> do nothing on UP.
> So introduce update_cfs_shares() for UP && FAIR_GROUP_SCHED.
> 
> This issue is found when enable autogroup, but also
> exists on cgroup.cpu on UP.
> 
> Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Mike Galbraith <efault@gmx.de>
> ---
> 
> Changelog from V2:
>   Move share caculation to calc_cfs_shares(), thus save
>   some duplicated code for update_cfs_shares().
>   Idea from Pekka Enberg.
> 
>  kernel/sched_fair.c |   78 ++++++++++++++++++++++++++++++++++----------------
>  1 files changed, 53 insertions(+), 25 deletions(-)
> 
> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> index 77e9166..3547699 100644
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -699,7 +699,8 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  	cfs_rq->nr_running--;
>  }
>  
> -#if defined CONFIG_SMP && defined CONFIG_FAIR_GROUP_SCHED
> +#ifdef CONFIG_FAIR_GROUP_SCHED
> +# ifdef CONFIG_SMP
>  static void update_cfs_rq_load_contribution(struct cfs_rq *cfs_rq,
>  					    int global_update)
>  {
> @@ -762,6 +763,51 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
>  		list_del_leaf_cfs_rq(cfs_rq);
>  }
>  
> +static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
> +				long weight_delta)
> +{
> +	long load_weight, load, shares;
> +
> +	load = cfs_rq->load.weight + weight_delta;
> +
> +	load_weight = atomic_read(&tg->load_weight);
> +	load_weight -= cfs_rq->load_contribution;
> +	load_weight += load;
> +
> +	shares = (tg->shares * load);
> +	if (load_weight)
> +		shares /= load_weight;
> +
> +	if (shares < MIN_SHARES)
> +		shares = MIN_SHARES;
> +	if (shares > tg->shares)
> +		shares = tg->shares;
> +
> +	return shares;
> +}
> +
> +static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
> +{
> +	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
> +		update_cfs_load(cfs_rq, 0);
> +		update_cfs_shares(cfs_rq, 0);
> +	}
> +}
> +# else /* CONFIG_SMP */
> +static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
> +{
> +}
> +
> +static inline long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
> +				long weight_delta)
> +{
> +	return tg->shares;
> +}
> +
> +static inline void update_entity_shares_tick(struct cfs_rq *cfs_rq)
> +{
> +}
> +# endif /* CONFIG_SMP */
>  static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
>  			    unsigned long weight)
>  {
> @@ -782,7 +828,7 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
>  {
>  	struct task_group *tg;
>  	struct sched_entity *se;
> -	long load_weight, load, shares;
> +	long shares;
>  
>  	if (!cfs_rq)
>  		return;
> @@ -791,32 +837,14 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
>  	se = tg->se[cpu_of(rq_of(cfs_rq))];
>  	if (!se)
>  		return;
> -
> -	load = cfs_rq->load.weight + weight_delta;
> -
> -	load_weight = atomic_read(&tg->load_weight);
> -	load_weight -= cfs_rq->load_contribution;
> -	load_weight += load;
> -
> -	shares = (tg->shares * load);
> -	if (load_weight)
> -		shares /= load_weight;
> -
> -	if (shares < MIN_SHARES)
> -		shares = MIN_SHARES;
> -	if (shares > tg->shares)
> -		shares = tg->shares;
> +#ifndef CONFIG_SMP
> +	if (likely(se->load.weight == tg->shares))
> +		return;
> +#endif
> +	shares = calc_cfs_shares(cfs_rq, tg, weight_delta);
>  
>  	reweight_entity(cfs_rq_of(se), se, shares);
>  }
> -
> -static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
> -{
> -	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
> -		update_cfs_load(cfs_rq, 0);
> -		update_cfs_shares(cfs_rq, 0);
> -	}
> -}
>  #else /* CONFIG_FAIR_GROUP_SCHED */
>  static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
>  {

[tip:sched/urgent] sched: Fix poor interactivity on UP systems due to group scheduler nice tune bug

[tip-bot for Yong Zhang]

Commit-ID:  3ff6dcac735704824c1dff64dc6863c390d364cc
Gitweb:     http://git.kernel.org/tip/3ff6dcac735704824c1dff64dc6863c390d364cc
Author:     Yong Zhang <yong.zhang0@gmail.com>
AuthorDate: Mon, 24 Jan 2011 15:33:52 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 24 Jan 2011 11:47:50 +0100

sched: Fix poor interactivity on UP systems due to group scheduler nice tune bug

Michael Witten and Christian Kujau reported that the autogroup
scheduling feature hurts interactivity on their UP systems.

It turns out that this is an older bug in the group scheduling code,
and the wider appeal provided by the autogroup feature exposed it
more prominently.

When on UP with FAIR_GROUP_SCHED enabled, tune shares
only affect tg->shares, but is not reflected in
tg->se->load. The reason is that update_cfs_shares()
does nothing on UP.

So introduce update_cfs_shares() for UP && FAIR_GROUP_SCHED.

This issue was found when enable autogroup scheduling was enabled,
but it is an older bug that also exists on cgroup.cpu on UP.

Reported-and-Tested-by: Michael Witten <mfwitten@gmail.com>
Reported-and-Tested-by: Christian Kujau <christian@nerdbynature.de>
Signed-off-by: Yong Zhang <yong.zhang0@gmail.com>
Acked-by: Pekka Enberg <penberg@kernel.org>
Acked-by: Mike Galbraith <efault@gmx.de>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
LKML-Reference: <20110124073352.GA24186@windriver.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched_fair.c |   78 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 53 insertions(+), 25 deletions(-)

diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
index 77e9166..3547699 100644
--- a/kernel/sched_fair.c
+++ b/kernel/sched_fair.c
@@ -699,7 +699,8 @@ account_entity_dequeue(struct cfs_rq *cfs_rq, struct sched_entity *se)
 	cfs_rq->nr_running--;
 }
 
-#if defined CONFIG_SMP && defined CONFIG_FAIR_GROUP_SCHED
+#ifdef CONFIG_FAIR_GROUP_SCHED
+# ifdef CONFIG_SMP
 static void update_cfs_rq_load_contribution(struct cfs_rq *cfs_rq,
 					    int global_update)
 {
@@ -762,6 +763,51 @@ static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 		list_del_leaf_cfs_rq(cfs_rq);
 }
 
+static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
+				long weight_delta)
+{
+	long load_weight, load, shares;
+
+	load = cfs_rq->load.weight + weight_delta;
+
+	load_weight = atomic_read(&tg->load_weight);
+	load_weight -= cfs_rq->load_contribution;
+	load_weight += load;
+
+	shares = (tg->shares * load);
+	if (load_weight)
+		shares /= load_weight;
+
+	if (shares < MIN_SHARES)
+		shares = MIN_SHARES;
+	if (shares > tg->shares)
+		shares = tg->shares;
+
+	return shares;
+}
+
+static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
+{
+	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
+		update_cfs_load(cfs_rq, 0);
+		update_cfs_shares(cfs_rq, 0);
+	}
+}
+# else /* CONFIG_SMP */
+static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
+{
+}
+
+static inline long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg,
+				long weight_delta)
+{
+	return tg->shares;
+}
+
+static inline void update_entity_shares_tick(struct cfs_rq *cfs_rq)
+{
+}
+# endif /* CONFIG_SMP */
 static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se,
 			    unsigned long weight)
 {
@@ -782,7 +828,7 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 {
 	struct task_group *tg;
 	struct sched_entity *se;
-	long load_weight, load, shares;
+	long shares;
 
 	if (!cfs_rq)
 		return;
@@ -791,32 +837,14 @@ static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta)
 	se = tg->se[cpu_of(rq_of(cfs_rq))];
 	if (!se)
 		return;
-
-	load = cfs_rq->load.weight + weight_delta;
-
-	load_weight = atomic_read(&tg->load_weight);
-	load_weight -= cfs_rq->load_contribution;
-	load_weight += load;
-
-	shares = (tg->shares * load);
-	if (load_weight)
-		shares /= load_weight;
-
-	if (shares < MIN_SHARES)
-		shares = MIN_SHARES;
-	if (shares > tg->shares)
-		shares = tg->shares;
+#ifndef CONFIG_SMP
+	if (likely(se->load.weight == tg->shares))
+		return;
+#endif
+	shares = calc_cfs_shares(cfs_rq, tg, weight_delta);
 
 	reweight_entity(cfs_rq_of(se), se, shares);
 }
-
-static void update_entity_shares_tick(struct cfs_rq *cfs_rq)
-{
-	if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) {
-		update_cfs_load(cfs_rq, 0);
-		update_cfs_shares(cfs_rq, 0);
-	}
-}
 #else /* CONFIG_FAIR_GROUP_SCHED */
 static void update_cfs_load(struct cfs_rq *cfs_rq, int global_update)
 {