* 'autogroup' sched code KILLING responsiveness @ 2011-01-21 18:20 Michael Witten 2011-01-21 22:27 ` Mike Galbraith 2011-01-23 10:50 ` Christian Kujau 0 siblings, 2 replies; 25+ messages in thread From: Michael Witten @ 2011-01-21 18:20 UTC (permalink / raw) To: linux-kernel 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-21 18:20 'autogroup' sched code KILLING responsiveness Michael Witten @ 2011-01-21 22:27 ` Mike Galbraith 2011-01-21 22:39 ` Michael Witten 2011-01-22 21:23 ` Michael Witten 2011-01-23 10:50 ` Christian Kujau 1 sibling, 2 replies; 25+ messages in thread From: Mike Galbraith @ 2011-01-21 22:27 UTC (permalink / raw) To: Michael Witten; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-21 22:27 ` Mike Galbraith @ 2011-01-21 22:39 ` Michael Witten 2011-01-22 3:22 ` Mike Galbraith 2011-01-22 21:23 ` Michael Witten 1 sibling, 1 reply; 25+ messages in thread From: Michael Witten @ 2011-01-21 22:39 UTC (permalink / raw) To: Mike Galbraith; +Cc: linux-kernel 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? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-21 22:39 ` Michael Witten @ 2011-01-22 3:22 ` Mike Galbraith 0 siblings, 0 replies; 25+ messages in thread From: Mike Galbraith @ 2011-01-22 3:22 UTC (permalink / raw) To: Michael Witten; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-21 22:27 ` Mike Galbraith 2011-01-21 22:39 ` Michael Witten @ 2011-01-22 21:23 ` Michael Witten 2011-01-23 3:32 ` Michael Witten 1 sibling, 1 reply; 25+ messages in thread From: Michael Witten @ 2011-01-22 21:23 UTC (permalink / raw) To: Mike Galbraith; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-22 21:23 ` Michael Witten @ 2011-01-23 3:32 ` Michael Witten 2011-01-23 5:42 ` Mike Galbraith 0 siblings, 1 reply; 25+ messages in thread From: Michael Witten @ 2011-01-23 3:32 UTC (permalink / raw) To: Mike Galbraith; +Cc: linux-kernel 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. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 3:32 ` Michael Witten @ 2011-01-23 5:42 ` Mike Galbraith 0 siblings, 0 replies; 25+ messages in thread From: Mike Galbraith @ 2011-01-23 5:42 UTC (permalink / raw) To: Michael Witten; +Cc: linux-kernel 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-21 18:20 'autogroup' sched code KILLING responsiveness Michael Witten 2011-01-21 22:27 ` Mike Galbraith @ 2011-01-23 10:50 ` Christian Kujau 2011-01-23 11:19 ` Christian Kujau 2011-01-23 14:54 ` Yong Zhang 1 sibling, 2 replies; 25+ messages in thread From: Christian Kujau @ 2011-01-23 10:50 UTC (permalink / raw) To: Michael Witten; +Cc: linux-kernel 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. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 10:50 ` Christian Kujau @ 2011-01-23 11:19 ` Christian Kujau 2011-01-23 14:54 ` Yong Zhang 1 sibling, 0 replies; 25+ messages in thread From: Christian Kujau @ 2011-01-23 11:19 UTC (permalink / raw) To: efault, Michael Witten; +Cc: LKML 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! ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 10:50 ` Christian Kujau 2011-01-23 11:19 ` Christian Kujau @ 2011-01-23 14:54 ` Yong Zhang 2011-01-23 15:03 ` [PATCH] sched: fix autogroup nice tune on UP Yong Zhang 2011-01-23 15:15 ` 'autogroup' sched code KILLING responsiveness Ingo Molnar 1 sibling, 2 replies; 25+ messages in thread From: Yong Zhang @ 2011-01-23 14:54 UTC (permalink / raw) To: Christian Kujau Cc: Michael Witten, linux-kernel, Peter Zijlstra, Mike Galbraith, Ingo Molnar 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH] sched: fix autogroup nice tune on UP 2011-01-23 14:54 ` Yong Zhang @ 2011-01-23 15:03 ` Yong Zhang 2011-01-23 15:16 ` Pekka Enberg 2011-01-24 5:40 ` [PATCH V2] " Yong Zhang 2011-01-23 15:15 ` 'autogroup' sched code KILLING responsiveness Ingo Molnar 1 sibling, 2 replies; 25+ messages in thread From: Yong Zhang @ 2011-01-23 15:03 UTC (permalink / raw) To: Peter Zijlstra Cc: Michael Witten, linux-kernel, Christian Kujau, Mike Galbraith, Ingo Molnar 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH] sched: fix autogroup nice tune on UP 2011-01-23 15:03 ` [PATCH] sched: fix autogroup nice tune on UP Yong Zhang @ 2011-01-23 15:16 ` Pekka Enberg 2011-01-24 3:17 ` Yong Zhang 2011-01-24 5:40 ` [PATCH V2] " Yong Zhang 1 sibling, 1 reply; 25+ messages in thread From: Pekka Enberg @ 2011-01-23 15:16 UTC (permalink / raw) To: Yong Zhang Cc: Peter Zijlstra, Michael Witten, linux-kernel, Christian Kujau, Mike Galbraith, Ingo Molnar 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? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH] sched: fix autogroup nice tune on UP 2011-01-23 15:16 ` Pekka Enberg @ 2011-01-24 3:17 ` Yong Zhang 0 siblings, 0 replies; 25+ messages in thread From: Yong Zhang @ 2011-01-24 3:17 UTC (permalink / raw) To: Pekka Enberg Cc: Peter Zijlstra, Michael Witten, linux-kernel, Christian Kujau, Mike Galbraith, Ingo Molnar 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V2] sched: fix autogroup nice tune on UP 2011-01-23 15:03 ` [PATCH] sched: fix autogroup nice tune on UP Yong Zhang 2011-01-23 15:16 ` Pekka Enberg @ 2011-01-24 5:40 ` Yong Zhang 2011-01-24 5:54 ` Pekka Enberg 1 sibling, 1 reply; 25+ messages in thread From: Yong Zhang @ 2011-01-24 5:40 UTC (permalink / raw) To: linux-kernel Cc: mfwitten, christian, penberg, a.p.zijlstra, Ingo Molnar, Peter Zijlstra, Mike Galbraith 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V2] sched: fix autogroup nice tune on UP 2011-01-24 5:40 ` [PATCH V2] " Yong Zhang @ 2011-01-24 5:54 ` Pekka Enberg 2011-01-24 6:11 ` Yong Zhang 0 siblings, 1 reply; 25+ messages in thread From: Pekka Enberg @ 2011-01-24 5:54 UTC (permalink / raw) To: Yong Zhang Cc: linux-kernel, mfwitten, christian, a.p.zijlstra, Ingo Molnar, Peter Zijlstra, Mike Galbraith 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? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V2] sched: fix autogroup nice tune on UP 2011-01-24 5:54 ` Pekka Enberg @ 2011-01-24 6:11 ` Yong Zhang 2011-01-24 6:18 ` Pekka Enberg 0 siblings, 1 reply; 25+ messages in thread From: Yong Zhang @ 2011-01-24 6:11 UTC (permalink / raw) To: Pekka Enberg Cc: linux-kernel, mfwitten, christian, a.p.zijlstra, Ingo Molnar, Peter Zijlstra, Mike Galbraith 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V2] sched: fix autogroup nice tune on UP 2011-01-24 6:11 ` Yong Zhang @ 2011-01-24 6:18 ` Pekka Enberg 2011-01-24 7:33 ` [PATCH V3] " Yong Zhang 0 siblings, 1 reply; 25+ messages in thread From: Pekka Enberg @ 2011-01-24 6:18 UTC (permalink / raw) To: Yong Zhang Cc: linux-kernel, mfwitten, christian, a.p.zijlstra, Ingo Molnar, Peter Zijlstra, Mike Galbraith 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); } ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH V3] sched: fix autogroup nice tune on UP 2011-01-24 6:18 ` Pekka Enberg @ 2011-01-24 7:33 ` Yong Zhang 2011-01-24 8:01 ` Pekka Enberg ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Yong Zhang @ 2011-01-24 7:33 UTC (permalink / raw) To: Pekka Enberg Cc: linux-kernel, mfwitten, christian, a.p.zijlstra, Ingo Molnar, Peter Zijlstra, Mike Galbraith 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH V3] sched: fix autogroup nice tune on UP 2011-01-24 7:33 ` [PATCH V3] " Yong Zhang @ 2011-01-24 8:01 ` Pekka Enberg 2011-01-24 9:00 ` Mike Galbraith 2011-01-24 10:51 ` [tip:sched/urgent] sched: Fix poor interactivity on UP systems due to group scheduler nice tune bug tip-bot for Yong Zhang 2 siblings, 0 replies; 25+ messages in thread From: Pekka Enberg @ 2011-01-24 8:01 UTC (permalink / raw) To: Yong Zhang Cc: linux-kernel, mfwitten, christian, a.p.zijlstra, Ingo Molnar, Peter Zijlstra, Mike Galbraith 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> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH V3] sched: fix autogroup nice tune on UP 2011-01-24 7:33 ` [PATCH V3] " Yong Zhang 2011-01-24 8:01 ` Pekka Enberg @ 2011-01-24 9:00 ` Mike Galbraith 2011-01-24 10:51 ` [tip:sched/urgent] sched: Fix poor interactivity on UP systems due to group scheduler nice tune bug tip-bot for Yong Zhang 2 siblings, 0 replies; 25+ messages in thread From: Mike Galbraith @ 2011-01-24 9:00 UTC (permalink / raw) To: Yong Zhang Cc: Pekka Enberg, linux-kernel, mfwitten, christian, a.p.zijlstra, Ingo Molnar, Peter Zijlstra 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) > { ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tip:sched/urgent] sched: Fix poor interactivity on UP systems due to group scheduler nice tune bug 2011-01-24 7:33 ` [PATCH V3] " Yong Zhang 2011-01-24 8:01 ` Pekka Enberg 2011-01-24 9:00 ` Mike Galbraith @ 2011-01-24 10:51 ` tip-bot for Yong Zhang 2 siblings, 0 replies; 25+ messages in thread From: tip-bot for Yong Zhang @ 2011-01-24 10:51 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, torvalds, peterz, efault, penberg, christian, mfwitten, tglx, yong.zhang0, mingo 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) { ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 14:54 ` Yong Zhang 2011-01-23 15:03 ` [PATCH] sched: fix autogroup nice tune on UP Yong Zhang @ 2011-01-23 15:15 ` Ingo Molnar 2011-01-23 15:53 ` Michael Witten ` (2 more replies) 1 sibling, 3 replies; 25+ messages in thread From: Ingo Molnar @ 2011-01-23 15:15 UTC (permalink / raw) To: Yong Zhang Cc: Christian Kujau, Michael Witten, linux-kernel, Peter Zijlstra, Mike Galbraith * 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 15:15 ` 'autogroup' sched code KILLING responsiveness Ingo Molnar @ 2011-01-23 15:53 ` Michael Witten 2011-01-23 18:52 ` Andreas Mohr 2011-01-23 23:57 ` Christian Kujau 2 siblings, 0 replies; 25+ messages in thread From: Michael Witten @ 2011-01-23 15:53 UTC (permalink / raw) To: Ingo Molnar Cc: Yong Zhang, Christian Kujau, linux-kernel, Peter Zijlstra, Mike Galbraith 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 15:15 ` 'autogroup' sched code KILLING responsiveness Ingo Molnar 2011-01-23 15:53 ` Michael Witten @ 2011-01-23 18:52 ` Andreas Mohr 2011-01-23 23:57 ` Christian Kujau 2 siblings, 0 replies; 25+ messages in thread From: Andreas Mohr @ 2011-01-23 18:52 UTC (permalink / raw) To: Ingo Molnar Cc: Yong Zhang, Christian Kujau, Michael Witten, linux-kernel, Peter Zijlstra, Mike Galbraith > 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: 'autogroup' sched code KILLING responsiveness 2011-01-23 15:15 ` 'autogroup' sched code KILLING responsiveness Ingo Molnar 2011-01-23 15:53 ` Michael Witten 2011-01-23 18:52 ` Andreas Mohr @ 2011-01-23 23:57 ` Christian Kujau 2 siblings, 0 replies; 25+ messages in thread From: Christian Kujau @ 2011-01-23 23:57 UTC (permalink / raw) To: Ingo Molnar Cc: Yong Zhang, Michael Witten, LKML, Peter Zijlstra, Mike Galbraith 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 ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2011-01-24 10:53 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-01-21 18:20 'autogroup' sched code KILLING responsiveness Michael Witten 2011-01-21 22:27 ` Mike Galbraith 2011-01-21 22:39 ` Michael Witten 2011-01-22 3:22 ` Mike Galbraith 2011-01-22 21:23 ` Michael Witten 2011-01-23 3:32 ` Michael Witten 2011-01-23 5:42 ` Mike Galbraith 2011-01-23 10:50 ` Christian Kujau 2011-01-23 11:19 ` Christian Kujau 2011-01-23 14:54 ` Yong Zhang 2011-01-23 15:03 ` [PATCH] sched: fix autogroup nice tune on UP Yong Zhang 2011-01-23 15:16 ` Pekka Enberg 2011-01-24 3:17 ` Yong Zhang 2011-01-24 5:40 ` [PATCH V2] " Yong Zhang 2011-01-24 5:54 ` Pekka Enberg 2011-01-24 6:11 ` Yong Zhang 2011-01-24 6:18 ` Pekka Enberg 2011-01-24 7:33 ` [PATCH V3] " Yong Zhang 2011-01-24 8:01 ` Pekka Enberg 2011-01-24 9:00 ` Mike Galbraith 2011-01-24 10:51 ` [tip:sched/urgent] sched: Fix poor interactivity on UP systems due to group scheduler nice tune bug tip-bot for Yong Zhang 2011-01-23 15:15 ` 'autogroup' sched code KILLING responsiveness Ingo Molnar 2011-01-23 15:53 ` Michael Witten 2011-01-23 18:52 ` Andreas Mohr 2011-01-23 23:57 ` Christian Kujau
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.