[PATCH v2 1/2] sparc: revive __HAVE_ARCH_STRLEN for 32bit sparc

[PATCH v2 1/2] sparc: revive __HAVE_ARCH_STRLEN for 32bit sparc

[Masahiro Yamada]

Prior to commit 70a6fcf3283a ("[sparc] unify 32bit and 64bit string.h"),
__HAVE_ARCH_STRLEN was defined in both of string_32.h and string_64.h

It did not unify __HAVE_ARCH_STRLEN, but deleted it from string_32.h

This issue was reported by the kbuild test robot in the trial of
forcible linking of $(lib-y) to vmlinux.

Fixes: 70a6fcf3283a ("[sparc] unify 32bit and 64bit string.h")
Reported-by: kbuild test robot <lkp@intel.com>
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - Insert a new patch to avoid sparc32 build error

 arch/sparc/include/asm/string.h    | 4 ++++
 arch/sparc/include/asm/string_64.h | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/sparc/include/asm/string.h b/arch/sparc/include/asm/string.h
index 3d9cd082716b..001a17baf2d5 100644
--- a/arch/sparc/include/asm/string.h
+++ b/arch/sparc/include/asm/string.h
@@ -37,6 +37,10 @@ void *memmove(void *, const void *, __kernel_size_t);
 #define __HAVE_ARCH_MEMCMP
 int memcmp(const void *,const void *,__kernel_size_t);
 
+/* Now the str*() stuff... */
+#define __HAVE_ARCH_STRLEN
+__kernel_size_t strlen(const char *);
+
 #define __HAVE_ARCH_STRNCMP
 int strncmp(const char *, const char *, __kernel_size_t);
 
diff --git a/arch/sparc/include/asm/string_64.h b/arch/sparc/include/asm/string_64.h
index ee9ba67321bd..d5c563058a5b 100644
--- a/arch/sparc/include/asm/string_64.h
+++ b/arch/sparc/include/asm/string_64.h
@@ -12,8 +12,4 @@
 
 #include <asm/asi.h>
 
-/* Now the str*() stuff... */
-#define __HAVE_ARCH_STRLEN
-__kernel_size_t strlen(const char *);
-
 #endif /* !(__SPARC64_STRING_H__) */
-- 
2.17.1

[PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Masahiro Yamada]

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>
Reported-by: kbuild test robot <lkp@intel.com>
---

Changes in v2: None

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

diff --git a/Makefile b/Makefile
index 6a538b79e61e..2cdbd4b3e36d 100644
--- a/Makefile
+++ b/Makefile
@@ -1034,8 +1034,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

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[kbuild test robot]

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

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on kbuild/for-next]
[also build test ERROR on v5.6-rc5 next-20200311]
[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/sparc-revive-__HAVE_ARCH_STRLEN-for-32bit-sparc/20200312-073459
base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
config: mips-cavium_octeon_defconfig (attached as .config)
compiler: mips64-linux-gcc (GCC) 9.2.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=9.2.0 make.cross ARCH=mips 

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

All errors (new ones prefixed by >>):

   mips64-linux-ld: arch/mips/fw/lib/cmdline.o: in function `fw_init_cmdline':
>> cmdline.c:(.init.text+0x0): multiple definition of `fw_init_cmdline'; arch/mips/cavium-octeon/setup.o:setup.c:(.init.text+0xad8): first defined here
   mips64-linux-ld: arch/mips/lib/delay.o: in function `__delay':
>> delay.c:(.text+0x0): multiple definition of `__delay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x80): first defined here
   mips64-linux-ld: arch/mips/lib/delay.o: in function `__udelay':
>> delay.c:(.text+0x10): multiple definition of `__udelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x8): first defined here
   mips64-linux-ld: arch/mips/lib/delay.o: in function `__ndelay':
>> delay.c:(.text+0x50): multiple definition of `__ndelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x40): first defined here
   mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memmove':
>> (.text+0x0): multiple definition of `memmove'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3a0): first defined here
   mips64-linux-ld: arch/mips/lib/memcpy.o: in function `__rmemcpy':
>> (.text+0x20): multiple definition of `__rmemcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3c0): first defined here
   mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
>> (.text+0x80): multiple definition of `memcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x0): first defined here
   mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
>> (.text+0x84): multiple definition of `__copy_user'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x4): 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: 18326 bytes --]

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Masahiro Yamada]

Hi MIPS forks,


I got the following report from 0-day bot.
Please advise me how to fix it.


I am not sure how multi-platform works in MIPS.

The cavium-octeon platform has its own implementation
of various functions.

So, vmlinux links different library routines
depending on whether CONFIG_CAVIUM_OCTEON_SOC, correct?



fw_init_cmdline():
arch/mips/cavium-octeon/setup.c
arch/mips/fw/lib/cmdline.c


__delay(), __udelay(), __ndelay():
arch/mips/cavium-octeon/csrc-octeon.c
arch/mips/lib/delay.S


memcpy(), memmove():
arch/mips/cavium-octeon/octeon-memcpy.S
arch/mips/lib/memcpy.S



FWIW, the following fixes the multiple definition errors.



diff --git a/arch/mips/fw/lib/cmdline.c b/arch/mips/fw/lib/cmdline.c
index 6ecda64ad184..6ac6e0493e1f 100644
--- a/arch/mips/fw/lib/cmdline.c
+++ b/arch/mips/fw/lib/cmdline.c
@@ -16,6 +16,7 @@ int fw_argc;
 int *_fw_argv;
 int *_fw_envp;

+#ifndef CONFIG_CAVIUM_OCTEON_SOC
 void __init fw_init_cmdline(void)
 {
        int i;
@@ -41,6 +42,7 @@ void __init fw_init_cmdline(void)
                        strlcat(arcs_cmdline, " ", COMMAND_LINE_SIZE);
        }
 }
+#endif

 char * __init fw_getcmdline(void)
 {
diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
index 479f50559c83..4cc98af4161a 100644
--- a/arch/mips/lib/Makefile
+++ b/arch/mips/lib/Makefile
@@ -3,10 +3,14 @@
 # Makefile for MIPS-specific library files..
 #

-lib-y  += bitops.o csum_partial.o delay.o memcpy.o memset.o \
+lib-y  += bitops.o csum_partial.o memset.o \
           mips-atomic.o strncpy_user.o \
           strnlen_user.o uncached.o

+ifneq ($(CONFIG_CAVIUM_OCTEON_SOC),y)
+lib-y  += delay.o memcpy.o
+endif
+
 obj-y                  += iomap_copy.o
 obj-$(CONFIG_PCI)      += iomap-pci.o
 lib-$(CONFIG_GENERIC_CSUM)     := $(filter-out csum_partial.o, $(lib-y))



On Thu, Mar 12, 2020 at 1:31 PM 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 v5.6-rc5 next-20200311]
> [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/sparc-revive-__HAVE_ARCH_STRLEN-for-32bit-sparc/20200312-073459
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> config: mips-cavium_octeon_defconfig (attached as .config)
> compiler: mips64-linux-gcc (GCC) 9.2.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=9.2.0 make.cross ARCH=mips
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    mips64-linux-ld: arch/mips/fw/lib/cmdline.o: in function `fw_init_cmdline':
> >> cmdline.c:(.init.text+0x0): multiple definition of `fw_init_cmdline'; arch/mips/cavium-octeon/setup.o:setup.c:(.init.text+0xad8): first defined here
>    mips64-linux-ld: arch/mips/lib/delay.o: in function `__delay':
> >> delay.c:(.text+0x0): multiple definition of `__delay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x80): first defined here
>    mips64-linux-ld: arch/mips/lib/delay.o: in function `__udelay':
> >> delay.c:(.text+0x10): multiple definition of `__udelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x8): first defined here
>    mips64-linux-ld: arch/mips/lib/delay.o: in function `__ndelay':
> >> delay.c:(.text+0x50): multiple definition of `__ndelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x40): first defined here
>    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memmove':
> >> (.text+0x0): multiple definition of `memmove'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3a0): first defined here
>    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `__rmemcpy':
> >> (.text+0x20): multiple definition of `__rmemcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3c0): first defined here
>    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
> >> (.text+0x80): multiple definition of `memcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x0): first defined here
>    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
> >> (.text+0x84): multiple definition of `__copy_user'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x4): first defined here
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org



--
Best Regards
Masahiro Yamada

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Thomas Bogendoerfer]

On Thu, Mar 12, 2020 at 03:12:28PM +0900, Masahiro Yamada wrote:
> I got the following report from 0-day bot.
> Please advise me how to fix it.
> 
> 
> I am not sure how multi-platform works in MIPS.
> 
> The cavium-octeon platform has its own implementation
> of various functions.
> 
> So, vmlinux links different library routines
> depending on whether CONFIG_CAVIUM_OCTEON_SOC, correct?

for cavium memcpy is directly linked in via octeon-memcpy.o, while for
every other platform it's coming from lib/lib.a(memcpy.o).

What have you changed, that this doesn't work anymore ?

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Masahiro Yamada]

Hi Thomas,

On Thu, Mar 12, 2020 at 5:40 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Thu, Mar 12, 2020 at 03:12:28PM +0900, Masahiro Yamada wrote:
> > I got the following report from 0-day bot.
> > Please advise me how to fix it.
> >
> >
> > I am not sure how multi-platform works in MIPS.
> >
> > The cavium-octeon platform has its own implementation
> > of various functions.
> >
> > So, vmlinux links different library routines
> > depending on whether CONFIG_CAVIUM_OCTEON_SOC, correct?
>
> for cavium memcpy is directly linked in via octeon-memcpy.o, while for
> every other platform it's coming from lib/lib.a(memcpy.o).
>
> What have you changed, that this doesn't work anymore ?
>
> Thomas.


I want to change all objects from lib-y
to be linked to vmlinux (exactly like obj-y )
if CONFIG_MODULES is enabled.

https://patchwork.kernel.org/patch/11432969/


EXPORT_SYMBOL in libraries
must be linked to vmlinux all the time,
even if there is no call-site in vmlinux.
I believe this is a good simplification because
EXPORT_SYMBOL is interface to loadable modules.

As it turned out, lib-y is (ab)used to avoid
multiple definition errors.

The 0-day detected a bug of 32-bit sparc:
https://patchwork.kernel.org/patch/11432969/

And, another is this one.

MIPS relies on that
arch/mips/lib/lib.a is weaker than octeon ones.

So, annotating __weak is a good solution
(thanks Nick!).

If I send a patch, is it acceptable?


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Thomas Bogendoerfer]

On Fri, Mar 20, 2020 at 12:48:20AM +0900, Masahiro Yamada wrote:
> Hi Thomas,
> 
> On Thu, Mar 12, 2020 at 5:40 PM Thomas Bogendoerfer
> <tsbogend@alpha.franken.de> wrote:
> >
> > On Thu, Mar 12, 2020 at 03:12:28PM +0900, Masahiro Yamada wrote:
> > > I got the following report from 0-day bot.
> > > Please advise me how to fix it.
> > >
> > >
> > > I am not sure how multi-platform works in MIPS.
> > >
> > > The cavium-octeon platform has its own implementation
> > > of various functions.
> > >
> > > So, vmlinux links different library routines
> > > depending on whether CONFIG_CAVIUM_OCTEON_SOC, correct?
> >
> > for cavium memcpy is directly linked in via octeon-memcpy.o, while for
> > every other platform it's coming from lib/lib.a(memcpy.o).
> >
> > What have you changed, that this doesn't work anymore ?
> >
> > Thomas.
> 
> 
> I want to change all objects from lib-y
> to be linked to vmlinux (exactly like obj-y )
> if CONFIG_MODULES is enabled.

ic

> So, annotating __weak is a good solution
> (thanks Nick!).
> 
> If I send a patch, is it acceptable?

sure.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Nick Desaulniers]

On Wed, Mar 11, 2020 at 11:13 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Hi MIPS forks,
>
>
> I got the following report from 0-day bot.
> Please advise me how to fix it.
>
>
> I am not sure how multi-platform works in MIPS.
>
> The cavium-octeon platform has its own implementation
> of various functions.
>
> So, vmlinux links different library routines
> depending on whether CONFIG_CAVIUM_OCTEON_SOC, correct?
>
>
>
> fw_init_cmdline():
> arch/mips/cavium-octeon/setup.c
> arch/mips/fw/lib/cmdline.c
>
>
> __delay(), __udelay(), __ndelay():
> arch/mips/cavium-octeon/csrc-octeon.c
> arch/mips/lib/delay.S
>
>
> memcpy(), memmove():
> arch/mips/cavium-octeon/octeon-memcpy.S
> arch/mips/lib/memcpy.S
>
>
>
> FWIW, the following fixes the multiple definition errors.
>
>
>
> diff --git a/arch/mips/fw/lib/cmdline.c b/arch/mips/fw/lib/cmdline.c
> index 6ecda64ad184..6ac6e0493e1f 100644
> --- a/arch/mips/fw/lib/cmdline.c
> +++ b/arch/mips/fw/lib/cmdline.c
> @@ -16,6 +16,7 @@ int fw_argc;
>  int *_fw_argv;
>  int *_fw_envp;
>
> +#ifndef CONFIG_CAVIUM_OCTEON_SOC
>  void __init fw_init_cmdline(void)

Alternatively, you could define this fw_init_cmdline as __weak, then
let the strong definition in arch/mips/cavium-octeon/setup.c override
it. But both should work, so:
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Eitherway, octeon has some multiple definition errors that should get fixed.

>  {
>         int i;
> @@ -41,6 +42,7 @@ void __init fw_init_cmdline(void)
>                         strlcat(arcs_cmdline, " ", COMMAND_LINE_SIZE);
>         }
>  }
> +#endif
>
>  char * __init fw_getcmdline(void)
>  {
> diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
> index 479f50559c83..4cc98af4161a 100644
> --- a/arch/mips/lib/Makefile
> +++ b/arch/mips/lib/Makefile
> @@ -3,10 +3,14 @@
>  # Makefile for MIPS-specific library files..
>  #
>
> -lib-y  += bitops.o csum_partial.o delay.o memcpy.o memset.o \
> +lib-y  += bitops.o csum_partial.o memset.o \
>            mips-atomic.o strncpy_user.o \
>            strnlen_user.o uncached.o
>
> +ifneq ($(CONFIG_CAVIUM_OCTEON_SOC),y)
> +lib-y  += delay.o memcpy.o
> +endif
> +
>  obj-y                  += iomap_copy.o
>  obj-$(CONFIG_PCI)      += iomap-pci.o
>  lib-$(CONFIG_GENERIC_CSUM)     := $(filter-out csum_partial.o, $(lib-y))
>
>
>
> On Thu, Mar 12, 2020 at 1:31 PM 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 v5.6-rc5 next-20200311]
> > [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/sparc-revive-__HAVE_ARCH_STRLEN-for-32bit-sparc/20200312-073459
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> > config: mips-cavium_octeon_defconfig (attached as .config)
> > compiler: mips64-linux-gcc (GCC) 9.2.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=9.2.0 make.cross ARCH=mips
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> >
> > All errors (new ones prefixed by >>):
> >
> >    mips64-linux-ld: arch/mips/fw/lib/cmdline.o: in function `fw_init_cmdline':
> > >> cmdline.c:(.init.text+0x0): multiple definition of `fw_init_cmdline'; arch/mips/cavium-octeon/setup.o:setup.c:(.init.text+0xad8): first defined here
> >    mips64-linux-ld: arch/mips/lib/delay.o: in function `__delay':
> > >> delay.c:(.text+0x0): multiple definition of `__delay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x80): first defined here
> >    mips64-linux-ld: arch/mips/lib/delay.o: in function `__udelay':
> > >> delay.c:(.text+0x10): multiple definition of `__udelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x8): first defined here
> >    mips64-linux-ld: arch/mips/lib/delay.o: in function `__ndelay':
> > >> delay.c:(.text+0x50): multiple definition of `__ndelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x40): first defined here
> >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memmove':
> > >> (.text+0x0): multiple definition of `memmove'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3a0): first defined here
> >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `__rmemcpy':
> > >> (.text+0x20): multiple definition of `__rmemcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3c0): first defined here
> >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
> > >> (.text+0x80): multiple definition of `memcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x0): first defined here
> >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
> > >> (.text+0x84): multiple definition of `__copy_user'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x4): first defined here
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Nick Desaulniers]

+ linux-mips this time

On Mon, Mar 16, 2020 at 4:13 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Mar 11, 2020 at 11:13 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Hi MIPS forks,
> >
> >
> > I got the following report from 0-day bot.
> > Please advise me how to fix it.
> >
> >
> > I am not sure how multi-platform works in MIPS.
> >
> > The cavium-octeon platform has its own implementation
> > of various functions.
> >
> > So, vmlinux links different library routines
> > depending on whether CONFIG_CAVIUM_OCTEON_SOC, correct?
> >
> >
> >
> > fw_init_cmdline():
> > arch/mips/cavium-octeon/setup.c
> > arch/mips/fw/lib/cmdline.c
> >
> >
> > __delay(), __udelay(), __ndelay():
> > arch/mips/cavium-octeon/csrc-octeon.c
> > arch/mips/lib/delay.S
> >
> >
> > memcpy(), memmove():
> > arch/mips/cavium-octeon/octeon-memcpy.S
> > arch/mips/lib/memcpy.S
> >
> >
> >
> > FWIW, the following fixes the multiple definition errors.
> >
> >
> >
> > diff --git a/arch/mips/fw/lib/cmdline.c b/arch/mips/fw/lib/cmdline.c
> > index 6ecda64ad184..6ac6e0493e1f 100644
> > --- a/arch/mips/fw/lib/cmdline.c
> > +++ b/arch/mips/fw/lib/cmdline.c
> > @@ -16,6 +16,7 @@ int fw_argc;
> >  int *_fw_argv;
> >  int *_fw_envp;
> >
> > +#ifndef CONFIG_CAVIUM_OCTEON_SOC
> >  void __init fw_init_cmdline(void)
>
> Alternatively, you could define this fw_init_cmdline as __weak, then
> let the strong definition in arch/mips/cavium-octeon/setup.c override
> it. But both should work, so:
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Eitherway, octeon has some multiple definition errors that should get fixed.
>
> >  {
> >         int i;
> > @@ -41,6 +42,7 @@ void __init fw_init_cmdline(void)
> >                         strlcat(arcs_cmdline, " ", COMMAND_LINE_SIZE);
> >         }
> >  }
> > +#endif
> >
> >  char * __init fw_getcmdline(void)
> >  {
> > diff --git a/arch/mips/lib/Makefile b/arch/mips/lib/Makefile
> > index 479f50559c83..4cc98af4161a 100644
> > --- a/arch/mips/lib/Makefile
> > +++ b/arch/mips/lib/Makefile
> > @@ -3,10 +3,14 @@
> >  # Makefile for MIPS-specific library files..
> >  #
> >
> > -lib-y  += bitops.o csum_partial.o delay.o memcpy.o memset.o \
> > +lib-y  += bitops.o csum_partial.o memset.o \
> >            mips-atomic.o strncpy_user.o \
> >            strnlen_user.o uncached.o
> >
> > +ifneq ($(CONFIG_CAVIUM_OCTEON_SOC),y)
> > +lib-y  += delay.o memcpy.o
> > +endif
> > +
> >  obj-y                  += iomap_copy.o
> >  obj-$(CONFIG_PCI)      += iomap-pci.o
> >  lib-$(CONFIG_GENERIC_CSUM)     := $(filter-out csum_partial.o, $(lib-y))
> >
> >
> >
> > On Thu, Mar 12, 2020 at 1:31 PM 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 v5.6-rc5 next-20200311]
> > > [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/sparc-revive-__HAVE_ARCH_STRLEN-for-32bit-sparc/20200312-073459
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> > > config: mips-cavium_octeon_defconfig (attached as .config)
> > > compiler: mips64-linux-gcc (GCC) 9.2.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=9.2.0 make.cross ARCH=mips
> > >
> > > If you fix the issue, kindly add following tag
> > > Reported-by: kbuild test robot <lkp@intel.com>
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > >    mips64-linux-ld: arch/mips/fw/lib/cmdline.o: in function `fw_init_cmdline':
> > > >> cmdline.c:(.init.text+0x0): multiple definition of `fw_init_cmdline'; arch/mips/cavium-octeon/setup.o:setup.c:(.init.text+0xad8): first defined here
> > >    mips64-linux-ld: arch/mips/lib/delay.o: in function `__delay':
> > > >> delay.c:(.text+0x0): multiple definition of `__delay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x80): first defined here
> > >    mips64-linux-ld: arch/mips/lib/delay.o: in function `__udelay':
> > > >> delay.c:(.text+0x10): multiple definition of `__udelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x8): first defined here
> > >    mips64-linux-ld: arch/mips/lib/delay.o: in function `__ndelay':
> > > >> delay.c:(.text+0x50): multiple definition of `__ndelay'; arch/mips/cavium-octeon/csrc-octeon.o:csrc-octeon.c:(.text+0x40): first defined here
> > >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memmove':
> > > >> (.text+0x0): multiple definition of `memmove'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3a0): first defined here
> > >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `__rmemcpy':
> > > >> (.text+0x20): multiple definition of `__rmemcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x3c0): first defined here
> > >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
> > > >> (.text+0x80): multiple definition of `memcpy'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x0): first defined here
> > >    mips64-linux-ld: arch/mips/lib/memcpy.o: in function `memcpy':
> > > >> (.text+0x84): multiple definition of `__copy_user'; arch/mips/cavium-octeon/octeon-memcpy.o:(.text+0x4): first defined here
> > >
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 2/2] kbuild: link lib-y objects to vmlinux forcibly when CONFIG_MODULES=y

[Masahiro Yamada]

On Thu, Mar 12, 2020 at 7:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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>
> Reported-by: kbuild test robot <lkp@intel.com>
> ---

Applied to linux-kbuild.

I will rebase my branch during this MW,
so the commit ID will be unstable.
Please do not record it until it lands in Linus' tree.





-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 1/2] sparc: revive __HAVE_ARCH_STRLEN for 32bit sparc

[Nick Desaulniers]

On Wed, Mar 11, 2020 at 3:37 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Prior to commit 70a6fcf3283a ("[sparc] unify 32bit and 64bit string.h"),
> __HAVE_ARCH_STRLEN was defined in both of string_32.h and string_64.h
>
> It did not unify __HAVE_ARCH_STRLEN, but deleted it from string_32.h
>
> This issue was reported by the kbuild test robot in the trial of
> forcible linking of $(lib-y) to vmlinux.
>
> Fixes: 70a6fcf3283a ("[sparc] unify 32bit and 64bit string.h")

Nice find with the above commit.  Thanks for the patch.  Without it,
looks like 32b sparc is using strlen from lib/string.c.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> Reported-by: kbuild test robot <lkp@intel.com>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
>   - Insert a new patch to avoid sparc32 build error
>
>  arch/sparc/include/asm/string.h    | 4 ++++
>  arch/sparc/include/asm/string_64.h | 4 ----
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/sparc/include/asm/string.h b/arch/sparc/include/asm/string.h
> index 3d9cd082716b..001a17baf2d5 100644
> --- a/arch/sparc/include/asm/string.h
> +++ b/arch/sparc/include/asm/string.h
> @@ -37,6 +37,10 @@ void *memmove(void *, const void *, __kernel_size_t);
>  #define __HAVE_ARCH_MEMCMP
>  int memcmp(const void *,const void *,__kernel_size_t);
>
> +/* Now the str*() stuff... */
> +#define __HAVE_ARCH_STRLEN
> +__kernel_size_t strlen(const char *);
> +
>  #define __HAVE_ARCH_STRNCMP
>  int strncmp(const char *, const char *, __kernel_size_t);
>
> diff --git a/arch/sparc/include/asm/string_64.h b/arch/sparc/include/asm/string_64.h
> index ee9ba67321bd..d5c563058a5b 100644
> --- a/arch/sparc/include/asm/string_64.h
> +++ b/arch/sparc/include/asm/string_64.h
> @@ -12,8 +12,4 @@
>
>  #include <asm/asi.h>
>
> -/* Now the str*() stuff... */
> -#define __HAVE_ARCH_STRLEN
> -__kernel_size_t strlen(const char *);
> -
>  #endif /* !(__SPARC64_STRING_H__) */
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 1/2] sparc: revive __HAVE_ARCH_STRLEN for 32bit sparc

[David Miller]

From: Masahiro Yamada <masahiroy@kernel.org>
Date: Thu, 12 Mar 2020 07:37:24 +0900

> Prior to commit 70a6fcf3283a ("[sparc] unify 32bit and 64bit string.h"),
> __HAVE_ARCH_STRLEN was defined in both of string_32.h and string_64.h
> 
> It did not unify __HAVE_ARCH_STRLEN, but deleted it from string_32.h
> 
> This issue was reported by the kbuild test robot in the trial of
> forcible linking of $(lib-y) to vmlinux.
> 
> Fixes: 70a6fcf3283a ("[sparc] unify 32bit and 64bit string.h")
> Reported-by: kbuild test robot <lkp@intel.com>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

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