I finally got a chance to go through the comments and work on a follow-up to this series, but it probably makes sense to get this sorted ahead of the follow-up (if possible). On Wed, May 24, 2023 at 2:02 AM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote: > > On Thu, Apr 06, 2023 at 02:00:27PM -0500, Allen Webb wrote: > > Generate modules.builtin.alias using modpost and install it with the > > modules. > > Why? This is probably one of the more important commits and the > commit log is pretty slim. > > > Signed-off-by: Allen Webb <allenwebb@xxxxxxxxxx> > > --- > > .gitignore | 1 + > > Makefile | 1 + > > scripts/Makefile.modpost | 15 +++++++++++++++ > > 3 files changed, 17 insertions(+) > > > > diff --git a/.gitignore b/.gitignore > > index 13a7f08a3d73..ddaa622bddac 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -71,6 +71,7 @@ modules.order > > /System.map > > /Module.markers > > /modules.builtin > > +/modules.builtin.alias > > /modules.builtin.modinfo > > /modules.nsdeps > > > > diff --git a/Makefile b/Makefile > > index a2c310df2145..43dcc1ea5fcf 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1578,6 +1578,7 @@ __modinst_pre: > > fi > > @sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order > > @cp -f modules.builtin $(MODLIB)/ > > + @cp -f modules.builtin.alias $(MODLIB)/ > > @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/ > > > > endif # CONFIG_MODULES > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > > index 0980c58d8afc..e3ecc17a7a19 100644 > > --- a/scripts/Makefile.modpost > > +++ b/scripts/Makefile.modpost > > @@ -15,6 +15,7 @@ > > # 2) modpost is then used to > > # 3) create one <module>.mod.c file per module > > # 4) create one Module.symvers file with CRC for all exported symbols > > +# 5) create modules.builtin.alias the aliases for built-in modules > > Does everyone want that file? Not everyone needs it so we could exclude it, but the cost of adding it isn't that high. I am fine with putting it behind a config, though we would need to decide whether to have it default on/off. > > > # Step 3 is used to place certain information in the module's ELF > > # section, including information such as: > > @@ -63,6 +64,20 @@ modpost-args += -T $(MODORDER) > > modpost-deps += $(MODORDER) > > endif > > > > +ifneq ($(wildcard vmlinux.o),) > > +output-builtin.alias := modules.builtin.alias > > +modpost-args += -b .modules.builtin.alias.in > > +.modules.builtin.alias.in: $(output-symdump) > > + @# Building $(output-symdump) generates .modules.builtin.alias.in as a > > + @# side effect. > > + @[ -e $@ ] || $(MODPOST) -b .modules.builtin.alias.in vmlinux.o > > Does using -b create a delay in builds ? What is the effect on build > time on a typical 4-core or 8-core build? Does everyone want it? Here is some data I collected related to build time and memory usage impact: Without builtin.alias: TIME="real %e\nuser %U\nsys %S\nres-max %M" time scripts/mod/modpost -E -o Module.symvers -T modules.order ERROR: modpost: "__x86_return_thunk" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "kernel_fpu_end" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "hchacha_block_generic" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "boot_cpu_data" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "static_key_enable" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "cpu_has_xfeatures" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "crypto_register_skciphers" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "crypto_unregister_skciphers" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "__stack_chk_fail" [arch/x86/crypto/chacha-x86_64.ko] undefined! ERROR: modpost: "memset" [arch/x86/crypto/chacha-x86_64.ko] undefined! WARNING: modpost: suppressed 17432 unresolved symbol warnings because there were too many) Command exited with non-zero status 1 real 0.44 user 0.43 sys 0.01 res-max 4896 With builtin.alias: TIME="real %e\nuser %U\nsys %S\nres-max %M" time scripts/mod/modpost -E -o Module.symvers -T modules.order -b .modules.builtin.alias.in vmlinux.o real 1.43 user 1.38 sys 0.05 res-max 51920 Notice that modpost only uses a single core, so multicore isn't really as much of a factor here. While it more than triples the time required for the modpost operation the difference is only about one second of CPU time. The memory usage is much larger when generating modules.builtin.alias because of the size of vmlinux.o. My biggest performance related concern is actually the size difference of vmlinux caused by the modules.h changes, but it looks like that is negligible (24KiB): Without builtin.alias: du vmlinux.o 663048 vmlinux.o With builtin.alias: du vmlinux.o 663072 vmlinux.o > > Should we add a new option which lets people decide if they want this > at build time or not? I don't feel strongly that there should or should not be a config for this. On the side for a config the extra second of CPU time and space taken up by the modules.builtin.alias file would add up across all the builds and machines, so removing it where it isn't used would help mitigate that. On the flip side if it isn't used widely enough, it is more likely that breakages are missed until someone who actually uses it notices. Please let me know if you feel strongly either way given the data. Thanks, Allen > > Luis > > > + > > +$(output-builtin.alias): .modules.builtin.alias.in > > + sort -o $@ $^ > > + > > +__modpost: $(output-builtin.alias) > > +endif > > + > > ifeq ($(KBUILD_EXTMOD),) > > > > # Generate the list of in-tree objects in vmlinux > > -- > > 2.39.2 > >
