Re: [PATCH 2/2] tools/memory-model: Add write ordering by release-acquire and by locks

From: Will Deacon <will.deacon@arm.com>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Andrea Parri <andrea.parri@amarulasolutions.com>,
	LKMM Maintainers -- Akira Yokosawa <akiyks@gmail.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	David Howells <dhowells@redhat.com>,
	Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	Nicholas Piggin <npiggin@gmail.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Kernel development list <linux-kernel@vger.kernel.org>,
	dlustig@nvidia.com
Subject: Re: [PATCH 2/2] tools/memory-model: Add write ordering by release-acquire and by locks
Date: Mon, 9 Jul 2018 17:52:00 +0100	[thread overview]
Message-ID: <20180709165200.GA4689@arm.com> (raw)
In-Reply-To: <20180706211055.GN3593@linux.vnet.ibm.com>

On Fri, Jul 06, 2018 at 02:10:55PM -0700, Paul E. McKenney wrote:
> On Fri, Jul 06, 2018 at 04:37:21PM -0400, Alan Stern wrote:
> > On Thu, 5 Jul 2018, Andrea Parri wrote:
> > 
> > > > At any rate, it looks like instead of strengthening the relation, I
> > > > should write a patch that removes it entirely.  I also will add new,
> > > > stronger relations for use with locking, essentially making spin_lock
> > > > and spin_unlock be RCsc.
> > > 
> > > Thank you.
> > > 
> > > Ah let me put this forward: please keep an eye on the (generic)
> > > 
> > >   queued_spin_lock()
> > >   queued_spin_unlock()
> > > 
> > > (just to point out an example). Their implementation (in part.,
> > > the fast-path) suggests that if we will stick to RCsc lock then
> > > we should also stick to RCsc acq. load from RMW and rel. store.
> > 
> > A very good point.  The implementation of those routines uses
> > atomic_cmpxchg_acquire() to acquire the lock.  Unless this is
> > implemented with an operation or fence that provides write-write
> > ordering (in conjunction with a suitable release), qspinlocks won't
> > have the ordering properties that we want.
> > 
> > I'm going to assume that the release operations used for unlocking
> > don't need to have any extra properties; only the lock-acquire
> > operations need to be special (i.e., stronger than a normal
> > smp_load_acquire). This suggests that atomic RMW functions with acquire
> > semantics should also use this stronger form of acquire.
> > 
> > Does anybody have a different suggestion?
> 
> The approach you suggest makes sense to me.  Will, Peter, Daniel, any
> reasons why this approach would be a problem for you guys?

qspinlock is very much opt-in per arch, so we can simply require that
an architecture must have RCsc RmW atomics if they want to use qspinlock.
Should an architecture arise where that isn't the case, then we could
consider an arch hook in the qspinlock code, but I don't think we have
to solve that yet.

Will