From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicholas Piggin Subject: Re: [PATCH 1/5] kbuild: thin archives final link close --whole-archives option Date: Wed, 21 Jun 2017 12:47:08 +1000 Message-ID: <20170621124708.6bce7692@roar.ozlabs.ibm.com> References: <20170609052417.561-1-npiggin@gmail.com> <20170609052417.561-2-npiggin@gmail.com> <20170619165100.3ce26d00@roar.ozlabs.ibm.com> <20170619182714.2cb60e17@roar.ozlabs.ibm.com> <20170620015205.329519b2@roar.ozlabs.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-kbuild-owner@vger.kernel.org To: Masahiro Yamada Cc: Linux Kbuild mailing list , linux-arch , Michal Marek , Linus Torvalds , Stephen Rothwell , kbuild test robot , Josh Triplett List-Id: linux-arch.vger.kernel.org On Wed, 21 Jun 2017 10:17:11 +0900 Masahiro Yamada wrote: > Hi Nicholas, > > 2017-06-20 0:52 GMT+09:00 Nicholas Piggin : > >> > >> masahiro@pug:~$ xtensa-linux-ld -v > >> GNU ld (GNU Binutils) 2.24 > > > > Ah, thank you. It must have been that the 0day build (cc'ed) did > > not return any error to be from the ld internal error. > > > > This has led me to the true cause of the error which is the way > > external archives are handled. After implementing the fix, I think > > the lib.a size regression for thin archives should be solved as > > well. > > > Thanks for figuring this out! > > > > This patch should be added between patches 2 and 3 of this series. > > > > Done. > > > I applied all 6 patches to linux-kbuild/thin-ar. > > I fixed up some: > > [1] fix a typo "tihs" -> "this" > [2] reword the document as "The build system has, as of 4.13, switched > to using..." > [3] Add "P" to link_vmlinux.sh as well > > > Could you double-check if I did them correctly? Yes these look good to me, thank you. > > --- a/Makefile > > +++ b/Makefile > > @@ -952,19 +952,20 @@ core-y := $(patsubst %/, %/built-in.o, $(core-y)) > > drivers-y := $(patsubst %/, %/built-in.o, $(drivers-y)) > > net-y := $(patsubst %/, %/built-in.o, $(net-y)) > > libs-y1 := $(patsubst %/, %/lib.a, $(libs-y)) > > -libs-y2 := $(patsubst %/, %/built-in.o, $(libs-y)) > > +libs-y2 := $(filter-out %.a, $(patsubst %/, %/built-in.o, $(libs-y))) > > libs-y := $(libs-y1) $(libs-y2) > > > After I applied this patch, I noticed one more thing. > > With this patch, I think "libs-y" will be unnecessary. > > If you ack me to fix-up locally, I will remove: > libs-y := $(libs-y1) $(libs-y2) Yes I suppose that can be removed. I wonder if the names could be a bit more descriptive? libs-y-liba libs-y-builtin ? > > @@ -50,7 +57,7 @@ archive_builtin() > > if [ -n "${CONFIG_THIN_ARCHIVES}" ]; then > > info AR built-in.o > > rm -f built-in.o; > > - ${AR} rcsT${KBUILD_ARFLAGS} built-in.o \ > > + ${AR} rcsTP${KBUILD_ARFLAGS} built-in.o \ > > ${KBUILD_VMLINUX_INIT} \ > > ${KBUILD_VMLINUX_MAIN} > > fi > > I moved this hunk to 2/6 "thin archives use P option to ar". Thanks, Nick From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f67.google.com ([74.125.83.67]:33241 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752983AbdFUCrU (ORCPT ); Tue, 20 Jun 2017 22:47:20 -0400 Date: Wed, 21 Jun 2017 12:47:08 +1000 From: Nicholas Piggin Subject: Re: [PATCH 1/5] kbuild: thin archives final link close --whole-archives option Message-ID: <20170621124708.6bce7692@roar.ozlabs.ibm.com> In-Reply-To: References: <20170609052417.561-1-npiggin@gmail.com> <20170609052417.561-2-npiggin@gmail.com> <20170619165100.3ce26d00@roar.ozlabs.ibm.com> <20170619182714.2cb60e17@roar.ozlabs.ibm.com> <20170620015205.329519b2@roar.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Masahiro Yamada Cc: Linux Kbuild mailing list , linux-arch , Michal Marek , Linus Torvalds , Stephen Rothwell , kbuild test robot , Josh Triplett Message-ID: <20170621024708.quKKWd54S5_dPrdQMOXMO0uW8HEUbNSyzwxlhKYpcE0@z> On Wed, 21 Jun 2017 10:17:11 +0900 Masahiro Yamada wrote: > Hi Nicholas, > > 2017-06-20 0:52 GMT+09:00 Nicholas Piggin : > >> > >> masahiro@pug:~$ xtensa-linux-ld -v > >> GNU ld (GNU Binutils) 2.24 > > > > Ah, thank you. It must have been that the 0day build (cc'ed) did > > not return any error to be from the ld internal error. > > > > This has led me to the true cause of the error which is the way > > external archives are handled. After implementing the fix, I think > > the lib.a size regression for thin archives should be solved as > > well. > > > Thanks for figuring this out! > > > > This patch should be added between patches 2 and 3 of this series. > > > > Done. > > > I applied all 6 patches to linux-kbuild/thin-ar. > > I fixed up some: > > [1] fix a typo "tihs" -> "this" > [2] reword the document as "The build system has, as of 4.13, switched > to using..." > [3] Add "P" to link_vmlinux.sh as well > > > Could you double-check if I did them correctly? Yes these look good to me, thank you. > > --- a/Makefile > > +++ b/Makefile > > @@ -952,19 +952,20 @@ core-y := $(patsubst %/, %/built-in.o, $(core-y)) > > drivers-y := $(patsubst %/, %/built-in.o, $(drivers-y)) > > net-y := $(patsubst %/, %/built-in.o, $(net-y)) > > libs-y1 := $(patsubst %/, %/lib.a, $(libs-y)) > > -libs-y2 := $(patsubst %/, %/built-in.o, $(libs-y)) > > +libs-y2 := $(filter-out %.a, $(patsubst %/, %/built-in.o, $(libs-y))) > > libs-y := $(libs-y1) $(libs-y2) > > > After I applied this patch, I noticed one more thing. > > With this patch, I think "libs-y" will be unnecessary. > > If you ack me to fix-up locally, I will remove: > libs-y := $(libs-y1) $(libs-y2) Yes I suppose that can be removed. I wonder if the names could be a bit more descriptive? libs-y-liba libs-y-builtin ? > > @@ -50,7 +57,7 @@ archive_builtin() > > if [ -n "${CONFIG_THIN_ARCHIVES}" ]; then > > info AR built-in.o > > rm -f built-in.o; > > - ${AR} rcsT${KBUILD_ARFLAGS} built-in.o \ > > + ${AR} rcsTP${KBUILD_ARFLAGS} built-in.o \ > > ${KBUILD_VMLINUX_INIT} \ > > ${KBUILD_VMLINUX_MAIN} > > fi > > I moved this hunk to 2/6 "thin archives use P option to ar". Thanks, Nick