linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Torvald Riegel <triegel@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Jan Kara <jack@suse.cz>, LKML <linux-kernel@vger.kernel.org>,
	linux-ia64@vger.kernel.org, dsterba@suse.cz, ptesarik@suse.cz,
	rguenther@suse.de, gcc@gcc.gnu.org
Subject: Re: Memory corruption due to word sharing
Date: Wed, 01 Feb 2012 22:37:46 +0100	[thread overview]
Message-ID: <1328132266.15992.6528.camel@triegel.csb> (raw)
In-Reply-To: <CA+55aFx=4AhdFEgjY3b=85__mGYX8BKcaXpFC=1XZzoFFjeTrw@mail.gmail.com>

On Wed, 2012-02-01 at 13:20 -0800, Linus Torvalds wrote:
> On Wed, Feb 1, 2012 at 12:53 PM, Torvald Riegel <triegel@redhat.com> wrote:
> >
> > For volatile, I agree.
> >
> > However, the original btrfs example was *without* a volatile, and that's
> > why I raised the memory model point.  This triggered an error in a
> > concurrent execution, so that's memory model land, at least in C
> > language standard.
> 
> Sure. The thing is, if you fix the volatile problem, you'll almost
> certainly fix our problem too.
> 
> The whole "compilers actually do reasonable things" approach really
> does work in reality. It in fact works a lot better than reading some
> spec and trying to figure out if something is "valid" or not, and
> having fifteen different compiler writers and users disagree about
> what the meaning of the word "is" is in some part of it.

That's why researchers have formalized the model in order to verify
whether it's sane.  And they found bugs / ambiguities in it:
http://www.cl.cam.ac.uk/~pes20/cpp/

The standards text might still be a typical standards text for various
reasons. But it enables having a formal version of it too, and
discussions about this formal version.  Personally, I find this more
formal model easier to work with, exactly because it is more precise
than prose.

> 
> >> We do end up doing
> >> much more aggressive threading, with models that C11 simply doesn't
> >> cover.
> >
> > Any specific examples for that would be interesting.
> 
> Oh, one of my favorite (NOT!) pieces of code in the kernel is the
> implementation of the
> 
>    smp_read_barrier_depends()
> 
> macro, which on every single architecture except for one (alpha) is a no-op.
> 
> We have basically 30 or so empty definitions for it, and I think we
> have something like five uses of it. One of them, I think, is
> performance crticial, and the reason for that macro existing.
> 
> What does it do? The semantics is that it's a read barrier between two
> different reads that we want to happen in order wrt two writes on the
> writing side (the writing side also has to have a "smp_wmb()" to order
> those writes). But the reason it isn't a simple read barrier is that
> the reads are actually causally *dependent*, ie we have code like
> 
>    first_read = read_pointer;
>    smp_read_barrier_depends();
>    second_read = *first_read;
> 
> and it turns out that on pretty much all architectures (except for
> alpha), the *data*dependency* will already guarantee that the CPU
> reads the thing in order. And because a read barrier can actually be
> quite expensive, we don't want to have a read barrier for this case.

I don't have time to look at this in detail right now, but it looks
roughly close to C++11's memory_order_consume to me, which is somehwat
like an acquire, but just for subsequent data-dependent loads.  Added
for performance reasons on some architecture AFAIR.

> You really want to try to describe issues like this in your memory
> consistency model? No you don't. Nobody will ever really care, except
> for crazy kernel people. And quite frankly, not even kernel people
> care: we have a fairly big kernel developer community, and the people
> who actually talk about memory ordering issues can be counted on one
> hand. There's the "RCU guy" who writes the RCU helper functions, and
> hides the proper serializing code into those helpers, so that normal
> mortal kernel people don't have to care, and don't even have to *know*
> how ignorant they are about the things.

I'm not a kernel person, and I do care about it.  Userspace is
synchronizing too, and not just with pthread mutexes.

> And that's also why the compiler shouldn't have to care. It's a really
> small esoteric detail, and it can be hidden in a header file and a set
> of library routines. Teaching the compiler about crazy memory ordering
> would just not be worth it. 99.99% of all programmers will never need
> to understand any of it, they'll use the locking primitives and follow
> the rules, and the code that makes it all work is basically invisible
> to them.

I disagree (though it would be nice if it were that esoteric), but
that's off-topic...


  reply	other threads:[~2012-02-01 21:38 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-01 15:19 Memory corruption due to word sharing Jan Kara
2012-02-01 15:34 ` Markus Trippelsdorf
2012-02-01 16:37 ` Colin Walters
2012-02-01 16:56   ` Linus Torvalds
2012-02-01 17:11     ` Jiri Kosina
2012-02-01 17:37       ` Linus Torvalds
2012-02-01 17:41       ` Michael Matz
2012-02-01 18:09         ` David Miller
2012-02-01 18:45           ` Jeff Law
2012-02-01 19:09             ` Linus Torvalds
2012-02-02 15:51               ` Jeff Garzik
2012-02-01 18:57           ` Linus Torvalds
2012-02-01 19:04           ` Peter Bergner
2012-02-01 18:52         ` Linus Torvalds
2012-02-02  9:35           ` Richard Guenther
2012-02-02  9:37           ` Richard Guenther
2012-02-02 13:43           ` Michael Matz
2012-02-01 16:41 ` Linus Torvalds
2012-02-01 17:42   ` Torvald Riegel
2012-02-01 19:40     ` Jakub Jelinek
2012-02-01 20:01       ` Linus Torvalds
2012-02-01 20:16         ` Jakub Jelinek
2012-02-01 20:44           ` Linus Torvalds
2012-02-02 15:58             ` Aldy Hernandez
2012-02-02 16:28               ` Michael Matz
2012-02-02 17:51                 ` Linus Torvalds
2012-02-01 20:19         ` Linus Torvalds
2012-02-02  9:46           ` Richard Guenther
2012-02-01 19:44     ` Boehm, Hans
2012-02-01 19:54       ` Jeff Law
2012-02-01 19:47     ` Linus Torvalds
2012-02-01 19:58       ` Alan Cox
2012-02-01 20:41       ` Torvald Riegel
2012-02-01 20:59         ` Linus Torvalds
2012-02-01 21:24           ` Torvald Riegel
2012-02-01 21:55             ` Linus Torvalds
2012-02-01 21:25           ` Boehm, Hans
2012-02-01 22:27             ` Linus Torvalds
2012-02-01 22:45           ` Paul E. McKenney
2012-02-01 23:11             ` Linus Torvalds
2012-02-02 18:42               ` Paul E. McKenney
2012-02-02 19:08                 ` Linus Torvalds
2012-02-02 19:37                   ` Paul E. McKenney
2012-02-03 16:38                     ` Andrew MacLeod
2012-02-03 17:16                       ` Linus Torvalds
2012-02-03 19:16                         ` Andrew MacLeod
2012-02-03 20:00                           ` Linus Torvalds
2012-02-03 20:19                             ` Paul E. McKenney
2012-02-06 15:38                             ` Torvald Riegel
2012-02-10 19:27                             ` Richard Henderson
2012-02-02 11:19           ` Ingo Molnar
2012-02-01 21:04       ` Boehm, Hans
2012-02-02  9:28         ` Bernd Petrovitsch
2012-02-01 17:08 ` Torvald Riegel
2012-02-01 17:29   ` Linus Torvalds
2012-02-01 20:53     ` Torvald Riegel
2012-02-01 21:20       ` Linus Torvalds
2012-02-01 21:37         ` Torvald Riegel [this message]
2012-02-01 22:18           ` Boehm, Hans
2012-02-02 11:11 ` James Courtier-Dutton
2012-02-02 11:24   ` Richard Guenther
2012-02-02 11:13 ` David Sterba
2012-02-02 11:23   ` Richard Guenther
2012-02-03  6:45 ` DJ Delorie
2012-02-03  9:37   ` Richard Guenther
2012-02-03 10:03     ` Matthew Gretton-Dann
2012-02-01 17:52 Dennis Clarke

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=1328132266.15992.6528.camel@triegel.csb \
    --to=triegel@redhat.com \
    --cc=dsterba@suse.cz \
    --cc=gcc@gcc.gnu.org \
    --cc=jack@suse.cz \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ptesarik@suse.cz \
    --cc=rguenther@suse.de \
    --cc=torvalds@linux-foundation.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).