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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.02.1109061704330.2723@ionos>
Date:	Tue, 6 Sep 2011 17:28:57 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Andi Kleen <andi@...stfloor.org>
cc:	LKML <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@...ux.intel.com>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH 2/3] broadcast-tick: Move oneshot broadcast mask to per
 cpu variables v2

On Mon, 29 Aug 2011, Andi Kleen wrote:

> From: Andi Kleen <ak@...ux.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/