[v3,07/12] kbuild: modpost: read modules.order instead of $(MODVERDIR)/*.mod
diff mbox series

Message ID 20190717061800.10018-8-yamada.masahiro@socionext.com
State New
Headers show
Series
  • kbuild: create *.mod with directory path and remove MODVERDIR
Related show

Commit Message

Masahiro Yamada July 17, 2019, 6:17 a.m. UTC
Towards the goal of removing MODVERDIR, read out modules.order to get
the list of modules to be processed. This is simpler than parsing *.mod
files in $(MODVERDIR).

For external modules, $(KBUILD_EXTMOD)/modules.order should be read.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

Changes in v3:
  - Add ifdef CONFIG_MODULES to avoid warning

Changes in v2: None

 scripts/Makefile.modpost | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Jan Kiszka July 25, 2019, 8:39 a.m. UTC | #1
On 17.07.19 08:17, Masahiro Yamada wrote:
> Towards the goal of removing MODVERDIR, read out modules.order to get
> the list of modules to be processed. This is simpler than parsing *.mod
> files in $(MODVERDIR).
> 
> For external modules, $(KBUILD_EXTMOD)/modules.order should be read.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> Changes in v3:
>   - Add ifdef CONFIG_MODULES to avoid warning
> 
> Changes in v2: None
> 
>  scripts/Makefile.modpost | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index fec6ec2ffa47..5841508ffca9 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -8,9 +8,10 @@
>  # b) A <module>.o file which is the .o files above linked together
>  # c) A <module>.mod file in $(MODVERDIR)/, listing the name of the
>  #    the preliminary <module>.o file, plus all .o files
> +# d) modules.order, which lists all the modules
>  
>  # Stage 2 is handled by this file and does the following
> -# 1) Find all modules from the files listed in $(MODVERDIR)/
> +# 1) Find all modules listed in modules.order
>  # 2) modpost is then used to
>  # 3)  create one <module>.mod.c file pr. module
>  # 4)  create one Module.symvers file with CRC for all exported symbols
> @@ -60,10 +61,12 @@ include scripts/Makefile.lib
>  kernelsymfile := $(objtree)/Module.symvers
>  modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
>  
> -# Step 1), find all modules listed in $(MODVERDIR)/
> -MODLISTCMD := find $(MODVERDIR) -name '*.mod' | xargs -r grep -h '\.ko$$' | sort -u
> -__modules := $(shell $(MODLISTCMD))
> -modules   := $(patsubst %.o,%.ko, $(wildcard $(__modules:.ko=.o)))
> +modorder := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)modules.order
> +
> +# Step 1), find all modules listed in modules.order
> +ifdef CONFIG_MODULES
> +modules := $(sort $(shell cat $(modorder)))
> +endif
>  
>  # Stop after building .o files if NOFINAL is set. Makes compile tests quicker
>  _modpost: $(if $(KBUILD_MODPOST_NOFINAL), $(modules:.ko:.o),$(modules))
> @@ -84,7 +87,7 @@ MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
>  
>  # We can go over command line length here, so be careful.
>  quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
> -      cmd_modpost = $(MODLISTCMD) | sed 's/\.ko$$/.o/' | $(modpost) $(MODPOST_OPT) -s -T -
> +      cmd_modpost = sed 's/ko$$/o/' $(modorder) | $(modpost) $(MODPOST_OPT) -s -T -
>  
>  PHONY += __modpost
>  __modpost: $(modules:.ko=.o) FORCE
> 

This affects also external modules builds: I have patterns here that do

[Makefile]
subdir-y := some-module

[some-module/Makefile]
obj-m := some-module.o

and since this patch, the final some-module.ko is no longer built. Am I missing
something in the kbuild makefiles, or is this a regression?

Jan
Masahiro Yamada July 25, 2019, 3:18 p.m. UTC | #2
Hi Jan,

On Thu, Jul 25, 2019 at 5:39 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >
>
> This affects also external modules builds: I have patterns here that do
>
> [Makefile]
> subdir-y := some-module
>
> [some-module/Makefile]
> obj-m := some-module.o
>
> and since this patch, the final some-module.ko is no longer built. Am I missing
> something in the kbuild makefiles, or is this a regression?

Thanks for the report.
Interesting. I have never imagined that Makefiles were written like that.

I just wrote a fix-up, but I have not determined to apply it.
https://patchwork.kernel.org/patch/11059033/

It is easy to fixup your Makefile, though.

--
Best Regards
Masahiro Yamada
Jan Kiszka July 25, 2019, 3:27 p.m. UTC | #3
On 25.07.19 17:18, Masahiro Yamada wrote:
> Hi Jan,
> 
> On Thu, Jul 25, 2019 at 5:39 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>
>>
>> This affects also external modules builds: I have patterns here that do
>>
>> [Makefile]
>> subdir-y := some-module
>>
>> [some-module/Makefile]
>> obj-m := some-module.o
>>
>> and since this patch, the final some-module.ko is no longer built. Am I missing
>> something in the kbuild makefiles, or is this a regression?
> 
> Thanks for the report.
> Interesting. I have never imagined that Makefiles were written like that.
> 
> I just wrote a fix-up, but I have not determined to apply it.
> https://patchwork.kernel.org/patch/11059033/
> 
> It is easy to fixup your Makefile, though.

Thanks for addressing this quickly! I'm happy to adjust our code [1]. Is the
suggested pattern usable with recent stable kernels as well, say down to 4.4 at
least?

Jan

[1] https://github.com/siemens/jailhouse/blob/master/Kbuild#L54
Masahiro Yamada July 25, 2019, 3:49 p.m. UTC | #4
On Fri, Jul 26, 2019 at 12:27 AM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 25.07.19 17:18, Masahiro Yamada wrote:
> > Hi Jan,
> >
> > On Thu, Jul 25, 2019 at 5:39 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>>
> >>
> >> This affects also external modules builds: I have patterns here that do
> >>
> >> [Makefile]
> >> subdir-y := some-module
> >>
> >> [some-module/Makefile]
> >> obj-m := some-module.o
> >>
> >> and since this patch, the final some-module.ko is no longer built. Am I missing
> >> something in the kbuild makefiles, or is this a regression?
> >
> > Thanks for the report.
> > Interesting. I have never imagined that Makefiles were written like that.
> >
> > I just wrote a fix-up, but I have not determined to apply it.
> > https://patchwork.kernel.org/patch/11059033/
> >
> > It is easy to fixup your Makefile, though.
>
> Thanks for addressing this quickly! I'm happy to adjust our code [1]. Is the
> suggested pattern usable with recent stable kernels as well, say down to 4.4 at
> least?

Yes, should work for any kernel version.



> Jan
>
> [1] https://github.com/siemens/jailhouse/blob/master/Kbuild#L54

Maybe,

obj-m := driver/ hypervisor/ configs/ inmates/ tools/

   and

$(patsubst %/,$(obj)/%,$(obj-m)): $(GEN_CONFIG_MK)


Not tested at all.

Patch
diff mbox series

diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index fec6ec2ffa47..5841508ffca9 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -8,9 +8,10 @@ 
 # b) A <module>.o file which is the .o files above linked together
 # c) A <module>.mod file in $(MODVERDIR)/, listing the name of the
 #    the preliminary <module>.o file, plus all .o files
+# d) modules.order, which lists all the modules
 
 # Stage 2 is handled by this file and does the following
-# 1) Find all modules from the files listed in $(MODVERDIR)/
+# 1) Find all modules listed in modules.order
 # 2) modpost is then used to
 # 3)  create one <module>.mod.c file pr. module
 # 4)  create one Module.symvers file with CRC for all exported symbols
@@ -60,10 +61,12 @@  include scripts/Makefile.lib
 kernelsymfile := $(objtree)/Module.symvers
 modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
 
-# Step 1), find all modules listed in $(MODVERDIR)/
-MODLISTCMD := find $(MODVERDIR) -name '*.mod' | xargs -r grep -h '\.ko$$' | sort -u
-__modules := $(shell $(MODLISTCMD))
-modules   := $(patsubst %.o,%.ko, $(wildcard $(__modules:.ko=.o)))
+modorder := $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)modules.order
+
+# Step 1), find all modules listed in modules.order
+ifdef CONFIG_MODULES
+modules := $(sort $(shell cat $(modorder)))
+endif
 
 # Stop after building .o files if NOFINAL is set. Makes compile tests quicker
 _modpost: $(if $(KBUILD_MODPOST_NOFINAL), $(modules:.ko:.o),$(modules))
@@ -84,7 +87,7 @@  MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
 
 # We can go over command line length here, so be careful.
 quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
-      cmd_modpost = $(MODLISTCMD) | sed 's/\.ko$$/.o/' | $(modpost) $(MODPOST_OPT) -s -T -
+      cmd_modpost = sed 's/ko$$/o/' $(modorder) | $(modpost) $(MODPOST_OPT) -s -T -
 
 PHONY += __modpost
 __modpost: $(modules:.ko=.o) FORCE