| Message ID | 20170529081103.29999-1-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Nick, On Mon, 29 May 2017 18:11:03 +1000 Nicholas Piggin <npiggin@gmail.com> wrote: > > diff --git a/arch/Kconfig b/arch/Kconfig > index 6c00e5b00f8b..28e64cb65dd5 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -511,10 +511,23 @@ config CC_STACKPROTECTOR_STRONG > endchoice > > config THIN_ARCHIVES > - bool > + bool "Build the kernel using thin archives" > + default n Of course, this will be enabled by allmodconfig, etc builds ...
On Mon, 29 May 2017 20:33:55 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > Hi Nick, > > On Mon, 29 May 2017 18:11:03 +1000 Nicholas Piggin <npiggin@gmail.com> wrote: > > > > diff --git a/arch/Kconfig b/arch/Kconfig > > index 6c00e5b00f8b..28e64cb65dd5 100644 > > --- a/arch/Kconfig > > +++ b/arch/Kconfig > > @@ -511,10 +511,23 @@ config CC_STACKPROTECTOR_STRONG > > endchoice > > > > config THIN_ARCHIVES > > - bool > > + bool "Build the kernel using thin archives" > > + default n > > Of course, this will be enabled by allmodconfig, etc builds ... > Yes that is true. Thanks for making a note of it -- I guess the changelog could be interpreted as implying no change to any existing build without arch enablement. I didn't intend to mislead there. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 29, 2017 at 1:11 AM, Nicholas Piggin <npiggin@gmail.com> wrote: > > The next step will be to have archs always select THIN_ARCHIVES > when they are known to work. Then remove the option entirely. Honestly, why even do that? If we really think that thin archives is the way to go, and it's known to work on ppc, x86 and arm, why not just do it unconditionally? I hate our kconfig process as-is, and asking about odd binutils things that nobody knows the answer to is just rude. Plus it will actually be really really painful to find problems triggered by this, because it will be some magical config detail that nobody thinks of. In contrast, if we just enable this unconditionally, and it causes problems, they will show up the culprit trivially with "git bisect". So either thin archives work or they don't. Don't make it some painful drawn-out process that actually makes it harder to test rather than easier. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 29, 2017 at 10:03 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > So either thin archives work or they don't. Don't make it some painful > drawn-out process that actually makes it harder to test rather than > easier. Side note: what would make them not work? Are there known bugs in 'ar' wrt thin archives, versioning issues, or other issues? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Nicholas. > Supporting two different intermediate-artifact packaging schemes > was only ever intended as a temporary transition. > > This has so far caused no problems for powerpc, after a small fix > for how the arch invoked ar. So now allow any arch to select the > option, continue defaulting to N. Alan Modra recommended this approach several years ago, and I think Stephen was the first to implement this for the kernel. It would be good to have the rational whay ar is better than ld -r included in the commit message for later reference. I also recall that using ar gave some small kernel size reductions from last time we played with this. This info could also be nice to give a rough idea of the impact. Added Alan to list of receivers. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 29 May 2017 10:14:50 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Mon, May 29, 2017 at 10:03 AM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > So either thin archives work or they don't. Don't make it some painful > > drawn-out process that actually makes it harder to test rather than > > easier. > > Side note: what would make them not work? Are there known bugs in 'ar' > wrt thin archives, versioning issues, or other issues? We haven't run into any bugs AFAIK. It's been pretty solid. *thin* archives does require binutils 2.19. That's nearly 10 years old, but we advertise 2.12 minimum at the moment. We'd have make "T" an optional argument if we turn it on unconditionally. Regular archives shouldn't be significantly worse than ld -r though, so yeah maybe we could do that. powerpc required a build fix: 43c9127d94d -override AR := GNUTARGET=elf$(BITS)-$(GNUTARGET) $(AR) +KBUILD_ARFLAGS += --target=elf$(BITS)-$(GNUTARGET) So there could be a couple of small things like this. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 29 May 2017 20:52:51 +0200 Sam Ravnborg <sam@ravnborg.org> wrote: > Hi Nicholas. > > > Supporting two different intermediate-artifact packaging schemes > > was only ever intended as a temporary transition. > > > > This has so far caused no problems for powerpc, after a small fix > > for how the arch invoked ar. So now allow any arch to select the > > option, continue defaulting to N. > > Alan Modra recommended this approach several years ago, and I think > Stephen was the first to implement this for the kernel. > It would be good to have the rational whay ar is better than ld -r > included in the commit message for later reference. We *are* using Stephen's thin archives build infrastructure in the kernel already. Only powerpc is using it so far: a5967db9af The big one for powerpc build is that the linker keeps relative location of input sections the same, so as you link into larger built-in.o files, there is less opportunity to place code optimally. x86 may not care so much, but some other archs will. The build output size improvement is nice for everyone though. > I also recall that using ar gave some small kernel size reductions > from last time we played with this. > This info could also be nice to give a rough idea of the impact. There is some explanation in sfr's patch. It's not thin archives as such, but rather we have to link with --whole-archive, but it was a very small impact. IIRC today's final link is technically buggy without that option, it's just that in practice the top-level built-in.o files pull in so much that the linker will never discard one. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 29, 2017 at 4:13 PM, Nicholas Piggin <npiggin@gmail.com> wrote: > > *thin* archives does require binutils 2.19. That's nearly 10 years > old, but we advertise 2.12 minimum at the moment. I think we're ok with upgrading that. We've been allowing some ridiculously old tools at times. I think "feature is 10 years old and actively used by other projects" (Qt uses it from some googling, for example) is more than sufficient. If somebody has user space older than that, they presumably aren't building new kernels either. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 29, 2017 at 4:13 PM, Nicholas Piggin <npiggin@gmail.com> wrote: > > We haven't run into any bugs AFAIK. It's been pretty solid. I tried just changing the THIN_ARCHIVES "bool" into "def_bool y" to test it on my laptop, and certainly saw neither build problems nor problems with the resulting kernel image. So honestly, I'd really rather just see the change happen unconditionally, and finding problems much quicker and easier that way (in addition to making the conversion much simpler and not asking people odd questions). Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 29 May 2017 20:22:41 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Mon, May 29, 2017 at 4:13 PM, Nicholas Piggin <npiggin@gmail.com> wrote: > > > > *thin* archives does require binutils 2.19. That's nearly 10 years > > old, but we advertise 2.12 minimum at the moment. > > I think we're ok with upgrading that. We've been allowing some > ridiculously old tools at times. I think "feature is 10 years old and > actively used by other projects" (Qt uses it from some googling, for > example) is more than sufficient. Well we might have to. I couldn't make thick archives work easily because we have multiple levels of built-in.o, and with thick you get an archive of an archive which the linker does not understand. Fixing that would mean unpacking the archive and repacking it for every built-in.o input you link -- too painful. I suspect Google uses the format internally too because they added the initial support. https://sourceware.org/ml/binutils/2008-03/msg00150.html Support has been pervasively added to binutils -- nm, objdump, ld, etc. I think it falls into the category of well supported. > > If somebody has user space older than that, they presumably aren't > building new kernels either. Yeah I think it's reasonable to drop support for such old toolchain if we have a reason for it. I'll send a patch to 0day and if that passes, I'll see if it's something Stephen would put into linux-next. Thanks, Nick -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 29, 2017 at 10:11 AM, Nicholas Piggin <npiggin@gmail.com> wrote: > Supporting two different intermediate-artifact packaging schemes > was only ever intended as a temporary transition. > > This has so far caused no problems for powerpc, after a small fix > for how the arch invoked ar. So now allow any arch to select the > option, continue defaulting to N. > > Cc: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > The next step will be to have archs always select THIN_ARCHIVES > when they are known to work. Then remove the option entirely. > > x86 has always just worked for me, so that should be easy. I have build-tested many thousand randconfig kernels on arm32 with this option enabled, and did not run into build-time regressions besides some initial problems from a broken binutils snapshot (all released binutils versions should be fine). Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/Kconfig b/arch/Kconfig index 6c00e5b00f8b..28e64cb65dd5 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -511,10 +511,23 @@ config CC_STACKPROTECTOR_STRONG endchoice config THIN_ARCHIVES - bool + bool "Build the kernel using thin archives" + default n help - Select this if the architecture wants to use thin archives - instead of ld -r to create the built-in.o files. + Enable this if you want to use thin archives (binutils `ar` thin + archive format) rather than `ld -r`, to create the built-in.o and + other intermediate packaging steps in the kernel build. + + This option reduces disk space for builds, especially important for + debug builds and IO-constrained environments. It gives the linker + more flexibility in assembling sections. And it is more amenable to + link-time-optimization that may be implemented in future. + + The intention is for all architectures to move to thin archives + soon, and the ld -r support code removed. + + If unsure, say N. + Kernel hackers, say Y and test/fix your arch! config LD_DEAD_CODE_DATA_ELIMINATION bool diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 6eb70c96ec5e..9274d9faf122 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -483,14 +483,6 @@ config MPROFILE_KERNEL depends on PPC64 && CPU_LITTLE_ENDIAN def_bool !DISABLE_MPROFILE_KERNEL -config USE_THIN_ARCHIVES - bool "Build the kernel using thin archives" - default n - select THIN_ARCHIVES - help - Build the kernel using thin archives. - If you're unsure say N. - config IOMMU_HELPER def_bool PPC64
Supporting two different intermediate-artifact packaging schemes was only ever intended as a temporary transition. This has so far caused no problems for powerpc, after a small fix for how the arch invoked ar. So now allow any arch to select the option, continue defaulting to N. Cc: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- The next step will be to have archs always select THIN_ARCHIVES when they are known to work. Then remove the option entirely. x86 has always just worked for me, so that should be easy. arch/Kconfig | 19 ++++++++++++++++--- arch/powerpc/Kconfig | 8 -------- 2 files changed, 16 insertions(+), 11 deletions(-)