'autogroup' sched code KILLING responsiveness

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

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                                                                  

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

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

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

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.

"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.

BOFH excuse #356:

the daemons! the daemons! the terrible daemons!

I think below patch will fix it.

---
From: Yong Zhang <[email protected]>
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.

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

index 18d38e4..3ec2c6c 100644
@@ -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

And there is another issue on UP when FAIR_GROUP_SCHED enabled.

---
From: Yong Zhang <[email protected]>
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.

---
 kernel/sched.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

index 3ec2c6c..b8ed459 100644
@@ -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

Typo here? You mean UP, right?

Ah, Yeah. Thanks for pointing it.

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

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.

---
 kernel/sched_fair.c |   61 ++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 44 insertions(+), 17 deletions(-)

index 77e9166..166892f 100644
@@ -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

Hi!

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.

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

Hi,

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);
}

Thanks for your example.
So something like this?

---
From: Yong Zhang <[email protected]>
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.

---

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(-)

index 77e9166..3547699 100644
@@ -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

Hi!

Looks good to me! FWIW,

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

-Mike

Commit-ID:  3ff6dcac735704824c1dff64dc6863c390d364cc
Gitweb:     http://git.kernel.org/tip/3ff6dcac735704824c1dff64dc6863c390d364cc
Author:     Yong Zhang <[email protected]>
AuthorDate: Mon, 24 Jan 2011 15:33:52 +0800
Committer:  Ingo Molnar <[email protected]>
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.

LKML-Reference: <[email protected]>

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

index 77e9166..3547699 100644
@@ -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)
 {

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

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

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

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:

Thanks to all involved,
Christian.

BOFH excuse #431:

Borg implants are failing