All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Peter Hurley <peter@hurleysoftware.com>,
	Will Deacon <will.deacon@arm.com>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [RFC PATCH v2] memory-barriers: remove smp_mb__after_unlock_lock()
Date: Mon, 13 Jul 2015 15:53:43 -0700	[thread overview]
Message-ID: <20150713225343.GA3717@linux.vnet.ibm.com> (raw)
In-Reply-To: <20150713221503.GD19282@twins.programming.kicks-ass.net>

On Tue, Jul 14, 2015 at 12:15:03AM +0200, Peter Zijlstra wrote:
> On Mon, Jul 13, 2015 at 01:16:42PM -0700, Paul E. McKenney wrote:
> > On Mon, Jul 13, 2015 at 03:41:53PM -0400, Peter Hurley wrote:
> > > > Does that answer the question, or am I missing the point?
> > > 
> > > Yes, it shows that smp_mb__after_unlock_lock() has no purpose, since it
> > > is defined only for PowerPC and your test above just showed that for
> > > the sequence
> 
> The only purpose is to provide transitivity, but the documentation fails
> to explicitly call that out.

It does say that it is a full barrier, but I added explicit mention of
transitivity.

> > > 
> > >   store a
> > >   UNLOCK M
> > >   LOCK N
> > >   store b
> > > 
> > > a and b is always observed as an ordered pair {a,b}.
> > 
> > Not quite.
> > 
> > This is instead the sequence that is of concern:
> > 
> > 	store a
> > 	unlock M
> > 	lock N
> > 	load b
> 
> So its late and that table didn't parse, but that should be ordered too.
> The load of b should not be able to escape the lock N.
> 
> If only because LWSYNC is a valid RMB and any LOCK implementation must
> load the lock state to observe it unlocked.

If you actually hold a given lock, then yes, you will observe anything
previously done while holding that same lock, even if you don't use
smp_mb__after_unlock_lock().  The smp_mb__after_unlock_lock() comes into
play when code not holding a lock needs to see the ordering.  RCU needs
this because of the strong ordering that grace periods must provide:
regardless of who started or ended the grace period, anything on any
CPU preceding a given grace period is fully ordered before anything on
any CPU following that same grace period.  It is not clear to me that
anything else would need such strong ordering.

> > > Additionally, the assertion in Documentation/memory_barriers.txt that
> > > the sequence above can be reordered as
> > > 
> > >   LOCK N
> > >   store b
> > >   store a
> > >   UNLOCK M
> > > 
> > > is not true on any existing arch in Linux.
> > 
> > It was at one time and might be again.
> 
> What would be required to make this true? I'm having a hard time seeing
> how things can get reordered like that.

You are right, I failed to merge current and past knowledge.  At one time,
Itanium was said to allow things to bleed into lock-based critical sections.
However, we now know that ld,acq and st,rel really do full ordering.

Compilers might one day do this sort of reordering, but I would guess
that Linux kernel builds would disable this sort of thing.  Something
about wanting critical sections to remain small.

							Thanx, Paul


  parent reply	other threads:[~2015-07-13 22:53 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-13 12:15 [RFC PATCH v2] memory-barriers: remove smp_mb__after_unlock_lock() Will Deacon
2015-07-13 13:09 ` Peter Hurley
2015-07-13 14:24   ` Will Deacon
2015-07-13 15:56     ` Peter Zijlstra
2015-07-13 13:11 ` Peter Zijlstra
2015-07-13 14:09   ` Will Deacon
2015-07-13 14:21     ` Will Deacon
2015-07-13 15:54       ` Peter Zijlstra
2015-07-13 17:50         ` Will Deacon
2015-07-13 20:20           ` Paul E. McKenney
2015-07-13 22:23             ` Peter Zijlstra
2015-07-13 23:04               ` Paul E. McKenney
2015-07-14 10:04                 ` Will Deacon
2015-07-14 12:45                   ` Paul E. McKenney
2015-07-14 12:51                     ` Will Deacon
2015-07-14 14:00                       ` Paul E. McKenney
2015-07-14 14:12                         ` Will Deacon
2015-07-14 19:31                           ` Paul E. McKenney
2015-07-15  1:38                             ` Paul E. McKenney
2015-07-15 10:51                               ` Will Deacon
2015-07-15 13:12                                 ` Paul E. McKenney
2015-07-24 11:31                                   ` Will Deacon
2015-07-24 15:30                                     ` Paul E. McKenney
2015-08-12 13:44                                       ` Will Deacon
2015-08-12 15:43                                         ` Paul E. McKenney
2015-08-12 17:59                                           ` Paul E. McKenney
2015-08-13 10:49                                             ` Will Deacon
2015-08-13 13:10                                               ` Paul E. McKenney
2015-08-17  4:06                                           ` Michael Ellerman
2015-08-17  6:15                                             ` Paul E. McKenney
2015-08-17  8:57                                               ` Will Deacon
2015-08-18  1:50                                                 ` Michael Ellerman
2015-08-18  8:37                                                   ` Will Deacon
2015-08-20  9:45                                                     ` Michael Ellerman
2015-08-20 15:56                                                       ` Will Deacon
2015-08-26  0:27                                                         ` Paul E. McKenney
2015-08-26  4:06                                                           ` Michael Ellerman
2015-07-13 18:23         ` Paul E. McKenney
2015-07-13 19:41           ` Peter Hurley
2015-07-13 20:16             ` Paul E. McKenney
2015-07-13 22:15               ` Peter Zijlstra
2015-07-13 22:43                 ` Benjamin Herrenschmidt
2015-07-14  8:34                   ` Peter Zijlstra
2015-07-13 22:53                 ` Paul E. McKenney [this message]
2015-07-13 22:37         ` Benjamin Herrenschmidt
2015-07-13 22:31 ` Benjamin Herrenschmidt
2015-07-14 10:16   ` Will Deacon
2015-07-15  3:06   ` Michael Ellerman
2015-07-15 10:44     ` Will Deacon
2015-07-16  2:00       ` Michael Ellerman
2015-07-16  5:03         ` Benjamin Herrenschmidt
2015-07-16  5:14           ` Benjamin Herrenschmidt
2015-07-16 15:11             ` Paul E. McKenney
2015-07-16 22:54               ` Benjamin Herrenschmidt
2015-07-17  9:32                 ` Will Deacon
2015-07-17 10:15                   ` Peter Zijlstra
2015-07-17 12:40                     ` Paul E. McKenney
2015-07-17 22:14                   ` Benjamin Herrenschmidt
2015-07-20 13:39                     ` Will Deacon
2015-07-20 13:48                       ` Paul E. McKenney
2015-07-20 13:56                         ` Will Deacon
2015-07-20 21:18                       ` Benjamin Herrenschmidt
2015-07-22 16:49                         ` Will Deacon
2015-07-22 16:49                           ` Will Deacon
2015-07-22 16:49                           ` Will Deacon
2015-09-01  2:57             ` Paul Mackerras
2015-07-15 14:18     ` Paul E. McKenney
2015-07-16  1:34       ` Michael Ellerman

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=20150713225343.GA3717@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peter@hurleysoftware.com \
    --cc=peterz@infradead.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.