All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>,
	Eric Biggers <ebiggers@kernel.org>,
	"Tobin C. Harding" <me@tobin.cc>,
	linux-fsdevel@vger.kernel.org
Subject: Re: dcache locking question
Date: Sun, 17 Mar 2019 17:35:14 -0700	[thread overview]
Message-ID: <20190318003514.GD4102@linux.ibm.com> (raw)
In-Reply-To: <1552796596.6551.17.camel@HansenPartnership.com>

On Sat, Mar 16, 2019 at 09:23:16PM -0700, James Bottomley wrote:
> On Sun, 2019-03-17 at 03:06 +0000, Al Viro wrote:
> > On Sat, Mar 16, 2019 at 07:20:20PM -0700, James Bottomley wrote:
> > > On Sat, 2019-03-16 at 17:50 -0700, Paul E. McKenney wrote:
> > > [...]
> > > >  I -have- seen stores of constant values be torn, but not stores
> > > > of runtime-variable values and not loads.  Still, such tearing is
> > > > permitted, and including the READ_ONCE() is making it easier for
> > > > things like thread sanitizers.  In addition, the READ_ONCE()
> > > > makes it clear that the value being loaded is unstable, which can
> > > > be useful documentation.
> > > 
> > > Um, just so I'm clear, because this assumption permeates all our
> > > code: load or store tearing can never occur if we're doing load or
> > > store of a 32 bit value which is naturally aligned.  Where
> > > naturally aligned is within the gift of the CPU to determine but
> > > which the compiler or kernel will always ensure for us unless we
> > > pack the structure or deliberately misalign the allocation.

A non-volatile store of certain 32-bit constants can and does tear
on some architectures.  These architectures would be the ones with a
store-immediate instruction with a small immediate field, and where the
32-bit constant is such that a pair of 16-bit immediate store instructions
can store that value.

There was a bug in an old version of GCC where even volatile 32-bit stores
of these constants would tear.  They did fix the bug, but it took some
time to find a GCC person who understood that this was in fact a bug.

Hence my preference for READ_ONCE() and WRITE_ONCE() for data-racing
loads and stores.

> > Wait a sec; are there any 64bit architectures where the same is not
> > guaranteed for dereferencing properly aligned void **?
> 
> Yes, naturally alligned void * dereference shouldn't tear either.  I
> was just using 32 bit as my example because 64 bit accesses will tear
> on 32 bit architectures but 64 bit naturally aligned accesses shouldn't
> tear on 64 bit architectures.  However, since we can't guarantee the 64
> bitness of the architecture 32 bit or void * is our gold standard for
> not tearing.

For stores of quantities not known at compiler time, agreed.  But that
same store-immediate situation could happen on 64-bit systems.

> James
> 
> 
> > If that's the case, I can think of quite a few places that are rather
> > dubious, and I don't see how READ_ONCE() could help in those - e.g.
> > if an architecture only has 32bit loads, rcu list traversals are
> > not going to be doable without one hell of an extra headache.

All the 64-bit systems that run the Linux kernel do have 64-bit load
instructions and rcu_dereference() uses READ_ONCE() internally, so we
should be fine with RCU list traverals.

							Thanx, Paul


  reply	other threads:[~2019-03-18  0:34 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-14 22:56 dcache locking question Tobin C. Harding
2019-03-14 23:09 ` Matthew Wilcox
2019-03-15  1:38   ` Tobin C. Harding
2019-03-14 23:19 ` Tobin C. Harding
2019-03-15  1:50   ` Al Viro
2019-03-15 17:38     ` Eric Biggers
2019-03-15 18:54       ` Al Viro
2019-03-16 22:31         ` Paul E. McKenney
2019-03-17  0:18           ` Al Viro
2019-03-17  0:50             ` Paul E. McKenney
2019-03-17  2:20               ` James Bottomley
2019-03-17  3:06                 ` Al Viro
2019-03-17  4:23                   ` James Bottomley
2019-03-18  0:35                     ` Paul E. McKenney [this message]
2019-03-18 16:26                       ` James Bottomley
2019-03-18 17:11                         ` Paul E. McKenney
2019-03-19 15:45                           ` 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=20190318003514.GD4102@linux.ibm.com \
    --to=paulmck@linux.ibm.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=ebiggers@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=me@tobin.cc \
    --cc=viro@zeniv.linux.org.uk \
    /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.