Re: modpost warning by default on missing Module.symvers

[William McVicker <willmcvicker@google.com>]

On 01/11/2023, Masahiro Yamada wrote:
> On Wed, Jan 11, 2023 at 3:43 AM William McVicker
> <willmcvicker@google.com> wrote:
> >
> > On 01/10/2023, Masahiro Yamada wrote:
> > > On Tue, Jan 10, 2023 at 5:45 AM William McVicker
> > > <willmcvicker@google.com> wrote:
> > > >
> > > > Hi Masahiro,
> > > >
> > > > I recently noticed that in commit 4475dff55c54 ("kbuild: fix false-positive
> > > > modpost warning when all symbols are trimmed") [1] you modified the modpost
> > > > behavior to always warn (by passing `-w`) when there are missing Module.symver
> > > > files in order to allow module builds to continue building with warnings
> > > > instead of errors. I'm curious why you decided to not continue to rely on
> > > > KBUILD_MODPOST_WARN to enable/disable that functionality?
> > > >
> > > > I personally find it useful to keep these types of warnings as errors in order
> > > > to catch missing dependencies at build time (ideally by the CI build) instead
> > > > of at runtime when a module fails to load due to a missing symbol dependency.
> > > >
> > > > Let me know your thoughts on this and I'll try to come up with a solution to
> > > > factor in any concerns you have.
> > > >
> > > > Thanks,
> > > > Will
> > > >
> > > > [1] https://lore.kernel.org/all/20210325185412.2352951-3-masahiroy@kernel.org/
> > >
> > >
> > >
> > > Good point.
> > >
> > > I think we can always require KBUILD_MODPOST_WARN=1 explicitly.
> > >
> > > Skipping unresolved symbols is not a good idea.
> > > Users can proceed if they want,
> > > but they should be aware of what they are doing, at least.
> > >
> > >
> > > How about something like this?
> > >
> > >
> > >
> > >
> > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> > > index 43343e13c542..34baef239816 100644
> > > --- a/scripts/Makefile.modpost
> > > +++ b/scripts/Makefile.modpost
> > > @@ -121,16 +121,14 @@ modpost-args += -e $(addprefix -i ,
> > > $(KBUILD_EXTRA_SYMBOLS))
> > >
> > >  endif # ($(KBUILD_EXTMOD),)
> > >
> > > -ifneq ($(missing-input),)
> > > -modpost-args += -w
> > > -endif
> > > -
> > >  quiet_cmd_modpost = MODPOST $@
> > >        cmd_modpost = \
> > >         $(if $(missing-input), \
> > >                 echo >&2 "WARNING: $(missing-input) is missing."; \
> > >                 echo >&2 "         Modules may not have dependencies
> > > or modversions."; \
> > > -               echo >&2 "         You may get many unresolved symbol
> > > warnings.";) \
> > > +               echo >&2 "         You may get many unresolved symbol
> > > errors.";) \
> > > +               echo >&2 "         You can set KBUILD_MODPOST_WARN=1
> > > to turn errors into warning"; \
> > > +               echo >&2 "         if you know what you are doing."; \
> > >         $(MODPOST) $(modpost-args)
> > >
> > >  targets += $(output-symdump)
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > --
> > > Best Regards
> > >
> > > Masahiro Yamada
> >
> > That looks good to me! You do mention in [1] that there's a case where
> > unresolved symbols are expected. Can you clarify that? Why would you want to
> > build a kernel or module with unresolved symbols?
> >
> > [1] https://lore.kernel.org/all/20230104140459.1147626-1-masahiroy@kernel.org/
> >
> > Thanks,
> > Will
> 
> 
> 
> What I have in mind is the following cases.
> We cannot check unresolved symbols due to missing vmlinux.
> 
> 
> 
> [1] Build in-tree modules without building vmlinux
> 
>     $ make defconfig
>     $ make modules
> 
> 
>    Perhaps, this is useful for people who are only interested
>    in particular modules, but not the entire kernel?
> 
> 
> [2] Build external modules with minimal setups
> 
>     $ make defconfig
>     $ make modules_prepare
>     $ make M=<path/to/eternal/module>
> 
>    This is useful if people want to compile their modules quicily?
> 
> 
> [3] Build single *.ko
> 
>    $ make defconfig
>    $ make <path/to/a/module>.ko
> 
>    Perhaps, this is useful for people who are only interested
>    in modules they maintain.
> 
> 
> 
> 
> I am not a big fan of any of them, but those have been available
> since before I became the maintainer.
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada
> 
> -- 
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
> 

Sorry for the delay in getting back to you.

This sounds good to me. I personally always build my modules with the
vmlinux so that I can detect at build time if there are any issues, e.g.
unresolved symbols. For these 3 cases, would your change require devs to
set KBUILD_MODPOST_WARN=1 if they want to ignore the unresolved symbols
error?

Thanks,
Will