On 9/23/26 7:18 PM, Lorenzo Stoakes (ARM) wrote:
> For every relocation modpost invokes check_section_mismatch() to determine
> whether there is any kind of mismatch between the source and destination,
> and if so which classification applies.
>
> Each time it does this it invokes section_mismatch() which iterates
> through the sectioncheck[] array every time it's called.
>
> When walked a relocation section the source is fixed and there aren't many
> targets, so the same names are looked up over and over again.
>
> Therefore cache not only mismatch categorisation but also whether a
> mismatch even exists for a given section and look up the sections in the
> cache.
>
> Special indices (undefined, absolute, common) take the uncached path as
> before.
>
> This results in very significant speed ups for allmodconfig incremental
> builds.
>
> modpost is on the serial tail of every build that links vmlinux.
>
> Whole build, 128-thread Threadripper 9980X, best of N runs:
>
> before after delta
> -------------------------------
> x86 defconfig, touch mm/vma.c, gcc 8.5s 8.3s -0.14s (-2%)
> x86 defconfig, touch mm/vma.c, clang 7.7s 7.6s -0.13s (-2%)
> x86 allmodconfig, touch mm/vma.c, gcc 33.9s 29.4s -4.5s (-13%)
> x86 allmodconfig, touch mm/vma.c, clang 31.9s 27.9s -4.0s (-13%)
>
> Assisted-by: LLM
> Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
This looks mostly ok to me. Below are some nits you might want to
consider addressing.
> ---
> scripts/mod/modpost.c | 66
> +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 75374c64b8cc..0fd43c8a89ea 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1155,12 +1155,67 @@ static void check_export_symbol(struct module *mod,
> struct elf_info *elf,
> name);
> }
>
> +/*
> + * mismatch_cache[section index] ->
> + * 0 - uncached.
> + * -1 - no mismatch.
> + * >0 - mismatch index + 1.
> + */
> +static int *mismatch_cache;
It might be cleaner to maintain this cache per elf_info, since it
logically belongs to the processing of an individual ELF file. This
would also be consistent with the existing symsearch cache, which uses
elf::symsearch.
The description could use 'to_secndx' instead of 'section index' to
clarify whether it refers to the source or destination section of
a relocation.
> +
> +static void init_mismatch_cache(unsigned int num_sections)
> +{
> + mismatch_cache = xcalloc(num_sections, sizeof(*mismatch_cache));
> +}
> +
> +static void reset_mismatch_cache(unsigned int num_sections)
> +{
> + memset(mismatch_cache, 0, num_sections * sizeof(*mismatch_cache));
> +}
> +
> +static void free_mismatch_cache(void)
> +{
> + free(mismatch_cache);
> + mismatch_cache = NULL;
> +}
> +
> +static const struct sectioncheck
> +*cache_mismatch(unsigned int secndx, const struct sectioncheck *mismatch)
Similarly here, s/secndx/to_secndx/.
> +{
> + if (!mismatch) {
> + mismatch_cache[secndx] = -1;
> + return NULL;
> + }
> +
> + mismatch_cache[secndx] = (mismatch - sectioncheck) + 1;
> + return mismatch;
> +}
> +
> +static const struct sectioncheck *get_section_mismatch(const char *fromsec,
> + const struct elf_info *elf, unsigned int secndx)
s/secndx/to_secndx/, and the fromsec and elf arguments could be swapped
for consistency with the rest of the code.
> +{
> + int cached;
> +
> + if (secndx >= elf->num_sections)
> + return section_mismatch(fromsec, sec_name(elf, secndx));
> +
> + cached = mismatch_cache[secndx];
> + if (cached < 0)
> + return NULL;
> + if (cached > 0)
> + return §ioncheck[cached - 1];
> +
> + return cache_mismatch(secndx,
> + section_mismatch(fromsec, sec_name(elf, secndx)));
> +}
> +
> static void check_section_mismatch(struct module *mod, struct elf_info *elf,
> Elf_Sym *sym,
> unsigned int fsecndx, const char *fromsec,
> Elf_Addr faddr, Elf_Addr taddr)
> {
> - const char *tosec = sec_name(elf, get_secindex(elf, sym));
> + const unsigned int to_secndx = get_secindex(elf, sym);
The const qualifier can be dropped from the to_secndx definition to
match the usual kernel style.
--
Thanks,
Petr
