diff mbox

[RFC,2/2] Kbuild: avoid partial linking of drivers/built-in.o

Message ID 1427716167-25078-3-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel March 30, 2015, 11:49 a.m. UTC
The recursive partial linking of vmlinux can result in a
drivers/built-in.o that is so huge that it interferes with
the ability of the linker to emit veneers in the final link
stage if the symbols are out of reach. This is caused by the
fact that those veneers, which should be emitted close enough
to the original call site, can only be emitted after the .text
section of drivers/built-in.o, whose size pushes those veneers
out of range.

So instead, avoid building drivers/built-in.o, and instead, add
the constituent parts to the command line of the final link.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Makefile | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Ard Biesheuvel March 30, 2015, 12:54 p.m. UTC | #1
On 30 March 2015 at 14:38, Michal Marek <mmarek@suse.cz> wrote:
> On 2015-03-30 13:49, Ard Biesheuvel wrote:
>> The recursive partial linking of vmlinux can result in a
>> drivers/built-in.o that is so huge that it interferes with
>> the ability of the linker to emit veneers in the final link
>> stage if the symbols are out of reach. This is caused by the
>> fact that those veneers, which should be emitted close enough
>> to the original call site, can only be emitted after the .text
>> section of drivers/built-in.o, whose size pushes those veneers
>> out of range.
>
> Is this a limitation of a particular ARM ABI or a limitation of a state
> of the art ARM linker or something else? If such a hack is necessary, it
> needs to be accompanied with an explanation as to in which environments
> it is needed, whether it can be removed at some point in future, what is
> the exact error it causes, etc. Also, are you able to gauge the
> limitation? Will it at some point affect fs/built-in.o or
> drivers/net/built-in.o?
>

The limitation results from the fact that ARM branch instructions are
limited to 24-bits of relative offset, and depending on the
instruction set (ARM or Thumb) this translates to +/- 32 MB or +/- 16
MB respectively.

Note that using -ffunction-sections works around the problem as well,
but it typically uses more space. There are other archs that add this,
to work around a similar issue.

The errors it causes are build time linker errors, i.e., after
crossing a certain size threshold, you just cannot build vmlinux
anymore.

It is not something that we would able to remove in the future, as it
is simply a result of the ISA capabilities and the build strategy that
relies heavily on partial linking. Other linkers would not be able to
do a better job, as the drivers/built-in.o(.text) section just exceeds
the maximum size that allows a branch instruction to jump out of it.

I am not sure whether it would affect other subdirs, but it is not
entirely unlikely.

A more structural approach would be to limit the contents of the
built-in.o files to the mod_init/_exit/_etc input sections, and let
those pull in everything else through a libbuilt-in.a at the same
level that contains the .text/.data etc, each of which would be
combined with the constituent ones as we do for built-in.o now. I
haven't played around with this extensively, though.

>
>> So instead, avoid building drivers/built-in.o, and instead, add
>> the constituent parts to the command line of the final link.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  Makefile | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile b/Makefile
>> index e734965b1604..1eb6c246a586 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -558,7 +558,7 @@ scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
>>
>>  # Objects we will link into vmlinux / subdirs we need to visit
>>  init-y               := init/
>> -drivers-y    := drivers/ sound/ firmware/
>> +drivers-y    := sound/ firmware/
>>  net-y                := net/
>>  libs-y               := lib/
>>  core-y               := usr/
>> @@ -569,6 +569,16 @@ ifeq ($(dot-config),1)
>>  -include include/config/auto.conf
>>
>>  ifeq ($(KBUILD_EXTMOD),)
>> +
>> +# drivers/built-in.o can become huge, which interferes with the linker's
>> +# ability to emit stubs for branch targets that are out of reach for the
>> +# ordinary relative branch instructions
>> +include $(srctree)/drivers/Makefile
>> +drivers-y += $(addprefix drivers/,$(sort $(obj-y)))
>> +drivers-m += $(addprefix drivers/,$(sort $(obj-m)))
>
> I think this will break should we ever put a .c file in drivers/ directly.
>

Yes, it will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Ard Biesheuvel March 30, 2015, 1:31 p.m. UTC | #2
On 30 March 2015 at 15:26, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Mar 30, 2015 at 02:38:35PM +0200, Michal Marek wrote:
>> Is this a limitation of a particular ARM ABI or a limitation of a state
>> of the art ARM linker or something else?
>
> It's a limitation of the ARM ISA.
>
> Normal PC-relative branches, which are emitted by the C compiler, can
> branch +/- 32MB for ARM, or +/- 16MB of Thumb.  Beyond that, the address
> offset is not representable in the instruction.
>
> To get around that "modern" linkers are able to insert stubs as
> necessary inbetween the object .text sections to extend the range of
> these branches (by emitting a chunk of code possibly with some data to
> extend the range of the branch.)
>
> Obviously, gcc can't know before hand that the sum total of all the
> small object files is going to cause problems, so the compiler can't
> do this on an "as necessary basis".
>
> For most practical kernels, this is not a problem; they normally fit
> within the PC-relative range.  However, the exception is allyesconfig.
>
> The question is: how far do we go with allyesconfig... do we want it
> to work, or is reaching the final link sufficient?  If we do tweak
> stuff to allow the link to work, are we going to try running it?
>

That is an excellent question, hence the RFC in the subject line.

Note that the other patch, the one against kallsyms, addresses the
issue where the distance between the beginning of .text and the end of
.init.text  exceeds this limit, which is not as unlikely as the issue
that this patch addresses, where just drivers/built-in.o in isolation
already exceeds this limit.

So I am quite happy to drop this, especially as we can add
-ffunction-sections as well. (Haven't tested this myself though: after
building allyesconfig on my poor laptop a couple of times, I am
sending it on a well deserved vacation for the rest of the week)

Thanks,
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Ard Biesheuvel March 30, 2015, 5:04 p.m. UTC | #3
On 30 March 2015 at 16:13, Michal Marek <mmarek@suse.cz> wrote:
> On 2015-03-30 15:31, Ard Biesheuvel wrote:
>> On 30 March 2015 at 15:26, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Mon, Mar 30, 2015 at 02:38:35PM +0200, Michal Marek wrote:
>>>> Is this a limitation of a particular ARM ABI or a limitation of a state
>>>> of the art ARM linker or something else?
>>>
>>> It's a limitation of the ARM ISA.
>>>
>>> Normal PC-relative branches, which are emitted by the C compiler, can
>>> branch +/- 32MB for ARM, or +/- 16MB of Thumb.  Beyond that, the address
>>> offset is not representable in the instruction.
>
> Thank you both for the explanation!
>
>
>>> The question is: how far do we go with allyesconfig... do we want it
>>> to work, or is reaching the final link sufficient?
>
> It certainly is more useful as a test tool if the baseline is a
> successful compile and link. Because you can have genuine link errors
> due to missing symbols.
>

Agreed

>
>>> If we do tweak
>>> stuff to allow the link to work, are we going to try running it?
>
> Good question. I myself always treated all{yes,mod}config as a build
> test only and never dared to run it. Allyesconfig produces a giant
> kernel image and allmodconfig builds binfmt_script as a module. And if
> people used all*config for boot tests, they would probably be sending
> patches to tweak the Kconfigs for that purpose. And this is not the case
> as far as I can tell.
>

Russell should confirm this, but I think running such a large kernel
is non-trivial on ARM, since the decompressor should make room for the
decompressed image by moving itself upward in memory, and it may
overwrite the device tree binary in the process.

>
>> That is an excellent question, hence the RFC in the subject line.
>>
>> Note that the other patch, the one against kallsyms, addresses the
>> issue where the distance between the beginning of .text and the end of
>> .init.text  exceeds this limit, which is not as unlikely as the issue
>> that this patch addresses, where just drivers/built-in.o in isolation
>> already exceeds this limit.
>>
>> So I am quite happy to drop this, especially as we can add
>> -ffunction-sections as well.
>
> What you could do is to add a Kconfig option to arch/arm/Kconfig adding
> -ffunction-sections to the compiler flags. Then allyesconfig would
> select it and work around the problem in a somewhat elegant way.
>

Excellent idea! Arnd hasn't chimed in yet, but he is the one doing
lots and lots of randconfig builds and other test builds, so I will
wait for him to confirm that this is a useful thing to have.

Thanks,
Ard.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre March 31, 2015, 3:22 p.m. UTC | #4
On Mon, 30 Mar 2015, Ard Biesheuvel wrote:

> On 30 March 2015 at 16:13, Michal Marek <mmarek@suse.cz> wrote:
> > On 2015-03-30 15:31, Ard Biesheuvel wrote:
> >> On 30 March 2015 at 15:26, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >>> On Mon, Mar 30, 2015 at 02:38:35PM +0200, Michal Marek wrote:
> >>>> Is this a limitation of a particular ARM ABI or a limitation of a state
> >>>> of the art ARM linker or something else?
> >>>
> >>> It's a limitation of the ARM ISA.
> >>>
> >>> Normal PC-relative branches, which are emitted by the C compiler, can
> >>> branch +/- 32MB for ARM, or +/- 16MB of Thumb.  Beyond that, the address
> >>> offset is not representable in the instruction.
> >
> > Thank you both for the explanation!
> >
> >
> >>> The question is: how far do we go with allyesconfig... do we want it
> >>> to work, or is reaching the final link sufficient?
> >
> > It certainly is more useful as a test tool if the baseline is a
> > successful compile and link. Because you can have genuine link errors
> > due to missing symbols.
> >
> 
> Agreed
> 
> >
> >>> If we do tweak
> >>> stuff to allow the link to work, are we going to try running it?
> >
> > Good question. I myself always treated all{yes,mod}config as a build
> > test only and never dared to run it. Allyesconfig produces a giant
> > kernel image and allmodconfig builds binfmt_script as a module. And if
> > people used all*config for boot tests, they would probably be sending
> > patches to tweak the Kconfigs for that purpose. And this is not the case
> > as far as I can tell.
> >
> 
> Russell should confirm this, but I think running such a large kernel
> is non-trivial on ARM, since the decompressor should make room for the
> decompressed image by moving itself upward in memory, and it may
> overwrite the device tree binary in the process.

Loading the device tree at a different address should be easy.  You just 
need to load it at the top of RAM. You may also start by attempting to 
boot a plain Image (uncompressed) kernel binary.

> >> That is an excellent question, hence the RFC in the subject line.
> >>
> >> Note that the other patch, the one against kallsyms, addresses the
> >> issue where the distance between the beginning of .text and the end of
> >> .init.text  exceeds this limit, which is not as unlikely as the issue
> >> that this patch addresses, where just drivers/built-in.o in isolation
> >> already exceeds this limit.
> >>
> >> So I am quite happy to drop this, especially as we can add
> >> -ffunction-sections as well.
> >
> > What you could do is to add a Kconfig option to arch/arm/Kconfig adding
> > -ffunction-sections to the compiler flags. Then allyesconfig would
> > select it and work around the problem in a somewhat elegant way.
> >
> 
> Excellent idea! Arnd hasn't chimed in yet, but he is the one doing
> lots and lots of randconfig builds and other test builds, so I will
> wait for him to confirm that this is a useful thing to have.

I'm using -ffunction-sections as well for the kernel size reduction work 
I'm currently doing.  The linker script has to be adapted so .text.* is 
specified along .text otherwise those functions end up appended at the 
end of the binary.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre March 31, 2015, 6:42 p.m. UTC | #5
On Tue, 31 Mar 2015, Dave Martin wrote:

> On Tue, Mar 31, 2015 at 11:22:29AM -0400, Nicolas Pitre wrote:
> > On Mon, 30 Mar 2015, Ard Biesheuvel wrote:
> > 
> > > On 30 March 2015 at 16:13, Michal Marek <mmarek@suse.cz> wrote:
> 
> [...]
> 
> > > > What you could do is to add a Kconfig option to arch/arm/Kconfig adding
> > > > -ffunction-sections to the compiler flags. Then allyesconfig would
> > > > select it and work around the problem in a somewhat elegant way.
> > > >
> > > 
> > > Excellent idea! Arnd hasn't chimed in yet, but he is the one doing
> > > lots and lots of randconfig builds and other test builds, so I will
> > > wait for him to confirm that this is a useful thing to have.
> > 
> > I'm using -ffunction-sections as well for the kernel size reduction work 
> > I'm currently doing.  The linker script has to be adapted so .text.* is 
> > specified along .text otherwise those functions end up appended at the 
> > end of the binary.
> 
> Interesting ... do you also mean using --gc-sections at link time?

Yes.

> We'd need to avoid pruning needed code that has no explicit caller,
> and functions that are part of the kernel/module ABI but not used
> within vmlinux.

Those are usually located in special sections, like the initcall table, 
the CPU entry table, etc. The linker allows for those sections to be 
marked with KEEP() not to prune them.

> The GCC docs suggest that -ffunction-sections may impact performance
> and/or increase code size, but I don't know by how much.  Maybe it
> interferes with inling.

It doesn't interfere with inlining. However it impose a section 
alignment on every function. That still can be overriden though. Also, I 
suppose that gcc may not assume that calls to a global function that 
happens to be located in the same C file will be close by anymore, 
however I don't see this having any impact on ARM code generation.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre March 31, 2015, 6:46 p.m. UTC | #6
On Tue, 31 Mar 2015, Russell King - ARM Linux wrote:

> On Tue, Mar 31, 2015 at 05:27:22PM +0100, Dave Martin wrote:
> > On Tue, Mar 31, 2015 at 11:22:29AM -0400, Nicolas Pitre wrote:
> > > I'm using -ffunction-sections as well for the kernel size reduction work 
> > > I'm currently doing.  The linker script has to be adapted so .text.* is 
> > > specified along .text otherwise those functions end up appended at the 
> > > end of the binary.
> > 
> > Interesting ... do you also mean using --gc-sections at link time?
> > We'd need to avoid pruning needed code that has no explicit caller,
> > and functions that are part of the kernel/module ABI but not used
> > within vmlinux.
> > 
> > The GCC docs suggest that -ffunction-sections may impact performance
> > and/or increase code size, but I don't know by how much.  Maybe it
> > interferes with inling.
> 
> I've mentioned these options over the course of the last couple of
> months.  David Woodhouse replied in February, so it's worth reading
> up on that work.

Yup, I did.

> David Woodhouse wrote:
> | In many kernel configurations there are actually quite a lot of
> | functions that are never called, and I was quite surprised the first
> | time I played with this stuff.
> | 
> | There are a few ways of dealing with it. One is to use
> | -ffunction-section -fdata-sections --gc-sections as you noted. I once
> | also played with using GCC's --combine during the brief period that it
> | was supported and not *entirely* broken, with similar effects:
> | https://lwn.net/Articles/197097/
> | 
> | These days, the better answer is probably LTO. We could potentially
> | still look at --gc-sections, but I suspect we're better off using LTO
> | and just filing toolchain bugs until everything that --gc-sections
> | *would* have dropped is also dropped from the LTO build :)
> | 
> | Unless --gc-sections actually speeds up the build in a significant way;
> | a full LTO link of the kernel takes insane amounts of memory IIRC.

LTO indirectly achieve the same effect (and more), however it is 
frigging heavy and unwieldy for kernel builds.  And it has many subtle 
issues of its own that --gc-sections doesn't have.  I'm therefore 
looking at both in parallel.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/Makefile b/Makefile
index e734965b1604..1eb6c246a586 100644
--- a/Makefile
+++ b/Makefile
@@ -558,7 +558,7 @@  scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
 
 # Objects we will link into vmlinux / subdirs we need to visit
 init-y		:= init/
-drivers-y	:= drivers/ sound/ firmware/
+drivers-y	:= sound/ firmware/
 net-y		:= net/
 libs-y		:= lib/
 core-y		:= usr/
@@ -569,6 +569,16 @@  ifeq ($(dot-config),1)
 -include include/config/auto.conf
 
 ifeq ($(KBUILD_EXTMOD),)
+
+# drivers/built-in.o can become huge, which interferes with the linker's
+# ability to emit stubs for branch targets that are out of reach for the
+# ordinary relative branch instructions
+include $(srctree)/drivers/Makefile
+drivers-y += $(addprefix drivers/,$(sort $(obj-y)))
+drivers-m += $(addprefix drivers/,$(sort $(obj-m)))
+obj-y :=
+obj-m :=
+
 # Read in dependencies to all Kconfig* files, make sure to run
 # oldconfig if changes are detected.
 -include include/config/auto.conf.cmd