Re: [PATCH] tools/memory-model: remove rb-dep, smp_read_barrier_depends, and lockless_dereference

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Andrea Parri <parri.andrea@gmail.com>,
	Akira Yokosawa <akiyks@gmail.com>,
	Kernel development list <linux-kernel@vger.kernel.org>,
	mingo@kernel.org, Will Deacon <will.deacon@arm.com>,
	boqun.feng@gmail.com, npiggin@gmail.com, dhowells@redhat.com,
	Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	Patrick Bellasi <patrick.bellasi@arm.com>
Subject: Re: [PATCH] tools/memory-model: remove rb-dep, smp_read_barrier_depends, and lockless_dereference
Date: Tue, 20 Feb 2018 08:10:30 -0800	[thread overview]
Message-ID: <20180220161030.GJ3617@linux.vnet.ibm.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1802201005590.1428-100000@iolanthe.rowland.org>

On Tue, Feb 20, 2018 at 10:11:06AM -0500, Alan Stern wrote:
> On Tue, 20 Feb 2018, Paul E. McKenney wrote:
> 
> > On Mon, Feb 19, 2018 at 09:28:44PM +0100, Peter Zijlstra wrote:
> > > On Mon, Feb 19, 2018 at 11:41:23AM -0800, Paul E. McKenney wrote:
> > > > On Mon, Feb 19, 2018 at 12:14:45PM -0500, Alan Stern wrote:
> > > > > This leaves us with a question: Do we want to change the kernel by
> > > > > adding memory barriers after unsuccessful RMW operations on Alpha, or
> > > > > do we want to change the model by excluding such operations from
> > > > > address dependencies?
> > > > 
> > > > I vote for adding the barrier on Alpha.  However, I don't know of any
> > > > code in the Linux kernel that relies on read-to-read address dependency
> > > > ordering headed by a failing RMW operation, so I don't feel all that
> > > > strongly about this.
> > > 
> > > Right, but not knowing doesn't mean doesn't exist, and most certainly
> > > doesn't mean will never exist.
> > 
> > Fair enough, safety first!
> > 
> > > > > Note that operations like atomic_add_unless() already include memory 
> > > > > barriers.
> > > > 
> > > > And I don't see an atomic_add_unless_relaxed(), so we are good on this
> > > > one.  So far, anyway!  ;-)
> > > 
> > > Not the point, add_unless() is a conditional operation, and therefore
> > > doesn't need to imply anything when failing.
> > 
> > Plus it doesn't return a pointer, so there is no problem with dereferences.
> > Unless someone wants to use its return value as an array index and rely
> > on dependency ordering to the array, but I would NAK that use case.
> 
> You may not get the chance to NAK it.
> 
> We need to be consistent.  Array indexing is indeed a form of address
> dependency, so either we assert that the dependency is enforced when
> the array index is derived from a failed atomic operation, or else we
> assert that failed atomic operations do not create address
> dependencies.

The problem is that the compiler can get up to a great deal more mischief
with integers than it can with pointers.  I agree that it would be quite
challenging to represent this distinction between integers and pointers in
the model, but then again herd does not yet support array indexing anyway.

So for the time being, anyway, I must stand by my NAK.

							Thanx, Paul