On Thu, Oct 15, 2026 at 05:42:45PM +0200, Petr Pavlu wrote:
> 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.
Thanks! :)
>
> > ---
> > 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.
Ack, makes sense, moved the decl inside elf_info.
>
> The description could use 'to_secndx' instead of 'section index' to
> clarify whether it refers to the source or destination section of
> a relocation.
Ack, updated for v5.
>
> > +
> > +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/.
Ack, done.
>
> > +{
> > + 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.
Ack, done.
>
> > +{
> > + 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.
Ack, done.
>
> --
> Thanks,
> Petr
Thanks for the review!
--
Cheers, Lorenzo
