Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
set which happens per default when building modules out of the tree.
This change gives the opportunity to define module build behaving also
in case of out of tree builds and default will become exit on error.
Errors which can be detected by the build should be trapped out of the box
there, unless somebody wants to notice broken stuff later at runtime.
Signed-off-by: Wladislav Wiebe <[email protected]>
---
scripts/Makefile.modpost | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 6b7f354f189a..fec6ec2ffa47 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -78,7 +78,7 @@ modpost = scripts/mod/modpost \
$(if $(KBUILD_EXTRA_SYMBOLS), $(patsubst %, -e %,$(KBUILD_EXTRA_SYMBOLS))) \
$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
$(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
- $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
+ $(if $(KBUILD_MODPOST_WARN),-w)
MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
--
2.19.2
(+CC Jonas Gorski)
On Tue, Mar 26, 2019 at 6:58 PM Wiebe, Wladislav (Nokia - DE/Ulm)
<[email protected]> wrote:
>
> Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
> was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
> Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
> set which happens per default when building modules out of the tree.
>
> This change gives the opportunity to define module build behaving also
> in case of out of tree builds and default will become exit on error.
> Errors which can be detected by the build should be trapped out of the box
> there, unless somebody wants to notice broken stuff later at runtime.
If an external module refers to a symbol
provided by another external module,
this patch will turn the warning into the error by default,
which is probably a bad idea.
I suggested a desired change in the following.
https://patchwork.kernel.org/patch/9980691/
I am poking Jonas Gorski
if he plans to send v2.
> Signed-off-by: Wladislav Wiebe <[email protected]>
> ---
> scripts/Makefile.modpost | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 6b7f354f189a..fec6ec2ffa47 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -78,7 +78,7 @@ modpost = scripts/mod/modpost \
> $(if $(KBUILD_EXTRA_SYMBOLS), $(patsubst %, -e %,$(KBUILD_EXTRA_SYMBOLS))) \
> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
> - $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
> + $(if $(KBUILD_MODPOST_WARN),-w)
>
> MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
>
> --
> 2.19.2
--
Best Regards
Masahiro Yamada
Hi!
On 07.04.2019 11:04, Masahiro Yamada wrote:
> (+CC Jonas Gorski)
>
>
> On Tue, Mar 26, 2019 at 6:58 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> <[email protected]> wrote:
>>
>> Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
>> was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
>> Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
>> set which happens per default when building modules out of the tree.
>>
>> This change gives the opportunity to define module build behaving also
>> in case of out of tree builds and default will become exit on error.
>> Errors which can be detected by the build should be trapped out of the box
>> there, unless somebody wants to notice broken stuff later at runtime.
>
> If an external module refers to a symbol
> provided by another external module,
> this patch will turn the warning into the error by default,
> which is probably a bad idea.
Indeed, exactly this should happen. You should fix your external module
dependencies by providing their symbols. Please use e.g.
KBUILD_EXTRA_SYMBOLS instead of converting errors to warning and hoping
that things will work. Or how do you want to make sure module A still
delivers all symbols needed by module B after an update/changes?
Manually comparing the logs after an update or waiting until it turns
out broken during run-time? I wouldn't say this is a good idea :-)
> I suggested a desired change in the following.
> https://patchwork.kernel.org/patch/9980691/
>
> I am poking Jonas Gorski
> if he plans to send v2.
I wouldn't vote for v2 based on explanation above.
- Wladislav
>
>
>> Signed-off-by: Wladislav Wiebe <[email protected]>
>> ---
>> scripts/Makefile.modpost | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
>> index 6b7f354f189a..fec6ec2ffa47 100644
>> --- a/scripts/Makefile.modpost
>> +++ b/scripts/Makefile.modpost
>> @@ -78,7 +78,7 @@ modpost = scripts/mod/modpost \
>> $(if $(KBUILD_EXTRA_SYMBOLS), $(patsubst %, -e %,$(KBUILD_EXTRA_SYMBOLS))) \
>> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
>> $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
>> - $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
>> + $(if $(KBUILD_MODPOST_WARN),-w)
>>
>> MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
>>
>> --
>> 2.19.2
>
>
>
Hi.
On Mon, Apr 8, 2019 at 5:03 PM Wiebe, Wladislav (Nokia - DE/Ulm)
<[email protected]> wrote:
>
> Hi!
>
> On 07.04.2019 11:04, Masahiro Yamada wrote:
> > (+CC Jonas Gorski)
> >
> >
> > On Tue, Mar 26, 2019 at 6:58 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> > <[email protected]> wrote:
> >>
> >> Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
> >> was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
> >> Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
> >> set which happens per default when building modules out of the tree.
> >>
> >> This change gives the opportunity to define module build behaving also
> >> in case of out of tree builds and default will become exit on error.
> >> Errors which can be detected by the build should be trapped out of the box
> >> there, unless somebody wants to notice broken stuff later at runtime.
> >
> > If an external module refers to a symbol
> > provided by another external module,
> > this patch will turn the warning into the error by default,
> > which is probably a bad idea.
>
> Indeed, exactly this should happen. You should fix your external module
> dependencies by providing their symbols. Please use e.g.
> KBUILD_EXTRA_SYMBOLS instead of converting errors to warning and hoping
> that things will work.
I know this option.
What I want to say is, this patch changes the behavior, and
may annoy some people.
> Or how do you want to make sure module A still
> delivers all symbols needed by module B after an update/changes?
> Manually comparing the logs after an update or waiting until it turns
> out broken during run-time? I wouldn't say this is a good idea :-)
OK, so the commit log should record both the behavior change
and workarounds.
- If an external module being built refers to symbols
in another external module, Kbuild previously showed a warning,
but going forward it will turn it into an error.
- To work around this, you should pass a symbol table via KBUILD_EXTRA_SYMBOLS,
or KBUILD_EXTRA_SYMBOLS=1 to turn the error into a warning.
Thanks.
>
> > I suggested a desired change in the following.
> > https://patchwork.kernel.org/patch/9980691/
> >
> > I am poking Jonas Gorski
> > if he plans to send v2.
>
> I wouldn't vote for v2 based on explanation above.
>
> - Wladislav
>
> >
> >
> >> Signed-off-by: Wladislav Wiebe <[email protected]>
> >> ---
> >> scripts/Makefile.modpost | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> >> index 6b7f354f189a..fec6ec2ffa47 100644
> >> --- a/scripts/Makefile.modpost
> >> +++ b/scripts/Makefile.modpost
> >> @@ -78,7 +78,7 @@ modpost = scripts/mod/modpost \
> >> $(if $(KBUILD_EXTRA_SYMBOLS), $(patsubst %, -e %,$(KBUILD_EXTRA_SYMBOLS))) \
> >> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> >> $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
> >> - $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
> >> + $(if $(KBUILD_MODPOST_WARN),-w)
> >>
> >> MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
> >>
> >> --
> >> 2.19.2
> >
> >
> >
--
Best Regards
Masahiro Yamada
Hi,
本当に申し訳ありません, I got sidetracked and completely forgot about it. I
actually still have my old tree with the suggested changes for v2.
On Tue, 9 Apr 2019 at 11:01, Masahiro Yamada
<[email protected]> wrote:
>
> Hi.
>
> On Mon, Apr 8, 2019 at 5:03 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> <[email protected]> wrote:
> >
> > Hi!
> >
> > On 07.04.2019 11:04, Masahiro Yamada wrote:
> > > (+CC Jonas Gorski)
> > >
> > >
> > > On Tue, Mar 26, 2019 at 6:58 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> > > <[email protected]> wrote:
> > >>
> > >> Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
> > >> was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
> > >> Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
> > >> set which happens per default when building modules out of the tree.
> > >>
> > >> This change gives the opportunity to define module build behaving also
> > >> in case of out of tree builds and default will become exit on error.
> > >> Errors which can be detected by the build should be trapped out of the box
> > >> there, unless somebody wants to notice broken stuff later at runtime.
> > >
> > > If an external module refers to a symbol
> > > provided by another external module,
> > > this patch will turn the warning into the error by default,
> > > which is probably a bad idea.
> >
> > Indeed, exactly this should happen. You should fix your external module
> > dependencies by providing their symbols. Please use e.g.
> > KBUILD_EXTRA_SYMBOLS instead of converting errors to warning and hoping
> > that things will work.
>
> I know this option.
> What I want to say is, this patch changes the behavior, and
> may annoy some people.
>
> > Or how do you want to make sure module A still
> > delivers all symbols needed by module B after an update/changes?
> > Manually comparing the logs after an update or waiting until it turns
> > out broken during run-time? I wouldn't say this is a good idea :-)
>
> OK, so the commit log should record both the behavior change
> and workarounds.
>
> - If an external module being built refers to symbols
> in another external module, Kbuild previously showed a warning,
> but going forward it will turn it into an error.
>
> - To work around this, you should pass a symbol table via KBUILD_EXTRA_SYMBOLS,
> or KBUILD_EXTRA_SYMBOLS=1 to turn the error into a warning.
This sounds like a good middle ground. When you do the effort of
setting KBUILD_EXTRA_SYMBOLS, you are likely interested in proper
dependency resolution and being notified if it fails.
I would probably still use KBUILD_MODPOST_WARN=0 to force the warnings
as errors, to have it as the central switch for the behaviour. So the
behaviour would then become
- If KBUILD_MODPOST_WARN is set to any value, set -w accordingly
- else, set -w only when building for an external module and
KBUILD_EXTRA_SYMBOLS is empty
or
ifdef KBUILD_MODPOST_WARN
KBUILD_MODPOST_WARN:=$(filter-out 0,$(KBUILD_MODPOST_WARN))
else
KBUILD_MODPOST_WARN:=$(if $(KBUILD_EXTMOD),$(if $(KBUILD_EXTRA_SYMBOLS),,1))
endif
(completely untested)
Regards
Jonas
Hi,
On 09.04.2019 11:00, Masahiro Yamada wrote:
> Hi.
>
> On Mon, Apr 8, 2019 at 5:03 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> <[email protected]> wrote:
>>
>> Hi!
>>
>> On 07.04.2019 11:04, Masahiro Yamada wrote:
>>> (+CC Jonas Gorski)
>>>
>>>
>>> On Tue, Mar 26, 2019 at 6:58 PM Wiebe, Wladislav (Nokia - DE/Ulm)
>>> <[email protected]> wrote:
>>>>
>>>> Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
>>>> was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
>>>> Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
>>>> set which happens per default when building modules out of the tree.
>>>>
>>>> This change gives the opportunity to define module build behaving also
>>>> in case of out of tree builds and default will become exit on error.
>>>> Errors which can be detected by the build should be trapped out of the box
>>>> there, unless somebody wants to notice broken stuff later at runtime.
>>>
>>> If an external module refers to a symbol
>>> provided by another external module,
>>> this patch will turn the warning into the error by default,
>>> which is probably a bad idea.
>>
>> Indeed, exactly this should happen. You should fix your external module
>> dependencies by providing their symbols. Please use e.g.
>> KBUILD_EXTRA_SYMBOLS instead of converting errors to warning and hoping
>> that things will work.
>
> I know this option.
> What I want to say is, this patch changes the behavior, and
> may annoy some people.
>
>> Or how do you want to make sure module A still
>> delivers all symbols needed by module B after an update/changes?
>> Manually comparing the logs after an update or waiting until it turns
>> out broken during run-time? I wouldn't say this is a good idea :-)
>
> OK, so the commit log should record both the behavior change
> and workarounds.
>
> - If an external module being built refers to symbols
> in another external module, Kbuild previously showed a warning,
> but going forward it will turn it into an error.
>
> - To work around this, you should pass a symbol table via KBUILD_EXTRA_SYMBOLS,
> or KBUILD_EXTRA_SYMBOLS=1 to turn the error into a warning.
Ok, I've posted v2:
https://lore.kernel.org/patchwork/patch/1059985/
Thanks!
- Wladislav
> Thanks.
>
>>
>>> I suggested a desired change in the following.
>>> https://patchwork.kernel.org/patch/9980691/
>>>
>>> I am poking Jonas Gorski
>>> if he plans to send v2.
>>
>> I wouldn't vote for v2 based on explanation above.
>>
>> - Wladislav
>>
>>>
>>>
>>>> Signed-off-by: Wladislav Wiebe <[email protected]>
>>>> ---
>>>> scripts/Makefile.modpost | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
>>>> index 6b7f354f189a..fec6ec2ffa47 100644
>>>> --- a/scripts/Makefile.modpost
>>>> +++ b/scripts/Makefile.modpost
>>>> @@ -78,7 +78,7 @@ modpost = scripts/mod/modpost \
>>>> $(if $(KBUILD_EXTRA_SYMBOLS), $(patsubst %, -e %,$(KBUILD_EXTRA_SYMBOLS))) \
>>>> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
>>>> $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
>>>> - $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
>>>> + $(if $(KBUILD_MODPOST_WARN),-w)
>>>>
>>>> MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
>>>>
>>>> --
>>>> 2.19.2
>>>
>>>
>>>
>
>
> --
> Best Regards
> Masahiro Yamada
>
On Tue, Apr 9, 2019 at 7:31 PM Jonas Gorski <[email protected]> wrote:
>
> Hi,
>
> 本当に申し訳ありません, I got sidetracked and completely forgot about it. I
> actually still have my old tree with the suggested changes for v2.
>
> On Tue, 9 Apr 2019 at 11:01, Masahiro Yamada
> <[email protected]> wrote:
> >
> > Hi.
> >
> > On Mon, Apr 8, 2019 at 5:03 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> > <[email protected]> wrote:
> > >
> > > Hi!
> > >
> > > On 07.04.2019 11:04, Masahiro Yamada wrote:
> > > > (+CC Jonas Gorski)
> > > >
> > > >
> > > > On Tue, Mar 26, 2019 at 6:58 PM Wiebe, Wladislav (Nokia - DE/Ulm)
> > > > <[email protected]> wrote:
> > > >>
> > > >> Commit ea837f1c0503 ("kbuild: make modpost processing configurable")
> > > >> was intended to give KBUILD_MODPOST_WARN flexibility to be configurable.
> > > >> Right now KBUILD_MODPOST_WARN gets just ignored when KBUILD_EXTMOD is
> > > >> set which happens per default when building modules out of the tree.
> > > >>
> > > >> This change gives the opportunity to define module build behaving also
> > > >> in case of out of tree builds and default will become exit on error.
> > > >> Errors which can be detected by the build should be trapped out of the box
> > > >> there, unless somebody wants to notice broken stuff later at runtime.
> > > >
> > > > If an external module refers to a symbol
> > > > provided by another external module,
> > > > this patch will turn the warning into the error by default,
> > > > which is probably a bad idea.
> > >
> > > Indeed, exactly this should happen. You should fix your external module
> > > dependencies by providing their symbols. Please use e.g.
> > > KBUILD_EXTRA_SYMBOLS instead of converting errors to warning and hoping
> > > that things will work.
> >
> > I know this option.
> > What I want to say is, this patch changes the behavior, and
> > may annoy some people.
> >
> > > Or how do you want to make sure module A still
> > > delivers all symbols needed by module B after an update/changes?
> > > Manually comparing the logs after an update or waiting until it turns
> > > out broken during run-time? I wouldn't say this is a good idea :-)
> >
> > OK, so the commit log should record both the behavior change
> > and workarounds.
> >
> > - If an external module being built refers to symbols
> > in another external module, Kbuild previously showed a warning,
> > but going forward it will turn it into an error.
> >
> > - To work around this, you should pass a symbol table via KBUILD_EXTRA_SYMBOLS,
> > or KBUILD_EXTRA_SYMBOLS=1 to turn the error into a warning.
>
> This sounds like a good middle ground. When you do the effort of
> setting KBUILD_EXTRA_SYMBOLS, you are likely interested in proper
> dependency resolution and being notified if it fails.
>
> I would probably still use KBUILD_MODPOST_WARN=0 to force the warnings
> as errors, to have it as the central switch for the behaviour. So the
> behaviour would then become
>
> - If KBUILD_MODPOST_WARN is set to any value, set -w accordingly
> - else, set -w only when building for an external module and
> KBUILD_EXTRA_SYMBOLS is empty
>
> or
>
> ifdef KBUILD_MODPOST_WARN
> KBUILD_MODPOST_WARN:=$(filter-out 0,$(KBUILD_MODPOST_WARN))
> else
> KBUILD_MODPOST_WARN:=$(if $(KBUILD_EXTMOD),$(if $(KBUILD_EXTRA_SYMBOLS),,1))
> endif
Sorry, I think this is too complicated.
I applied this since it is simple.
https://patchwork.kernel.org/patch/10895465/
--
Best Regards
Masahiro Yamada