From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Torvald Riegel <triegel@redhat.com>, 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, 1 Feb 2012 14:45:54 -0800 [thread overview]
Message-ID: <20120201224554.GK2382@linux.vnet.ibm.com> (raw)
In-Reply-To: <CA+55aFx4SdtZcYaWZ-=JG3yVFJxsBa-Yqn0m+h4Y=QHdRjx6_w@mail.gmail.com>
On Wed, Feb 01, 2012 at 12:59:24PM -0800, Linus Torvalds wrote:
> On Wed, Feb 1, 2012 at 12:41 PM, Torvald Riegel <triegel@redhat.com> wrote:
> >
> > You do rely on the compiler to do common transformations I suppose:
> > hoist loads out of loops, CSE, etc. How do you expect the compiler to
> > know whether they are allowed for a particular piece of code or not?
>
> We have barriers.
>
> Compiler barriers are cheap. They look like this:
>
> include/linux/compiler-gcc.h:
> #define barrier() __asm__ __volatile__("": : :"memory")
>
> and if we don't allow hoisting, we'll often use something like that.
>
> It's not the only thing we do. We have cases where it's not that you
> can't hoist things outside of loops, it's that you have to read things
> exactly *once*, and then use that particular value (ie the compiler
> can't be allowed to reload the value because it may change). So any
> *particular* value we read is valid, but we can't allow the compiler
> to do anything but a single access.
>
> So we have this beauty:
>
> include/linux/compiler.h:
> #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
My (perhaps forlorn and naive) hope is that C++11 memory_order_relaxed
will eventually allow ACCESS_ONCE() to be upgraded so that (for example)
access-once increments can generate a single increment-memory instruction
on x86.
> which does that for us (quite often, it's not used as-is: a more
> common case is using the "rcu_access_pointer()" inline function that
> not only does that ACCESS_ONCE() but also has the capability to enable
> run-time dynamic verification that you are in a proper RCU read locked
> section etc)
And I also hope that rcu_access_pointer(), rcu_dereference(), and
friends can eventually be written in terms of C++11 memory_order_consume
so that the some of the less-sane optimizations can actually be applied
without breaking the kernel.
Of course, this cannot happen immediately. First, gcc must fully support
the C++11 features of interest, the inevitable bugs must be found and
fixed, and the optimizer will no doubt need to be reworked a bit before
the kernel can make use of these features.
New architectures might eventually might define things like atomic_inc()
in terms of C++11 atomics, but let's start with the straightforward stuff
as and if it makes sense.
Thanx, Paul
> We also have tons of (very subtle) code that actually creates locks
> and memory ordering etc. They usually just use the big "barrier()"
> approach to telling the compiler not to combine or move memory
> accesses around it.
>
> And I realize that compiler people tend to think that loop hoisting
> etc is absolutely critical for performance, and some big hammer like
> "barrier()" makes a compiler person wince. You think it results in
> horrible code generation problems.
>
> It really doesn't. Loops are fairly unusual in the kernel to begin
> with, and the compiler barriers are a total non-issue. We have much
> more problems with the actual CPU barriers that can be *very*
> expensive on some architectures, and we work a lot at avoiding those
> and avoiding cacheline ping-pong issues etc.
>
> >> > No vague assumptions with lots of hand-waving.
> >>
> >> So here's basically what the kernel needs:
> >>
> >> - if we don't touch a field, the compiler doesn't touch it.
> >
> > "we don't touch a field": what does that mean precisely? Never touch it
> > in the same thread? Same function? Same basic block? Between two
> > sequence points? When crossing synchronizing code? For example, in the
> > above code, can we touch it earlier/later?
>
> Why are you trying to make it more complex than it is?
>
> If the source code doesn't write to it, the assembly the compiler
> generates doesn't write to it.
>
> Don't try to make it anything more complicated. This has *nothing* to
> do with threads or functions or anything else.
>
> If you do massive inlining, and you don't see any barriers or
> conditionals or other reasons not to write to it, just write to it.
>
> Don't try to appear smart and make this into something it isn't.
>
> Look at the damn five-line example of the bug. FIX THE BUG. Don't try
> to make it anything bigger than a stupid compiler bug. Don't try to
> make this into a problem it simply isn't.
>
> Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
next prev parent reply other threads:[~2012-02-01 22:47 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 [this message]
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
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=20120201224554.GK2382@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.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 \
--cc=triegel@redhat.com \
/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).