linux-kernel - Re: [PATCH 06/16] SPARC: use ACCESS

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.01.0911181000020.4644@localhost.localdomain>
Date:	Wed, 18 Nov 2009 10:09:05 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	David Miller <davem@...emloft.net>
cc:	jslaby@...ell.com, jirislaby@...il.com, mingo@...e.hu,
	nhorman@...driver.com, sfr@...b.auug.org.au,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	marcin.slusarz@...il.com, tglx@...utronix.de, mingo@...hat.com,
	hpa@...or.com, jmorris@...ei.org, heiko.carstens@...ibm.com,
	sparclinux@...r.kernel.org
Subject: Re: [PATCH 06/16] SPARC: use ACCESS_ONCE for rlimits

On Wed, 18 Nov 2009, David Miller wrote:
> 
> But I wonder have we really seen the compiler create this
> kind of situation?  Or is this patch series based upon the
> fact that it "could happen"?

We have seen things like that in practice - where the compiler re-loads a 
value twice, rather than use a copy like the source code did.

That said, it's rare, to the point of being _almost_ unheard of. It's much 
more common that gcc generates bad code by doing the reverse (trying to 
keep things in registers and spilling, instead of just re-generating the 
value). There are very very few cases where ACCESS_ONCE() actually matters 
for correctness.

Because in practice, the value is either modified some way (and spilling 
it is cheaper than re-computing the modification), or there's just some 
operation that might act as a memory barrier and alias the original memory 
location so gcc wouldn't dare re-load anyway.

However, one of the nice things about ACCESS_ONCE() is that it's also a 
big flag for "this value is loaded without locking, on purpose".

So even if it doesn't then actually change code generation significantly 
(most common end result especially on x86 that has most ALU instructions 
taking memory operations: gcc generates slightly worse code due to getting 
nervous about 'volatile' and not combining instructions), it's a big 
honking piece of programmer documentation: look out!

It's basically a heads-up for lockless programming like RCU. As such, it 
can be something scary, but when it's done right, it's a good thing. And I 
think that for rlimits, we do have a good reason to say "sure, somebody 
else may change the limit values concurrently, but we don't really care: 
we just want _one_ value, whether it's the old or the new one".

That said, the patch you Ack'ed is in the series of patches that I hated, 
and Nak'ed for other reasons (namely "-EEXPRESSIONTOOCOMPLICATEDTOLIVE").

			Linus
--
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/