Re: [PATCH v2 13/21] modpost: emit module descriptors as assembly

[Lorenzo Stoakes (ARM)]

(note there's a v3 see [0])

[0]: https://lore.kernel.org/linux-kbuild/20260917-build-speedup-v3-0-9ecf4163ff36@xxxxxxxxxx/

On Mon, Sep 21, 2026 at 01:25:09PM +0200, Petr Pavlu wrote:
> On 9/14/26 11:22 AM, Lorenzo Stoakes (ARM) wrote:
> > modpost generates a descriptor for every module in the form of a
> > <module>.mod.c file with .modinfo strings, the __this_module descriptor,
> > exported symbol tables and (with CONFIG_MODVERSIONS set), the CRC of
> > imported symbols.
> >
> > These files are compiled like any other kernel C file with all of the
> > -include preamble, as well as including linux/module.h, header dependencies
> > generated by fixdep of a few hundred headers, an objtool run and if LTO is
> > being performed, a link is performed to generate native code.
>
> As a side note, I'm currently working on cleaning up what linux/module.h
> includes. This was also originally motivated by an effort to reduce the
> compilation time of .mod.c files, though cleaning up that header makes
> sense in general, since it is the most explicitly included file in the
> kernel.

Nice :) and yeah it seems like that would be useful more generally.

>
> >
> > On an x86-64 allmodconfig build 11,189 *.mod.c files are built, each
> > taking ~0.24s of CPU time to compile, and module finalisation as a whole
> > 6,300 CPU seconds, or 64 seconds of wall time when run over 128 threads.
> >
> > It also generates ~1.3 GiB of *.mod.o.cmd files that every subsequent build
> > has to read back.
> >
> > Avoid all this by emitting the descriptors as assembly instead.
> >
> > The layout required (size and alignment of struct module, struct
> > modversion_info, the module's name offsets, init, and exit fields and
> > whether the architecture uses PREL32 ksymtab references) can all be derived
> > from scripts/mod/module-offsets.h.
> >
> > The fields of __this_module are emitted in offset order rather than
> > declaration order, as CONFIG_RANDSTRUCT shuffles struct module so the name,
> > init and exit fields can land anywhere.
> >
> > An assembly file avoids all of the issues previously mentioned so this
> > conversion results in a very significant performance win on kernel build.
> >
> > As a consequence of this change, since module-offsets.c includes
> > linux/module.h, scripts/mod is now built after the generated headers in
> > prepare0, rather than before.
> >
> > Also update .gitignore and make clean to handle .mod.S files, but keep
> > .mod.c files there to ensure that users do not end up with untracked
> > changes/dirty trees after the change takes effect.
> >
> > The sections were confirmed to be byte-for-byte identical to the C version
> > produced - each of .modinfo, .gnu.linkonce.this_module, __ksymtab*,
> > __ksymtab_strings, __kcrctab*, __kflagstab*, __versions,
> > __version_ext_crcs, __version_ext_names and their relocations - for all
> > 8,135 modules of a clang allmodconfig build with CONFIG_COMPILE_TEST off
> > and CONFIG_MODVERSIONS, CONFIG_EXTENDED_MODVERSIONS and
> > CONFIG_MODULE_SRCVERSION_ALL on, and for a sample built with gcc.
>
> Lowering the data to assembly loses debug information for __this_module,
> specifically that it has type `struct module`. It might be worth

No, it keeps it - every module includes linux/module.h, and the extern
declaration of __this_module carries the type, so all of the stuff gdb
needs (e.g. ptype, p & field access), DWARF etc. is all there in the same
way as with .mod.c.

Have confirmed locally.

> restoring that information in same way. A small hack would be to add
> something like the following to scripts/module-common.c:
>
> __weak __section(".discard.this_module") struct module __this_module;

That isn't necessary it turns out given the above.

>
> > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> > index 01a37ec872b9..75e9effdf02c 100644
> > --- a/scripts/Makefile.modfinal
> > +++ b/scripts/Makefile.modfinal
> > @@ -20,10 +20,14 @@ __modfinal: $(modules:%.o=%.ko)
> >  modname = $(notdir $(@:.mod.o=))
> >  part-of-module = y
> >  GCOV_PROFILE := n
> > -ccflags-remove-y := $(CC_FLAGS_CFI)
> >
> > -%.mod.o: %.mod.c FORCE
> > -	$(call if_changed_rule,cc_o_c)
> > +# modpost lays the <module>.mod.S out completely (write_mod_S_file()), so it
> > +# needs only the assembler and no dependency tracking.
> > +quiet_cmd_as_mod_o = AS [M]  $@
> > +      cmd_as_mod_o = $(CC) $(_a_flags) $(modkern_aflags) -c -o $@ $<
> > +
> > +%.mod.o: %.mod.S FORCE
> > +	$(call if_changed,as_mod_o)
> >
> >  .module-common.o: $(srctree)/scripts/module-common.c FORCE
> >  	$(call if_changed_rule,cc_o_c)
>
> If the <module>.mod.S files are fully self-contained and do not even use
> something like KBUILD_MODNAME, would it be possible to name them
> <module>.mod.s and skip the C preprocessor as well?

Ack will do for v4!

>
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 0fd43c8a89ea..b4550b545330 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -2191,30 +2042,483 @@ static void write_vmlinux_export_c_file(struct module *mod)
> >  	free(buf.p);
> >  }
> >
> > -/* do sanity checks, and generate *.mod.c file */
> > -static void write_mod_c_file(struct module *mod)
> > +#if MOD_SIZEOF_LONG == 8
> > +#define MOD_PTR_DIRECTIVE	".quad"
> > +#else
> > +#define MOD_PTR_DIRECTIVE	".long"
> > +#endif
> > +
> > +/* See KSYM_FUNC() in include/linux/export-internal.h. */
> > +#if MOD_FUNC_PLABEL
> > +#define MOD_FUNC_PREFIX		"P%"
> > +#else
> > +#define MOD_FUNC_PREFIX		""
> > +#endif
> > +
> > +/* See __KSYM_ALIGN in include/linux/export-internal.h. */
> > +#if MOD_PREL32_RELOCATIONS || MOD_SIZEOF_LONG == 4
> > +#define KSYM_ALIGN		4
> > +#else
> > +#define KSYM_ALIGN		8
> > +#endif
> > +
> > +/* Append the body of an assembler string literal, escaped as needed. */
> > +static void buf_escaped(struct buffer *buf, const char *str)
> > +{
> > +	unsigned char chr;
> > +
> > +	while ((chr = *str++)) {
> > +		if (chr == '"' || chr == '\\')
> > +			buf_printf(buf, "\\%c", chr);
> > +		else if (isprint(chr))
> > +			buf_printf(buf, "%c", chr);
> > +		else
> > +			buf_printf(buf, "\\%03o", chr);
> > +	}
> > +}
> > +
> > +static void buf_asciz(struct buffer *buf, const char *str)
> >  {
> > -	struct buffer buf = { };
> > -	struct module_alias *alias, *next;
> > -	char fname[PATH_MAX];
> > -	int ret;
> > +	buf_printf(buf, "\t.asciz \"");
> > +	buf_escaped(buf, str);
> > +	buf_printf(buf, "\"\n");
> > +}
> >
> > -	add_header(&buf, mod);
> > -	add_exported_symbols(&buf, mod);
> > -	add_versions(&buf, mod);
> > -	add_extended_versions(&buf, mod);
> > -	add_depends(&buf, mod);
> > +/* The equivalent of MODULE_INFO(tag, info). */
> > +static void add_asm_modinfo(struct buffer *buf, const char *tag,
> > +			    const char *info)
> > +{
> > +	buf_printf(buf, "\t.section .modinfo,\"a\",%%progbits\n");
> > +	buf_printf(buf, "\t.asciz \"%s=", tag);
> > +	buf_escaped(buf, info);
> > +	buf_printf(buf, "\"\n");
> > +}
> >
> > -	buf_printf(&buf, "\n");
> > +/* See __KSYM_REF() in include/linux/export-internal.h. */
> > +static void add_asm_ksym_ref(struct buffer *buf, const char *prefix,
> > +			     const char *sym)
> > +{
> > +#if MOD_PREL32_RELOCATIONS
> > +	buf_printf(buf, "\t.long %s%s - .\n", prefix, sym);
> > +#else
> > +	buf_printf(buf, "\t" MOD_PTR_DIRECTIVE " %s%s\n", prefix, sym);
> > +#endif
> > +}
> > +
> > +/* The name and namespace strings a ksymtab entry refers to. */
> > +static void add_asm_kstrtab(struct buffer *buf, const struct symbol *sym)
> > +{
> > +	buf_printf(buf, "\t.section \"__ksymtab_strings\",\"aMS\",%%progbits,1\n");
> > +	buf_printf(buf, "__kstrtab_%s:\n", sym->name);
> > +	buf_asciz(buf, sym->name);
> > +	buf_printf(buf, "__kstrtabns_%s:\n", sym->name);
> > +	buf_asciz(buf, sym->namespace);
> > +	buf_printf(buf, "\t.previous\n");
> > +}
> > +
> > +/* The equivalent of SYMBOL_FLAGS(). */
> > +static void add_asm_kflagstab(struct buffer *buf, const struct symbol *sym)
> > +{
> > +	buf_printf(buf, "\t.section \"___kflagstab+%s\", \"a\"\n", sym->name);
> > +	buf_printf(buf, "__flags_%s:\n", sym->name);
> > +	buf_printf(buf, "\t.byte 0x%02x\n", get_symbol_flags(sym));
> > +	buf_printf(buf, "\t.previous\n");
> > +}
> > +
> > +/* The equivalent of KSYMTAB_FUNC()/KSYMTAB_DATA(). */
> > +static void add_asm_ksymtab(struct buffer *buf, const struct symbol *sym)
> > +{
> > +	const char *name = sym->name;
> > +
> > +	add_asm_kstrtab(buf, sym);
> > +
> > +	buf_printf(buf, "\t.section \"___ksymtab+%s\", \"a\"\n", name);
> > +	buf_printf(buf, "\t.balign %d\n", KSYM_ALIGN);
> > +	buf_printf(buf, "__ksymtab_%s:\n", name);
> > +	add_asm_ksym_ref(buf, sym->is_func ? MOD_FUNC_PREFIX : "", name);
> > +	add_asm_ksym_ref(buf, "__kstrtab_", name);
> > +	add_asm_ksym_ref(buf, "__kstrtabns_", name);
> > +	buf_printf(buf, "\t.previous\n");
> > +
> > +	add_asm_kflagstab(buf, sym);
> > +}
> > +
> > +/* The equivalent of SYMBOL_CRC(). */
> > +static void add_asm_crc(struct buffer *buf, const struct symbol *sym)
> > +{
> > +	buf_printf(buf, "\t.section \"___kcrctab+%s\",\"a\"\n", sym->name);
> > +	buf_printf(buf, "\t.balign 4\n");
> > +	buf_printf(buf, "__crc_%s:\n", sym->name);
> > +	buf_printf(buf, "\t.long 0x%08x\n", sym->crc);
> > +	buf_printf(buf, "\t.previous\n");
> > +}
>
> It would be good to avoid duplicating these symbol-related
> implementations in include/linux/export-internal.h and
> scripts/mod/modpost.c. Perhaps .vmlinux.export.c could also be lowered
> to assembly, so the implementation in include/linux/export-internal.h
> can be removed and scripts/mod/modpost.c becomes the only source of
> truth.

Ack, sounds reasonable, but seems sensible as a follow-up I think?

>
> > +
> > +static bool export_is_kept(const struct symbol *sym)
> > +{
> > +	return !trim_unused_exports || sym->used;
> > +}
> > +
> > +/* Record the CRCs of the exported symbols. */
> > +static void add_asm_crcs(struct buffer *buf, struct module *mod)
> > +{
> > +	struct symbol *sym;
> > +
> > +	list_for_each_entry(sym, &mod->exported_symbols, list) {
> > +		if (!export_is_kept(sym))
> > +			continue;
> > +
> > +		if (!sym->crc_valid)
> > +			mod_warn(mod, "EXPORT symbol '%s' version generation failed, symbol will not be versioned.\n"
> > +				 "Is '%s' prototyped in <asm/asm-prototypes.h>?\n",
> > +				 sym->name, sym->name);
> > +		add_asm_crc(buf, sym);
> > +	}
> > +}
> > +
> > +static void add_asm_exported_symbols(struct buffer *buf, struct module *mod)
> > +{
> > +	struct symbol *sym;
> > +
> > +	list_for_each_entry(sym, &mod->exported_symbols, list) {
> > +		if (export_is_kept(sym))
> > +			add_asm_ksymtab(buf, sym);
> > +	}
> > +
> > +	if (modversions)
> > +		add_asm_crcs(buf, mod);
> > +}
>
> Nit: This could be kept shorter as a single function, similarly to how
> the original C version was implemented solely as add_exported_symbols().
>
> static void add_asm_exported_symbols(struct buffer *buf, struct module *mod)
> {
> 	struct symbol *sym;
>
> 	list_for_each_entry(sym, &mod->exported_symbols, list) {
> 		if (trim_unused_exports && !sym->used)
> 			continue;
>
> 		add_asm_ksymtab(buf, sym);
>
> 		if (!modversions)
> 			continue;
>
> 		if (!sym->crc_valid)
> 			mod_warn(mod, "EXPORT symbol '%s' version generation failed, symbol will not be versioned.\n"
> 				 "Is '%s' prototyped in <asm/asm-prototypes.h>?\n",
> 				 sym->name, sym->name);
> 		add_asm_crc(buf, sym);
> 	}
> }

Ack will do for v4.

>
> > +/*
> > + * An unresolved symbol without a module is not versioned; one without a CRC
> > + * cannot be, so warn about it.
> > + */
> > +static bool skip_unversioned(struct module *mod, const struct symbol *sym)
> > +{
> > +	if (!sym->module)
> > +		return true;
> > +	if (sym->crc_valid)
> > +		return false;
> > +
> > +	mod_warn(mod, "symbol '%s' has no CRC!\n", sym->name);
> > +	return true;
> > +}
> > +
> > +/* One struct modversion_info: the CRC, then the name padded to the end. */
> > +static void add_asm_version(struct buffer *buf, const struct symbol *sym)
> > +{
> > +	buf_printf(buf, "\t" MOD_PTR_DIRECTIVE " 0x%08x\n", sym->crc);
> > +	buf_printf(buf, "\t.ascii \"%s\"\n", sym->name);
> > +	buf_printf(buf, "\t.skip %zu\n", MOD_SIZEOF_struct_modversion_info -
> > +		   MOD_OFF_modversion_info_name - strlen(sym->name));
> > +}
> > +
> > +/*
> > + * The equivalent of:
> > + *
> > + *	static const struct modversion_info ____versions[]
> > + *	__used __section("__versions") = { { crc, "name" }, ... };
> > + *
> > + * for unresolved symbols.
> > + */
> > +static void add_asm_versions(struct buffer *buf, struct module *mod)
> > +{
> > +	struct symbol *sym;
> > +
> > +	if (!basic_modversions)
> > +		return;
> > +
> > +	buf_printf(buf, "\n\t.section __versions,\"a\",%%progbits\n");
> > +	buf_printf(buf, "\t.balign %d\n", MOD_ALIGNOF_struct_modversion_info);
> > +	list_for_each_entry(sym, &mod->unresolved_symbols, list) {
> > +		if (skip_unversioned(mod, sym))
> > +			continue;
> > +
> > +		if (strlen(sym->name) >= MOD_NAME_LEN) {
> > +			/* Only the extended table can hold it. */
> > +			if (extended_modversions)
> > +				continue;
> > +
> > +			mod_error(mod, "too long symbol '%s'\n", sym->name);
> > +			break;
> > +		}
> > +
> > +		add_asm_version(buf, sym);
> > +	}
> > +}
> > +
> > +static void add_asm_version_ext_crcs(struct buffer *buf, struct module *mod)
> > +{
> > +	struct symbol *sym;
> > +
> > +	buf_printf(buf, "\n\t.section __version_ext_crcs,\"a\",%%progbits\n");
> > +	buf_printf(buf, "\t.balign 4\n");
> > +	list_for_each_entry(sym, &mod->unresolved_symbols, list) {
> > +		if (skip_unversioned(mod, sym))
> > +			continue;
> > +
> > +		buf_printf(buf, "\t.long 0x%08x\n", sym->crc);
> > +	}
> > +}
> > +
> > +/*
> > + * A symbol without a CRC was warned about with the CRCs, and is skipped here
> > + * too so that the names line up with them.
> > + */
> > +static void add_asm_version_ext_names(struct buffer *buf, struct module *mod)
> > +{
> > +	struct symbol *sym;
> > +
> > +	buf_printf(buf, "\t.section __version_ext_names,\"a\",%%progbits\n");
> > +	list_for_each_entry(sym, &mod->unresolved_symbols, list) {
> > +		if (!sym->module || !sym->crc_valid)
> > +			continue;
> > +
> > +		buf_asciz(buf, sym->name);
> > +	}
> > +	/* The terminator of the string literal this used to be. */
> > +	buf_printf(buf, "\t.byte 0\n");
>
> This is ok since the conversion aims to be byte-identical. In the
> future, I believe the extra NUL byte should be removed and
> elf_validity_cache_index_versions() also be updated to report when
> __version_ext_names contains more names than there are CRCs in
> __version_ext_crcs.

Ack, I agree one for a follow up!

>
> > +}
> > +
> > +/*
> > + * The equivalent of:
> > + *	static const u32 ____version_ext_crcs[] __section("__version_ext_crcs") = { crc, ... };
> > + *	static const char ____version_ext_names[] __section("__version_ext_names") = "name\0" ...;
> > + *
> > + * for unresolved symbols.
> > + */
> > +static void add_asm_extended_versions(struct buffer *buf, struct module *mod)
> > +{
> > +	if (!extended_modversions)
> > +		return;
> > +
> > +	add_asm_version_ext_crcs(buf, mod);
> > +	add_asm_version_ext_names(buf, mod);
> > +}
>
> Nit: add_asm_version_ext_crcs() and add_asm_version_ext_names() are both
> fairly short and are called only from add_asm_extended_versions(), so
> I think they could be inlined into that function. This would be similar
> to the original implementation, which does all the work in
> add_extended_versions().

Ack will do for v4!

>
> > +
> > +/* Clear ->seen of the modules that own symbols this one needs. */
> > +static void clear_seen_dependencies(struct module *mod)
> > +{
> > +	struct symbol *sym;
> > +
> > +	list_for_each_entry(sym, &mod->unresolved_symbols, list) {
> > +		if (sym->module)
> > +			sym->module->seen = sym->module->is_vmlinux;
> > +	}
> > +}
> > +
> > +/* The modules this one depends on, each once, comma separated. */
> > +static void collect_dependencies(struct module *mod, struct buffer *deps)
> > +{
> > +	struct symbol *sym;
> > +	bool first = true;
> > +
> > +	clear_seen_dependencies(mod);
> > +
> > +	list_for_each_entry(sym, &mod->unresolved_symbols, list) {
> > +		struct module *owner = sym->module;
> > +
> > +		if (!owner || owner->seen)
> > +			continue;
> > +
> > +		owner->seen = true;
> > +		buf_printf(deps, "%s%s", first ? "" : ",",
> > +			   get_basename(owner->name));
> > +		first = false;
> > +	}
> > +	buf_write(deps, "", 1);
> > +}
> > +
> > +static void add_asm_depends(struct buffer *buf, struct module *mod)
> > +{
> > +	struct buffer deps = { };
> > +
> > +	collect_dependencies(mod, &deps);
> > +	buf_printf(buf, "\n");
> > +	add_asm_modinfo(buf, "depends", deps.p);
> > +	free(deps.p);
> > +}
>
> Nit: Same here. I think this would be more straightforward if
> collect_dependencies() and clear_seen_dependencies() were inlined into
> add_asm_depends(), matching the original add_depends().

Ack one for v4 too! :)

>
> --
> Thanks,
> Petr

--
Cheers, Lorenzo