LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y
@ 2020-03-09  2:39 Masahiro Yamada
  2020-03-09 10:55 ` kbuild test robot
  0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2020-03-09  2:39 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Al Viro, clang-built-linux,
	F���ng-ru���
	S���ng, Ilie Halip, Nick Desaulniers,
	Nathan Chancellor, Masahiro Yamada, Michal Marek

Kbuild supports not only obj-y but also lib-y to list objects linked to
vmlinux.

The difference between them is that all the objects from obj-y are
forcibly linked to vmlinux by using --whole-archive, whereas the objects
from lib-y are linked as needed; if there is no user of a lib-y object,
it is not linked.

lib-y is intended to list utility functions that may be called from all
over the place (and may be unused at all), but it is a problem for
EXPORT_SYMBOL(). Even if there is no call-site in the vmlinux, we need
to keep exported symbols for the use from loadable modules.

Commit 7f2084fa55e6 ("[kbuild] handle exports in lib-y objects reliably")
worked around it by linking a dummy object, lib-ksyms.o, which contains
references to all the symbols exported from lib.a in that directory.
It uses the linker script command, EXTERN. Unfortunately, the meaning of
EXTERN of ld.lld is different from that of ld.bfd. Therefore, this does
not work with LD=ld.lld (CBL issue #515).

Anyway, the build rule of lib-ksyms.o is somewhat tricky. So, I want to
get rid of it.

At first, I was thinking of accumulating lib-y objects into obj-y
(or even replacing lib-y with obj-y entirely), but the lib-y syntax
is used beyond the ordinary use in lib/ and arch/*/lib/.

Examples:

 - drivers/firmware/efi/libstub/Makefile builds lib.a, which is linked
   into vmlinux in the own way (arm64), or linked to the decompressor
   (arm, x86).

 - arch/alpha/lib/Makefile builds lib.a which is linked not only to
   vmlinux, but also to bootloaders in arch/alpha/boot/Makefile.

 - arch/xtensa/boot/lib/Makefile builds lib.a for use from
   arch/xtensa/boot/boot-redboot/Makefile.

One more thing, adding everything to obj-y would increase the vmlinux
size of allnoconfig (or tinyconfig).

For less impact, I tweaked the destination of lib.a at the top Makefile;
when CONFIG_MODULES=y, lib.a goes to KBUILD_VMLINUX_OBJS, which is
forcibly linked to vmlinux, otherwise lib.a goes to KBUILD_VMLINUX_LIBS
as before.

The size impact for normal usecases is quite small since at lease one
symbol in every lib-y object is eventually called by someone. In case
you are intrested, here are the figures.

x86_64_defconfig:

   text	   data	    bss	    dec	    hex	filename
19566602 5422072 1589328 26578002 1958c52 vmlinux.before
19566932 5422104 1589328 26578364 1958dbc vmlinux.after

The case with the biggest impact is allnoconfig + CONFIG_MODULES=y.

ARCH=x86 allnoconfig + CONFIG_MODULES=y:

   text	   data	    bss	    dec	    hex	filename
1175162	 254740	1220608	2650510	 28718e	vmlinux.before
1177974	 254836	1220608	2653418	 287cea	vmlinux.after

Hopefully this is still not a big deal. The per-file trimming with the
static library is not so effective after all.

If fine-grained optimization is desired, some architectures support
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION, which trims dead code per-symbol
basis. When LTO is supported in mainline, even better optimization will
be possible.

Link: https://github.com/ClangBuiltLinux/linux/issues/515
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile               |  7 ++++++-
 scripts/Makefile.build | 17 -----------------
 2 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index 86035d866f2c..07f89d2a581a 100644
--- a/Makefile
+++ b/Makefile
@@ -1031,8 +1031,13 @@ init-y		:= $(patsubst %/, %/built-in.a, $(init-y))
 core-y		:= $(patsubst %/, %/built-in.a, $(core-y))
 drivers-y	:= $(patsubst %/, %/built-in.a, $(drivers-y))
 net-y		:= $(patsubst %/, %/built-in.a, $(net-y))
+libs-y2		:= $(patsubst %/, %/built-in.a, $(filter %/, $(libs-y)))
+ifdef CONFIG_MODULES
+libs-y1		:= $(filter-out %/, $(libs-y))
+libs-y2		+= $(patsubst %/, %/lib.a, $(filter %/, $(libs-y)))
+else
 libs-y1		:= $(patsubst %/, %/lib.a, $(libs-y))
-libs-y2		:= $(patsubst %/, %/built-in.a, $(filter-out %.a, $(libs-y)))
+endif
 virt-y		:= $(patsubst %/, %/built-in.a, $(virt-y))
 
 # Externally visible symbols (used by link-vmlinux.sh)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a1730d42e5f3..356601994f3a 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -65,7 +65,6 @@ endif
 
 ifneq ($(strip $(lib-y) $(lib-m) $(lib-)),)
 lib-target := $(obj)/lib.a
-real-obj-y += $(obj)/lib-ksyms.o
 endif
 
 ifdef need-builtin
@@ -410,22 +409,6 @@ $(lib-target): $(lib-y) FORCE
 
 targets += $(lib-target)
 
-dummy-object = $(obj)/.lib_exports.o
-ksyms-lds = $(dot-target).lds
-
-quiet_cmd_export_list = EXPORTS $@
-cmd_export_list = $(OBJDUMP) -h $< | \
-	sed -ne '/___ksymtab/s/.*+\([^ ]*\).*/EXTERN(\1)/p' >$(ksyms-lds);\
-	rm -f $(dummy-object);\
-	echo | $(CC) $(a_flags) -c -o $(dummy-object) -x assembler -;\
-	$(LD) $(ld_flags) -r -o $@ -T $(ksyms-lds) $(dummy-object);\
-	rm $(dummy-object) $(ksyms-lds)
-
-$(obj)/lib-ksyms.o: $(lib-target) FORCE
-	$(call if_changed,export_list)
-
-targets += $(obj)/lib-ksyms.o
-
 endif
 
 # NOTE:
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y
  2020-03-09  2:39 [PATCH] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y Masahiro Yamada
@ 2020-03-09 10:55 ` kbuild test robot
  2020-03-09 19:59   ` Nick Desaulniers
  0 siblings, 1 reply; 4+ messages in thread
From: kbuild test robot @ 2020-03-09 10:55 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: kbuild-all, linux-kbuild, linux-kernel, Al Viro,
	clang-built-linux,
	F���ng-ru���
	S���ng, Ilie Halip, Nick Desaulniers,
	Nathan Chancellor, Masahiro Yamada, Michal Marek

[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on kbuild/for-next]
[also build test ERROR on linux/master linus/master v5.6-rc5 next-20200306]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-link-lib-y-objects-to-vmlinux-forcibly-when-CONFIG_MODULES-y/20200309-115312
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
config: sparc-defconfig (attached as .config)
compiler: sparc-linux-gcc (GCC) 7.5.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.5.0 make.cross ARCH=sparc 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/sparc/lib/strlen.o: In function `strlen':
>> (.text+0x0): multiple definition of `strlen'
   lib/string.o:string.c:(.text+0x2b0): first defined here

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12974 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y
  2020-03-09 10:55 ` kbuild test robot
@ 2020-03-09 19:59   ` Nick Desaulniers
  2020-03-10  1:14     ` Masahiro Yamada
  0 siblings, 1 reply; 4+ messages in thread
From: Nick Desaulniers @ 2020-03-09 19:59 UTC (permalink / raw)
  To: David S. Miller, Masahiro Yamada
  Cc: kbuild-all, Linux Kbuild mailing list, LKML, Al Viro,
	clang-built-linux,
	F���ng-ru���
	S���ng, Ilie Halip, Nathan Chancellor,
	Michal Marek, kbuild test robot

On Mon, Mar 9, 2020 at 3:55 AM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Masahiro,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on kbuild/for-next]
> [also build test ERROR on linux/master linus/master v5.6-rc5 next-20200306]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-link-lib-y-objects-to-vmlinux-forcibly-when-CONFIG_MODULES-y/20200309-115312
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> config: sparc-defconfig (attached as .config)
> compiler: sparc-linux-gcc (GCC) 7.5.0
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         GCC_VERSION=7.5.0 make.cross ARCH=sparc
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    arch/sparc/lib/strlen.o: In function `strlen':
> >> (.text+0x0): multiple definition of `strlen'
>    lib/string.o:string.c:(.text+0x2b0): first defined here

So this looks like a bug in 32b Sparc, that we should fix first.

arch/sparc/lib/strlen.S provides a definition of strlen, but it does
not define the preprocessor token __HAVE_ARCH_STRLEN to avoid multiple
definitions from producing a link error.

Or rather, __HAVE_ARCH_STRLEN is provided in include/asm/string_64.h,
but not for 32b sparc.  arch/sparc/strlen.o is unconditionally
required in lib-y in arch/sparc/lib/Makefile.

Either:
1. arch/sparc/lib/strlen.S supports 32b sparc, then __HAVE_ARCH_STRLEN
and a forward declaration of strlen should be provided in
arch/sparc/include/asm/string.h rather than
arch/sparc/include/asm/string_64.h, or...
2. arch/sparc/lib/strlen.S does not support 32b sparc, then the
inclusion of strlen.o in arch/sparc/lib/Makefile should be predicated
on CONFIG_SPARC64.

+ Dave who maybe can provide guidance on how to proceed?  The use of
the BRANCH32 macro in arch/sparc/lib/strlen.S seems to have different
definitions based on CONFIG_SPARC64 vs CONFIG_SPARC32, which makes me
thing it's case 1 above, but I'm not familiar with Sparc assembly to
be confident.
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y
  2020-03-09 19:59   ` Nick Desaulniers
@ 2020-03-10  1:14     ` Masahiro Yamada
  0 siblings, 0 replies; 4+ messages in thread
From: Masahiro Yamada @ 2020-03-10  1:14 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: David S. Miller, kbuild-all, Linux Kbuild mailing list, LKML,
	Al Viro, clang-built-linux,
	F���ng-ru���
	S���ng, Ilie Halip, Nathan Chancellor,
	Michal Marek, kbuild test robot

Hi Nick, David,

On Tue, Mar 10, 2020 at 4:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Mar 9, 2020 at 3:55 AM kbuild test robot <lkp@intel.com> wrote:
> >
> > Hi Masahiro,
> >
> > I love your patch! Yet something to improve:
> >
> > [auto build test ERROR on kbuild/for-next]
> > [also build test ERROR on linux/master linus/master v5.6-rc5 next-20200306]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-link-lib-y-objects-to-vmlinux-forcibly-when-CONFIG_MODULES-y/20200309-115312
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> > config: sparc-defconfig (attached as .config)
> > compiler: sparc-linux-gcc (GCC) 7.5.0
> > reproduce:
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # save the attached .config to linux build tree
> >         GCC_VERSION=7.5.0 make.cross ARCH=sparc
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    arch/sparc/lib/strlen.o: In function `strlen':
> > >> (.text+0x0): multiple definition of `strlen'
> >    lib/string.o:string.c:(.text+0x2b0): first defined here
>
> So this looks like a bug in 32b Sparc, that we should fix first.
>
> arch/sparc/lib/strlen.S provides a definition of strlen, but it does
> not define the preprocessor token __HAVE_ARCH_STRLEN to avoid multiple
> definitions from producing a link error.
>
> Or rather, __HAVE_ARCH_STRLEN is provided in include/asm/string_64.h,
> but not for 32b sparc.  arch/sparc/strlen.o is unconditionally
> required in lib-y in arch/sparc/lib/Makefile.
>
> Either:
> 1. arch/sparc/lib/strlen.S supports 32b sparc, then __HAVE_ARCH_STRLEN
> and a forward declaration of strlen should be provided in
> arch/sparc/include/asm/string.h rather than
> arch/sparc/include/asm/string_64.h, or...
> 2. arch/sparc/lib/strlen.S does not support 32b sparc, then the
> inclusion of strlen.o in arch/sparc/lib/Makefile should be predicated
> on CONFIG_SPARC64.
>
> + Dave who maybe can provide guidance on how to proceed?  The use of
> the BRANCH32 macro in arch/sparc/lib/strlen.S seems to have different
> definitions based on CONFIG_SPARC64 vs CONFIG_SPARC32, which makes me
> thing it's case 1 above, but I'm not familiar with Sparc assembly to
> be confident.



I agree.
The former is probably the intention
according to this commit:

commit ae984d72e0632782dd98c3fcf469b9045ad0d449
Author: David S. Miller <davem@davemloft.net>
Date:   Tue Dec 9 01:07:09 2008 -0800

    sparc: Unify strlen assembler.

    Use the new asm/asm.h header to help commonize the
    strlen assembler between 32-bit and 64-bit

    While we're here, use proper linux/linkage.h macros
    instead of by-hand stuff.

    Signed-off-by: David S. Miller <davem@davemloft.net>



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-03-10  1:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09  2:39 [PATCH] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y Masahiro Yamada
2020-03-09 10:55 ` kbuild test robot
2020-03-09 19:59   ` Nick Desaulniers
2020-03-10  1:14     ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).