Re: [PATCH v2] tools/memory-model: Add extra ordering for locks and remove it for ordinary release/acquire

[Will Deacon <will.deacon@arm.com>]

On Fri, Jul 13, 2018 at 11:07:11AM +0200, Andrea Parri wrote:
> On Thu, Jul 12, 2018 at 07:05:39PM -0700, Daniel Lustig wrote:
> > On 7/12/2018 11:10 AM, Linus Torvalds wrote:
> > > On Thu, Jul 12, 2018 at 11:05 AM Peter Zijlstra <peterz@infradead.org> wrote:
> > >>
> > >> The locking pattern is fairly simple and shows where RCpc comes apart
> > >> from expectation real nice.
> > > 
> > > So who does RCpc right now for the unlock-lock sequence? Somebody
> > > mentioned powerpc. Anybody else?
> > > 
> > > How nasty would be be to make powerpc conform? I will always advocate
> > > tighter locking and ordering rules over looser ones..
> > > 
> > >             Linus
> > 
> > RISC-V probably would have been RCpc if we weren't having this discussion.
> > Depending on how we map atomics/acquire/release/unlock/lock, we can end up
> > producing RCpc, "RCtso" (feel free to find a better name here...), or RCsc
> > behaviors, and we're trying to figure out which we actually need.
> > 
> > I think the debate is this:
> > 
> > Obviously programmers would prefer just to have RCsc and not have to figure out
> > all the complexity of the other options.  On x86 or architectures with native
> > RCsc operations (like ARMv8), that's generally easy enough to get.
> > 
> > For weakly-ordered architectures that use fences for ordering (including
> > PowerPC and sometimes RISC-V, see below), though, it takes extra fences to go
> > from RCpc to either "RCtso" or RCsc.  People using these architectures are
> > concerned about whether there's a negative performance impact from those extra
> > fences.
> > 
> > However, some scheduler code, some RCU code, and probably some other examples
> > already implicitly or explicitly assume unlock()/lock() provides stronger
> > ordering than RCpc.  So, we have to decide whether to:
> > 1) define unlock()/lock() to enforce "RCtso" or RCsc, insert more fences on
> > PowerPC and RISC-V accordingly, and probably negatively regress PowerPC
> > 2) leave unlock()/lock() as enforcing only RCpc, fix any code that currently
> > assumes something stronger than RCpc is being provided, and hope people don't
> > get it wrong in the future
> > 3) some mixture like having unlock()/lock() be "RCtso" but smp_store_release()/
> > smp_cond_load_acquire() be only RCpc
> > 
> > Also, FWIW, if other weakly-ordered architectures come along in the future and
> > also use any kind of lightweight fence rather than native RCsc operations,
> > they'll likely be in the same boat as RISC-V and Power here, in the sense of
> > not providing RCsc by default either.
> > 
> > Is that a fair assessment everyone?
> 
> It's for me, thank you!  And as we've seen, there are arguments for each of
> the above three choices.  I'm afraid that (despite Linus's statement  ;-)),
> my preference would currently go to (2).

And, since we're stating preferences, I'll reiterate my preference towards:

	* RCsc unlock/lock
	* RCpc release/acquire
	* Not fussed about atomic rmws, but having them closer to RCsc would
	  make it easier to implement and reason about generic locking
	  implementations (i.e. reduce the number of special ordering cases
	  and/or magic barrier macros)

Will