Re: [PATCH 09/23] kbuild: do not allocate .modinfo in vmlinux

[Nathan Chancellor]

> Commit 3e86e4d74c04 ("kbuild: keep .modinfo section in vmlinux.unstripped")
> placed MODULE_INFO() strings in the .modinfo section in vmlinux.unstripped,
> however it left the section allocatable, so it gets assigned an address and
> on arm64, arm and riscv, the section is tagged as PT_LOAD.
> 
> This is useless as the data is ultimately stripped anyway.
> 
> Doing this results in a lot of unnecessary work - each pass copies the
> whole file, 450 MIB with relocations for an x86 allmodconfig build and 250
> MiB for an arm64 allmodconfig build.
> 
> This adds ~0.7s on the serial tail of every build for arm64 and ~0.2s for
> x86 (the tail is single-threaded work done after parallel work has
> finished).
> 
> Nothing requires .modinfo to exist at an address, so fix this by using
> --dump-section which prevents the allocation.
> 
> Image, bzImage, System.map and modules.builtin.modinfo are unchanged and
> the stripped vmlinux differs only in its program headers.
> 
> The objcopy passes are on the serial tail of every build that links
> vmlinux, no-op builds are unchanged.
> 
> Whole build, 128-thread Threadripper 9980X, best of N runs:
> 
>                                          before   after     delta
>                                          -------------------------------
>   x86 defconfig, touch mm/vma.c, gcc        8.9s     8.8s    -0.16s (-2%)
>   x86 defconfig, touch mm/vma.c, clang      8.1s     7.9s    -0.20s (-2%)
>   x86 defconfig, clean, gcc                27.6s    27.5s    -0.13s (0%)
>   x86 defconfig, clean, clang              27.2s    27.0s    -0.18s (-1%)
>   x86 allmodconfig, touch mm/vma.c, gcc    42.0s    41.6s    -0.40s (-1%)
> 
> Assisted-by: LLM
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index b2988aa12f66..7be9e0321867 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -855,7 +855,7 @@
>  		KLP_SYMID
>  
>  #define MODINFO								\
> -		.modinfo : { *(.modinfo) . = ALIGN(8); }
> +		.modinfo (INFO) : { *(.modinfo) . = ALIGN(8); }
>  
>  #ifdef CONFIG_GENERIC_BUG
>  #define BUG_TABLE							\
> diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
> index 4b54aaeca65d..1fc7a0ca8eaf 100644
> --- a/scripts/Makefile.vmlinux
> +++ b/scripts/Makefile.vmlinux
> @@ -90,11 +90,9 @@ remove-section-$(CONFIG_ARCH_VMLINUX_NEEDS_RELOCS) += '.rel.*'
>  
>  remove-symbols := -w --strip-unneeded-symbol='__mod_device_table__*'
>  
> -# To avoid warnings: "empty loadable segment detected at ..." from GNU objcopy,
> -# it is necessary to remove the PT_LOAD flag from the segment.
> +# none of the removed sections is allocated, so no segment is left empty

I think we could just drop this comment, I don't think it is
particularly helpful

>  quiet_cmd_strip_relocs = OBJCOPY $@
> -      cmd_strip_relocs = $(OBJCOPY) $(patsubst %,--set-section-flags %=noload,$(remove-section-y)) $< $@; \
> -                         $(OBJCOPY) $(addprefix --remove-section=,$(remove-section-y)) $(remove-symbols) $@
> +      cmd_strip_relocs = $(OBJCOPY) $(addprefix --remove-section=,$(remove-section-y)) $(remove-symbols) $< $@
>  
>  targets += vmlinux
>  vmlinux: vmlinux.unstripped FORCE
> @@ -103,21 +101,11 @@ vmlinux: vmlinux.unstripped FORCE
>  # modules.builtin.modinfo
>  # ---------------------------------------------------------------------------
>  
> -# .modinfo in vmlinux.unstripped is aligned to 8 bytes for compatibility with
> -# tools that expect vmlinux to have sufficiently aligned sections but the
> -# additional bytes used for padding .modinfo to satisfy this requirement break
> -# certain versions of kmod with
> -#
> -#   depmod: ERROR: kmod_builtin_iter_next: unexpected string without modname prefix
> -#
> -# Strip the trailing padding bytes after extracting .modinfo to comply with
> -# what kmod expects to parse.
> +# use --dump-section to include non-allocated sections

Same thing here, I am not sure anyone will question '--dump-section'
here. Otherwise, this seems reasonable. I was curious about the $@.tmp
until I read the objcopy manual but that probably is not worth a
comment.

I can probably take this one faster than the rest if you want to put it
early in the stack since it is pretty low hanging fruit and earlier it
gets exposure to -next, the better.

-- 
Cheers,
Nathan