On Wed, Mar 18, 2015 at 02:31:02PM +0800, Xunlei Pang wrote: > From: Xunlei Pang <pang.xun...@linaro.org> > > In load_balance(), some members of lb_env will be assigned with > new values in LBF_DST_PINNED case. But lb_env::flags may still > retain LBF_ALL_PINNED if no proper tasks were found afterwards > due to another balance, task affinity changing, etc, which can > really happen because busiest rq lock has already been released.
Sure.. > This is wrong, for example with env.dst_cpu assigned new_dst_cpu > when going back to "redo" label, it may cause should_we_balance() > to return false which is unreasonable. Why? You've got a very unlikely, very hard case, its unlikely that anything we do will substantially improve the situation, but you make the code uglier for it. > This patch restores proper status of env before "goto redo", and > improves "out_all_pinned" and "out_one_pinned" labels. That doesn't even begin to explain half of what the patch does. > @@ -6977,12 +6978,19 @@ more_balance: > /* All tasks on this runqueue were pinned by CPU affinity */ > if (unlikely(env.flags & LBF_ALL_PINNED)) { > cpumask_clear_cpu(cpu_of(busiest), cpus); > - if (!cpumask_empty(cpus)) { > - env.loop = 0; > - env.loop_break = sched_nr_migrate_break; > - goto redo; > + if (env.new_dst_cpu != -1) { I really don't get this, how can this not be? > + env.new_dst_cpu = -1; > + cpumask_or(cpus, cpus, > + sched_group_cpus(sd->groups)); > + cpumask_and(cpus, cpus, cpu_active_mask); More unexplained magic, why is this right? The rest of the patch isn't much better. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/