linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Will Deacon <will.deacon@arm.com>
Cc: David Howells <dhowells@redhat.com>,
	linux-arch@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, ramana.radhakrishnan@arm.com,
	dwmw2@infradead.org
Subject: Re: [RFC PATCH 00/15] Provide atomics and bitops implemented with ISO C++11 atomics
Date: Wed, 8 Jun 2016 13:01:14 -0700	[thread overview]
Message-ID: <20160608200114.GN4104@linux.vnet.ibm.com> (raw)
In-Reply-To: <20160601144545.GG355@arm.com>

On Wed, Jun 01, 2016 at 03:45:45PM +0100, Will Deacon wrote:
> Hi David,
> 
> On Wed, May 18, 2016 at 04:10:37PM +0100, David Howells wrote:
> > 
> > Here's a set of patches to provide kernel atomics and bitops implemented
> > with ISO C++11 atomic intrinsics.  The second part of the set makes the x86
> > arch use the implementation.
> 
> As you know, I'm really not a big fan of this :)
> 
> Whilst you're seeing some advantages in using this on x86, I suspect
> that's because the vast majority of memory models out there end up using
> similar instructions sequences on that architecture (i.e. MOV and a very
> occasional mfence). For weakly ordered architectures such as arm64, the
> kernel memory model is noticeably different to that offered by C11 and
> I'd be hesitant to map the two as you're proposing here, for the following
> reasons:
> 
>   (1) C11's SC RMW operations are weaker than our full barrier atomics
> 
>   (2) There is no high quality implementation of consume loads, so we'd
>       either need to continue using our existing rcu_deference code or
>       be forced to use acquire loads
> 
>   (3) wmb/rmb don't exist in C11
> 
>   (4) We patch our atomics at runtime based on the CPU capabilites, since
>       we require a single binary kernel Image
> 
>   (5) Even recent versions of GCC have been found to have serious issues
>       generating correct (let alone performant) code [1]
> 
>   (6) If we start mixing and patching C11 atomics with homebrew atomics
>       in an attempt to address some of the issues above, we open ourselves
>       up to potential data races (i.e. undefined behaviour), but I doubt
>       existing compilers actually manage to detect this.

One of the big short-term benefits of David's work is the resulting
bug reports against gcc on sub-optimal code, a number of which are
now fixed, if I remember correctly.  I do agree that the differences
between C11's and the Linux kernel's memory models mean that you have
to be quite careful when using C11 atomics in the Linux kernel.
Even ignoring the self-modifying Linux kernels.  ;-)

> Now, given all of that, you might be surprised to hear that I'm not
> completely against some usage of C11 atomics in the kernel! What I think
> would work quite nicely is defining an asm-generic interface built solely
> out of the C11 _relaxed atomics and SC fences. Would it be efficient? Almost
> certainly not. Would it be useful for new architecture ports to get up and
> running quickly? Definitely.

I agree that might be very hard for the C11 intrinsics to beat tightly
coded asms.  But it might not be all that long before the compilers can
beat straightforward hand-written assembly.  And the compiler might well
eventually be able to beat even tightly code asms in the more complex
cases such as cmpxchg loops.

> In my opinion, if an architecture wants to go further than that (like you've
> proposed here), then the code should be entirely confined to the relevant
> arch/ directory and not advertised as a general, portable mapping between
> the memory models.

Agreed, at least in the near term.

							Thanx, Paul

> Will
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69875
> 

      parent reply	other threads:[~2016-06-08 20:01 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-18 15:10 [RFC PATCH 00/15] Provide atomics and bitops implemented with ISO C++11 atomics David Howells
2016-05-18 15:10 ` [RFC PATCH 01/15] cmpxchg_local() is not signed-value safe, so fix generic atomics David Howells
2016-05-18 15:10   ` David Howells
2016-05-18 15:29   ` Arnd Bergmann
2016-05-18 15:10 ` [RFC PATCH 02/15] tty: ldsem_cmpxchg() should use cmpxchg() not atomic_long_cmpxchg() David Howells
2016-05-18 15:10 ` [RFC PATCH 03/15] Provide atomic_t functions implemented with ISO-C++11 atomics David Howells
2016-05-18 15:10   ` David Howells
2016-05-18 17:31   ` Peter Zijlstra
2016-05-18 17:32   ` Peter Zijlstra
2016-05-18 17:32     ` Peter Zijlstra
2016-05-19  7:36     ` David Woodhouse
2016-05-19  7:45       ` Peter Zijlstra
2016-05-18 17:33   ` Peter Zijlstra
2016-05-19  9:52   ` David Howells
2016-05-19 10:50     ` Peter Zijlstra
2016-05-19 11:31       ` Peter Zijlstra
2016-05-19 11:33         ` Peter Zijlstra
2016-05-19 14:22       ` Paul E. McKenney
2016-05-19 14:41         ` Peter Zijlstra
2016-05-19 15:00           ` Paul E. McKenney
2016-05-20  9:32             ` Michael Ellerman
2016-05-20  9:32               ` Michael Ellerman
2016-05-23 18:39               ` Paul E. McKenney
2016-06-01 14:16     ` Will Deacon
2016-06-01 14:16       ` Will Deacon
2016-05-18 15:11 ` [RFC PATCH 04/15] Convert 32-bit ISO atomics into a template David Howells
2016-05-18 15:11   ` David Howells
2016-05-18 15:11 ` [RFC PATCH 05/15] Provide atomic64_t and atomic_long_t using ISO atomics David Howells
2016-05-18 15:11   ` David Howells
2016-05-18 15:11 ` [RFC PATCH 06/15] Provide 16-bit " David Howells
2016-05-18 15:11   ` David Howells
2016-05-18 17:28   ` Peter Zijlstra
2016-05-18 15:11 ` [RFC PATCH 07/15] Provide cmpxchg(), xchg(), xadd() and __add() based on ISO C++11 intrinsics David Howells
2016-05-18 15:11   ` David Howells
2016-05-18 15:11 ` [RFC PATCH 08/15] Provide an implementation of bitops using C++11 atomics David Howells
2016-05-18 15:11   ` David Howells
2016-05-18 15:11 ` [RFC PATCH 09/15] Make the ISO bitops use 32-bit values internally David Howells
2016-05-18 15:11   ` David Howells
2016-05-18 15:11 ` [RFC PATCH 10/15] x86: Use ISO atomics David Howells
2016-05-18 15:12 ` [RFC PATCH 11/15] x86: Use ISO bitops David Howells
2016-05-18 15:12   ` David Howells
2016-05-18 15:12 ` [RFC PATCH 12/15] x86: Use ISO xchg(), cmpxchg() and friends David Howells
2016-05-18 15:12   ` David Howells
2016-05-18 15:12 ` [RFC PATCH 13/15] x86: Improve spinlocks using ISO C++11 intrinsic atomics David Howells
2016-05-18 15:12   ` David Howells
2016-05-18 17:37   ` Peter Zijlstra
2016-05-18 15:12 ` [RFC PATCH 14/15] x86: Make the mutex implementation use ISO atomic ops David Howells
2016-05-18 15:12   ` David Howells
2016-05-18 15:12 ` [RFC PATCH 15/15] x86: Fix misc cmpxchg() and atomic_cmpxchg() calls to use try/return variants David Howells
2016-05-18 15:12   ` David Howells
2016-05-18 17:22 ` [RFC PATCH 00/15] Provide atomics and bitops implemented with ISO C++11 atomics Peter Zijlstra
2016-05-18 17:45 ` Peter Zijlstra
2016-05-18 18:05 ` Peter Zijlstra
2016-05-19  0:23 ` Paul E. McKenney
2016-06-01 14:45 ` Will Deacon
2016-06-01 14:45   ` Will Deacon
2016-06-08 20:01   ` Paul E. McKenney [this message]

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=20160608200114.GN4104@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ramana.radhakrishnan@arm.com \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).