2022-12-11 13:51:46

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/2] kbuild: change module.order to list *.o instead of *.ko

scripts/Makefile.build replaces the suffix .o with .ko, then
scripts/Makefile.modpost calls the sed command to change .ko back
to the original .o suffix.

Instead of converting the suffixes back-and-forth, store the .o paths
in modules.order, and replace it with .ko in 'make modules_install'.

This avoids the unneeded sed command.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 2 +-
scripts/Makefile.build | 2 +-
scripts/Makefile.modfinal | 6 +++---
scripts/Makefile.modinst | 2 +-
scripts/Makefile.modpost | 7 +++++--
scripts/clang-tools/gen_compile_commands.py | 8 ++++----
scripts/mod/modpost.c | 11 ++++-------
scripts/modules-check.sh | 2 +-
8 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/Makefile b/Makefile
index 8b5930d521fc..669e25970917 100644
--- a/Makefile
+++ b/Makefile
@@ -1569,7 +1569,7 @@ __modinst_pre:
rm -f $(MODLIB)/build ; \
ln -s $(CURDIR) $(MODLIB)/build ; \
fi
- @sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
+ @sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order
@cp -f modules.builtin $(MODLIB)/
@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 799df12b53f3..267eb7aac5b2 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -435,7 +435,7 @@ $(obj)/built-in.a: $(real-obj-y) FORCE
# modules.order unless contained modules are updated.

cmd_modules_order = { $(foreach m, $(real-prereqs), \
- $(if $(filter %/modules.order, $m), cat $m, echo $(patsubst %.o,%.ko,$m));) :; } \
+ $(if $(filter %/modules.order, $m), cat $m, echo $m);) :; } \
> $@

$(obj)/modules.order: $(obj-m) FORCE
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 83f2797e530c..a30d5b08eee9 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -15,7 +15,7 @@ include $(srctree)/scripts/Makefile.lib
# find all modules listed in modules.order
modules := $(call read-file, $(MODORDER))

-__modfinal: $(modules)
+__modfinal: $(modules:%.o=%.ko)
@:

# modname and part-of-module are set to make c_flags define proper module flags
@@ -57,13 +57,13 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)

# Re-generate module BTFs if either module's .ko or vmlinux changed
-$(modules): %.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE
+%.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE
+$(call if_changed_except,ld_ko_o,vmlinux)
ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+$(if $(newer-prereqs),$(call cmd,btf_ko))
endif

-targets += $(modules) $(modules:.ko=.mod.o)
+targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)

# Add FORCE to the prequisites of a target to force it to be always rebuilt.
# ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index 65aac6be78ec..836391e5d209 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -26,7 +26,7 @@ suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst

-modules := $(patsubst $(extmod_prefix)%, $(dst)/%$(suffix-y), $(modules))
+modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))

__modinst: $(modules)
@:
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 55a72f5eb76d..f814a6acd200 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -107,7 +107,10 @@ ifneq ($(KBUILD_MODPOST_WARN)$(missing-input),)
modpost-args += -w
endif

-modorder-if-needed := $(if $(KBUILD_MODULES), $(MODORDER))
+ifdef KBUILD_MODULES
+modorder-if-needed := $(MODORDER)
+modpost-args += -T $(MODORDER)
+endif

MODPOST = scripts/mod/modpost

@@ -119,7 +122,7 @@ quiet_cmd_modpost = MODPOST $@
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.";) \
- sed 's/ko$$/o/' $(or $(modorder-if-needed), /dev/null) | $(MODPOST) $(modpost-args) -T - $(vmlinux.o-if-present)
+ $(MODPOST) $(modpost-args) $(vmlinux.o-if-present)

targets += $(output-symdump)
$(output-symdump): $(modorder-if-needed) $(vmlinux.o-if-present) $(module.symvers-if-present) $(MODPOST) FORCE
diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
index d800b2c0af97..0227522959a4 100755
--- a/scripts/clang-tools/gen_compile_commands.py
+++ b/scripts/clang-tools/gen_compile_commands.py
@@ -138,10 +138,10 @@ def cmdfiles_for_modorder(modorder):
"""
with open(modorder) as f:
for line in f:
- ko = line.rstrip()
- base, ext = os.path.splitext(ko)
- if ext != '.ko':
- sys.exit('{}: module path must end with .ko'.format(ko))
+ obj = line.rstrip()
+ base, ext = os.path.splitext(obj)
+ if ext != '.o':
+ sys.exit('{}: module path must end with .o'.format(obj))
mod = base + '.mod'
# Read from *.mod, to get a list of objects that compose the module.
with open(mod) as m:
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 56d856f2e511..b48838a71bf6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1856,11 +1856,9 @@ static void read_symbols_from_files(const char *filename)
FILE *in = stdin;
char fname[PATH_MAX];

- if (strcmp(filename, "-") != 0) {
- in = fopen(filename, "r");
- if (!in)
- fatal("Can't open filenames file %s: %m", filename);
- }
+ in = fopen(filename, "r");
+ if (!in)
+ fatal("Can't open filenames file %s: %m", filename);

while (fgets(fname, PATH_MAX, in) != NULL) {
if (strends(fname, "\n"))
@@ -1868,8 +1866,7 @@ static void read_symbols_from_files(const char *filename)
read_symbols(fname);
}

- if (in != stdin)
- fclose(in);
+ fclose(in);
}

#define SZ 500
diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
index e06327722263..4c8da90de78e 100755
--- a/scripts/modules-check.sh
+++ b/scripts/modules-check.sh
@@ -16,7 +16,7 @@ check_same_name_modules()
for m in $(sed 's:.*/::' "$1" | sort | uniq -d)
do
echo "error: the following would cause module name conflict:" >&2
- sed -n "/\/$m/s:^: :p" "$1" >&2
+ sed -n "/\/$m/s:^\(.*\)\.o\$: \1.ko:p" "$1" >&2
exit_code=1
done
}
--
2.34.1


2022-12-12 20:24:26

by Luis Chamberlain

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: change module.order to list *.o instead of *.ko

On Sun, Dec 11, 2022 at 10:04:07PM +0900, Masahiro Yamada wrote:
> scripts/Makefile.build replaces the suffix .o with .ko, then
> scripts/Makefile.modpost calls the sed command to change .ko back
> to the original .o suffix.
>
> Instead of converting the suffixes back-and-forth, store the .o paths
> in modules.order, and replace it with .ko in 'make modules_install'.
>
> This avoids the unneeded sed command.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

Reviewed-by: Luis Chamberlain <[email protected]>

Luis

2022-12-14 07:03:21

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: change module.order to list *.o instead of *.ko

On Sun, Dec 11, 2022 at 10:04 PM Masahiro Yamada <[email protected]> wrote:
>
> scripts/Makefile.build replaces the suffix .o with .ko, then
> scripts/Makefile.modpost calls the sed command to change .ko back
> to the original .o suffix.
>
> Instead of converting the suffixes back-and-forth, store the .o paths
> in modules.order, and replace it with .ko in 'make modules_install'.
>
> This avoids the unneeded sed command.
>
> Signed-off-by: Masahiro Yamada <[email protected]>



0day bot reported a breakage when CONFIG_TRIM_UNUSED_KSYMS=y

I will squash the following diff.




diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
index 653fadbad302..12bcfae940ee 100755
--- a/scripts/gen_autoksyms.sh
+++ b/scripts/gen_autoksyms.sh
@@ -48,7 +48,7 @@ cat > "$output_file" << EOT
EOT

{
- [ -n "${read_modorder}" ] && sed 's/ko$/usyms/' modules.order
| xargs cat
+ [ -n "${read_modorder}" ] && sed 's/o$/usyms/' modules.order | xargs cat
echo "$needed_symbols"
[ -n "$ksym_wl" ] && cat "$ksym_wl"
} | sed -e 's/ /\n/g' | sed -n -e '/^$/!p' |





> ---
>
> Makefile | 2 +-
> scripts/Makefile.build | 2 +-
> scripts/Makefile.modfinal | 6 +++---
> scripts/Makefile.modinst | 2 +-
> scripts/Makefile.modpost | 7 +++++--
> scripts/clang-tools/gen_compile_commands.py | 8 ++++----
> scripts/mod/modpost.c | 11 ++++-------
> scripts/modules-check.sh | 2 +-
> 8 files changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 8b5930d521fc..669e25970917 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1569,7 +1569,7 @@ __modinst_pre:
> rm -f $(MODLIB)/build ; \
> ln -s $(CURDIR) $(MODLIB)/build ; \
> fi
> - @sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
> + @sed 's:^\(.*\)\.o$$:kernel/\1.ko:' modules.order > $(MODLIB)/modules.order
> @cp -f modules.builtin $(MODLIB)/
> @cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 799df12b53f3..267eb7aac5b2 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -435,7 +435,7 @@ $(obj)/built-in.a: $(real-obj-y) FORCE
> # modules.order unless contained modules are updated.
>
> cmd_modules_order = { $(foreach m, $(real-prereqs), \
> - $(if $(filter %/modules.order, $m), cat $m, echo $(patsubst %.o,%.ko,$m));) :; } \
> + $(if $(filter %/modules.order, $m), cat $m, echo $m);) :; } \
> > $@
>
> $(obj)/modules.order: $(obj-m) FORCE
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 83f2797e530c..a30d5b08eee9 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -15,7 +15,7 @@ include $(srctree)/scripts/Makefile.lib
> # find all modules listed in modules.order
> modules := $(call read-file, $(MODORDER))
>
> -__modfinal: $(modules)
> +__modfinal: $(modules:%.o=%.ko)
> @:
>
> # modname and part-of-module are set to make c_flags define proper module flags
> @@ -57,13 +57,13 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \
> printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
>
> # Re-generate module BTFs if either module's .ko or vmlinux changed
> -$(modules): %.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE
> +%.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE
> +$(call if_changed_except,ld_ko_o,vmlinux)
> ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +$(if $(newer-prereqs),$(call cmd,btf_ko))
> endif
>
> -targets += $(modules) $(modules:.ko=.mod.o)
> +targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o)
>
> # Add FORCE to the prequisites of a target to force it to be always rebuilt.
> # ---------------------------------------------------------------------------
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index 65aac6be78ec..836391e5d209 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -26,7 +26,7 @@ suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
> suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
> suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
>
> -modules := $(patsubst $(extmod_prefix)%, $(dst)/%$(suffix-y), $(modules))
> +modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
>
> __modinst: $(modules)
> @:
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 55a72f5eb76d..f814a6acd200 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -107,7 +107,10 @@ ifneq ($(KBUILD_MODPOST_WARN)$(missing-input),)
> modpost-args += -w
> endif
>
> -modorder-if-needed := $(if $(KBUILD_MODULES), $(MODORDER))
> +ifdef KBUILD_MODULES
> +modorder-if-needed := $(MODORDER)
> +modpost-args += -T $(MODORDER)
> +endif
>
> MODPOST = scripts/mod/modpost
>
> @@ -119,7 +122,7 @@ quiet_cmd_modpost = MODPOST $@
> 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.";) \
> - sed 's/ko$$/o/' $(or $(modorder-if-needed), /dev/null) | $(MODPOST) $(modpost-args) -T - $(vmlinux.o-if-present)
> + $(MODPOST) $(modpost-args) $(vmlinux.o-if-present)
>
> targets += $(output-symdump)
> $(output-symdump): $(modorder-if-needed) $(vmlinux.o-if-present) $(module.symvers-if-present) $(MODPOST) FORCE
> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
> index d800b2c0af97..0227522959a4 100755
> --- a/scripts/clang-tools/gen_compile_commands.py
> +++ b/scripts/clang-tools/gen_compile_commands.py
> @@ -138,10 +138,10 @@ def cmdfiles_for_modorder(modorder):
> """
> with open(modorder) as f:
> for line in f:
> - ko = line.rstrip()
> - base, ext = os.path.splitext(ko)
> - if ext != '.ko':
> - sys.exit('{}: module path must end with .ko'.format(ko))
> + obj = line.rstrip()
> + base, ext = os.path.splitext(obj)
> + if ext != '.o':
> + sys.exit('{}: module path must end with .o'.format(obj))
> mod = base + '.mod'
> # Read from *.mod, to get a list of objects that compose the module.
> with open(mod) as m:
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 56d856f2e511..b48838a71bf6 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1856,11 +1856,9 @@ static void read_symbols_from_files(const char *filename)
> FILE *in = stdin;
> char fname[PATH_MAX];
>
> - if (strcmp(filename, "-") != 0) {
> - in = fopen(filename, "r");
> - if (!in)
> - fatal("Can't open filenames file %s: %m", filename);
> - }
> + in = fopen(filename, "r");
> + if (!in)
> + fatal("Can't open filenames file %s: %m", filename);
>
> while (fgets(fname, PATH_MAX, in) != NULL) {
> if (strends(fname, "\n"))
> @@ -1868,8 +1866,7 @@ static void read_symbols_from_files(const char *filename)
> read_symbols(fname);
> }
>
> - if (in != stdin)
> - fclose(in);
> + fclose(in);
> }
>
> #define SZ 500
> diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
> index e06327722263..4c8da90de78e 100755
> --- a/scripts/modules-check.sh
> +++ b/scripts/modules-check.sh
> @@ -16,7 +16,7 @@ check_same_name_modules()
> for m in $(sed 's:.*/::' "$1" | sort | uniq -d)
> do
> echo "error: the following would cause module name conflict:" >&2
> - sed -n "/\/$m/s:^: :p" "$1" >&2
> + sed -n "/\/$m/s:^\(.*\)\.o\$: \1.ko:p" "$1" >&2
> exit_code=1
> done
> }
> --
> 2.34.1
>


--
Best Regards
Masahiro Yamada

2022-12-31 18:28:21

by Marc Kleine-Budde

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: change module.order to list *.o instead of *.ko

On 11.12.2022 22:04:07, Masahiro Yamada wrote:
> scripts/Makefile.build replaces the suffix .o with .ko, then
> scripts/Makefile.modpost calls the sed command to change .ko back
> to the original .o suffix.
>
> Instead of converting the suffixes back-and-forth, store the .o paths
> in modules.order, and replace it with .ko in 'make modules_install'.
>
> This avoids the unneeded sed command.

This breaks direct compilation of kernel modules (on current Linus's
master):

| $ make drivers/net/can/dev/can-dev.ko
| [...]
| CC [M] drivers/net/can/dev/skb.o
| CC [M] drivers/net/can/dev/calc_bittiming.o
| CC [M] drivers/net/can/dev/bittiming.o
| CC [M] drivers/net/can/dev/dev.o
| CC [M] drivers/net/can/dev/length.o
| CC [M] drivers/net/can/dev/netlink.o
| LD [M] drivers/net/can/dev/can-dev.o
| make[5]: 'drivers/net/can/dev/can-dev.mod' is up to date.
| LDS scripts/module.lds
| MODPOST Module.symvers
| drivers/net/can/dev/can-dev.ko: No such file or directory
| make[1]: *** [.../linux/scripts/Makefile.modpost:129: Module.symvers] Error 1
| make: *** [.../linux/Makefile:1982: single_modules] Error 2

According to "make help" it should be possible:

| dir/file.ko - Build module including final link

I've bisected it to:

| first bad commit: [f65a486821cfd363833079b2a7b0769250ee21c9] kbuild: change module.order to list *.o instead of *.ko

regards,
Marc

--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |


Attachments:
(No filename) (1.67 kB)
signature.asc (499.00 B)
Download all attachments

2023-01-01 06:51:55

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/2] kbuild: change module.order to list *.o instead of *.ko

On Sun, Jan 1, 2023 at 2:48 AM Marc Kleine-Budde <[email protected]> wrote:
>
> On 11.12.2022 22:04:07, Masahiro Yamada wrote:
> > scripts/Makefile.build replaces the suffix .o with .ko, then
> > scripts/Makefile.modpost calls the sed command to change .ko back
> > to the original .o suffix.
> >
> > Instead of converting the suffixes back-and-forth, store the .o paths
> > in modules.order, and replace it with .ko in 'make modules_install'.
> >
> > This avoids the unneeded sed command.
>
> This breaks direct compilation of kernel modules (on current Linus's
> master):
>
> | $ make drivers/net/can/dev/can-dev.ko
> | [...]
> | CC [M] drivers/net/can/dev/skb.o
> | CC [M] drivers/net/can/dev/calc_bittiming.o
> | CC [M] drivers/net/can/dev/bittiming.o
> | CC [M] drivers/net/can/dev/dev.o
> | CC [M] drivers/net/can/dev/length.o
> | CC [M] drivers/net/can/dev/netlink.o
> | LD [M] drivers/net/can/dev/can-dev.o
> | make[5]: 'drivers/net/can/dev/can-dev.mod' is up to date.
> | LDS scripts/module.lds
> | MODPOST Module.symvers
> | drivers/net/can/dev/can-dev.ko: No such file or directory
> | make[1]: *** [.../linux/scripts/Makefile.modpost:129: Module.symvers] Error 1
> | make: *** [.../linux/Makefile:1982: single_modules] Error 2
>
> According to "make help" it should be possible:
>
> | dir/file.ko - Build module including final link
>
> I've bisected it to:
>
> | first bad commit: [f65a486821cfd363833079b2a7b0769250ee21c9] kbuild: change module.order to list *.o instead of *.ko
>
> regards,
> Marc



Thanks for the report. I submitted a fix.

https://lore.kernel.org/lkml/[email protected]/T/#u




>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Embedded Linux | https://www.pengutronix.de |
> Vertretung West/Dortmund | Phone: +49-231-2826-924 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |



--
Best Regards
Masahiro Yamada