Re: [PATCH 2/3] broadcast-tick: Move oneshot broadcast mask to per cpu variables v2

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, 29 Aug 2011, Andi Kleen wrote:

> From: Andi Kleen <ak@linux.intel.com>
> 
> Avoid a global cache line hotspot in the oneshot cpu mask. Maintain
> this information in per cpu variables instead.

<SNIP>

> @@ -411,7 +418,9 @@ again:
>  	cpumask_clear(to_cpumask(tmpmask));
>  	now = ktime_get();
>  	/* Find all expired events */
> -	for_each_cpu(cpu, tick_get_broadcast_oneshot_mask()) {
> +	for_each_online_cpu(cpu) {
> +		if (!per_cpu(state, cpu).need_oneshot)
> +			continue;

So we iterate over each online CPU instead. I'm really not convinced
that this is better performing especially if the number of CPUs in
broadcast mode is way smaller than the number of online CPUs.

Also having a separate per cpu variable which is just a boolean marker
is silly. If we really want move that to per cpu storage then the
marker should simply be the expiry time of that CPU so you don't have
to evaluate two per cpu variables, which are in completely different
cachelines. If a CPU is not in that mode that expiry time should
simply read KTIME_MAX.

But again, I'm not convinced that iterating over a large number of
CPUs to find a single one in oneshot mode is a good idea.

Thanks,

	tglx