LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Andrea Parri <andrea.parri@amarulasolutions.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Will Deacon <will.deacon@arm.com>,
	Daniel Lustig <dlustig@nvidia.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	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>,
	Nick Piggin <npiggin@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] tools/memory-model: Add extra ordering for locks and remove it for ordinary release/acquire
Date: Fri, 13 Jul 2018 21:51:29 -0400 (EDT)
Message-ID: <Pine.LNX.4.44L0.1807132133330.26947-100000@netrider.rowland.org> (raw)
In-Reply-To: <20180713190638.GA4269@andrea>

On Fri, 13 Jul 2018, Andrea Parri wrote:

> On Fri, Jul 13, 2018 at 10:16:48AM -0700, Linus Torvalds wrote:
> > On Fri, Jul 13, 2018 at 2:34 AM Will Deacon <will.deacon@arm.com> wrote:
> > >
> > > And, since we're stating preferences, I'll reiterate my preference towards:
> > >
> > >         * RCsc unlock/lock
> > >         * RCpc release/acquire
> > 
> > Yes, I think this would be best. We *used* to have pretty heavy-weight
> > locking rules for various reasons, and we relaxed them for reasons
> > that weren't perhaps always the right ones.
> > 
> > Locking is pretty heavy-weight in general, and meant to be the "I
> > don't really have to think about this very much" option. Then not
> > being serializing enough to confuse people when it allows odd behavior
> > (on _some_ architectures) does not sound like a great idea.
> > 
> > In contrast, when you do release/acquire or any of the other "I know
> > what I'm doing" things, I think we want the minimal serialization
> > implied by the very specialized op.
> 
> The changes under discussion are _not_ affecting uses such as:
> 
>   P0:
>   spin_lock(s);
>   UPDATE data_struct
>   spin_unlock(s);
> 
>   P1:
>   spin_lock(s);
>   UPDATE data_struct
>   spin_unlock(s);
> 
>   [...]
> 
> (most common use case for locking?):  these uses work just _fine_ with 
> the current implementations and in LKMM.
> 
> OTOH, these changes are going to affect uses where threads interact by
> "mixing" locking and _other_ synchronization primitives such as in:
> 
>   { x = 0; y = 0; }
> 
>   P0:
>   spin_lock(s);
>   WRITE_ONCE(x, 1);
>   spin_unlock(s);
> 
>   P1:
>   spin_lock(s);
>   r0 = READ_ONCE(x);
>   WRITE_ONCE(y, 1);
>   spin_unlock(s);
> 
>   P2:
>   r1 = smp_load_acquire(&y);
>   r2 = READ_ONCE(x);
> 
>   BUG_ON(r0 == 1 && r1 == 1 && r2 == 0)
> 
> and
> 
>   { x = 0; y = 0; z = 0; }
> 
>   P0:
>   spin_lock(s);
>   WRITE_ONCE(x, 1);
>   r0 = READ_ONCE(y);
>   spin_unlock(s);
> 
>   P1:
>   spin_lock(s);
>   WRITE_ONCE(y, 1);
>   r1 = READ_ONCE(z);
>   spin_unlock(s);
> 
>   P2
>   WRITE_ONCE(z, 1);
>   smp_mb();
>   r2 = READ_ONCE(x);
> 
>   BUG_ON(r0 == 0 && r1 == 0 && r2 == 0)
> 
> (inspired from __two__ uses in kernel/{sched,rcu}).  Even if someone were
> to tell me that locks serialize enough, I'd still be prompted to say "yes,
> but do / can my BUG_ON()s fire?".

The point being that the scenarios under discussion in this thread all 
fall most definitely into the "Non-standard usage; you'd better know 
exactly what you're doing" category.

Which suggests, by Linus's reasoning, that locking should be as
lightweight as possible while still being able to perform its basic job
of defining critical sections.  In other words, RCpc.

And which would still leave smp_mb__after_unlock_lock available for
more esoteric usages.  Although it provides RCsc ordering, I assume the
overhead wouldn't be prohibitive in situations where only RCtso
ordering is needed.

Alan

> Actually, my very first reaction, before starting what does appear to be
> indeed a long and complex conversation, would probably be to run/check the
> above snippets against the (latest) LKMM, by using the associated tool.
> 
> Once "checked" with both people and automated models, I'd probably remain
> suspicious about my "magic" code so that I most likely will be prompted to
> dig into each single arch. implementation / reference manual...
> 
> ... Time's up!
> 
>   Andrea



  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
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 [this message]
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=Pine.LNX.4.44L0.1807132133330.26947-100000@netrider.rowland.org \
    --to=stern@rowland.harvard.edu \
    --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=peterz@infradead.org \
    --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