LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Andrea Parri <andrea.parri@amarulasolutions.com>
Cc: Will Deacon <will.deacon@arm.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	LKMM Maintainers -- Akira Yokosawa <akiyks@gmail.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Daniel Lustig <dlustig@nvidia.com>,
	David Howells <dhowells@redhat.com>,
	Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	Nicholas Piggin <npiggin@gmail.com>,
	Kernel development list <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v2] tools/memory-model: Add extra ordering for locks and remove it for ordinary release/acquire
Date: Thu, 12 Jul 2018 15:48:21 +0200
Message-ID: <20180712134821.GT2494@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20180712115249.GA6201@andrea>

On Thu, Jul 12, 2018 at 01:52:49PM +0200, Andrea Parri wrote:
> On Thu, Jul 12, 2018 at 09:40:40AM +0200, Peter Zijlstra wrote:
> > On Wed, Jul 11, 2018 at 02:34:21PM +0200, Andrea Parri wrote:

> > >   2) Resolve the above mentioned controversy (the inconsistency between
> > >      - locking operations and atomic RMWs on one side, and their actual
> > >      implementation in generic code on the other), thus enabling the use
> > >      of LKMM _and_ its tools for the analysis/reviewing of the latter.
> > 
> > This is a good point; so lets see if there is something we can do to
> > strengthen the model so it all works again.
> > 
> > And I think if we raise atomic*_acquire() to require TSO (but ideally
> > raise it to RCsc) we're there.
> > 

> You mean: "when paired with a po-earlier release to the same memory
> location", right?  I am afraid that neither arm64 nor riscv current
> implementations would ensure "(r1 == 1 && r2 == 0) forbidden" if we
> removed the po-earlier spin_unlock()...

Yes indeed. More on this below.

> But again, these are stuble patterns, and my guess is that several/
> most kernel developers really won't care about such guarantees (and
> if some will do, they'll have the tools to figure out what they can
> actually rely on ...)

Yes it is subtle, yes most people won't care, however the problem is
that it is subtly the wrong way around. People expect causality, this is
a human failing perhaps, but that's how it is.

And I strongly feel we should have our locks be such that they don't
subtly break things.

Take for instance the pattern where RCU relies on RCsc locks, this is an
entirely simple and straight forward use of locks, yet completely fails
on this subtle point.

And people will not even try and use complicated tools for apparently
simple things. They'll say, oh of course this simple thing will work
right.

I'm still hoping we can convince the PowerPC people that they're wrong,
and get rid of this wart and just call all locks RCsc.

> OTOH (as I pointed out earlier) the strengthening we're configuring
> will prevent some arch. (riscv being just the example of today!) to
> go "full RCpc", and this will inevitably "complicate" both the LKMM
> and the reviewing process of related changes (atomics, locking, ...;
> c.f., this debate), apparently, just because you  ;-) want to "care"
> about these guarantees.

It's not just me btw, Linus also cares about these matters. Widely used
primitives such as spinlocks, should not have subtle and
counter-intuitive behaviour such as RCpc.



Anyway, back to the problem of being able to use the memory model to
describe locks. This is I think a useful property.

My earlier reasoning was that:

  - smp_store_release() + smp_load_acquire() := RCpc

  - we use smp_store_release() as unlock()

Therefore, if we want unlock+lock to imply at least TSO (ideally
smp_mb()) we need lock to make up for whatever unlock lacks.

Hence my proposal to strenghten rmw-acquire, because that is the basic
primitive used to implement lock.

But as you (and Will) point out, we don't so much care about rmw-acquire
semantics as much as that we care about unlock+lock behaviour. Another
way to look at this is to define:

  smp-store-release + rmw-acquire := TSO (ideally smp_mb)

But then we also have to look at:

  rmw-release + smp-load-acquire
  rmw-release + rmw-acquire

for completeness sake, and I would suggest they result in (at least) the
same (TSO) ordering as the one we really care about.

One alternative is to no longer use smp_store_release() for unlock(),
and say define atomic_set_release() to be in the rmw-release class
instead of being a simple smp_store_release().

Another, and I like this proposal least, is to introduce a new barrier
to make this all work.

  parent reply index

Thread overview: 84+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-09 20:01 Alan Stern
2018-07-09 21:45 ` Paul E. McKenney
2018-07-10 13:57   ` Alan Stern
2018-07-10 16:25     ` Paul E. McKenney
     [not found]       ` <Pine.LNX.4.44L0.1807101416390.1449-100000@iolanthe.rowland.org>
2018-07-10 19:58         ` [PATCH v3] " Paul E. McKenney
2018-07-10 20:24           ` Alan Stern
2018-07-10 20:31             ` Paul E. McKenney
2018-07-11  9:43         ` Will Deacon
2018-07-11 15:42           ` Paul E. McKenney
2018-07-11 16:17             ` Andrea Parri
2018-07-11 18:03               ` Paul E. McKenney
2018-07-11 16:34           ` Peter Zijlstra
2018-07-11 18:10             ` Paul E. McKenney
2018-07-10  9:38 ` [PATCH v2] " Andrea Parri
2018-07-10 14:48   ` Alan Stern
2018-07-10 15:24     ` Andrea Parri
2018-07-10 15:34       ` Alan Stern
2018-07-10 23:14         ` Andrea Parri
2018-07-11  9:43   ` Will Deacon
2018-07-11 12:34     ` Andrea Parri
2018-07-11 12:54       ` Andrea Parri
2018-07-11 15:57       ` Will Deacon
2018-07-11 16:28         ` Andrea Parri
2018-07-11 17:00         ` Peter Zijlstra
2018-07-11 17:50           ` Daniel Lustig
2018-07-12  8:34             ` Andrea Parri
2018-07-12  9:29             ` Peter Zijlstra
2018-07-12  7:40       ` Peter Zijlstra
2018-07-12  9:34         ` Peter Zijlstra
2018-07-12  9:45           ` Will Deacon
2018-07-13  2:17             ` Daniel Lustig
2018-07-12 11:52         ` Andrea Parri
2018-07-12 12:01           ` Andrea Parri
2018-07-12 12:11             ` Peter Zijlstra
2018-07-12 13:48           ` Peter Zijlstra [this message]
2018-07-12 16:19             ` Paul E. McKenney
2018-07-12 17:04             ` Alan Stern
2018-07-12 17:14               ` Will Deacon
2018-07-12 17:28               ` Paul E. McKenney
2018-07-12 18:05                 ` Peter Zijlstra
2018-07-12 18:10                   ` Linus Torvalds
2018-07-12 19:52                     ` Andrea Parri
2018-07-12 20:24                       ` Andrea Parri
2018-07-13  2:05                     ` Daniel Lustig
2018-07-13  4:03                       ` Paul E. McKenney
2018-07-13  9:07                       ` Andrea Parri
2018-07-13  9:35                         ` Will Deacon
2018-07-13 17:16                           ` Linus Torvalds
2018-07-13 19:06                             ` Andrea Parri
2018-07-14  1:51                               ` Alan Stern
2018-07-14  2:58                                 ` Linus Torvalds
2018-07-16  2:31                                   ` Paul E. McKenney
2018-07-13 11:08                     ` Peter Zijlstra
2018-07-13 13:15                       ` Michael Ellerman
2018-07-13 16:42                         ` Peter Zijlstra
2018-07-13 19:56                           ` Andrea Parri
2018-07-16 14:40                           ` Michael Ellerman
2018-07-16 19:01                             ` Peter Zijlstra
2018-07-16 19:30                             ` Linus Torvalds
2018-07-17 14:45                               ` Michael Ellerman
2018-07-17 16:19                                 ` Linus Torvalds
2018-07-17 18:33                                   ` Paul E. McKenney
2018-07-17 18:42                                     ` Peter Zijlstra
2018-07-17 19:40                                       ` Paul E. McKenney
2018-07-17 19:47                                       ` Alan Stern
2018-07-17 18:44                                     ` Linus Torvalds
2018-07-17 18:49                                       ` Linus Torvalds
2018-07-17 19:42                                         ` Paul E. McKenney
2018-07-17 19:37                                       ` Alan Stern
2018-07-17 20:13                                         ` Linus Torvalds
2018-07-17 19:38                                       ` Paul E. McKenney
2018-07-17 19:40                                     ` Andrea Parri
2018-07-17 19:52                                       ` Paul E. McKenney
2018-07-18 12:31                                   ` Michael Ellerman
2018-07-18 13:16                             ` Michael Ellerman
2018-07-12 17:52               ` Andrea Parri
2018-07-12 20:43                 ` Alan Stern
2018-07-12 21:13                   ` Andrea Parri
2018-07-12 21:23                     ` Andrea Parri
2018-07-12 18:33               ` Peter Zijlstra
2018-07-12 17:45             ` Andrea Parri
2018-07-10 16:56 ` Daniel Lustig
     [not found]   ` <Pine.LNX.4.44L0.1807101315140.1449-100000@iolanthe.rowland.org>
2018-07-10 23:31     ` Andrea Parri
2018-07-11 14:19       ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180712134821.GT2494@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akiyks@gmail.com \
    --cc=andrea.parri@amarulasolutions.com \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=dlustig@nvidia.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=npiggin@gmail.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.org \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git