From: Ingo Molnar <mingo@elte.hu>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: akpm@linux-foundation.org, torvalds@linux-foundation.org,
andi@firstfloor.org, ak@linux.intel.com, cl@linux-foundation.org,
rth@twiddle.net, sfr@canb.auug.org.au, travis@sgi.com,
linux-kernel@vger.kernel.org
Subject: Re: [patch 2/8] compiler-gcc.h: add more comments to RELOC_HIDE
Date: Sat, 10 Jan 2009 13:29:45 +0100 [thread overview]
Message-ID: <20090110122945.GA28033@elte.hu> (raw)
In-Reply-To: <200901102211.45603.rusty@rustcorp.com.au>
* Rusty Russell <rusty@rustcorp.com.au> wrote:
> On Saturday 10 January 2009 11:10:53 akpm@linux-foundation.org wrote:
> > From: Andi Kleen <andi@firstfloor.org>
> ...
> > + * This is needed because the C standard makes it undefined to do
> > + * pointer arithmetic on "objects" outside their boundaries and the
> > + * gcc optimizers assume this is the case. In particular they
> > + * assume such arithmetic does not wrap.
that is a rather useless comment without real examples.
> ...
>
> That last sentence seems misleading to me; the case I know of gcc
> hurting us is the powerpc literal strcpy case. See prom_init.c where
> vars are not yet mapped at KERNEL_BASE, so they offset them manually:
> gcc turned a strcpy(dest, "literal"+KERNEL_BASE) into memcpy(dest,
> "literal"+KERNEL_BASE, 7-KERNEL_BASE). Boom.
>
> Richard came up with the RELOC_HIDE macro, to future-proof against other
> such optimizations. I can't remember if I was clever enough to use it
> for per-cpu on my own, or whether he pointed it out.
>
> I hope that clarifies once and for all!
Note that 32-bit x86 lived just fine for years without RELOC_HIDE() - and
it has similar large-offset pointer arithmetics both for PAGE_OFFSET and
for per-cpu accesses.
The obscurity and undocumentedness of RELOC_HIDE() turned it a bit of a
voodoo programming construct: it got introduced due to unspecified fear
but there's no real documented specifics of actual GCC breakage caused by
it in the field, other than a constant string put into a percpu area -
which is pretty stupid to do to begin with.
So it's a bit of a mystery construct, and if we add a comment, here are a
few examples that brings the above textbook definition closer to reality,
examples of what GCC _could_ do and how it could break without RELOC_HIDE
- and this might be much more useful for the commit log than the obscure
C-lawyering paragraph above:
1) String operations
For example if we do something like:
strcpy(tmp, "abc" + LARGE_OFFSET)
GCC could assume that "abc" is a 4 bytes long string, and could assume
that LARGE_OFFSET points within it, and could optimize this into:
memcpy(tmp, "abc" + LARGE_OFFSET, 4 - LARGE_OFFSET)
[ This is a bit far-fetched, as it would need us to put constant strings
into PER_CPU areas - but it's not actually wrong to do it so if it
happens it's nasty. OTOH, very clear crashes will point to it, so not
really relevant. ]
2) Special structure sizes
For example we want to load a value from "&var + LARGE_OFFSET" (common
percpu construct). If 'var' is a C structure that happens to have a size
of 255 bytes, then GCC could legitimately assume that 'LARGE_OFFSET' is
0..255 [inclusive], and could optimize LARGE_OFFSET to be truncated to a
single byte.
3) Memory object aliasing, alias analysis
A "bad" aliasing optimization happens when GCC detects that a %gs
referenced and a kernel linear pointer has the same value - and thus
mis-optimizes it assuming that already loaded values must be the same.
[ does not really happen as both in the current and in the zerobased case
the two memory spaces are clearly separated. ]
Or if GCC does not recognize that aliased accesses to the same object
involve the same object - and optimizes things assuming that it's two
separate objects - and could schedule instructions in a mixed up way,
corrupting the object.
Not a real issue either here: the %gs space is very separate from the
kernel linear address space, we never access the same object via two
similar-looking pointers at once. We just set up the percpu area and
leave those linear addresses alone.
But RELOC_HIDE() does not actually _prevent_ any useful optimizations from
being done - so it does not really hurt. But the comment added by this
patch is rather misleading as it's formulated way too generic way without
pointing out the obscurity of these cases, and thus hurts the end result
more than it helps.
Ingo
next parent reply other threads:[~2009-01-10 12:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <200901100040.n0A0eruc013680@imap1.linux-foundation.org>
[not found] ` <200901102211.45603.rusty@rustcorp.com.au>
2009-01-10 12:29 ` Ingo Molnar [this message]
2009-01-12 17:33 ` [patch 2/8] compiler-gcc.h: add more comments to RELOC_HIDE Christoph Lameter
[not found] ` <200901151227.27935.rusty@rustcorp.com.au>
2009-01-15 3:33 ` Linus Torvalds
2009-01-15 19:19 ` Christoph Lameter
2009-01-15 22:44 ` Andi Kleen
2009-01-15 20:29 ` Christoph Lameter
2009-01-15 23:15 ` Richard Henderson
2009-01-16 20:37 ` Christoph Lameter
2009-01-19 16:30 ` Richard Henderson
2009-01-21 13:50 ` Christoph Lameter
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=20090110122945.GA28033@elte.hu \
--to=mingo@elte.hu \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=cl@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rth@twiddle.net \
--cc=rusty@rustcorp.com.au \
--cc=sfr@canb.auug.org.au \
--cc=torvalds@linux-foundation.org \
--cc=travis@sgi.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