Re: [PATCH v8 2/3] modpost: Produce extended MODVERSIONS information

[Luis Chamberlain <mcgrof@kernel.org>]

On Wed, Nov 06, 2024 at 02:19:38PM -0800, Matthew Maurer wrote:
> >
> > > If booted against an old kernel, it will
> > > behave as though there is no modversions information.
> >
> > Huh? This I don't get. If you have the new libkmod and boot
> > an old kernel, that should just not break becauase well, long
> > symbols were not ever supported properly anyway, so no regression.
> 
> Specifically, if you set NO_BASIC_MODVERSIONS, build a module, and
> then load said module with a kernel *before* EXTENDED_MODVERSIONS
> existed, it will see no modversion info on the module to check. This
> will be true regardless of symbol length.

Isn't that just the same as disabling modverisons?

If you select modversions, you get the options to choose:

  - old modversions
  - old modversions + extended modversions
  - extended modversions only

> > I'm not quite sure I understood your last comment here though,
> > can you clarify what you meant?
> >
> > Anyway, so now that this is all cleared up, the next question I have
> > is, let's compare a NO_BASIC_MODVERSIONS world now, given that the
> > userspace requirements aren't large at all, what actual benefits does
> > using this new extended mod versions have? Why wouldn't a distro end
> > up preferring this for say a future release for all modules?
> 
> I think a distro will end up preferring using this for all modules,
> but was intending to put both in for a transitional period until the
> new format was more accepted.

The only thing left I think to test is the impact at runtime, and the
only thing I can think of is first we use find_symbol() on resolve_symbol() 
which it took me a while to review and realize that this just uses a
completely different ELF section, the the ksymtab sections which are split up
between the old and the gpl section. But after that we use check_version().
I suspect the major overhead here is in find_symbol() and that's in no way shape
or form affected by your changes, and I also suspect that since the
way you implemented for_each_modversion_info_ext() is just *one* search
there shouldn't be any penalty here at all. Given it took *me* a while
to review all this, I think it would be good for you to also expand your
cover letter to be crystal clear on these expectations to users and
developers and if anything expand on the Kconfig / and add documentation
if we don't document any of this.

I'd still like to see you guys test all this with the new TEST_KALLSYMS.

  Luis