linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Will Deacon <will@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	Andrea Parri <parri.andrea@gmail.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	Nick Piggin <npiggin@gmail.com>,
	David Howells <dhowells@redhat.com>,
	Jade Alglave <j.alglave@ucl.ac.uk>,
	Luc Maranget <luc.maranget@inria.fr>,
	Akira Yokosawa <akiyks@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-toolchains@vger.kernel.org,
	linux-arch <linux-arch@vger.kernel.org>
Subject: Re: [RFC] LKMM: Add volatile_if()
Date: Mon, 7 Jun 2021 11:08:26 -0700	[thread overview]
Message-ID: <20210607180826.GV4397@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <20210607160252.GA7580@willie-the-truck>

On Mon, Jun 07, 2021 at 05:02:53PM +0100, Will Deacon wrote:
> Hi Paul,
> 
> On Mon, Jun 07, 2021 at 08:25:33AM -0700, Paul E. McKenney wrote:
> > On Mon, Jun 07, 2021 at 12:52:35PM +0100, Will Deacon wrote:
> > > It's the conditional instructions that are more fun. For example, the CSEL
> > > instruction:
> > > 
> > > 	CSEL	X0, X1, X2, <cond>
> > > 
> > > basically says:
> > > 
> > > 	if (cond)
> > > 		X0 = X1;
> > > 	else
> > > 		X0 = X2;
> > > 
> > > these are just register-register operations, but the idea is that the CPU
> > > can predict that "branching event" inside the CSEL instruction and
> > > speculatively rename X0 while waiting for the condition to resolve.
> > > 
> > > So then you can add loads and stores to the mix along the lines of:
> > > 
> > > 	LDR	X0, [X1]		// X0 = *X1
> > > 	CMP	X0, X2
> > > 	CSEL	X3, X4, X5, EQ		// X3 = (X0 == X2) ? X4 : X5
> > > 	STR	X3, [X6]		// MUST BE ORDERED AFTER THE LOAD
> > > 	STR	X7, [X8]		// Can be reordered
> > > 
> > > (assuming X1, X6, X8 all point to different locations in memory)
> > > 
> > > So now we have a dependency from the load to the first store, but the
> > > interesting part is that the last store is _not_ ordered wrt either of the
> > > other two memory accesses, whereas it would be if we used a conditional
> > > branch instead of the CSEL. Make sense?
> > 
> > And if I remember correctly, this is why LKMM orders loads in the
> > "if" condition only with stores in the "then" and "else" clauses,
> > not with stores after the end of the "if" statement.  Or is there
> > some case that I am missing?
> 
> It's not clear to me that such a restriction prevents the compiler from
> using any of the arm64 conditional instructions in place of the conditional
> branch in such a way that you end up with an "independent" store in the
> assembly output constructed from two stores on the "then" and "else" paths
> which the compiler determined where the same.
> 
> > > Now, obviously the compiler is blissfully unaware that conditional
> > > data processing instructions can give rise to dependencies than
> > > conditional branches, so the question really is how much do we need to
> > > care in the kernel?
> > > 
> > > My preference is to use load-acquire instead of control dependencies so
> > > that we don't have to worry about this, or any future relaxations to the
> > > CPU architecture, at all.
> > 
> > From what I can see, ARMv8 has DMB(LD) and DMB(ST).  Does it have
> > something like a DMB(LD,ST) that would act something like powerpc lwsync?
> > 
> > Or are you proposing rewriting the "if" conditions to upgrade
> > READ_ONCE() to smp_load_acquire()?  Or something else?
> > 
> > Just trying to find out exactly what you are proposing.  ;-)
> 
> Some options are:
> 
>  (1) Do nothing until something actually goes wrong (and hope we spot/debug it)
> 
>  (2) Have volatile_if force a conditional branch, assuming that it solves
>      the problem and doesn't hurt codegen (I still haven't convinced myself
>      for either case)
> 
>  (3) Upgrade READ_ONCE() to RCpc acquire, relaxed atomic RMWs to RCsc
>      acquire on arm64
> 
>  (4) Introduce e.g. READ_ONCE_CTRL(), atomic_add_return_ctrl() etc
>      specifically for control dependencies and upgrade only those for
>      arm64
> 
>  (5) Work to get toolchain support for dependency ordering and use that
> 
> I'm suggesting (3) or (4) because, honestly, it feels like we're being
> squeezed from both sides with both the compiler and the hardware prepared
> to break control dependencies.

I will toss out this as well:

  (6) Create a volatile_if() that does not support an "else" clause,
      thus covering all current use cases and avoiding some of the
      same-store issues.  Which in the end might or might not help,
      but perhaps worth looking into.

							Thanx, Paul

  reply	other threads:[~2021-06-07 18:08 UTC|newest]

Thread overview: 121+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 10:12 Peter Zijlstra
2021-06-04 10:44 ` Will Deacon
2021-06-04 11:13   ` Will Deacon
2021-06-04 11:31   ` Peter Zijlstra
2021-06-04 13:44     ` Will Deacon
2021-06-04 13:56       ` Peter Zijlstra
2021-06-04 15:13         ` Will Deacon
2021-06-04 15:22           ` Peter Zijlstra
2021-06-04 15:36             ` Alan Stern
2021-06-04 15:42             ` Peter Zijlstra
2021-06-04 15:51               ` Alan Stern
2021-06-04 16:17                 ` Peter Zijlstra
2021-06-04 18:27                   ` Alan Stern
2021-06-04 19:09                     ` Linus Torvalds
2021-06-04 19:18                       ` Linus Torvalds
2021-06-04 20:56                         ` Paul E. McKenney
2021-06-04 21:27                           ` Linus Torvalds
2021-06-04 21:40                             ` Paul E. McKenney
2021-06-04 22:19                               ` Linus Torvalds
2021-06-05 14:57                                 ` Alan Stern
2021-06-06  0:14                                   ` Paul E. McKenney
2021-06-06  1:29                                     ` Alan Stern
2021-06-06  3:41                                       ` Linus Torvalds
2021-06-06  4:43                                         ` Paul E. McKenney
2021-06-06 13:17                                           ` Segher Boessenkool
2021-06-06 19:07                                             ` Paul E. McKenney
2021-06-06 12:59                                         ` Segher Boessenkool
2021-06-06 13:47                                           ` Alan Stern
2021-06-06 17:13                                             ` Segher Boessenkool
2021-06-06 18:25                                           ` Linus Torvalds
2021-06-06 19:19                                             ` Segher Boessenkool
2021-06-06 18:41                                         ` Alan Stern
2021-06-06 18:59                                         ` Jakub Jelinek
2021-06-06 19:15                                           ` Paul E. McKenney
2021-06-06 19:22                                           ` Linus Torvalds
2021-06-06 20:11                                             ` Segher Boessenkool
2021-06-06 21:19                                             ` Alexander Monakov
2021-06-06 22:38                                               ` Linus Torvalds
2021-06-06 23:39                                                 ` Rasmus Villemoes
2021-06-06 23:44                                                   ` Rasmus Villemoes
2021-06-07  8:01                                                 ` Alexander Monakov
2021-06-07  8:27                                                   ` Marco Elver
2021-06-07 15:28                                                     ` Paul E. McKenney
2021-06-07 17:04                                                       ` Marco Elver
2021-06-08  9:30                                                         ` Marco Elver
2021-06-08 11:22                                                           ` Peter Zijlstra
2021-06-08 15:28                                                             ` Segher Boessenkool
2021-06-09 12:44                                                               ` Marco Elver
2021-06-09 15:31                                                                 ` Segher Boessenkool
2021-06-09 16:13                                                                   ` Marco Elver
2021-06-09 17:14                                                                     ` Segher Boessenkool
2021-06-09 17:31                                                                       ` Nick Desaulniers
2021-06-09 20:24                                                                         ` Segher Boessenkool
2021-06-09 18:25                                                                     ` Linus Torvalds
2021-06-07 17:52                                                   ` Segher Boessenkool
2021-06-07 18:07                                                     ` Alexander Monakov
2021-06-07 18:18                                                       ` Segher Boessenkool
2021-06-07 17:42                                                 ` Segher Boessenkool
2021-06-07 20:31                                                   ` Linus Torvalds
2021-06-07 22:54                                                     ` Segher Boessenkool
2021-06-06 11:53                                       ` Segher Boessenkool
2021-06-06 13:45                                         ` Alan Stern
2021-06-06 18:04                                         ` Linus Torvalds
2021-06-06 18:22                                           ` Alan Stern
2021-06-06 18:43                                             ` Linus Torvalds
2021-06-07 10:43                                               ` Peter Zijlstra
2021-06-07 11:52                                                 ` Will Deacon
2021-06-07 15:25                                                   ` Paul E. McKenney
2021-06-07 16:02                                                     ` Will Deacon
2021-06-07 18:08                                                       ` Paul E. McKenney [this message]
     [not found]                                                   ` <20210730172020.GA32396@knuckles.cs.ucl.ac.uk>
2021-07-30 20:35                                                     ` Alan Stern
2021-08-02 21:18                                                     ` Alan Stern
2021-08-02 23:31                                                     ` Paul E. McKenney
2021-08-04 20:09                                                       ` Alan Stern
2021-08-05 19:47                                                     ` Alan Stern
2021-08-07  0:51                                                     ` Alan Stern
2021-06-06 18:40                                           ` Segher Boessenkool
2021-06-06 18:48                                             ` Linus Torvalds
2021-06-06 18:53                                               ` Linus Torvalds
2021-06-06 19:52                                               ` Segher Boessenkool
2021-06-06 20:11                                                 ` Linus Torvalds
2021-06-06 20:26                                                   ` Segher Boessenkool
2021-06-06 23:37                                                     ` Paul E. McKenney
2021-06-07 14:12                                                       ` Segher Boessenkool
2021-06-07 15:27                                                         ` Paul E. McKenney
2021-06-07 18:23                                                           ` Segher Boessenkool
2021-06-07 19:51                                                             ` Alan Stern
2021-06-07 20:16                                                               ` Paul E. McKenney
2021-06-07 22:40                                                                 ` Segher Boessenkool
2021-06-07 23:26                                                                   ` Paul E. McKenney
2021-06-07 10:52                                                     ` Peter Zijlstra
2021-06-07 14:16                                                       ` Segher Boessenkool
2021-06-04 22:05                             ` Peter Zijlstra
2021-06-05  3:14                       ` Alan Stern
2021-06-05 16:24                         ` Linus Torvalds
2021-06-04 15:50         ` Segher Boessenkool
2021-06-04 15:47     ` Segher Boessenkool
2021-06-04 11:44 ` Peter Zijlstra
2021-06-04 14:13   ` Paul E. McKenney
2021-06-04 15:35   ` Segher Boessenkool
2021-06-04 16:10     ` Peter Zijlstra
2021-06-04 16:40       ` Segher Boessenkool
2021-06-04 18:55         ` Paul E. McKenney
2021-06-04 19:53           ` Segher Boessenkool
2021-06-04 20:40             ` Paul E. McKenney
2021-06-06 11:36               ` Segher Boessenkool
2021-06-06 19:01                 ` Paul E. McKenney
2021-06-04 14:25 ` Alan Stern
2021-06-04 16:09 ` Segher Boessenkool
2021-06-04 16:33   ` Peter Zijlstra
2021-06-04 16:30 ` Linus Torvalds
2021-06-04 16:37   ` Peter Zijlstra
2021-06-04 16:52     ` Segher Boessenkool
2021-06-04 17:10     ` Linus Torvalds
2021-06-04 17:24       ` Segher Boessenkool
2021-06-04 17:38         ` Linus Torvalds
2021-06-04 18:25           ` Segher Boessenkool
2021-06-04 19:17         ` Peter Zijlstra
2021-06-04 20:43           ` Paul E. McKenney
2021-06-04 18:23       ` Alan Stern
2021-06-08 12:48 ` David Laight

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=20210607180826.GV4397@paulmck-ThinkPad-P17-Gen-1 \
    --to=paulmck@kernel.org \
    --cc=akiyks@gmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=npiggin@gmail.com \
    --cc=parri.andrea@gmail.com \
    --cc=peterz@infradead.org \
    --cc=segher@kernel.crashing.org \
    --cc=stern@rowland.harvard.edu \
    --cc=torvalds@linux-foundation.org \
    --cc=will@kernel.org \
    --subject='Re: [RFC] LKMM: Add volatile_if()' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox