On Fri, Feb 24, 2017 at 11:04:39PM -0600, Josh Poimboeuf wrote:
> On Thu, Feb 23, 2017 at 09:10:39PM +0100, Pavel Machek wrote:
> > Hi!
> >
> >
> > > > > > Somehow, startup_32_smp() is on the stack twice. The stack unwind led
> > > > > > to the startup_32_smp() frame at 0xf50cdf9c rather than the one at
> > > > > > 0xf50cdfa8 (which is where it should normally be). So the question is
> > > > > > how startup_32_smp() got executed the second time, with the wrong stack
> > > > > > offset.
> > > > >
> > > > > Not much idea... but this is stack dump, right? Just because some
> > > > > value is on the stack does not mean it is a return address, no?
> > > >
> > > > Right, but the one at 0xf50cdfa8 is where the startup_32_smp() is
> > > > *supposed* to be. If the unwinder had unwinded to that one, it wouldn't
> > > > have complained. So it looks to me like the CPU somehow booted twice:
> > > > the first time at the right stack address, and the second time it
> > > > somehow ended up with a different stack address.
> > > >
> > > > > And .... startup_32_smp is kind of "interesting" function. Take a
> > > > > look...
> > > >
> > > > Yes, it's used in bringing up the CPU.
> > >
> > > Can you share your .config?
> >
> > Here you go...
>
> What version of gcc are you using?
>
> Can you post a disassembly of the first 10 instructions of
> start_secondary()?
Pavel, ping? I'd like to try to get to the bottom of this issue soon.
I asked for the gcc version and the disassembly of start_secondary()
because I suspect gcc may have done a funky stack alignment prologue
which copies the return address on the stack a second time after
aligning it.
--
Josh
On Thu 2017-03-02 17:45:14, Josh Poimboeuf wrote:
> On Fri, Feb 24, 2017 at 11:04:39PM -0600, Josh Poimboeuf wrote:
> > On Thu, Feb 23, 2017 at 09:10:39PM +0100, Pavel Machek wrote:
> > > Hi!
> > >
> > >
> > > > > > > Somehow, startup_32_smp() is on the stack twice. The stack unwind led
> > > > > > > to the startup_32_smp() frame at 0xf50cdf9c rather than the one at
> > > > > > > 0xf50cdfa8 (which is where it should normally be). So the question is
> > > > > > > how startup_32_smp() got executed the second time, with the wrong stack
> > > > > > > offset.
> > > > > >
> > > > > > Not much idea... but this is stack dump, right? Just because some
> > > > > > value is on the stack does not mean it is a return address, no?
> > > > >
> > > > > Right, but the one at 0xf50cdfa8 is where the startup_32_smp() is
> > > > > *supposed* to be. If the unwinder had unwinded to that one, it wouldn't
> > > > > have complained. So it looks to me like the CPU somehow booted twice:
> > > > > the first time at the right stack address, and the second time it
> > > > > somehow ended up with a different stack address.
> > > > >
> > > > > > And .... startup_32_smp is kind of "interesting" function. Take a
> > > > > > look...
> > > > >
> > > > > Yes, it's used in bringing up the CPU.
> > > >
> > > > Can you share your .config?
> > >
> > > Here you go...
> >
> > What version of gcc are you using?
> >
> > Can you post a disassembly of the first 10 instructions of
> > start_secondary()?
>
> Pavel, ping? I'd like to try to get to the bottom of this issue soon.
>
> I asked for the gcc version and the disassembly of start_secondary()
> because I suspect gcc may have done a funky stack alignment prologue
> which copies the return address on the stack a second time after
> aligning it.
Sorry for the delay. This is on v4.11-rc1, but that should be similar.
pavel@duo:~$ gcc --version
gcc (Debian 4.9.2-10) 4.9.2
And here's the disassemble:
c402d200 <start_secondary>:
c402d200: 57 push %edi
c402d201: 8d 7c 24 08 lea 0x8(%esp),%edi
c402d205: 83 e4 f8 and $0xfffffff8,%esp
c402d208: ff 77 fc pushl -0x4(%edi)
c402d20b: 55 push %ebp
c402d20c: 89 e5 mov %esp,%ebp
c402d20e: 57 push %edi
c402d20f: 56 push %esi
c402d210: 83 ec 10 sub $0x10,%esp
c402d213: e8 78 78 ff ff call c4024a90 <cpu_init>
c402d218: ff 15 d0 d7 0c c5 call *0xc50cd7d0
c402d21e: 8b 15 00 53 05 c5 mov 0xc5055300,%edx
c402d224: 8d 75 e8 lea -0x18(%ebp),%esi
c402d227: 64 a1 f4 c0 1d c5 mov %fs:0xc51dc0f4,%eax
c402d22d: 89 45 e8 mov %eax,-0x18(%ebp)
c402d230: b8 20 00 00 00 mov $0x20,%eax
c402d235: ff 52 78 call *0x78(%edx)
c402d238: 8b 15 00 53 05 c5 mov 0xc5055300,%edx
c402d23e: ff 52 4c call *0x4c(%edx)
c402d241: e8 ea 2c 00 00 call c402ff30
<apic_ap_setup>
c402d246: 8b 45 e8 mov -0x18(%ebp),%eax
c402d249: e8 42 fb ff ff call c402cd90
<smp_store_cpu_info>
c402d24e: e8 5d 37 fd ff call c40009b0
<calibrate_delay>
c402d253: 8b 55 e8 mov -0x18(%ebp),%edx
c402d256: b8 00 c0 1d c5 mov $0xc51dc000,%eax
c402d25b: 8b 0d 88 d6 0b c5 mov 0xc50bd688,%ecx
c402d261: f6 05 fa fc 13 c5 04 testb $0x4,0xc513fcfa
c402d268: 8b 14 95 20 52 05 c5 mov
-0x3afaade0(,%edx,4),%edx
c402d26f: 89 8c 10 c4 00 00 00 mov %ecx,0xc4(%eax,%edx,1)
c402d276: 0f 85 24 01 00 00 jne c402d3a0
<start_secondary+0x1a0>
c402d27c: 64 a1 f4 c0 1d c5 mov %fs:0xc51dc0f4,%eax
c402d282: e8 49 fb ff ff call c402cdd0
<set_cpu_sibling_map>
Let me know if I should go back to v4.10 and retry.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Tue, Mar 07, 2017 at 12:28:55PM -0600, Josh Poimboeuf wrote:
> On Tue, Mar 07, 2017 at 09:59:44AM -0800, Andy Lutomirski wrote:
> > On Tue, Mar 7, 2017 at 9:52 AM, Linus Torvalds
> > <[email protected]> wrote:
> > > On Tue, Mar 7, 2017 at 9:38 AM, Josh Poimboeuf <[email protected]> wrote:
> > >>
> > >> So I'm thinking we should have -maccumulate-outgoing-args always enabled
> > >> on x86_32 just like we already do on x86_64.
> > >
> > > Ugh. I realize we have workarounds for bugs, but I think
> > > -maccumulate-outgoing-args is nasty. It just generates worse code by
> > > avoiding the much nicer push/pop sequences, afaik.
>
> Yes, maybe the pushes/pops around a function call are a little easier to
> read than movs.
>
> But the -maccumulate-outgoing-args realignment prologue is a *lot* worse
> for readability, IMO.
Er, the *NON* -maccumulate-outgoing-args realignment prologue.
> Also, the gcc documentation says -maccumulate-outgoing-args is
> "generally beneficial for performance and size."
>
> Not to mention the fact that -maccumulate-outgoing-args seems to already
> be enabled in most cases anyway. Having it uniformly enabled everywhere
> makes it less confusing overall when the rare divergences are
> encountered. From looking at some of the changes related to
> ADD_ACCUMULATE_OUTGOING_ARGS in arch/x86/Makefile_32.cpu, I can tell
> that several others before me have stumbled into this prologue issue.
>
> > > On x86-64 it's not such a big deal, because we pass the first six
> > > arguments in registers anyway, so the arguments on the stack is a
> > > fairly unusual special case.
> > >
> > > But on x86-32, we only have three argument registers, so this
> > > braindamage is potentially worse.
> > >
> > > I guess we already do this in most situations due to the gcc bugs, but
> > > I do think it's sad that we would do it for our _own_ bugs too.
> > >
> >
> > Is it our bug or a gcc bug? I would have thought
> > -fno-omit-frame-pointer meant that the call-frame-to-return-address
> > offset should be constant and -fomit-frame-pointer meant "do
> > whatever".
>
> I don't think it's a gcc bug because it doesn't seem to violate frame
> pointer conventions:
>
> pushl -0x4(%edi) # copy return address
> push %ebp
>
> The frame pointer and return address are still stored adjacently. And
> it normally allows unwinds to work fine.
>
> The problem is the kernel unwinder's assumption that the last frame
> pointer is at a certain address. That assumption breaks with the DRAP
> prologue.
>
> > Also, maybe I'm missing something, but does gcc's code even allow the
> > function to return sensibly? It could do it by a nasty calculation
> > involving backing out the old esp from edi, but that seems quite
> > overcomplicated.
>
> That's what it does:
>
> lea -0x8(%edi),%esp
> pop %edi
> ret
>
> --
> Josh
--
Josh
On Mon, Mar 06, 2017 at 05:38:07PM +0100, Pavel Machek wrote:
> Sorry for the delay. This is on v4.11-rc1, but that should be similar.
>
> pavel@duo:~$ gcc --version
> gcc (Debian 4.9.2-10) 4.9.2
>
> And here's the disassemble:
>
> c402d200 <start_secondary>:
> c402d200: 57 push %edi
> c402d201: 8d 7c 24 08 lea 0x8(%esp),%edi
> c402d205: 83 e4 f8 and $0xfffffff8,%esp
> c402d208: ff 77 fc pushl -0x4(%edi)
> c402d20b: 55 push %ebp
> c402d20c: 89 e5 mov %esp,%ebp
> c402d20e: 57 push %edi
> c402d20f: 56 push %esi
> c402d210: 83 ec 10 sub $0x10,%esp
Thanks. This confirms what I was thinking, the function prologue is
wack. It's realigning the stack, but it's not the "normal" realign
pattern. Instead it makes a fake frame header, which saves a duplicate
copy of the return address ("pushl -0x4(%edi)") in a place the unwinder
wasn't expecting.
I did some digging in gcc to figure out why this can happen. gcc uses
something called a Dynamic Realign Argument Pointer (DRAP), which, when
enabled, makes a prologue like the above. It's almost always enabled
for aligned stacks when -maccumulate-outgoing-args isn't set.
So I'm thinking we should have -maccumulate-outgoing-args always enabled
on x86_32 just like we already do on x86_64.
Can you verify the warning is fixed with the following patch?
-----
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..53ec1e4 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -61,7 +61,7 @@ ifeq ($(CONFIG_JUMP_LABEL), y)
ADD_ACCUMULATE_OUTGOING_ARGS := y
endif
-cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
+cflags-y += $(call cc-option,-maccumulate-outgoing-args)
# Bug fix for binutils: this option is required in order to keep
# binutils from generating NOPL instructions against our will.
On Tue, Mar 07, 2017 at 09:59:44AM -0800, Andy Lutomirski wrote:
> On Tue, Mar 7, 2017 at 9:52 AM, Linus Torvalds
> <[email protected]> wrote:
> > On Tue, Mar 7, 2017 at 9:38 AM, Josh Poimboeuf <[email protected]> wrote:
> >>
> >> So I'm thinking we should have -maccumulate-outgoing-args always enabled
> >> on x86_32 just like we already do on x86_64.
> >
> > Ugh. I realize we have workarounds for bugs, but I think
> > -maccumulate-outgoing-args is nasty. It just generates worse code by
> > avoiding the much nicer push/pop sequences, afaik.
Yes, maybe the pushes/pops around a function call are a little easier to
read than movs.
But the -maccumulate-outgoing-args realignment prologue is a *lot* worse
for readability, IMO.
Also, the gcc documentation says -maccumulate-outgoing-args is
"generally beneficial for performance and size."
Not to mention the fact that -maccumulate-outgoing-args seems to already
be enabled in most cases anyway. Having it uniformly enabled everywhere
makes it less confusing overall when the rare divergences are
encountered. From looking at some of the changes related to
ADD_ACCUMULATE_OUTGOING_ARGS in arch/x86/Makefile_32.cpu, I can tell
that several others before me have stumbled into this prologue issue.
> > On x86-64 it's not such a big deal, because we pass the first six
> > arguments in registers anyway, so the arguments on the stack is a
> > fairly unusual special case.
> >
> > But on x86-32, we only have three argument registers, so this
> > braindamage is potentially worse.
> >
> > I guess we already do this in most situations due to the gcc bugs, but
> > I do think it's sad that we would do it for our _own_ bugs too.
> >
>
> Is it our bug or a gcc bug? I would have thought
> -fno-omit-frame-pointer meant that the call-frame-to-return-address
> offset should be constant and -fomit-frame-pointer meant "do
> whatever".
I don't think it's a gcc bug because it doesn't seem to violate frame
pointer conventions:
pushl -0x4(%edi) # copy return address
push %ebp
The frame pointer and return address are still stored adjacently. And
it normally allows unwinds to work fine.
The problem is the kernel unwinder's assumption that the last frame
pointer is at a certain address. That assumption breaks with the DRAP
prologue.
> Also, maybe I'm missing something, but does gcc's code even allow the
> function to return sensibly? It could do it by a nasty calculation
> involving backing out the old esp from edi, but that seems quite
> overcomplicated.
That's what it does:
lea -0x8(%edi),%esp
pop %edi
ret
--
Josh
On Tue, Mar 7, 2017 at 10:28 AM, Josh Poimboeuf <[email protected]> wrote:
>
> Also, the gcc documentation says -maccumulate-outgoing-args is
> "generally beneficial for performance and size."
Hmm. I wonder how true that is. I'm pretty sure it generates bigger
code, although it's probably less noticeable in the kernel (as opposed
to the traditional x86 "push everything" model) due to having the
three register arguments.
And the "it's faster" is almost certainly garbage. It's true on P4 and
some older AMD cores that couldn't do push/pops quickly.
> Not to mention the fact that -maccumulate-outgoing-args seems to already
> be enabled in most cases anyway.
Yeah, that's the main argument for this patch, I think - just remove
the (unusual) special case.
Linus
On Tue, Mar 7, 2017 at 9:52 AM, Linus Torvalds
<[email protected]> wrote:
> On Tue, Mar 7, 2017 at 9:38 AM, Josh Poimboeuf <[email protected]> wrote:
>>
>> So I'm thinking we should have -maccumulate-outgoing-args always enabled
>> on x86_32 just like we already do on x86_64.
>
> Ugh. I realize we have workarounds for bugs, but I think
> -maccumulate-outgoing-args is nasty. It just generates worse code by
> avoiding the much nicer push/pop sequences, afaik.
>
> On x86-64 it's not such a big deal, because we pass the first six
> arguments in registers anyway, so the arguments on the stack is a
> fairly unusual special case.
>
> But on x86-32, we only have three argument registers, so this
> braindamage is potentially worse.
>
> I guess we already do this in most situations due to the gcc bugs, but
> I do think it's sad that we would do it for our _own_ bugs too.
>
Is it our bug or a gcc bug? I would have thought
-fno-omit-frame-pointer meant that the call-frame-to-return-address
offset should be constant and -fomit-frame-pointer meant "do
whatever".
Also, maybe I'm missing something, but does gcc's code even allow the
function to return sensibly? It could do it by a nasty calculation
involving backing out the old esp from edi, but that seems quite
overcomplicated.
--Andy
On Tue, Mar 7, 2017 at 9:38 AM, Josh Poimboeuf <[email protected]> wrote:
>
> So I'm thinking we should have -maccumulate-outgoing-args always enabled
> on x86_32 just like we already do on x86_64.
Ugh. I realize we have workarounds for bugs, but I think
-maccumulate-outgoing-args is nasty. It just generates worse code by
avoiding the much nicer push/pop sequences, afaik.
On x86-64 it's not such a big deal, because we pass the first six
arguments in registers anyway, so the arguments on the stack is a
fairly unusual special case.
But on x86-32, we only have three argument registers, so this
braindamage is potentially worse.
I guess we already do this in most situations due to the gcc bugs, but
I do think it's sad that we would do it for our _own_ bugs too.
Linus
On Tue, Mar 07, 2017 at 10:40:14AM -0800, Linus Torvalds wrote:
> On Tue, Mar 7, 2017 at 10:28 AM, Josh Poimboeuf <[email protected]> wrote:
> >
> > Also, the gcc documentation says -maccumulate-outgoing-args is
> > "generally beneficial for performance and size."
>
> Hmm. I wonder how true that is. I'm pretty sure it generates bigger
> code, although it's probably less noticeable in the kernel (as opposed
> to the traditional x86 "push everything" model) due to having the
> three register arguments.
It does seem to make it bigger. With Pavel's config on gcc 6, if I add
-maccumulate-outgoing-args:
text data bss dec hex filename
12692555 5550652 9146368 27389575 1a1ee87 vmlinux.before
13179531 5546556 9146368 27872455 1a94cc7 vmlinux.after
That's 3.8% more text on x86-32.
(FWIW, on x86-64, the size difference is negligible.)
> And the "it's faster" is almost certainly garbage. It's true on P4 and
> some older AMD cores that couldn't do push/pops quickly.
>
> > Not to mention the fact that -maccumulate-outgoing-args seems to already
> > be enabled in most cases anyway.
>
> Yeah, that's the main argument for this patch, I think - just remove
> the (unusual) special case.
As it turns out, when optimizing for size, gcc seems to ignore
-maccumulate-outgoing-args completely. So I guess we would have to live
with both cases anyway. Which means I'll need to make the unwinder
smart enough to deal with it.
But that brings up another question. If -maccumulate-outgoing-args is
ignored with CONFIG_CC_OPTIMIZE_FOR_SIZE=y, wouldn't using that option
break the things which require -maccumulate-outgoing-args?
So, looking deeper at the various reasons this flag is enabled, they
seem to be mostly obsolete.
- CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
where the stack gets aligned before the mcount call. This issue
should be mostly obsolete as most modern compilers now have -mfentry.
We could make it dependent on CC_USING_FENTRY.
- CONFIG_JUMP_LABEL sets it on x86-32 because of a bug in gcc <= 4.5.1
which has since been fixed with
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226. We could probably
make it gcc-version-dependent.
- x86-64 sets it to apparently make the no-longer-in-tree DWARF unwinder
happy with older versions of gcc.
So it looks like -maccumulate-outgoing-args isn't actually needed in
most cases.
--
Josh
On Wed, Mar 8, 2017 at 9:37 AM, Josh Poimboeuf <[email protected]> wrote:
>
> It does seem to make it bigger. With Pavel's config on gcc 6, if I add
> -maccumulate-outgoing-args:
>
> That's 3.8% more text on x86-32.
That's even more than I expected. I would have expected the
-mregparm=3 to catch so much of our stack setup that it wouldn't be
all that noticeable. But apparently we just have a ton of functions
with more than 3 arguments.
> (FWIW, on x86-64, the size difference is negligible.)
Yeah, I seriously hope we've actively tried to avoid the more than six
argument calling conventions. The mm code had some that grew over
time, but most of that got converted to passing a pointer to a
descriptor structure instead (ie "struct vm_fault" etc models).
> As it turns out, when optimizing for size, gcc seems to ignore
> -maccumulate-outgoing-args completely. So I guess we would have to live
> with both cases anyway. Which means I'll need to make the unwinder
> smart enough to deal with it.
>
> But that brings up another question. If -maccumulate-outgoing-args is
> ignored with CONFIG_CC_OPTIMIZE_FOR_SIZE=y, wouldn't using that option
> break the things which require -maccumulate-outgoing-args?
>
> So, looking deeper at the various reasons this flag is enabled, they
> seem to be mostly obsolete.
Good.
> - CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
> where the stack gets aligned before the mcount call. This issue
> should be mostly obsolete as most modern compilers now have -mfentry.
> We could make it dependent on CC_USING_FENTRY.
Yeah. At some point we might even upgrade the compiler requirements to
no longer accept the mcount model.
I think the fentry model is gcc-4.6.0 and up. Currently I guess we
support gcc-3.2+, which is fairly ridiculous considering that 4.6.0 is
from March, 2011. So it's over five years ago already.
gcc-3.2.0 is from 2002, I think. At some point you just have to say
"caring about a 15 year old compiler is ridiculous"
The main reason we have fairly aggressively supported old compilers
tends to be some odder architectures that don't have good support, so
people use various random "this works for me" versions.
We could easily make the gcc version checks much more strict on x86, I suspect.
> - CONFIG_JUMP_LABEL sets it on x86-32 because of a bug in gcc <= 4.5.1
> which has since been fixed with
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226. We could probably
> make it gcc-version-dependent.
Looks like we could just use the FENTRY test, since that's more recent.
> - x86-64 sets it to apparently make the no-longer-in-tree DWARF unwinder
> happy with older versions of gcc.
Ok. Since it's not as big of a deal on x86-64 I guess we don't care,
but on the other hand it would probably then be better to aim to
switch away from it entirely and just put that whole sorry thing
behind us.
> So it looks like -maccumulate-outgoing-args isn't actually needed in
> most cases.
That would be lovely indeed.
Linus
On Wed, Mar 8, 2017 at 10:25 AM, Linus Torvalds
<[email protected]> wrote:
> On Wed, Mar 8, 2017 at 9:37 AM, Josh Poimboeuf <[email protected]> wrote:
> Yeah. At some point we might even upgrade the compiler requirements to
> no longer accept the mcount model.
>
> I think the fentry model is gcc-4.6.0 and up. Currently I guess we
> support gcc-3.2+, which is fairly ridiculous considering that 4.6.0 is
> from March, 2011. So it's over five years ago already.
>
> gcc-3.2.0 is from 2002, I think. At some point you just have to say
> "caring about a 15 year old compiler is ridiculous"
>
> The main reason we have fairly aggressively supported old compilers
> tends to be some odder architectures that don't have good support, so
> people use various random "this works for me" versions.
I thought it was because akpm still used Fedora Core 6. :)
At some point, it would be nice to skip way forward and require a
compiler without the 16-byte-stack-alignment bug, too.
--Andy
[adding Steven Rostedt to CC as an FYI]
On Wed, Mar 08, 2017 at 10:25:01AM -0800, Linus Torvalds wrote:
> On Wed, Mar 8, 2017 at 9:37 AM, Josh Poimboeuf <[email protected]> wrote:
> > - CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
> > where the stack gets aligned before the mcount call. This issue
> > should be mostly obsolete as most modern compilers now have -mfentry.
> > We could make it dependent on CC_USING_FENTRY.
>
> Yeah. At some point we might even upgrade the compiler requirements to
> no longer accept the mcount model.
The plot slightly thickens...
So I was mistaken about this problem not existing with newer versions of
gcc, because the x86-32 ftrace code doesn't use -mfentry. It still
relies on mcount. So CONFIG_FUNCTION_GRAPH_TRACER will still need
-maccumulate-outgoing-args for *all* versions of gcc on x86-32.
(Of course, that situation would improve if ftrace on x86-32 were ported
to use -mfentry.)
Also, since -Os tells gcc to ignore -maccumulate-outgoing-args, this
means that CONFIG_FUNCTION_GRAPH_TRACER with mcount needs a dependency
on CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE.
I suspect these issues also affect x86-64 with gcc 4.4.x and 4.5.x,
which corresponds to the window after the funky DRAP prologue was
introduced but before -mfentry was introduced.
In summary, here are the changes I'm looking at:
- set -maccumulate-outgoing-args if CONFIG_FUNCTION_GRAPH_TRACER &&
!CC_USING_ENTRY
(for both 32- and 64-bit)
- somehow make CONFIG_FUNCTION_GRAPH_TRACER depend on either
CC_USING_FENTRY or CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE
(for both 32- and 64-bit)
(not sure how to do that -- maybe just fail the build in the
graph tracer + mcount + '-Os' case)
- set -maccumulate-outgoing-args if CONFIG_JUMP_LABEL && gcc < 4.5.2
(for both 32-bit and 64-bit)
--
Josh
Hi!
> > - CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
> > where the stack gets aligned before the mcount call. This issue
> > should be mostly obsolete as most modern compilers now have -mfentry.
> > We could make it dependent on CC_USING_FENTRY.
>
> Yeah. At some point we might even upgrade the compiler requirements to
> no longer accept the mcount model.
>
> I think the fentry model is gcc-4.6.0 and up. Currently I guess we
> support gcc-3.2+, which is fairly ridiculous considering that 4.6.0 is
> from March, 2011. So it's over five years ago already.
>
> gcc-3.2.0 is from 2002, I think. At some point you just have to say
> "caring about a 15 year old compiler is ridiculous"
>
> The main reason we have fairly aggressively supported old compilers
> tends to be some odder architectures that don't have good support, so
> people use various random "this works for me" versions.
>
> We could easily make the gcc version checks much more strict on x86,
> I suspect.
Well, I have fast CPUs, but most of the time they just compile
stuff. Especially bisect is compile-heavy. I suspect going back to
gcc-3.2 would bring me bigger advantages than CPU upgrade...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
On Wed, Mar 8, 2017 at 10:22 PM, Pavel Machek <[email protected]> wrote:
> Well, I have fast CPUs, but most of the time they just compile
> stuff. Especially bisect is compile-heavy. I suspect going back to
> gcc-3.2 would bring me bigger advantages than CPU upgrade...
I hope you do use ccache or distcc?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi!
> > > - CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
> > > where the stack gets aligned before the mcount call. This issue
> > > should be mostly obsolete as most modern compilers now have -mfentry.
> > > We could make it dependent on CC_USING_FENTRY.
> >
> > Yeah. At some point we might even upgrade the compiler requirements to
> > no longer accept the mcount model.
> >
> > I think the fentry model is gcc-4.6.0 and up. Currently I guess we
> > support gcc-3.2+, which is fairly ridiculous considering that 4.6.0 is
> > from March, 2011. So it's over five years ago already.
> >
> > gcc-3.2.0 is from 2002, I think. At some point you just have to say
> > "caring about a 15 year old compiler is ridiculous"
> >
> > The main reason we have fairly aggressively supported old compilers
> > tends to be some odder architectures that don't have good support, so
> > people use various random "this works for me" versions.
> >
> > We could easily make the gcc version checks much more strict on x86,
> > I suspect.
>
> Well, I have fast CPUs, but most of the time they just compile
> stuff. Especially bisect is compile-heavy. I suspect going back to
> gcc-3.2 would bring me bigger advantages than CPU upgrade...
Okay, would not it be nice if we supported gcc-3.3? It compiles about
twice the speed of gcc-4.9, across the board: (If we could compile at
-O1, we'd get 4 times the speed. At -O0, we'd be at cca 9 times the
speed; that would be useful for a bisect!)
Good news is that -Os is quite significantly faster than -O2 (and
already supported), so that should be simple way to optimize bisect performance.
(On thinkpad X220, compiling bzip2)
| mach | gcc | | | real | user | sys | $
| x220 | 4.9.2-10 | -O0 | bzip2.c caf036 | 0.644 | 0.54 | 0.03 | $
| | | -O1 | | 1.501 | | | $
| | | -O2 | | 2.607 | | | $
| | | -O3 | | 3.052 | | | $
| | | -Os | | 1.839 | | | $
| | 3.3.5-13 | -O0 | | 0.343 | 0.300 | 0.028 | $
| | | -O1 | | 0.721 | | | $
| | | -O2 | | 1.238 | | | $
| | | -O3 | | 1.598 | 1.508 | 0.032 | $
Unfortunately, 4.11-rc1 fails to compile on gcc 3.3.5.
> 1. None (CC_STACKPROTECTOR_NONE) (NEW)
is needed. Easy. But then I get
AS arch/x86/entry/entry_32.o
arch/x86/entry/entry_32.S: Assembler messages:
arch/x86/entry/entry_32.S:440: Error: invalid character '"' in
operand 1
from the ALTERNATIVE macro. It seems 3.3 just does not like " in macro
arguments.
arch/x86/boot/bioscall.S: Assembler messages:
arch/x86/boot/bioscall.S:68: Error: `68(%esp)' is not a valid 16 bit
base/index expression
Plus I get about milion of
from fs/fs-writeback.c:23:
include/linux/irq.h:419: warning: parameter has
incomplete type
include/linux/irq.h:420: warning: parameter has
incomplete type
... and problem with builtin_ffs in drm_blend.c, and others with
function alignment in drm.
lzo1x_compress needs __builtin_ctz. In the end, compilation fails with
mm/built-in.o(.text+0x2b714): In function `do_set_pmd':
: undefined reference to `__compiletime_assert_3034'
mm/built-in.o(.text+0x2c09a): In function `create_huge_pmd':
: undefined reference to `do_huge_pmd_anonymous_page'
mm/built-in.o(.text+0x2c0ca): In function `wp_huge_pmd':
: undefined reference to `do_huge_pmd_wp_page'
drivers/built-in.o(.text+0xe5a2b): In function
`cea_mode_alternate_timings':
: undefined reference to `__compiletime_assert_2638'
drivers/built-in.o(.text+0x3c969f): In function `sg_ioctl':
: undefined reference to `__divdi3'
But that looks fixable. But when I force the compilation, it is
actually _slower_ than recent gcc (23 minutes vs. 13
minutes). Interesting. If someone knows what old gcc versions actually
compile recent kernels, I'd like to know.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu 2017-03-09 10:38:46, Geert Uytterhoeven wrote:
> Hi Pavel,
>
> On Wed, Mar 8, 2017 at 10:22 PM, Pavel Machek <[email protected]> wrote:
> > Well, I have fast CPUs, but most of the time they just compile
> > stuff. Especially bisect is compile-heavy. I suspect going back to
> > gcc-3.2 would bring me bigger advantages than CPU upgrade...
>
> I hope you do use ccache or distcc?
I tried to use distcc before, but it was rather hard to maintain. No
ccache here. Hmm. I guess ccache really makes sense for bisect.
On the other hand... it should be possible to compile kernel 10 times
faster than we normally do, without powering up additional machines
and without caching tricks.
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
On Thu, Mar 9, 2017 at 11:56 AM, Pavel Machek <[email protected]> wrote:
> On Thu 2017-03-09 10:38:46, Geert Uytterhoeven wrote:
>> On Wed, Mar 8, 2017 at 10:22 PM, Pavel Machek <[email protected]> wrote:
>> > Well, I have fast CPUs, but most of the time they just compile
>> > stuff. Especially bisect is compile-heavy. I suspect going back to
>> > gcc-3.2 would bring me bigger advantages than CPU upgrade...
>>
>> I hope you do use ccache or distcc?
>
> I tried to use distcc before, but it was rather hard to maintain. No
> ccache here. Hmm. I guess ccache really makes sense for bisect.
Yes it does. So if you're not using it yet, do the below, today, not tomorrow.
If your distro supports it, prepend /usr/lib/ccache/ to your $PATH.
Create symlinks from the names of your favorite cross-compilers
to /usr/bin/ccache, and make sure they are early in your $PATH.
That's it! Enjoy!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Wed, 8 Mar 2017 15:29:59 -0600
Josh Poimboeuf <[email protected]> wrote:
> [adding Steven Rostedt to CC as an FYI]
>
> On Wed, Mar 08, 2017 at 10:25:01AM -0800, Linus Torvalds wrote:
> > On Wed, Mar 8, 2017 at 9:37 AM, Josh Poimboeuf <[email protected]> wrote:
> > > - CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
> > > where the stack gets aligned before the mcount call. This issue
> > > should be mostly obsolete as most modern compilers now have -mfentry.
> > > We could make it dependent on CC_USING_FENTRY.
> >
> > Yeah. At some point we might even upgrade the compiler requirements to
> > no longer accept the mcount model.
>
> The plot slightly thickens...
>
> So I was mistaken about this problem not existing with newer versions of
> gcc, because the x86-32 ftrace code doesn't use -mfentry. It still
> relies on mcount. So CONFIG_FUNCTION_GRAPH_TRACER will still need
> -maccumulate-outgoing-args for *all* versions of gcc on x86-32.
OK, I admit, I was lazy here. I thought, who cares about x86-32
anymore ;-)
>
> (Of course, that situation would improve if ftrace on x86-32 were ported
> to use -mfentry.)
That can easily be done.
>
> Also, since -Os tells gcc to ignore -maccumulate-outgoing-args, this
> means that CONFIG_FUNCTION_GRAPH_TRACER with mcount needs a dependency
> on CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE.
>
> I suspect these issues also affect x86-64 with gcc 4.4.x and 4.5.x,
> which corresponds to the window after the funky DRAP prologue was
> introduced but before -mfentry was introduced.
>
> In summary, here are the changes I'm looking at:
>
> - set -maccumulate-outgoing-args if CONFIG_FUNCTION_GRAPH_TRACER &&
> !CC_USING_ENTRY
> (for both 32- and 64-bit)
>
> - somehow make CONFIG_FUNCTION_GRAPH_TRACER depend on either
> CC_USING_FENTRY or CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE
> (for both 32- and 64-bit)
>
> (not sure how to do that -- maybe just fail the build in the
> graph tracer + mcount + '-Os' case)
Could just place something like this in the x86 code:
#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
!defined(CC_USING_FENTRY) && \
!defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
# error Your compiler doesn't support function graph tracing
#endif
-- Steve
>
> - set -maccumulate-outgoing-args if CONFIG_JUMP_LABEL && gcc < 4.5.2
> (for both 32-bit and 64-bit)
>
On Wed, Mar 08, 2017 at 10:22:53PM +0100, Pavel Machek wrote:
>
> Well, I have fast CPUs, but most of the time they just compile
> stuff. Especially bisect is compile-heavy. I suspect going back to
> gcc-3.2 would bring me bigger advantages than CPU upgrade...
>
But note that 3.2 compiles a distinctly different kernel from something
new and shiny. The kernel uses a lot of GCC features optimistically to
generate different code.
So if by some chance your error depends on one of the new features,
bisecting with some ancient compiler will not work.
And I cannot see that getting any better, only worse.
On Thu, Mar 9, 2017 at 2:49 AM, Pavel Machek <[email protected]> wrote:
>
> (On thinkpad X220, compiling bzip2)
You really shouldn't assume that the zlib build tracks the kernel build.
At least at some point, a noticeable part of the build cost for the
kernel was just parsing the fairly big source code. We have honking
big include files and deep nesting, and there is a lot of preprocessor
and just general parsing overhead for stuff that in most files don't
even generate code.
All those inline functions and type declarations for things that then
aren't actually used in most files means that you spend relatively
more time just parsing files than you spend on generating and
optimizing code.
So the trade-offs between different projects can be very different.
Some projects have huge tables with static initializers that gcc at
some point had serious quadratic-time issues with, and other code has
big functions where the actual optimization phase is the bulk of it.
And some projects have a lot of big and nested include files.
It's not nearly as bad as some C++ projects (where the header file
mess can often _easily_ be the dominant factor by far), but it's still
potentially completely different from something like building zlib.
Oh, and don't even bother looking at -O0 times. That's almost purely
parsing, but more importantly, the kernel has never in its lifetime
built without optimizations.
We basically rely on the compiler not being moronic crap. Always have,
always will.
> Unfortunately, 4.11-rc1 fails to compile on gcc 3.3.5.
>
>> 1. None (CC_STACKPROTECTOR_NONE) (NEW)
>
> is needed. Easy. But then I get
>
> AS arch/x86/entry/entry_32.o
> arch/x86/entry/entry_32.S: Assembler messages:
> arch/x86/entry/entry_32.S:440: Error: invalid character '"' in
> operand 1
>
> from the ALTERNATIVE macro. It seems 3.3 just does not like " in macro
> arguments.
Ok. Clearly our checks in <linux/compiler-gcc.h> are outdated, and we
"allow" compilers that don't actually work.
> But that looks fixable. But when I force the compilation, it is
> actually _slower_ than recent gcc (23 minutes vs. 13
> minutes). Interesting.
I forget when gcc got the "integrated preprocessor". It's a long time
ago. But that actually sped things up, because it basically halves (or
more) the overhead of parsing.
With an external preprocessor you obviously first have cpp doing its
parsing, then writing the preprocessed results out, and then you had
cc1 doing parsing again.
So yes, gcc has gotten a lot slower over time, but some things have
actually improved.
Linus
On Thu, Mar 09, 2017 at 09:14:47AM -0500, Steven Rostedt wrote:
> On Wed, 8 Mar 2017 15:29:59 -0600
> Josh Poimboeuf <[email protected]> wrote:
>
> > [adding Steven Rostedt to CC as an FYI]
> >
> > On Wed, Mar 08, 2017 at 10:25:01AM -0800, Linus Torvalds wrote:
> > > On Wed, Mar 8, 2017 at 9:37 AM, Josh Poimboeuf <[email protected]> wrote:
> > > > - CONFIG_FUNCTION_GRAPH_TRACER sets it on x86-32 because of a gcc bug
> > > > where the stack gets aligned before the mcount call. This issue
> > > > should be mostly obsolete as most modern compilers now have -mfentry.
> > > > We could make it dependent on CC_USING_FENTRY.
> > >
> > > Yeah. At some point we might even upgrade the compiler requirements to
> > > no longer accept the mcount model.
> >
> > The plot slightly thickens...
> >
> > So I was mistaken about this problem not existing with newer versions of
> > gcc, because the x86-32 ftrace code doesn't use -mfentry. It still
> > relies on mcount. So CONFIG_FUNCTION_GRAPH_TRACER will still need
> > -maccumulate-outgoing-args for *all* versions of gcc on x86-32.
>
> OK, I admit, I was lazy here. I thought, who cares about x86-32
> anymore ;-)
As we just saw in another thread where somebody ran into this problem
with -Os, apparently some people still do care...
> > (Of course, that situation would improve if ftrace on x86-32 were ported
> > to use -mfentry.)
>
> That can easily be done.
You weren't on CC earlier, so just to summarize the benefits of doing
fentry on x86-32, thus removing the need for -maccumulate-outgoing-args:
- graph tracer compatibility with -Os
- text size decrease of ~3%
- possible performance improvement
- more uniformity (-maccumulate-outgoing-args disabled everywhere for
modern gccs)
But either way I'll still work up a patch to make the changes I
suggested.
--
Josh
On Thu 2017-03-09 16:29:10, Peter Zijlstra wrote:
> On Wed, Mar 08, 2017 at 10:22:53PM +0100, Pavel Machek wrote:
> >
> > Well, I have fast CPUs, but most of the time they just compile
> > stuff. Especially bisect is compile-heavy. I suspect going back to
> > gcc-3.2 would bring me bigger advantages than CPU upgrade...
> >
>
> But note that 3.2 compiles a distinctly different kernel from something
> new and shiny. The kernel uses a lot of GCC features optimistically to
> generate different code.
>
> So if by some chance your error depends on one of the new features,
> bisecting with some ancient compiler will not work.
Well, yes, obviously different compilers generate different code.
OTOH for drivers (where most errors are) the difference should not be
significant.
And actually.. if you realize it bug is gcc version dependend, you'll
know where to look for the bug.
(Anyway, it looks like gcc-3.3 is not usable for kernel on x86, and it
is actually slower, too. So -- bad idea. gcc -O1 looks promising.)
Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu 2017-03-09 13:16:09, Geert Uytterhoeven wrote:
> Hi Pavel,
>
> On Thu, Mar 9, 2017 at 11:56 AM, Pavel Machek <[email protected]> wrote:
> > On Thu 2017-03-09 10:38:46, Geert Uytterhoeven wrote:
> >> On Wed, Mar 8, 2017 at 10:22 PM, Pavel Machek <[email protected]> wrote:
> >> > Well, I have fast CPUs, but most of the time they just compile
> >> > stuff. Especially bisect is compile-heavy. I suspect going back to
> >> > gcc-3.2 would bring me bigger advantages than CPU upgrade...
> >>
> >> I hope you do use ccache or distcc?
> >
> > I tried to use distcc before, but it was rather hard to maintain. No
> > ccache here. Hmm. I guess ccache really makes sense for bisect.
>
> Yes it does. So if you're not using it yet, do the below, today, not tomorrow.
>
> If your distro supports it, prepend /usr/lib/ccache/ to your $PATH.
> Create symlinks from the names of your favorite cross-compilers
> to /usr/bin/ccache, and make sure they are early in your $PATH.
>
> That's it! Enjoy!
Hmm. Installed, and seems to work. OTOH, compilation now seems to
produce 2-3MB/sec writing on spinning rust, and CPUs are no longer
fully loaded. (make -j 7 on 2 core HT machine). Any io load sends the
CPU utilization to cca 50% range... Compilation goes up from 9:13 to
11:40... to 23 minutes depending on situation. I guess it is still
worth it for the bisect, but it looks like ccache really needs an ssd.
On the other hand, switching to -O1 is really easy, and gets 15% or so
improvement.
Hmm. And killing chromium matters a lot for a compile time. I hate
modern web :-(.
Best regards, Pavel
--- a/Makefile
+++ b/Makefile
@@ -639,9 +639,9 @@ ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
KBUILD_CFLAGS += -Os $(call cc-disable-warning,maybe-uninitialized,)
else
ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS += -O2 $(call cc-disable-warning,maybe-uninitialized,)
+KBUILD_CFLAGS += -O1 $(call cc-disable-warning,maybe-uninitialized,)
else
-KBUILD_CFLAGS += -O2
+KBUILD_CFLAGS += -O1
endif
endif
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Hi Pavel,
On Fri, Mar 10, 2017 at 2:17 PM, Pavel Machek <[email protected]> wrote:
> On Thu 2017-03-09 13:16:09, Geert Uytterhoeven wrote:
>> On Thu, Mar 9, 2017 at 11:56 AM, Pavel Machek <[email protected]> wrote:
>> > On Thu 2017-03-09 10:38:46, Geert Uytterhoeven wrote:
>> >> I hope you do use ccache or distcc?
>> >
>> > I tried to use distcc before, but it was rather hard to maintain. No
>> > ccache here. Hmm. I guess ccache really makes sense for bisect.
>>
>> Yes it does. So if you're not using it yet, do the below, today, not tomorrow.
>>
>> If your distro supports it, prepend /usr/lib/ccache/ to your $PATH.
>> Create symlinks from the names of your favorite cross-compilers
>> to /usr/bin/ccache, and make sure they are early in your $PATH.
>>
>> That's it! Enjoy!
>
> Hmm. Installed, and seems to work. OTOH, compilation now seems to
> produce 2-3MB/sec writing on spinning rust, and CPUs are no longer
> fully loaded. (make -j 7 on 2 core HT machine). Any io load sends the
> CPU utilization to cca 50% range... Compilation goes up from 9:13 to
> 11:40... to 23 minutes depending on situation. I guess it is still
I guess that was the first build, with a clean cache?
Now run "make clean", and try again ;-)
BTW, I tend not to do -j beyond the number of cores/threads (i.e. -j 8
on the i7-4770), unless you just want to compile, and not enjoy other
interactive work ;-)
> worth it for the bisect, but it looks like ccache really needs an ssd.
Adding an SSD never hurts.
Although I have been a happy user of ccache since long before I got an SSD.
> Hmm. And killing chromium matters a lot for a compile time. I hate
> modern web :-(.
Adding (freeing) RAM also never hurts ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Pavel,
On Fri, Mar 10, 2017 at 02:17:51PM +0100, Pavel Machek wrote:
> On Thu 2017-03-09 13:16:09, Geert Uytterhoeven wrote:
> > Hi Pavel,
> >
> > On Thu, Mar 9, 2017 at 11:56 AM, Pavel Machek <[email protected]> wrote:
> > > On Thu 2017-03-09 10:38:46, Geert Uytterhoeven wrote:
> > >> On Wed, Mar 8, 2017 at 10:22 PM, Pavel Machek <[email protected]> wrote:
> > >> > Well, I have fast CPUs, but most of the time they just compile
> > >> > stuff. Especially bisect is compile-heavy. I suspect going back to
> > >> > gcc-3.2 would bring me bigger advantages than CPU upgrade...
> > >>
> > >> I hope you do use ccache or distcc?
> > >
> > > I tried to use distcc before, but it was rather hard to maintain. No
> > > ccache here. Hmm. I guess ccache really makes sense for bisect.
> >
> > Yes it does. So if you're not using it yet, do the below, today, not tomorrow.
> >
> > If your distro supports it, prepend /usr/lib/ccache/ to your $PATH.
> > Create symlinks from the names of your favorite cross-compilers
> > to /usr/bin/ccache, and make sure they are early in your $PATH.
> >
> > That's it! Enjoy!
>
> Hmm. Installed, and seems to work. OTOH, compilation now seems to
> produce 2-3MB/sec writing on spinning rust, and CPUs are no longer
> fully loaded. (make -j 7 on 2 core HT machine). Any io load sends the
> CPU utilization to cca 50% range... Compilation goes up from 9:13 to
> 11:40... to 23 minutes depending on situation. I guess it is still
> worth it for the bisect, but it looks like ccache really needs an ssd.
You need to put your cache in /dev/shm or some fast place not moving
heavy metallic heads.
That said, I have great success with distcc, I use it a lot with my build
farms (home and work) on some fanless machines [1]. I had to apply some
small updates recently to distcc because I noticed it would not delegate
files built with certain -Wa arguments as were recently added to the kernel.
I also found that by default it limits itself to 50 jobs which is often not
enough to keep your local machine busy.
The last 3.10.105-rc1 series I sent was build-tested this way, and it
builds allmodconfig in slightly less than 5 mn (4900 modules I believe),
with peaks up to 120 files per second. That's quite fast.
With distcc however you need a fast local machine because cpp is run
there, like a number of other tasks (eg: do not compress modules). You need
some RAM as well because you'll need a high parallel job count to keep your
machines busy (I build at -j60 which is the optimal value in my case). At
work only 4 small fanless ARM boards (cortex A17) cut my build time by 3
(local is a t430s with an i5-3320m) and at home 6 such boards cut the build
time by 2.5 (local is i7-6700K).
You cannot reasonably do that with too slow build nodes because you want
to limit the maximum build time for a single file. Here the cortex A17
at 1.8-2.0 GHz is perfect, it's the fastest fanless machine I found. Some
cheap x86 machines can work well also. In all case you must absolutely use
gigabit network or you'll constantly have some idle time as the traffic is
very bursty. I'm seeing up to 650 Mbps without compressing with LZO, and
between 70-150 Mbps with LZO, and it always builds faster with it.
> On the other hand, switching to -O1 is really easy, and gets 15% or so
> improvement.
I used to do such things in the past but certain level of optimizations
are useful to report warnings. For example I build haproxy daily at -O0,
but sometimes I discover later that I introduced some warnings that are
only detected at -O2.
> Hmm. And killing chromium matters a lot for a compile time. I hate
> modern web :-(.
killall -STOP. That's what I'm doing with firefox when I want some
resources.
Cheers,
Willy
---
[1] https://forum.mqmaker.com/t/miqi-based-build-farm-finally-up-and-running/605/24
Subject: [PATCH] x86: mostly disable '-maccumulate-outgoing-args'
The gcc '-maccumulate-outgoing-args' flag is enabled for most configs,
mostly because of issues which are no longer relevant. For most
configs, with most recent versions of gcc, it's no longer needed.
Clarify which cases need it, and only enable it for those cases. Also
produce a compile-time error for the ftrace graph + mcount + '-Os' case,
which will otherwise cause runtime failures.
The main benefit of '-maccumulate-outgoing-args' is that it prevents an
ugly prologue for functions which have aligned stacks. But removing the
option also has some benefits: more readable argument saves, smaller
text size, and (presumably) slightly improved performance.
Here are the object size savings for 32-bit and 64-bit defconfig
kernels:
text data bss dec hex filename
10006710 3543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
9706358 3547424 1773568 15027350 e54c96 vmlinux.x86-32.after
text data bss dec hex filename
10652105 4537576 843776 16033457 f4a6b1 vmlinux.x86-64.before
10639629 4537576 843776 16020981 f475f5 vmlinux.x86-64.after
That comes out to a 3% text size improvement on x86-32 and a 0.1% text
size improvement on x86-64.
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/Makefile | 29 +++++++++++++++++++++++++----
arch/x86/Makefile_32.cpu | 18 ------------------
arch/x86/kernel/ftrace.c | 6 ++++++
scripts/Kbuild.include | 4 ++++
4 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..fa45989b 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -120,10 +120,6 @@ else
# -funit-at-a-time shrinks the kernel .text considerably
# unfortunately it makes reading oopses harder.
KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
-
- # this works around some issues with generating unwind tables in older gccs
- # newer gccs do it by default
- KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
endif
ifdef CONFIG_X86_X32
@@ -147,6 +143,31 @@ ifeq ($(CONFIG_KMEMCHECK),y)
KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
endif
+# If the function graph tracer is used with mcount instead of fentry,
+# '-maccumulate-outgoing-args' is needed to prevent gcc bug
+# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109
+ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ ifndef CONFIG_HAVE_FENTRY
+ ACCUMULATE_OUTGOING_ARGS := 1
+ else
+ ifeq ($(call cc-option, -mfentry),)
+ ACCUMULATE_OUTGOING_ARGS := 1
+ endif
+ endif
+endif
+
+# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
+# gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226
+ifdef CONFIG_JUMP_LABEL
+ ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
+ endif
+endif
+
+ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ KBUILD_CFLAGS += -maccumulate-outgoing-args
+endif
+
# Stackpointer is addressed different for 32 bit and 64 bit x86
sp-$(CONFIG_X86_32) := esp
sp-$(CONFIG_X86_64) := rsp
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..a45eb15 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
# cpu entries
cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
-# Work around the pentium-mmx code generator madness of gcc4.4.x which
-# does stack alignment by generating horrible code _before_ the mcount
-# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
-# tracer assumptions. For i686, generic, core2 this is set by the
-# compiler anyway
-ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-# Work around to a bug with asm goto with first implementations of it
-# in gcc causing gcc to mess up the push and pop of the stack in some
-# uses of asm goto.
-ifeq ($(CONFIG_JUMP_LABEL), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
-
# Bug fix for binutils: this option is required in order to keep
# binutils from generating NOPL instructions against our will.
ifneq ($(CONFIG_X86_P6_NOP),y)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8f3d9cf..59f9b46 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -29,6 +29,12 @@
#include <asm/ftrace.h>
#include <asm/nops.h>
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
+ !defined(CC_USING_FENTRY) && \
+ !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
+# error Your compiler does not support function graph tracing
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
int ftrace_arch_code_modify_prepare(void)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index d6ca649..afe3fd3 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
+# cc-if-fullversion
+# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
+cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
+
# cc-ldoption
# Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
cc-ldoption = $(call try-run,\
--
2.7.4
On Thu, 16 Mar 2017 10:42:08 -0500
Josh Poimboeuf <[email protected]> wrote:
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> arch/x86/Makefile | 29 +++++++++++++++++++++++++----
> arch/x86/Makefile_32.cpu | 18 ------------------
> arch/x86/kernel/ftrace.c | 6 ++++++
> scripts/Kbuild.include | 4 ++++
> 4 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2d44933..fa45989b 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -120,10 +120,6 @@ else
> # -funit-at-a-time shrinks the kernel .text considerably
> # unfortunately it makes reading oopses harder.
> KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> -
> - # this works around some issues with generating unwind tables in older gccs
> - # newer gccs do it by default
> - KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> endif
>
> ifdef CONFIG_X86_X32
> @@ -147,6 +143,31 @@ ifeq ($(CONFIG_KMEMCHECK),y)
> KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
> endif
>
> +# If the function graph tracer is used with mcount instead of fentry,
> +# '-maccumulate-outgoing-args' is needed to prevent gcc bug
"to prevent a gcc bug"
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109
> +ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + ifndef CONFIG_HAVE_FENTRY
> + ACCUMULATE_OUTGOING_ARGS := 1
> + else
> + ifeq ($(call cc-option, -mfentry),)
Hmm, the blank entry makes me nervous. I wonder if it would be better
if we had ifneq ($(call cc-option-yn, -mfentry),y)
Unfortunately, there's one of each in the existing kernel, so there is
really no precedence.
> + ACCUMULATE_OUTGOING_ARGS := 1
> + endif
> + endif
> +endif
> +
> +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
Can we make a test instead? I hate testing versions, and things get
backported all the time. We usually like to have a test case instead of
relying on versions. Not to mention, a newer gcc may one day break.
-- Steve
> +# gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226
> +ifdef CONFIG_JUMP_LABEL
> + ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> + ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
> + endif
> +endif
> +
> +ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> + KBUILD_CFLAGS += -maccumulate-outgoing-args
> +endif
> +
> # Stackpointer is addressed different for 32 bit and 64 bit x86
> sp-$(CONFIG_X86_32) := esp
> sp-$(CONFIG_X86_64) := rsp
> diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
> index 6647ed4..a45eb15 100644
> --- a/arch/x86/Makefile_32.cpu
> +++ b/arch/x86/Makefile_32.cpu
> @@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
> # cpu entries
> cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
>
> -# Work around the pentium-mmx code generator madness of gcc4.4.x which
> -# does stack alignment by generating horrible code _before_ the mcount
> -# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
> -# tracer assumptions. For i686, generic, core2 this is set by the
> -# compiler anyway
> -ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
> -ADD_ACCUMULATE_OUTGOING_ARGS := y
> -endif
> -
> -# Work around to a bug with asm goto with first implementations of it
> -# in gcc causing gcc to mess up the push and pop of the stack in some
> -# uses of asm goto.
> -ifeq ($(CONFIG_JUMP_LABEL), y)
> -ADD_ACCUMULATE_OUTGOING_ARGS := y
> -endif
> -
> -cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
> -
> # Bug fix for binutils: this option is required in order to keep
> # binutils from generating NOPL instructions against our will.
> ifneq ($(CONFIG_X86_P6_NOP),y)
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 8f3d9cf..59f9b46 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -29,6 +29,12 @@
> #include <asm/ftrace.h>
> #include <asm/nops.h>
>
> +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
> + !defined(CC_USING_FENTRY) && \
> + !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
> +# error Your compiler does not support function graph tracing
> +#endif
> +
> #ifdef CONFIG_DYNAMIC_FTRACE
>
> int ftrace_arch_code_modify_prepare(void)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index d6ca649..afe3fd3 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
> # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
>
> +# cc-if-fullversion
> +# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
> +cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
> +
> # cc-ldoption
> # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
> cc-ldoption = $(call try-run,\
On Thu, Mar 16, 2017 at 01:32:01PM -0400, Steven Rostedt wrote:
> On Thu, 16 Mar 2017 10:42:08 -0500
> Josh Poimboeuf <[email protected]> wrote:
>
> > Signed-off-by: Josh Poimboeuf <[email protected]>
> > ---
> > arch/x86/Makefile | 29 +++++++++++++++++++++++++----
> > arch/x86/Makefile_32.cpu | 18 ------------------
> > arch/x86/kernel/ftrace.c | 6 ++++++
> > scripts/Kbuild.include | 4 ++++
> > 4 files changed, 35 insertions(+), 22 deletions(-)
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index 2d44933..fa45989b 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -120,10 +120,6 @@ else
> > # -funit-at-a-time shrinks the kernel .text considerably
> > # unfortunately it makes reading oopses harder.
> > KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> > -
> > - # this works around some issues with generating unwind tables in older gccs
> > - # newer gccs do it by default
> > - KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > endif
> >
> > ifdef CONFIG_X86_X32
> > @@ -147,6 +143,31 @@ ifeq ($(CONFIG_KMEMCHECK),y)
> > KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
> > endif
> >
> > +# If the function graph tracer is used with mcount instead of fentry,
> > +# '-maccumulate-outgoing-args' is needed to prevent gcc bug
>
> "to prevent a gcc bug"
It was
"to prevent gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109"
where "gcc bug" was an adjective and the URL was a noun. But yeah,
that's kind of confusing, and the line wrap made it more so. Maybe I'll
change it to
"to prevent a gcc bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)"
and a similar change for the jump label bug comment.
> > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109
> > +ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > + ifndef CONFIG_HAVE_FENTRY
> > + ACCUMULATE_OUTGOING_ARGS := 1
> > + else
> > + ifeq ($(call cc-option, -mfentry),)
>
> Hmm, the blank entry makes me nervous. I wonder if it would be better
> if we had ifneq ($(call cc-option-yn, -mfentry),y)
>
> Unfortunately, there's one of each in the existing kernel, so there is
> really no precedence.
Either way seems fine. I'll go with your suggested change.
> > + ACCUMULATE_OUTGOING_ARGS := 1
> > + endif
> > + endif
> > +endif
> > +
> > +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
>
> Can we make a test instead? I hate testing versions, and things get
> backported all the time. We usually like to have a test case instead of
> relying on versions. Not to mention, a newer gcc may one day break.
Tests are generally better, but I'm not sure how to test for this
cleanly. The test is rather big for embedding in a makefile:
https://gcc.gnu.org/bugzilla/attachment.cgi?id=22199
Any ideas?
--
Josh
On Thu, Mar 16, 2017 at 01:36:35PM -0500, Josh Poimboeuf wrote:
> On Thu, Mar 16, 2017 at 01:32:01PM -0400, Steven Rostedt wrote:
> > On Thu, 16 Mar 2017 10:42:08 -0500
> > Josh Poimboeuf <[email protected]> wrote:
> > > + ACCUMULATE_OUTGOING_ARGS := 1
> > > + endif
> > > + endif
> > > +endif
> > > +
> > > +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
> >
> > Can we make a test instead? I hate testing versions, and things get
> > backported all the time. We usually like to have a test case instead of
> > relying on versions. Not to mention, a newer gcc may one day break.
>
> Tests are generally better, but I'm not sure how to test for this
> cleanly. The test is rather big for embedding in a makefile:
>
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=22199
>
> Any ideas?
After some snooping I discovered there's some precedent for doing this
in the scripts/gcc-*.sh files. So maybe I'll add a test there and call
it from the Makefile.
--
Josh
On Thu, 16 Mar 2017 13:36:35 -0500
Josh Poimboeuf <[email protected]> wrote:
> On Thu, Mar 16, 2017 at 01:32:01PM -0400, Steven Rostedt wrote:
> > On Thu, 16 Mar 2017 10:42:08 -0500
> > Josh Poimboeuf <[email protected]> wrote:
> >
> > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > > ---
> > > arch/x86/Makefile | 29 +++++++++++++++++++++++++----
> > > arch/x86/Makefile_32.cpu | 18 ------------------
> > > arch/x86/kernel/ftrace.c | 6 ++++++
> > > scripts/Kbuild.include | 4 ++++
> > > 4 files changed, 35 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > > index 2d44933..fa45989b 100644
> > > --- a/arch/x86/Makefile
> > > +++ b/arch/x86/Makefile
> > > @@ -120,10 +120,6 @@ else
> > > # -funit-at-a-time shrinks the kernel .text considerably
> > > # unfortunately it makes reading oopses harder.
> > > KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> > > -
> > > - # this works around some issues with generating unwind tables in older gccs
> > > - # newer gccs do it by default
> > > - KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > > endif
> > >
> > > ifdef CONFIG_X86_X32
> > > @@ -147,6 +143,31 @@ ifeq ($(CONFIG_KMEMCHECK),y)
> > > KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
> > > endif
> > >
> > > +# If the function graph tracer is used with mcount instead of fentry,
> > > +# '-maccumulate-outgoing-args' is needed to prevent gcc bug
> >
> > "to prevent a gcc bug"
>
> It was
>
> "to prevent gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109"
>
> where "gcc bug" was an adjective and the URL was a noun. But yeah,
> that's kind of confusing, and the line wrap made it more so. Maybe I'll
> change it to
>
> "to prevent a gcc bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)"
Hmm, "the" would have made it work too.
>
> and a similar change for the jump label bug comment.
>
> > > +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109
> > > +ifdef CONFIG_FUNCTION_GRAPH_TRACER
> > > + ifndef CONFIG_HAVE_FENTRY
> > > + ACCUMULATE_OUTGOING_ARGS := 1
> > > + else
> > > + ifeq ($(call cc-option, -mfentry),)
> >
> > Hmm, the blank entry makes me nervous. I wonder if it would be better
> > if we had ifneq ($(call cc-option-yn, -mfentry),y)
> >
> > Unfortunately, there's one of each in the existing kernel, so there is
> > really no precedence.
>
> Either way seems fine. I'll go with your suggested change.
>
> > > + ACCUMULATE_OUTGOING_ARGS := 1
> > > + endif
> > > + endif
> > > +endif
> > > +
> > > +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
> >
> > Can we make a test instead? I hate testing versions, and things get
> > backported all the time. We usually like to have a test case instead of
> > relying on versions. Not to mention, a newer gcc may one day break.
>
> Tests are generally better, but I'm not sure how to test for this
> cleanly. The test is rather big for embedding in a makefile:
>
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=22199
>
> Any ideas?
>
I'd reply but I see you figured it out yourself.
-- Steve
On Thu, 16 Mar 2017 14:04:01 -0500
Josh Poimboeuf <[email protected]> wrote:
> > After some snooping I discovered there's some precedent for doing this
> > in the scripts/gcc-*.sh files. So maybe I'll add a test there and call
> > it from the Makefile.
>
> But now I realize that those other tests are just build tests, whereas
> this one needs to be executed. That's a no-go for cross compilers. So
> I think we need to do the version check after all.
>
Fine, but please add a comment saying such.
-- Steve
On Thu, Mar 16, 2017 at 01:53:05PM -0500, Josh Poimboeuf wrote:
> On Thu, Mar 16, 2017 at 01:36:35PM -0500, Josh Poimboeuf wrote:
> > On Thu, Mar 16, 2017 at 01:32:01PM -0400, Steven Rostedt wrote:
> > > On Thu, 16 Mar 2017 10:42:08 -0500
> > > Josh Poimboeuf <[email protected]> wrote:
> > > > + ACCUMULATE_OUTGOING_ARGS := 1
> > > > + endif
> > > > + endif
> > > > +endif
> > > > +
> > > > +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
> > >
> > > Can we make a test instead? I hate testing versions, and things get
> > > backported all the time. We usually like to have a test case instead of
> > > relying on versions. Not to mention, a newer gcc may one day break.
> >
> > Tests are generally better, but I'm not sure how to test for this
> > cleanly. The test is rather big for embedding in a makefile:
> >
> > https://gcc.gnu.org/bugzilla/attachment.cgi?id=22199
> >
> > Any ideas?
>
> After some snooping I discovered there's some precedent for doing this
> in the scripts/gcc-*.sh files. So maybe I'll add a test there and call
> it from the Makefile.
But now I realize that those other tests are just build tests, whereas
this one needs to be executed. That's a no-go for cross compilers. So
I think we need to do the version check after all.
--
Josh
The gcc '-maccumulate-outgoing-args' flag is enabled for most configs,
mostly because of issues which are no longer relevant. For most
configs, and with most recent versions of gcc, it's no longer needed.
Clarify which cases need it, and only enable it for those cases. Also
produce a compile-time error for the ftrace graph + mcount + '-Os' case,
which will otherwise cause runtime failures.
The main benefit of '-maccumulate-outgoing-args' is that it prevents an
ugly prologue for functions which have aligned stacks. But removing the
option also has some benefits: more readable argument saves, smaller
text size, and (presumably) slightly improved performance.
Here are the object size savings for 32-bit and 64-bit defconfig
kernels:
text data bss dec hex filename
10006710 3543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
9706358 3547424 1773568 15027350 e54c96 vmlinux.x86-32.after
text data bss dec hex filename
10652105 4537576 843776 16033457 f4a6b1 vmlinux.x86-64.before
10639629 4537576 843776 16020981 f475f5 vmlinux.x86-64.after
That comes out to a 3% text size improvement on x86-32 and a 0.1% text
size improvement on x86-64.
Signed-off-by: Josh Poimboeuf <[email protected]>
---
v2:
- improve readability of the comments
- add comment about why gcc version needs to be checked
- use cc-option-yn instead of cc-option
arch/x86/Makefile | 31 +++++++++++++++++++++++++++----
arch/x86/Makefile_32.cpu | 18 ------------------
arch/x86/kernel/ftrace.c | 6 ++++++
scripts/Kbuild.include | 4 ++++
4 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..04c87be 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -120,10 +120,6 @@ else
# -funit-at-a-time shrinks the kernel .text considerably
# unfortunately it makes reading oopses harder.
KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
-
- # this works around some issues with generating unwind tables in older gccs
- # newer gccs do it by default
- KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
endif
ifdef CONFIG_X86_X32
@@ -147,6 +143,33 @@ ifeq ($(CONFIG_KMEMCHECK),y)
KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
endif
+# If the function graph tracer is used with mcount instead of fentry,
+# '-maccumulate-outgoing-args' is needed to prevent a gcc bug
+# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)
+ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ ifndef CONFIG_HAVE_FENTRY
+ ACCUMULATE_OUTGOING_ARGS := 1
+ else
+ ifeq ($(call cc-option-yn, -mfentry), n)
+ ACCUMULATE_OUTGOING_ARGS := 1
+ endif
+ endif
+endif
+
+# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
+# gcc bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226). There's no way
+# to test for this bug at compile-time because the test case needs to execute,
+# which is a no-go for cross compilers. So check the gcc version instead.
+ifdef CONFIG_JUMP_LABEL
+ ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
+ endif
+endif
+
+ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ KBUILD_CFLAGS += -maccumulate-outgoing-args
+endif
+
# Stackpointer is addressed different for 32 bit and 64 bit x86
sp-$(CONFIG_X86_32) := esp
sp-$(CONFIG_X86_64) := rsp
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..a45eb15 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
# cpu entries
cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
-# Work around the pentium-mmx code generator madness of gcc4.4.x which
-# does stack alignment by generating horrible code _before_ the mcount
-# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
-# tracer assumptions. For i686, generic, core2 this is set by the
-# compiler anyway
-ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-# Work around to a bug with asm goto with first implementations of it
-# in gcc causing gcc to mess up the push and pop of the stack in some
-# uses of asm goto.
-ifeq ($(CONFIG_JUMP_LABEL), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
-
# Bug fix for binutils: this option is required in order to keep
# binutils from generating NOPL instructions against our will.
ifneq ($(CONFIG_X86_P6_NOP),y)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8f3d9cf..59f9b46 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -29,6 +29,12 @@
#include <asm/ftrace.h>
#include <asm/nops.h>
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
+ !defined(CC_USING_FENTRY) && \
+ !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
+# error Your compiler does not support function graph tracing
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
int ftrace_arch_code_modify_prepare(void)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index d6ca649..afe3fd3 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
+# cc-if-fullversion
+# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
+cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
+
# cc-ldoption
# Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
cc-ldoption = $(call try-run,\
--
2.7.4
* Josh Poimboeuf <[email protected]> wrote:
> The gcc '-maccumulate-outgoing-args' flag is enabled for most configs,
> mostly because of issues which are no longer relevant. For most
> configs, and with most recent versions of gcc, it's no longer needed.
>
> Clarify which cases need it, and only enable it for those cases. Also
> produce a compile-time error for the ftrace graph + mcount + '-Os' case,
> which will otherwise cause runtime failures.
>
> The main benefit of '-maccumulate-outgoing-args' is that it prevents an
> ugly prologue for functions which have aligned stacks. But removing the
> option also has some benefits: more readable argument saves, smaller
> text size, and (presumably) slightly improved performance.
>
> Here are the object size savings for 32-bit and 64-bit defconfig
> kernels:
>
> text data bss dec hex filename
> 10006710 3543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
> 9706358 3547424 1773568 15027350 e54c96 vmlinux.x86-32.after
>
> text data bss dec hex filename
> 10652105 4537576 843776 16033457 f4a6b1 vmlinux.x86-64.before
> 10639629 4537576 843776 16020981 f475f5 vmlinux.x86-64.after
>
> That comes out to a 3% text size improvement on x86-32 and a 0.1% text
> size improvement on x86-64.
>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> v2:
> - improve readability of the comments
> - add comment about why gcc version needs to be checked
> - use cc-option-yn instead of cc-option
>
> arch/x86/Makefile | 31 +++++++++++++++++++++++++++----
> arch/x86/Makefile_32.cpu | 18 ------------------
> arch/x86/kernel/ftrace.c | 6 ++++++
> scripts/Kbuild.include | 4 ++++
> 4 files changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2d44933..04c87be 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -120,10 +120,6 @@ else
> # -funit-at-a-time shrinks the kernel .text considerably
> # unfortunately it makes reading oopses harder.
> KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> -
> - # this works around some issues with generating unwind tables in older gccs
> - # newer gccs do it by default
> - KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> endif
>
> ifdef CONFIG_X86_X32
> @@ -147,6 +143,33 @@ ifeq ($(CONFIG_KMEMCHECK),y)
> KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
> endif
>
> +# If the function graph tracer is used with mcount instead of fentry,
> +# '-maccumulate-outgoing-args' is needed to prevent a gcc bug
> +# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)
> +ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + ifndef CONFIG_HAVE_FENTRY
> + ACCUMULATE_OUTGOING_ARGS := 1
> + else
> + ifeq ($(call cc-option-yn, -mfentry), n)
> + ACCUMULATE_OUTGOING_ARGS := 1
> + endif
> + endif
> +endif
> +
> +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
> +# gcc bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226). There's no way
> +# to test for this bug at compile-time because the test case needs to execute,
> +# which is a no-go for cross compilers. So check the gcc version instead.
In documentation please refer to GCC the compiler as 'GCC' (upper case), while gcc
the command as 'gcc'.
Also, even in Kbuild try to follow the kernel style - which for comments would be
something like:
#
# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
# GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226). There's no way
# to test for this bug at compile-time because the test case needs to execute,
# which is a no-go for cross compilers. So check the GCC version instead.
#
... even though there's plenty of bad example in the Makefile you are changing.
> +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
> + !defined(CC_USING_FENTRY) && \
> + !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
> +# error Your compiler does not support function graph tracing
> +#endif
Might make sense to add the compiler option that is missing, i.e. something like:
# error Your compiler does not support function graph tracing (-mfentry)
(or whatever compiler feature is missing.)
Thanks,
Ingo
On Wed, Mar 22, 2017 at 08:51:53AM +0100, Ingo Molnar wrote:
> > +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
> > + !defined(CC_USING_FENTRY) && \
> > + !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
> > +# error Your compiler does not support function graph tracing
> > +#endif
>
> Might make sense to add the compiler option that is missing, i.e. something like:
>
> # error Your compiler does not support function graph tracing (-mfentry)
>
> (or whatever compiler feature is missing.)
I left it vague because otherwise it would need a paragraph :-)
After Steven's latest patches which port fentry to x86-32, I think the
precise version would be:
# error The following combination is not supported: ((compiler missing -mfentry) || (CONFIG_X86_32 and !CONFIG_DYNAMIC_FTRACE)) && CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_CC_OPTIMIZE_FOR_SIZE.
--
Josh
Commit-ID: a14d30bdd58680dd9eb4d83600387f159a4b0f18
Gitweb: http://git.kernel.org/tip/a14d30bdd58680dd9eb4d83600387f159a4b0f18
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Thu, 16 Mar 2017 14:31:33 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 28 Mar 2017 09:33:02 +0200
x86/build: Mostly disable '-maccumulate-outgoing-args'
The GCC '-maccumulate-outgoing-args' flag is enabled for most configs,
mostly because of issues which are no longer relevant. For most
configs, and with most recent versions of GCC, it's no longer needed.
Clarify which cases need it, and only enable it for those cases. Also
produce a compile-time error for the ftrace graph + mcount + '-Os' case,
which will otherwise cause runtime failures.
The main benefit of '-maccumulate-outgoing-args' is that it prevents an
ugly prologue for functions which have aligned stacks. But removing the
option also has some benefits: more readable argument saves, smaller
text size, and (presumably) slightly improved performance.
Here are the object size savings for 32-bit and 64-bit defconfig
kernels:
text data bss dec hex filename
10006710 3543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
9706358 3547424 1773568 15027350 e54c96 vmlinux.x86-32.after
text data bss dec hex filename
10652105 4537576 843776 16033457 f4a6b1 vmlinux.x86-64.before
10639629 4537576 843776 16020981 f475f5 vmlinux.x86-64.after
That comes out to a 3% text size improvement on x86-32 and a 0.1% text
size improvement on x86-64.
Signed-off-by: Josh Poimboeuf <[email protected]>
Cc: Andrew Lutomirski <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/20170316193133.zrj6gug53766m6nn@treble
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/Makefile | 31 +++++++++++++++++++++++++++----
arch/x86/Makefile_32.cpu | 18 ------------------
arch/x86/kernel/ftrace.c | 6 ++++++
scripts/Kbuild.include | 4 ++++
4 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..07b6746 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -120,10 +120,6 @@ else
# -funit-at-a-time shrinks the kernel .text considerably
# unfortunately it makes reading oopses harder.
KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
-
- # this works around some issues with generating unwind tables in older gccs
- # newer gccs do it by default
- KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
endif
ifdef CONFIG_X86_X32
@@ -147,6 +143,33 @@ ifeq ($(CONFIG_KMEMCHECK),y)
KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
endif
+# If the function graph tracer is used with mcount instead of fentry,
+# '-maccumulate-outgoing-args' is needed to prevent a GCC bug
+# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)
+ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ ifndef CONFIG_HAVE_FENTRY
+ ACCUMULATE_OUTGOING_ARGS := 1
+ else
+ ifeq ($(call cc-option-yn, -mfentry), n)
+ ACCUMULATE_OUTGOING_ARGS := 1
+ endif
+ endif
+endif
+
+# Jump labels need '-maccumulate-outgoing-args' for GCC < 4.5.2 to prevent a
+# GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226). There's no way
+# to test for this bug at compile-time because the test case needs to execute,
+# which is a no-go for cross compilers. So check the GCC version instead.
+ifdef CONFIG_JUMP_LABEL
+ ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
+ endif
+endif
+
+ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ KBUILD_CFLAGS += -maccumulate-outgoing-args
+endif
+
# Stackpointer is addressed different for 32 bit and 64 bit x86
sp-$(CONFIG_X86_32) := esp
sp-$(CONFIG_X86_64) := rsp
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..a45eb15 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
# cpu entries
cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
-# Work around the pentium-mmx code generator madness of gcc4.4.x which
-# does stack alignment by generating horrible code _before_ the mcount
-# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
-# tracer assumptions. For i686, generic, core2 this is set by the
-# compiler anyway
-ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-# Work around to a bug with asm goto with first implementations of it
-# in gcc causing gcc to mess up the push and pop of the stack in some
-# uses of asm goto.
-ifeq ($(CONFIG_JUMP_LABEL), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
-
# Bug fix for binutils: this option is required in order to keep
# binutils from generating NOPL instructions against our will.
ifneq ($(CONFIG_X86_P6_NOP),y)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8f3d9cf..59f9b46 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -29,6 +29,12 @@
#include <asm/ftrace.h>
#include <asm/nops.h>
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
+ !defined(CC_USING_FENTRY) && \
+ !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
+# error Your compiler does not support function graph tracing
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
int ftrace_arch_code_modify_prepare(void)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index d6ca649..afe3fd3 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
+# cc-if-fullversion
+# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
+cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
+
# cc-ldoption
# Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
cc-ldoption = $(call try-run,\
Dear tip-bot,
If it's not too late, here's a version with Ingo's suggested changes
(improving the makefile comment readability and improving the ftrace
build error message).
----
From: Josh Poimboeuf <[email protected]>
Subject: [PATCH v3] x86: mostly disable '-maccumulate-outgoing-args'
The gcc '-maccumulate-outgoing-args' flag is enabled for most configs,
mostly because of issues which are no longer relevant. For most
configs, and with most recent versions of gcc, it's no longer needed.
Clarify which cases need it, and only enable it for those cases. Also
produce a compile-time error for the ftrace graph + mcount + '-Os' case,
which will otherwise cause runtime failures.
The main benefit of '-maccumulate-outgoing-args' is that it prevents an
ugly prologue for functions which have aligned stacks. But removing the
option also has some benefits: more readable argument saves, smaller
text size, and (presumably) slightly improved performance.
Here are the object size savings for 32-bit and 64-bit defconfig
kernels:
text data bss dec hex filename
10006710 3543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
9706358 3547424 1773568 15027350 e54c96 vmlinux.x86-32.after
text data bss dec hex filename
10652105 4537576 843776 16033457 f4a6b1 vmlinux.x86-64.before
10639629 4537576 843776 16020981 f475f5 vmlinux.x86-64.after
That comes out to a 3% text size improvement on x86-32 and a 0.1% text
size improvement on x86-64.
Signed-off-by: Josh Poimboeuf <[email protected]>
---
v3:
- improve makefile comment readability
- improve precision of ftrace build error message
arch/x86/Makefile | 35 +++++++++++++++++++++++++++++++----
arch/x86/Makefile_32.cpu | 18 ------------------
arch/x86/kernel/ftrace.c | 6 ++++++
scripts/Kbuild.include | 4 ++++
4 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..a94a4d1 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -120,10 +120,6 @@ else
# -funit-at-a-time shrinks the kernel .text considerably
# unfortunately it makes reading oopses harder.
KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
-
- # this works around some issues with generating unwind tables in older gccs
- # newer gccs do it by default
- KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
endif
ifdef CONFIG_X86_X32
@@ -147,6 +143,37 @@ ifeq ($(CONFIG_KMEMCHECK),y)
KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
endif
+#
+# If the function graph tracer is used with mcount instead of fentry,
+# '-maccumulate-outgoing-args' is needed to prevent a GCC bug
+# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)
+#
+ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ ifndef CONFIG_HAVE_FENTRY
+ ACCUMULATE_OUTGOING_ARGS := 1
+ else
+ ifeq ($(call cc-option-yn, -mfentry), n)
+ ACCUMULATE_OUTGOING_ARGS := 1
+ endif
+ endif
+endif
+
+#
+# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
+# GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226). There's no way
+# to test for this bug at compile-time because the test case needs to execute,
+# which is a no-go for cross compilers. So check the GCC version instead.
+#
+ifdef CONFIG_JUMP_LABEL
+ ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
+ endif
+endif
+
+ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ KBUILD_CFLAGS += -maccumulate-outgoing-args
+endif
+
# Stackpointer is addressed different for 32 bit and 64 bit x86
sp-$(CONFIG_X86_32) := esp
sp-$(CONFIG_X86_64) := rsp
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..a45eb15 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
# cpu entries
cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
-# Work around the pentium-mmx code generator madness of gcc4.4.x which
-# does stack alignment by generating horrible code _before_ the mcount
-# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
-# tracer assumptions. For i686, generic, core2 this is set by the
-# compiler anyway
-ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-# Work around to a bug with asm goto with first implementations of it
-# in gcc causing gcc to mess up the push and pop of the stack in some
-# uses of asm goto.
-ifeq ($(CONFIG_JUMP_LABEL), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
-
# Bug fix for binutils: this option is required in order to keep
# binutils from generating NOPL instructions against our will.
ifneq ($(CONFIG_X86_P6_NOP),y)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8f3d9cf..cbd73eb 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -29,6 +29,12 @@
#include <asm/ftrace.h>
#include <asm/nops.h>
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
+ !defined(CC_USING_FENTRY) && \
+ !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
+# error The following combination is not supported: ((compiler missing -mfentry) || (CONFIG_X86_32 and !CONFIG_DYNAMIC_FTRACE)) && CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_CC_OPTIMIZE_FOR_SIZE
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
int ftrace_arch_code_modify_prepare(void)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index d6ca649..afe3fd3 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
+# cc-if-fullversion
+# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
+cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
+
# cc-ldoption
# Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
cc-ldoption = $(call try-run,\
--
2.7.4
Commit-ID: 3f135e57a4f76d24ae8d8a490314331f0ced40c5
Gitweb: http://git.kernel.org/tip/3f135e57a4f76d24ae8d8a490314331f0ced40c5
Author: Josh Poimboeuf <[email protected]>
AuthorDate: Thu, 16 Mar 2017 14:31:33 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 30 Mar 2017 11:53:04 +0200
x86/build: Mostly disable '-maccumulate-outgoing-args'
The GCC '-maccumulate-outgoing-args' flag is enabled for most configs,
mostly because of issues which are no longer relevant. For most
configs, and with most recent versions of GCC, it's no longer needed.
Clarify which cases need it, and only enable it for those cases. Also
produce a compile-time error for the ftrace graph + mcount + '-Os' case,
which will otherwise cause runtime failures.
The main benefit of '-maccumulate-outgoing-args' is that it prevents an
ugly prologue for functions which have aligned stacks. But removing the
option also has some benefits: more readable argument saves, smaller
text size, and (presumably) slightly improved performance.
Here are the object size savings for 32-bit and 64-bit defconfig
kernels:
text data bss dec hex filename
10006710 3543328 1773568 15323606 e9d1d6 vmlinux.x86-32.before
9706358 3547424 1773568 15027350 e54c96 vmlinux.x86-32.after
text data bss dec hex filename
10652105 4537576 843776 16033457 f4a6b1 vmlinux.x86-64.before
10639629 4537576 843776 16020981 f475f5 vmlinux.x86-64.after
That comes out to a 3% text size improvement on x86-32 and a 0.1% text
size improvement on x86-64.
Signed-off-by: Josh Poimboeuf <[email protected]>
Cc: Andrew Lutomirski <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Brian Gerst <[email protected]>
Cc: Denys Vlasenko <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/20170316193133.zrj6gug53766m6nn@treble
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/Makefile | 35 +++++++++++++++++++++++++++++++----
arch/x86/Makefile_32.cpu | 18 ------------------
arch/x86/kernel/ftrace.c | 6 ++++++
scripts/Kbuild.include | 4 ++++
4 files changed, 41 insertions(+), 22 deletions(-)
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 2d44933..a94a4d1 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -120,10 +120,6 @@ else
# -funit-at-a-time shrinks the kernel .text considerably
# unfortunately it makes reading oopses harder.
KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
-
- # this works around some issues with generating unwind tables in older gccs
- # newer gccs do it by default
- KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
endif
ifdef CONFIG_X86_X32
@@ -147,6 +143,37 @@ ifeq ($(CONFIG_KMEMCHECK),y)
KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
endif
+#
+# If the function graph tracer is used with mcount instead of fentry,
+# '-maccumulate-outgoing-args' is needed to prevent a GCC bug
+# (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109)
+#
+ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ ifndef CONFIG_HAVE_FENTRY
+ ACCUMULATE_OUTGOING_ARGS := 1
+ else
+ ifeq ($(call cc-option-yn, -mfentry), n)
+ ACCUMULATE_OUTGOING_ARGS := 1
+ endif
+ endif
+endif
+
+#
+# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent a
+# GCC bug (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226). There's no way
+# to test for this bug at compile-time because the test case needs to execute,
+# which is a no-go for cross compilers. So check the GCC version instead.
+#
+ifdef CONFIG_JUMP_LABEL
+ ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
+ endif
+endif
+
+ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
+ KBUILD_CFLAGS += -maccumulate-outgoing-args
+endif
+
# Stackpointer is addressed different for 32 bit and 64 bit x86
sp-$(CONFIG_X86_32) := esp
sp-$(CONFIG_X86_64) := rsp
diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
index 6647ed4..a45eb15 100644
--- a/arch/x86/Makefile_32.cpu
+++ b/arch/x86/Makefile_32.cpu
@@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
# cpu entries
cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
-# Work around the pentium-mmx code generator madness of gcc4.4.x which
-# does stack alignment by generating horrible code _before_ the mcount
-# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
-# tracer assumptions. For i686, generic, core2 this is set by the
-# compiler anyway
-ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-# Work around to a bug with asm goto with first implementations of it
-# in gcc causing gcc to mess up the push and pop of the stack in some
-# uses of asm goto.
-ifeq ($(CONFIG_JUMP_LABEL), y)
-ADD_ACCUMULATE_OUTGOING_ARGS := y
-endif
-
-cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
-
# Bug fix for binutils: this option is required in order to keep
# binutils from generating NOPL instructions against our will.
ifneq ($(CONFIG_X86_P6_NOP),y)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8f3d9cf..cbd73eb 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -29,6 +29,12 @@
#include <asm/ftrace.h>
#include <asm/nops.h>
+#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
+ !defined(CC_USING_FENTRY) && \
+ !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
+# error The following combination is not supported: ((compiler missing -mfentry) || (CONFIG_X86_32 and !CONFIG_DYNAMIC_FTRACE)) && CONFIG_FUNCTION_GRAPH_TRACER && CONFIG_CC_OPTIMIZE_FOR_SIZE
+#endif
+
#ifdef CONFIG_DYNAMIC_FTRACE
int ftrace_arch_code_modify_prepare(void)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index d6ca649..afe3fd3 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
+# cc-if-fullversion
+# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
+cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
+
# cc-ldoption
# Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
cc-ldoption = $(call try-run,\