| Message ID | 20230123052653.711899-1-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: do not automatically add -w option to modpost | expand |
On 01/23/2023, Masahiro Yamada wrote: > When there is a missing input file (vmlinux.o or Module.symvers), you > are likely to get a ton of unresolved symbols. > > Currently, Kbuild automatically adds the -w option to allow module builds > to continue with warnings instead of errors. > > This may not be what the user expects because it is generally more useful > to catch all possible issues at build time instead of at run time. > > Let's not do what the user did not ask. > > If you still want to build modules anyway, you can proceed by explicitly > setting KBUILD_MODPOST_WARN=1. Since you may miss a real issue, you need > to be aware of what you are doing. > > Suggested-by: William McVicker <willmcvicker@google.com> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > scripts/Makefile.modpost | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > index 43343e13c542..9254ed811ddd 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.";) \ You need to move the closing parenthesis to come at the end of these echo messages. Otherwise you get this new message unconditionally. I also found during testing that the refactoring in commit f73edc8951b2 ("kbuild: unify two modpost invocations") dropped the check for missing KBUILD_EXTRA_SYMBOLS. That means if an external module depends on another external module and sets: KBUILD_EXTRA_SYMBOLS=/path/to/ext_module/Module.symvers ... then make will fail even with KBUILD_MODPOST_WARN=1 since we unconditionally add KBUILD_EXTRA_SYMBOLS to the modpost-args like this: modpost-args += -e $(addprefix -i , $(KBUILD_EXTRA_SYMBOLS)) To fix this, I suggest you also take the following patch so that KBUILD_MODPOST_WARN=1 will allow you to skip those unresolved symbols as well: diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index 931a3272a4ba..0e2f7fa58056 100644 --- a/scripts/Makefile.modpost +++ b/scripts/Makefile.modpost @@ -149,14 +149,12 @@ include $(kbuild-file) output-symdump := $(KBUILD_EXTMOD)/Module.symvers -ifeq ($(wildcard Module.symvers),) -missing-input := Module.symvers -else -modpost-args += -i Module.symvers -modpost-deps += Module.symvers -endif +input-symdump := Module.symvers $(KBUILD_EXTRA_SYMBOLS) +existing-input-symdump := $(wildcard $(input-symdump)) +missing-input := $(filter-out $(existing-input-symdump), $(input-symdump)) -modpost-args += -e $(addprefix -i , $(KBUILD_EXTRA_SYMBOLS)) +modpost-deps += $(existing-input-symdumps) +modpost-args += -e $(addprefix -i , $(existing-input-symdump)) endif # ($(KBUILD_EXTMOD),) With those changes, the patch works for me as expected. I verified I get a build time error when referencing unresolved symbols without setting KBUILD_MODPOST_WARN=1. And then I verified setting KBUILD_MODPOST_WARN=1 treated those errors as warnings. Thanks, Will > + echo >&2 " You can set KBUILD_MODPOST_WARN=1 to turn errors into warning"; \ > + echo >&2 " if you want to proceed at your own risk."; \ > $(MODPOST) $(modpost-args) > > targets += $(output-symdump) > -- > 2.34.1 >
On Tue, Jan 24, 2023 at 7:42 AM William McVicker <willmcvicker@google.com> wrote: > > On 01/23/2023, Masahiro Yamada wrote: > > When there is a missing input file (vmlinux.o or Module.symvers), you > > are likely to get a ton of unresolved symbols. > > > > Currently, Kbuild automatically adds the -w option to allow module builds > > to continue with warnings instead of errors. > > > > This may not be what the user expects because it is generally more useful > > to catch all possible issues at build time instead of at run time. > > > > Let's not do what the user did not ask. > > > > If you still want to build modules anyway, you can proceed by explicitly > > setting KBUILD_MODPOST_WARN=1. Since you may miss a real issue, you need > > to be aware of what you are doing. > > > > Suggested-by: William McVicker <willmcvicker@google.com> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > scripts/Makefile.modpost | 8 +++----- > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > > index 43343e13c542..9254ed811ddd 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.";) \ > > You need to move the closing parenthesis to come at the end of these > echo messages. Otherwise you get this new message unconditionally. Ah, thanks for catching it. > I also found during testing that the refactoring in commit f73edc8951b2 > ("kbuild: unify two modpost invocations") dropped the check for missing > KBUILD_EXTRA_SYMBOLS. That means if an external module depends on > another external module and sets: > > KBUILD_EXTRA_SYMBOLS=/path/to/ext_module/Module.symvers > > ... then make will fail even with KBUILD_MODPOST_WARN=1 since we > unconditionally add KBUILD_EXTRA_SYMBOLS to the modpost-args like this: > > modpost-args += -e $(addprefix -i , $(KBUILD_EXTRA_SYMBOLS)) > > To fix this, I suggest you also take the following patch so that > KBUILD_MODPOST_WARN=1 will allow you to skip those unresolved symbols as > well: How is this useful? KBUILD_EXTRA_SYMBOLS is explicitly specified by the user via the command line or the environment variable. If $(KBUILD_EXTRA_SYMBOLS) does not exist, it is a user's fault, isn't it?
On 01/24/2023, Masahiro Yamada wrote: > On Tue, Jan 24, 2023 at 7:42 AM William McVicker > <willmcvicker@google.com> wrote: > > > > On 01/23/2023, Masahiro Yamada wrote: > > > When there is a missing input file (vmlinux.o or Module.symvers), you > > > are likely to get a ton of unresolved symbols. > > > > > > Currently, Kbuild automatically adds the -w option to allow module builds > > > to continue with warnings instead of errors. > > > > > > This may not be what the user expects because it is generally more useful > > > to catch all possible issues at build time instead of at run time. > > > > > > Let's not do what the user did not ask. > > > > > > If you still want to build modules anyway, you can proceed by explicitly > > > setting KBUILD_MODPOST_WARN=1. Since you may miss a real issue, you need > > > to be aware of what you are doing. > > > > > > Suggested-by: William McVicker <willmcvicker@google.com> > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > scripts/Makefile.modpost | 8 +++----- > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost > > > index 43343e13c542..9254ed811ddd 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.";) \ > > > > You need to move the closing parenthesis to come at the end of these > > echo messages. Otherwise you get this new message unconditionally. > > > Ah, thanks for catching it. > > > > I also found during testing that the refactoring in commit f73edc8951b2 > > ("kbuild: unify two modpost invocations") dropped the check for missing > > KBUILD_EXTRA_SYMBOLS. That means if an external module depends on > > another external module and sets: > > > > KBUILD_EXTRA_SYMBOLS=/path/to/ext_module/Module.symvers > > > > ... then make will fail even with KBUILD_MODPOST_WARN=1 since we > > unconditionally add KBUILD_EXTRA_SYMBOLS to the modpost-args like this: > > > > modpost-args += -e $(addprefix -i , $(KBUILD_EXTRA_SYMBOLS)) > > > > To fix this, I suggest you also take the following patch so that > > KBUILD_MODPOST_WARN=1 will allow you to skip those unresolved symbols as > > well: > > > How is this useful? > > KBUILD_EXTRA_SYMBOLS is explicitly specified by the user > via the command line or the environment variable. > > If $(KBUILD_EXTRA_SYMBOLS) does not exist, > it is a user's fault, isn't it? Sort of, yes. One could argue that it's the same situation as missing the in-tree Module.symvers or the vmlinuux.o/vmlinux.symvers. Basically, if you keep it as is, then KBUILD_MODPOST_WARN=1 would only work if the user edits the Makefile to remove the KBUILD_EXTRA_SYMBOLS line. With my suggested patch, the user could build the module as is without any dependencies by setting KBUILD_MODPOST_WARN=1. I'm fine with whichever way you choose to support since I know this is an external modules edge-case. I personally like to build my modules with all the dependencies present to try an catch any issues at build time. Thanks, Will > > > > > > > -- > Best Regards > Masahiro Yamada
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost index 43343e13c542..9254ed811ddd 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 want to proceed at your own risk."; \ $(MODPOST) $(modpost-args) targets += $(output-symdump)
When there is a missing input file (vmlinux.o or Module.symvers), you are likely to get a ton of unresolved symbols. Currently, Kbuild automatically adds the -w option to allow module builds to continue with warnings instead of errors. This may not be what the user expects because it is generally more useful to catch all possible issues at build time instead of at run time. Let's not do what the user did not ask. If you still want to build modules anyway, you can proceed by explicitly setting KBUILD_MODPOST_WARN=1. Since you may miss a real issue, you need to be aware of what you are doing. Suggested-by: William McVicker <willmcvicker@google.com> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/Makefile.modpost | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)