linux-kernel - Re: local_add

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: <200812172153.05303.rusty@rustcorp.com.au>
Date:	Wed, 17 Dec 2008 21:53:04 +1030
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Paul Mackerras <paulus@...ba.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Christoph Lameter <christoph@...eter.com>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Martin Bligh <mbligh@...gle.com>
Subject: Re: local_add_return

On Wednesday 17 December 2008 02:55:32 Mathieu Desnoyers wrote:
> Specifically on this comment :
> 
> +/* There are three obvious ways to implement local_t on an arch which
> + * can't do single-instruction inc/dec etc.
> + * 1) atomic_long
> + * 2) irq_save/irq_restore
> + * 3) multiple counters.
> 
> Option 3) is not workable for tracers, because it's not safe against
> some exceptions (e.g. some hardware errors) nor NMIs.

Hmm, nor is option 2.  Now I understand where you were coming from and
I sympathize with your dilemna, but I don't think that non-x86 archs should
pay for it where local_t is used as intended, so I don't think local_t should
be (have been) hijacked for this.  nmi_safe_t?

> Also, local_t
> operations must have preemption disabled before playing on per-cpu data,
> which I don't see in your test. This has to be taken into account in the
> runtime cost.

atomic_long_t implementations don't have to.  local_irq_save does it
as a side effect.  You're right about multiple counters tho.  We can
either do it conditionally or unconditionally, but I think unconditional
makes sense (CONFIG_PREEMPT=y seems to be less popular than it was).

> the CPU_OPS work done by Christoph Lameter which use
> segments to address the per-cpu data, which effectively removes the need
> for disabling preemption around local_t operations because the CPU ID
> becomes encoded in a cpu register.

Well, we did this for 32-bit x86 some time ago, so that works today.
64-bit was delayed because of the stack protection code, which needs
a fixed offset for the canary so needs zero-based percpu, but IIRC
that's orthogonal to the CPU_OPS work itself.

Here's the timing diff when trivalue is fixed here (preempt on)
Before:
	local_inc=45 local_add=45 cpu_local_inc=6 local_read=21 local_add_return=127
After:
	local_inc=47 local_add=47 cpu_local_inc=6 local_read=41 local_add_return=127

Since sparc64 has CONFIG_PREEMPT=n in its defconfig, I think it is still
ahead with trivalue.

Thanks,
Rusty.
--
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/