Re: Soft lockup regression from today's sched.git merge.

Peter Zijlstra
2008-04-22T12:45:39Z

On Tue, 2008-04-22 at 03:05 -0700, David Miller wrote:

> BTW, I'm also getting cpu's wedged in the group aggregate code:
>
> [ 121.338742] TSTATE: 0000009980001602 TPC: 000000000054ea20 TNPC: 0000000000456828 Y: 00000000 Not tainted
> [ 121.338778] TPC: <__first_cpu+0x4/0x28>
> [ 121.338791] g0: 0000000000000000 g1: 0000000000000002 g2: 0000000000000000 g3: 0000000000000002
> [ 121.338809] g4: fffff803fe9b24c0 g5: fffff8001587c000 g6: fffff803fe9d0000 g7: 00000000007b7260
> [ 121.338827] o0: 0000000000000002 o1: 00000000007b7258 o2: 0000000000000000 o3: 00000000007b7800
> [ 121.338845] o4: 0000000000845000 o5: 0000000000000400 sp: fffff803fe9d2ed1 ret_pc: 0000000000456820
> [ 121.338879] RPC: <aggregate_group_shares+0x10c/0x16c>
> [ 121.338893] l0: 0000000000000400 l1: 000000000000000d l2: 00000000000003ff l3: 0000000000000000
> [ 121.338911] l4: 0000000000000000 l5: 0000000000000000 l6: fffff803fe9d0000 l7: 0000000080009002
> [ 121.338928] i0: 0000000000801c20 i1: fffff800161ca508 i2: 00000000000001d8 i3: 0000000000000001
> [ 121.338946] i4: fffff800161d9c00 i5: 0000000000000001 i6: fffff803fe9d2f91 i7: 0000000000456904
> [ 121.338968] I7: <aggregate_get_down+0x84/0x13c>
>
> I'm suspecting the deluge of cpumask changes that also went in today.
>
> I guess I'll be bisecting all day tomorrow too :-/

Sadly both the cpumask changes and the group load-balancer came in the
same batch - so its all new code. Also, it looks like the cpumask
changes are before the load balance changes - so bisecting this will be
'fun'.

That said; the code in question looks like:

static
void aggregate_group_shares(struct task_group *tg, struct sched_domain *sd)
{
unsigned long shares = 0;
int i;

again:
for_each_cpu_mask(i, sd->span)
shares += tg->cfs_rq[i]->shares;

/*
* When the span doesn't have any shares assigned, but does have
* tasks to run do a machine wide rebalance (should be rare).
*/
if (unlikely(!shares && aggregate(tg, sd)->rq_weight)) {
__aggregate_redistribute_shares(tg);
goto again;
}

aggregate(tg, sd)->shares = shares;
}

and the __first_cpu() call is from the for_each_cpu_mask() afaict. and
sd->span should be good - that's not new. So I'm a bit puzzled.

But you say they get wedged - so the above trace is a snapshot (NMI,
sysrq-[tw] or the like) that could also mean they get wedged in this
'again' loop we have here.

I have two patches; the first will stick in a printk() to see if it is
indeed the loop in this function. The second is an attempt at breaking
out of it.

BTW, what does the sched_domain tree look like on that 128-cpu machine?

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

Index: linux-2.6-2/kernel/printk.c
===================================================================
--- linux-2.6-2.orig/kernel/printk.c
+++ linux-2.6-2/kernel/printk.c
@@ -1020,8 +1020,6 @@ void release_console_sem(void)
console_locked = 0;
up(&console_sem);
spin_unlock_irqrestore(&logbuf_lock, flags);
- if (wake_klogd)
- wake_up_klogd();
}
EXPORT_SYMBOL(release_console_sem);

Index: linux-2.6-2/kernel/sched.c
===================================================================
--- linux-2.6-2.orig/kernel/sched.c
+++ linux-2.6-2/kernel/sched.c
@@ -1710,6 +1710,7 @@ again:
* tasks to run do a machine wide rebalance (should be rare).
*/
if (unlikely(!shares && aggregate(tg, sd)->rq_weight)) {
+ printk(KERN_EMERG "[%d] no sd shares\n", raw_smp_processor_id());
__aggregate_redistribute_shares(tg);
goto again;
}

---
kernel/sched.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

Index: linux-2.6-2/kernel/sched.c
===================================================================
--- linux-2.6-2.orig/kernel/sched.c
+++ linux-2.6-2/kernel/sched.c
@@ -1661,9 +1661,9 @@ void aggregate_group_weight(struct task_
*/
static void __aggregate_redistribute_shares(struct task_group *tg)
{
- int i, max_cpu = smp_processor_id();
+ int i;
unsigned long rq_weight = 0;
- unsigned long shares, max_shares = 0, shares_rem = tg->shares;
+ unsigned long shares, shares_rem = tg->shares;

for_each_possible_cpu(i)
rq_weight += tg->cfs_rq[i]->load.weight;
@@ -1677,10 +1677,6 @@ static void __aggregate_redistribute_sha

tg->cfs_rq[i]->shares = shares;

- if (shares > max_shares) {
- max_shares = shares;
- max_cpu = i;
- }
shares_rem -= shares;
}

@@ -1689,7 +1685,7 @@ static void __aggregate_redistribute_sha
* due to rounding down when computing the per-cpu shares.
*/
if (shares_rem)
- tg->cfs_rq[max_cpu]->shares += shares_rem;
+ tg->cfs_rq[smp_process_id()]->shares += shares_rem;
}

/*