Re: [PATCH 2/5] sched: factor out code to should_we_balance()

[Joonsoo Kim <iamjoonsoo.kim@lge.com>]

Hello, Peter.

On Tue, Apr 02, 2013 at 10:10:06AM +0200, Peter Zijlstra wrote:
> On Thu, 2013-03-28 at 16:58 +0900, Joonsoo Kim wrote:
> > Now checking that this cpu is appropriate to balance is embedded into
> > update_sg_lb_stats() and this checking has no direct relationship to
> > this
> > function.
> > 
> > There is not enough reason to place this checking at
> > update_sg_lb_stats(),
> > except saving one iteration for sched_group_cpus.
> 
> Its only one iteration if there's only 2 groups, but there can be more
> than 2, take any desktop Intel i7, it will have 4-8 cores, each with
> HT; thus the CPU domain will have 4-8 groups.
> 
> And note that local_group is always the first group of a domain, so
> we'd stop the balance at the first group and avoid touching the other
> 3-7, avoiding touching cachelines on 6-14 cpus.
> 
> So this short-cut does make sense.. its not pretty, granted, but
> killing it doesn't seem right.

It seems that there is some misunderstanding about this patch.
In this patch, we don't iterate all groups. Instead, we iterate on
cpus of local sched_group only. So there is no penalty you mentioned.

In summary, net effect is below.

* For cpus which are not proper to balance,
Reduce function call,
Reduce memset

* For cpus which should balance
Extra one iteration on cpus of local sched_group in should_we_balance()
Reduce some branch, so expect better optimization

Thanks.

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/