linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: David Howells <dhowells@redhat.com>
Cc: linux-arch@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, ramana.radhakrishnan@arm.com,
	paulmck@linux.vnet.ibm.com, dwmw2@infradead.org
Subject: Re: [RFC PATCH 00/15] Provide atomics and bitops implemented with ISO C++11 atomics
Date: Wed, 1 Jun 2016 15:45:45 +0100	[thread overview]
Message-ID: <20160601144545.GG355@arm.com> (raw)
In-Reply-To: <146358423711.8596.9104061348359986393.stgit@warthog.procyon.org.uk>

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.

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.

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.

Will

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

  parent reply	other threads:[~2016-06-01 14:45 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 [this message]
2016-06-01 14:45   ` Will Deacon
2016-06-08 20:01   ` Paul E. McKenney

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=20160601144545.GG355@arm.com \
    --to=will.deacon@arm.com \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=ramana.radhakrishnan@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).