linux-kernel - Re: [RFC][PATCH] mips: Fix arch_spin

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: <20160127114348.GF2390@arm.com>
Date:	Wed, 27 Jan 2016 11:43:48 +0000
From:	Will Deacon <will.deacon@....com>
To:	"Maciej W. Rozycki" <macro@...tec.com>
Cc:	David Daney <ddaney@...iumnetworks.com>,
	Måns Rullgård <mans@...sr.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	linux-kernel@...r.kernel.org,
	Paul McKenney <paulmck@...ux.vnet.ibm.com>,
	torvalds@...ux-foundation.org, boqun.feng@...il.com
Subject: Re: [RFC][PATCH] mips: Fix arch_spin_unlock()

Hi Maciej,

Thanks for digging all this up.

On Wed, Jan 27, 2016 at 09:57:24AM +0000, Maciej W. Rozycki wrote:
> On Thu, 12 Nov 2015, David Daney wrote:
> 
> > > > Certainly we can load up the code with "SYNC" all over the place, but
> > > > it will kill performance on SMP systems.  So, my vote would be to make
> > > > it as light weight as possible, but no lighter.  That will mean
> > > > inventing the proper barrier primitives.
> > > 
> > > It seems to me that the proper barrier here is a "SYNC 18" aka
> > > SYNC_RELEASE instruction, at least on CPUs that implement that variant.
> 
>  For the record, we've had "cooked" aliases in the toolchain for a short 
> while now -- since Sep 2010 or binutils 2.21 -- so for readability you can 
> actually use `sync_release' in your source code rather than obscure `sync 
> 18' (of course you could define a macro instead, but there's no need now), 
> and disassembly will show the "cooked" mnemonic too.
> 
>  Although Documentation/Changes still lists binutils 2.12 as the minimum, 
> so perhaps using macros is indeed the way to go now, at least for the time 
> being.
> 
> > Yes, unfortunately very few CPUs implement that.  It is an instruction that
> > MIPS invented only recently, so older CPUs need a different solution.
> 
>  Hmm, it looks to me we might actually be safe, although as often the 
> situation seems more complicated than it had to be.

[... trim ISA archaeology ...]

>  Overall I think it should be safe after all to use SYNC_RELEASE and other 
> modern lightweight barriers uncondtionally under the assumption that 
> architecture was meant to remain backward compatible.  Even though it 
> might be possible someone would implement unusual semantics for the then 
> undefined `stype' values, I highly doubt it as it would be extra effort 
> and hardware logic space for no gain.  We could try and reach architecture 
> overseers to double-check whether the `stype' encodings, somewhat 
> irregularly distributed, were indeed defined in a manner so as not to 
> clash with values implementers chose to use before rev. 2.61 of the 
> architecture specification.

Do you know whether a SYNC 18 (RELEASE) followed in program order by a
SYNC 17 (ACQUIRE) creates a full barrier (i.e. something like SYNC 16)?

If not, you may need to implement smp_mb__after_unlock_lock for RCU
to ensure globally transitive unlock->lock ordering should you decide
to relax your locking barriers.

Will