Messages in this thread Patch in this message | | | Subject | Re: [BUG] kernel freezes with latest tree | From | Peter Zijlstra <> | Date | Wed, 11 Jan 2012 17:58:22 +0100 |
| |
On Wed, 2012-01-11 at 08:31 -0800, Linus Torvalds wrote: > On Wed, Jan 11, 2012 at 8:05 AM, Peter Zijlstra <peterz@infradead.org> wrote: > > > > Ah, right! Silly me. One possibility is to rotate that list, except that > > won't work for the cgroup case where we have another iteration. > > I just wonder whether you *really* need that loop at all? > > If something went wrong with the attempted task move - you raced with > another cpu, or whatever - is there any real reason to even bother to > try again? > > It's just a heuristic, after all, and we'll come back to balancing later. > > The minimal patch looks good, but I did want to ask whether people > have considered just removing the looping entirely?
Yeah, I did consider it, and given the current code it doesn't really make a difference either way. But ideally we'll go fix the code to provide better progress over repeated attempts.
Esp for people who put the migration count rather low (-rt crackpots) multiple rounds (provided progress) make more sense.
Something like the below snippet improves the progress for !cgroup kernels, but it also defeats the regular termination condition since any list with two or more elements will endlessly rotate until we hit the break limit.
We could fix this by adding an iteration count and limit that to nr_running I guess, but since world and dog compiles kernels with cgroup muck these days we need something slightly more clever to deal with the for_each_leaf_cfs_rq() loop in load_balance_fair().
Will ponder more..
--- --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -3244,8 +3246,10 @@ balance_tasks(struct rq *this_rq, int th if ((p->se.load.weight >> 1) > rem_load_move || !can_migrate_task(p, busiest, this_cpu, sd, idle, - lb_flags)) + lb_flags)) { + list_move_tail(&p->se.group_node, &busiest_cfs_rq->tasks); continue; + } pull_task(busiest, p, this_rq, this_cpu); pulled++;
| |