diff mbox series

[09/13] kbuild: do not create built-in.a.symversions or lib.a.symversions

Message ID 20210819005744.644908-10-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series kbuild: refactoring after Clang LTO | expand

Commit Message

Masahiro Yamada Aug. 19, 2021, 12:57 a.m. UTC 
Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
merging them in the unit of built-in.a or lib.a.

This is a preparation for further code cleanups.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build  | 10 ++--------
 scripts/link-vmlinux.sh | 22 ++++++++++++++++++----
 2 files changed, 20 insertions(+), 12 deletions(-)

Comments

Kees Cook Aug. 19, 2021, 6:41 a.m. UTC | #1 
On Thu, Aug 19, 2021 at 09:57:40AM +0900, Masahiro Yamada wrote:
> Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
> merging them in the unit of built-in.a or lib.a.
> 
> This is a preparation for further code cleanups.

Looks good, though I wonder about this becoming serialized during the
link phase rather than doing the work per-target. I mean, it always had
to collect them all during the link phase (with "cat"), but before it
wasn't running $(AR) serially to do it.

I'll ponder how this might be made a little more parallel. But for now:

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  scripts/Makefile.build  | 10 ++--------
>  scripts/link-vmlinux.sh | 22 ++++++++++++++++++----
>  2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 585dae34746a..37d6f6da34d6 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -430,11 +430,8 @@ endif
>  quiet_cmd_ar_builtin = AR      $@
>        cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
>  
> -quiet_cmd_ar_and_symver = AR      $@
> -      cmd_ar_and_symver = $(cmd_update_lto_symversions); $(cmd_ar_builtin)
> -
>  $(obj)/built-in.a: $(real-obj-y) FORCE
> -	$(call if_changed,ar_and_symver)
> +	$(call if_changed,ar_builtin)
>  
>  #
>  # Rule to create modules.order file
> @@ -454,11 +451,8 @@ $(obj)/modules.order: $(obj-m) FORCE
>  #
>  # Rule to compile a set of .o files into one .a file (with symbol table)
>  #
> -quiet_cmd_ar_lib = AR      $@
> -      cmd_ar_lib = $(cmd_update_lto_symversions); $(cmd_ar)
> -
>  $(obj)/lib.a: $(lib-y) FORCE
> -	$(call if_changed,ar_lib)
> +	$(call if_changed,ar)
>  
>  # NOTE:
>  # Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index d74cee5c4326..17976609c2d8 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -52,6 +52,13 @@ gen_initcalls()
>  		> .tmp_initcalls.lds
>  }
>  
> +append_symversion()
> +{
> +	if [ -f ${1}.symversions ]; then
> +		cat ${1}.symversions >> .tmp_symversions.lds
> +	fi
> +}
> +
>  # If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
>  # .tmp_symversions.lds
>  gen_symversions()
> @@ -59,10 +66,17 @@ gen_symversions()
>  	info GEN .tmp_symversions.lds
>  	rm -f .tmp_symversions.lds
>  
> -	for o in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
> -		if [ -f ${o}.symversions ]; then
> -			cat ${o}.symversions >> .tmp_symversions.lds
> -		fi
> +	for a in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
> +		case $a in
> +		*.a)
> +			for o in $(${AR} t ${a}); do
> +				append_symversion ${o}
> +			done
> +			;;
> +		*)
> +			append_symversion ${a}
> +			;;
> +		esac
>  	done
>  }
>  
> -- 
> 2.30.2
>
Masahiro Yamada Aug. 28, 2021, 11:57 a.m. UTC | #2 
On Thu, Aug 19, 2021 at 3:41 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Aug 19, 2021 at 09:57:40AM +0900, Masahiro Yamada wrote:
> > Merge all *.o.symversions in scripts/link-vmlinux.sh instead of
> > merging them in the unit of built-in.a or lib.a.
> >
> > This is a preparation for further code cleanups.
>
> Looks good, though I wonder about this becoming serialized during the
> link phase rather than doing the work per-target. I mean, it always had
> to collect them all during the link phase (with "cat"), but before it
> wasn't running $(AR) serially to do it.
>
> I'll ponder how this might be made a little more parallel. But for now:
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> -Kees
>


I measured the cost of merging all the *.symversions.

For a typical use-case
(x86_64 defconfig + CONFIG_LTO_CLANG_THIN + CONFIG_MODVERSIONS),
my shell script took about 0.40 msec
for merging all the individual *.symversions files.

Most of the cost of 0.40 msec came from the 'cat' command.
The 'cat' command is kind of slow when you concatenate
a large number of files.

I implemented the equivalent functionality with a perl script,
which worked in only 0.04 msec.

I think 0.04 msec should be acceptable cost because
this commit eliminates all the intermediate built-in.a.symversions
and lib.a.symversions, saving disk space.

I also tried allyesconfig + CONFIG_LTO_CLANG_THIN + CONFIG_MODVERSIONS
(the heaviest load), but the result is similar.

This is because most of EXPORT_SYMBOL's come from the core part of
the kernel, and enabling drivers as built-in does not give much impact, I think.

So, I will plan to submit v2 with perl implementation.


The detailed test code is as follows:








masahiro@oscar:~/workspace/linux-kbuild$ cat scripts/merge-symvers.sh
#!/bin/sh

append_symversion()
{
        if [ -f ${1}.symversions ]; then
                cat ${1}.symversions >> .tmp_symversions.lds
        fi
}

# If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
# .tmp_symversions.lds
gen_symversions()
{
        rm -f .tmp_symversions.lds

        for a in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
                case $a in
                *.a)
                        for o in $(${AR} t ${a}); do
                                append_symversion ${o}
                        done
                        ;;
                *)
                        append_symversion ${a}
                        ;;
                esac
        done
}

gen_symversions



masahiro@oscar:~/workspace/linux-kbuild$ cat scripts/merge-symvers.pl
#!/usr/bin/env perl
# SPDX-License-Identifier: GPL-2.0-only

use autodie;
use strict;
use warnings;
use Getopt::Long 'GetOptions';

my $ar;
my $output;

GetOptions(
        'a|ar=s' => \$ar,
        'o|output=s'  => \$output,
);

# Collect all objects
my @objects;

foreach (@ARGV) {
        if (/\.o$/) {
                # Some objects (head-y) are linked to vmlinux directly.
                push(@objects, $_);
        } elsif (/\.a$/) {
                # Most of built-in objects are contained in built-in.a or lib.a.
                # Use 'ar -t' to get the list of the contained objects.
                $_ = `$ar -t $_`;
                push(@objects, split(/\n/));
        } else {
                die "$_: unknown file type\n";
        }
}

open(my $out_fh, '>', "$output");

foreach (@objects) {
        # The symbol CRCs for foo/bar/baz.o is output to
foo/bar/baz.o.symversions
        s/(.*)/$1.symversions/;

        if (! -e $_) {
                # .symversions does not exist if the object does not contain
                # EXPORT_SYMBOL at all. Skip it.
                next;
        }

        open(my $in_fh, '<', "$_");
        # Concatenate all the existing *.symversions files.
        print $out_fh do { local $/; <$in_fh> };
        close $in_fh;
}

close $out_fh;



masahiro@oscar:~/workspace/linux-kbuild$ git diff
diff --git a/Makefile b/Makefile
index 3ef3685b7e4a..5b8fe617769a 100644
--- a/Makefile
+++ b/Makefile
@@ -1175,6 +1175,14 @@ vmlinux: scripts/link-vmlinux.sh
autoksyms_recursive $(vmlinux-deps) FORCE

 targets := vmlinux

+PHONY += merge-symvers-by-shell merge-symvers-by-perl
+
+merge-symvers-by-shell:
+       time sh scripts/merge-symvers.sh
+
+merge-symvers-by-perl:
+       time perl scripts/merge-symvers.pl -a $(AR) -o
.tmp_symversions.lds $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)
+
 # The actual objects are generated when descending,
 # make sure no implicit rule kicks in
 $(sort $(vmlinux-deps) $(subdir-modorder)): descend ;




masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1 defconfig
*** Default configuration is based on 'x86_64_defconfig'
#
# configuration written to .config
#
masahiro@oscar:~/workspace/linux-kbuild$ ./scripts/config  -d
LTO_NONE -e LTO_CLANG_THIN -e MODVERSIONS
masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1  -s -j24
arch/x86/entry/vdso/Makefile:135: FORCE prerequisite is missing
arch/x86/entry/vdso/Makefile:135: FORCE prerequisite is missing


masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-shell
time sh scripts/merge-symvers.sh
0.40user 0.08system 0:00.47elapsed 101%CPU (0avgtext+0avgdata 7156maxresident)k
0inputs+896outputs (0major+90678minor)pagefaults 0swaps


masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-perl
time perl scripts/merge-symvers.pl -a llvm-ar -o .tmp_symversions.lds
arch/x86/kernel/head_64.o arch/x86/kernel/head64.o
arch/x86/kernel/ebda.o arch/x86/kernel/platform-quirks.o
init/built-in.a usr/built-in.a arch/x86/built-in.a kernel/built-in.a
certs/built-in.a mm/built-in.a fs/built-in.a ipc/built-in.a
security/built-in.a crypto/built-in.a block/built-in.a lib/built-in.a
arch/x86/lib/built-in.a  lib/lib.a  arch/x86/lib/lib.a
drivers/built-in.a sound/built-in.a net/built-in.a virt/built-in.a
arch/x86/pci/built-in.a arch/x86/power/built-in.a
0.04user 0.02system 0:00.06elapsed 101%CPU (0avgtext+0avgdata 10100maxresident)k
0inputs+896outputs (0major+8590minor)pagefaults 0swaps


masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   allyesconfig
#
# configuration written to .config
#
masahiro@oscar:~/workspace/linux-kbuild$ ./scripts/config  -d
LTO_NONE -e LTO_CLANG_THIN
masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1  -s -j24
  [ snip ]

masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-shell
time sh scripts/merge-symvers.sh
0.41user 0.09system 0:00.50elapsed 101%CPU (0avgtext+0avgdata 7172maxresident)k
0inputs+896outputs (0major+91425minor)pagefaults 0swaps

masahiro@oscar:~/workspace/linux-kbuild$ make LLVM=1   merge-symvers-by-perl
time perl scripts/merge-symvers.pl -a llvm-ar -o .tmp_symversions.lds
arch/x86/kernel/head_64.o arch/x86/kernel/head64.o
arch/x86/kernel/ebda.o arch/x86/kernel/platform-quirks.o
init/built-in.a usr/built-in.a arch/x86/built-in.a kernel/built-in.a
certs/built-in.a mm/built-in.a fs/built-in.a ipc/built-in.a
security/built-in.a crypto/built-in.a block/built-in.a lib/built-in.a
arch/x86/lib/built-in.a  lib/lib.a  arch/x86/lib/lib.a
drivers/built-in.a sound/built-in.a samples/built-in.a net/built-in.a
virt/built-in.a arch/x86/pci/built-in.a arch/x86/power/built-in.a
arch/x86/video/built-in.a
0.08user 0.02system 0:00.11elapsed 100%CPU (0avgtext+0avgdata 15984maxresident)k
0inputs+896outputs (0major+11506minor)pagefaults 0swaps
diff mbox series

Patch

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 585dae34746a..37d6f6da34d6 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -430,11 +430,8 @@  endif
 quiet_cmd_ar_builtin = AR      $@
       cmd_ar_builtin = rm -f $@; $(AR) cDPrST $@ $(real-prereqs)
 
-quiet_cmd_ar_and_symver = AR      $@
-      cmd_ar_and_symver = $(cmd_update_lto_symversions); $(cmd_ar_builtin)
-
 $(obj)/built-in.a: $(real-obj-y) FORCE
-	$(call if_changed,ar_and_symver)
+	$(call if_changed,ar_builtin)
 
 #
 # Rule to create modules.order file
@@ -454,11 +451,8 @@  $(obj)/modules.order: $(obj-m) FORCE
 #
 # Rule to compile a set of .o files into one .a file (with symbol table)
 #
-quiet_cmd_ar_lib = AR      $@
-      cmd_ar_lib = $(cmd_update_lto_symversions); $(cmd_ar)
-
 $(obj)/lib.a: $(lib-y) FORCE
-	$(call if_changed,ar_lib)
+	$(call if_changed,ar)
 
 # NOTE:
 # Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d74cee5c4326..17976609c2d8 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -52,6 +52,13 @@  gen_initcalls()
 		> .tmp_initcalls.lds
 }
 
+append_symversion()
+{
+	if [ -f ${1}.symversions ]; then
+		cat ${1}.symversions >> .tmp_symversions.lds
+	fi
+}
+
 # If CONFIG_LTO_CLANG is selected, collect generated symbol versions into
 # .tmp_symversions.lds
 gen_symversions()
@@ -59,10 +66,17 @@  gen_symversions()
 	info GEN .tmp_symversions.lds
 	rm -f .tmp_symversions.lds
 
-	for o in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
-		if [ -f ${o}.symversions ]; then
-			cat ${o}.symversions >> .tmp_symversions.lds
-		fi
+	for a in ${KBUILD_VMLINUX_OBJS} ${KBUILD_VMLINUX_LIBS}; do
+		case $a in
+		*.a)
+			for o in $(${AR} t ${a}); do
+				append_symversion ${o}
+			done
+			;;
+		*)
+			append_symversion ${a}
+			;;
+		esac
 	done
 }