modpost warning by default on missing Module.symvers

modpost warning by default on missing Module.symvers

[William McVicker]

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/

Re: modpost warning by default on missing Module.symvers

[Masahiro Yamada]

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

Re: modpost warning by default on missing Module.symvers

[William McVicker]

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

Re: modpost warning by default on missing Module.symvers

[Masahiro Yamada]

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

Re: modpost warning by default on missing Module.symvers

[William McVicker]

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