[PATCH 0/7] kbuild: more misc cleanups

[PATCH 0/7] kbuild: more misc cleanups

[Masahiro Yamada]

I sent the first batch of cleanups:
https://lore.kernel.org/linux-kbuild/20220405113359.2880241-1-masahiroy@kernel.org/T/#t

I took 01-06, 09-10.
I dropped 07, 08.

This is the second batch.



Masahiro Yamada (7):
  kbuild: reuse suffix-search to refactor multi_depend
  kbuild: make multi_depend work with targets in subdirectory
  kbuild: reuse real-search to simplify cmd_mod
  kbuild: split the second line of *.mod into *.usyms
  kbuild: get rid of duplication in *.mod files
  kbuild: make *.mod not depend on *.o
  kbuild: read *.mod to get objects passed to $(LD) or $(AR)

 .gitignore                  |  1 +
 Makefile                    |  5 +++--
 scripts/Makefile.build      | 31 ++++++++++++++-----------------
 scripts/Makefile.lib        |  6 +++---
 scripts/adjust_autoksyms.sh |  2 +-
 scripts/gen_autoksyms.sh    | 18 +++++++++++-------
 scripts/mod/sumversion.c    | 11 ++---------
 7 files changed, 35 insertions(+), 39 deletions(-)

-- 
2.32.0

[PATCH 1/7] kbuild: reuse suffix-search to refactor multi_depend

[Masahiro Yamada]

The complicated part of multi_depend is the same as suffix-search.

Reuse it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.lib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 9f69ecdd7977..d56cda3c1e8a 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -238,7 +238,7 @@ endif
 define multi_depend
 $(foreach m, $(notdir $1), \
 	$(eval $(obj)/$m: \
-	$(addprefix $(obj)/, $(foreach s, $3, $($(m:%$(strip $2)=%$(s)))))))
+	$(addprefix $(obj)/, $(call suffix-search, $m, $2, $3))))
 endef
 
 # Copy a file
-- 
2.32.0

[PATCH 2/7] kbuild: make multi_depend work with targets in subdirectory

[Masahiro Yamada]

Precisely speaking, when you get the stem of the path, you should use
$(patsubst $(obj)/%,%,...) instead of $(notdir ...).

I do not see this usecase, but if you create a composite object in a
subdirectory, the Makefile should look like this:

   obj-$(CONFIG_FOO) += dir/foo.o
   dir/foo-objs      := dir/foo1.o dir/foo2.o

The member objects should be assigned to dir/foo-objs instead of
foo-objs.

This syntax is more consistent with commit 54b8ae66ae1a ("kbuild:
change *FLAGS_<basetarget>.o to take the path relative to $(obj)").

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.lib | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index d56cda3c1e8a..0453a1904646 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -236,9 +236,9 @@ endif
 # Usage:
 #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
 define multi_depend
-$(foreach m, $(notdir $1), \
-	$(eval $(obj)/$m: \
-	$(addprefix $(obj)/, $(call suffix-search, $m, $2, $3))))
+$(foreach m, $1, \
+	$(eval $m: \
+	$(addprefix $(obj)/, $(call suffix-search, $(patsubst $(obj)/%,%,$m), $2, $3))))
 endef
 
 # Copy a file
-- 
2.32.0

[PATCH 3/7] kbuild: reuse real-search to simplify cmd_mod

[Masahiro Yamada]

The first command in cmd_mod is similar to the real-search macro.
Reuse it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f15c245dc17e..857329844789 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -306,7 +306,7 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
 endif
 
 cmd_mod = { \
-	echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), $(@:.mod=.o)); \
+	echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)); \
 	$(undefined_syms) echo; \
 	} > $@
 
-- 
2.32.0

[PATCH 4/7] kbuild: split the second line of *.mod into *.usyms

[Masahiro Yamada]

The *.mod files have two lines; the first line lists the member objects
of the module, and the second line, if CONFIG_TRIM_UNUSED_KSYMS=y, lists
the undefined symbols.

Currently, we generate *.mod after constructing composite modules,
otherwise, we cannot compute the second line. No prerequisite is
required to print the first line.

They are orthogonal. Splitting them into separate commands will ease
further cleanups.

This commit splits the list of undefined symbols out to *.usyms files.

Previously, the list of undefined symbols ended up with a very long
line, but now it has one symbol per line.

Use sed like we did before commit 7d32358be8ac ("kbuild: avoid split
lines in .mod files").

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
---

 .gitignore                  |  1 +
 Makefile                    |  2 +-
 scripts/Makefile.build      | 17 +++++++++--------
 scripts/adjust_autoksyms.sh |  2 +-
 scripts/gen_autoksyms.sh    | 18 +++++++++++-------
 scripts/mod/sumversion.c    | 11 ++---------
 6 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/.gitignore b/.gitignore
index 7afd412dadd2..265959544978 100644
--- a/.gitignore
+++ b/.gitignore
@@ -45,6 +45,7 @@
 *.symversions
 *.tab.[ch]
 *.tar
+*.usyms
 *.xz
 *.zst
 Module.symvers
diff --git a/Makefile b/Makefile
index d9336e783be3..82ee893909e9 100644
--- a/Makefile
+++ b/Makefile
@@ -1848,7 +1848,7 @@ clean: $(clean-dirs)
 		-o -name '*.ko.*' \
 		-o -name '*.dtb' -o -name '*.dtbo' -o -name '*.dtb.S' -o -name '*.dt.yaml' \
 		-o -name '*.dwo' -o -name '*.lst' \
-		-o -name '*.su' -o -name '*.mod' \
+		-o -name '*.su' -o -name '*.mod' -o -name '*.usyms' \
 		-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
 		-o -name '*.lex.c' -o -name '*.tab.[ch]' \
 		-o -name '*.asn1.[ch]' \
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 857329844789..6ae92d119dfa 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -85,7 +85,8 @@ ifdef need-builtin
 targets-for-builtin += $(obj)/built-in.a
 endif
 
-targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
+targets-for-modules := $(foreach x, mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
+				$(patsubst %.o, %.$x, $(filter %.o, $(obj-m))))
 
 ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
 targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
@@ -256,9 +257,6 @@ endif
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 cmd_gen_ksymdeps = \
 	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
-
-# List module undefined symbols
-undefined_syms = $(NM) $< | $(AWK) '$$1 == "U" { printf("%s%s", x++ ? " " : "", $$2) }';
 endif
 
 define rule_cc_o_c
@@ -305,14 +303,17 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
 	$(call if_changed,cc_prelink_modules)
 endif
 
-cmd_mod = { \
-	echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)); \
-	$(undefined_syms) echo; \
-	} > $@
+cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) > $@
 
 $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
 	$(call if_changed,mod)
 
+# List module undefined symbols
+cmd_undefined_syms = $(NM) $< | sed -n 's/^  *U //p' > $@
+
+$(obj)/%.usyms: $(obj)/%$(mod-prelink-ext).o FORCE
+	$(call if_changed,undefined_syms)
+
 quiet_cmd_cc_lst_c = MKLST   $@
       cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
 		     $(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 59fdb875e818..f1b5ac818411 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -35,7 +35,7 @@ case "$KBUILD_VERBOSE" in
 esac
 
 # Generate a new symbol list file
-$CONFIG_SHELL $srctree/scripts/gen_autoksyms.sh "$new_ksyms_file"
+$CONFIG_SHELL $srctree/scripts/gen_autoksyms.sh --modorder "$new_ksyms_file"
 
 # Extract changes between old and new list and touch corresponding
 # dependency files.
diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
index 120225c541c5..faacf7062122 100755
--- a/scripts/gen_autoksyms.sh
+++ b/scripts/gen_autoksyms.sh
@@ -2,13 +2,10 @@
 # SPDX-License-Identifier: GPL-2.0-only
 
 # Create an autoksyms.h header file from the list of all module's needed symbols
-# as recorded on the second line of *.mod files and the user-provided symbol
-# whitelist.
+# as recorded in *.usyms files and the user-provided symbol whitelist.
 
 set -e
 
-output_file="$1"
-
 # Use "make V=1" to debug this script.
 case "$KBUILD_VERBOSE" in
 *1*)
@@ -16,6 +13,15 @@ case "$KBUILD_VERBOSE" in
 	;;
 esac
 
+read_modorder=
+
+if [ "$1" = --modorder ]; then
+	shift
+	read_modorder=1
+fi
+
+output_file="$1"
+
 needed_symbols=
 
 # Special case for modversions (see modpost.c)
@@ -41,10 +47,8 @@ cat > "$output_file" << EOT
 
 EOT
 
-[ -f modules.order ] && modlist=modules.order || modlist=/dev/null
-
 {
-	sed 's/ko$/mod/' $modlist | xargs -n1 sed -n -e '2p'
+	[ -n "${read_modorder}" ] && sed 's/ko$/usyms/' modules.order | xargs cat
 	echo "$needed_symbols"
 	[ -n "$ksym_wl" ] && cat "$ksym_wl"
 } | sed -e 's/ /\n/g' | sed -n -e '/^$/!p' |
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 905c0ec291e1..0125698f2037 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -387,7 +387,7 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
 /* Calc and record src checksum. */
 void get_src_version(const char *modname, char sum[], unsigned sumlen)
 {
-	char *buf, *pos, *firstline;
+	char *buf;
 	struct md4_ctx md;
 	char *fname;
 	char filelist[PATH_MAX + 1];
@@ -397,15 +397,8 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
 
 	buf = read_text_file(filelist);
 
-	pos = buf;
-	firstline = get_line(&pos);
-	if (!firstline) {
-		warn("bad ending versions file for %s\n", modname);
-		goto free;
-	}
-
 	md4_init(&md);
-	while ((fname = strsep(&firstline, " "))) {
+	while ((fname = strsep(&buf, " \n"))) {
 		if (!*fname)
 			continue;
 		if (!(is_static_library(fname)) &&
-- 
2.32.0

[PATCH 5/7] kbuild: get rid of duplication in *.mod files

[Masahiro Yamada]

It is allowed to add the same objects multiple times to obj-y / obj-m:

  obj-y += foo.o foo.o foo.o
  obj-m += bar.o bar.o bar.o

It is also allowed to add the same objects multiple times to a composite
module:

  obj-m    += foo.o
  foo-objs := foo1.o foo2.o foo2.o foo1.o

This flexibility is useful because the same object might be selected by
different CONFIG options, like this:

  obj-m               += foo.o
  foo-y               := foo1.o
  foo-$(CONFIG_FOO_X) += foo2.o
  foo-$(CONFIG_FOO_Y) += foo2.o

The duplicated objects are omitted at link time. It works naturally in
Makefiles because GNU Make removes duplication in $^ without changing
the order.

It is working well, almost...

A small flaw I notice is, *.mod contains duplication in such a case.

This is probably not a big deal. As far as I know, the only small
problem is scripts/mod/sumversion.c parses the same file multiple
times.

I am fixing this because I plan to reuse *.mod for other purposes,
where the duplication can be problematic.

The code change is quite simple. We already use awk to drop duplicated
lines in modules.order (see cmd_modules_order in the same file).
I copied the code, but changed RS to use spaces as record separators.

I also changed the file format to list one object per line.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build   | 3 ++-
 scripts/mod/sumversion.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 6ae92d119dfa..f7a30f378e20 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -303,7 +303,8 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
 	$(call if_changed,cc_prelink_modules)
 endif
 
-cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) > $@
+cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
+	$(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
 
 $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
 	$(call if_changed,mod)
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 0125698f2037..79bb9eaa65ac 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -398,7 +398,7 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
 	buf = read_text_file(filelist);
 
 	md4_init(&md);
-	while ((fname = strsep(&buf, " \n"))) {
+	while ((fname = strsep(&buf, "\n"))) {
 		if (!*fname)
 			continue;
 		if (!(is_static_library(fname)) &&
-- 
2.32.0

[PATCH 6/7] kbuild: make *.mod not depend on *.o

[Masahiro Yamada]

The dependency

    $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o

... exists because *.mod files previously contained undefined symbols,
which are computed from *.o files when CONFIG_TRIM_UNUSED_KSYMS=y.

Now that the undefined symbols are put into separate *.usyms files,
there is no reason to make *.mod depend on *.o files.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 Makefile               | 3 ++-
 scripts/Makefile.build | 5 ++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 82ee893909e9..e915aacd02b0 100644
--- a/Makefile
+++ b/Makefile
@@ -1792,7 +1792,8 @@ ifdef single-build
 
 # .ko is special because modpost is needed
 single-ko := $(sort $(filter %.ko, $(MAKECMDGOALS)))
-single-no-ko := $(sort $(patsubst %.ko,%.mod, $(MAKECMDGOALS)))
+single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS)) \
+		$(foreach x, o mod, $(patsubst %.ko, %.$x, $(single-ko)))
 
 $(single-ko): single_modpost
 	@:
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f7a30f378e20..3da731cf6978 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -85,7 +85,7 @@ ifdef need-builtin
 targets-for-builtin += $(obj)/built-in.a
 endif
 
-targets-for-modules := $(foreach x, mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
+targets-for-modules := $(foreach x, o mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
 				$(patsubst %.o, %.$x, $(filter %.o, $(obj-m))))
 
 ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
@@ -306,7 +306,7 @@ endif
 cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
 	$(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
 
-$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
+$(obj)/%.mod: FORCE
 	$(call if_changed,mod)
 
 # List module undefined symbols
@@ -469,7 +469,6 @@ $(multi-obj-m): FORCE
 	$(call if_changed,link_multi-m)
 $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
 
-targets += $(multi-obj-m)
 targets := $(filter-out $(PHONY), $(targets))
 
 # Add intermediate targets:
-- 
2.32.0

[PATCH 7/7] kbuild: read *.mod to get objects passed to $(LD) or $(AR)

[Masahiro Yamada]

ld and ar support @file, which command-line options are read from.

Now that *.mod lists the member objects in the correct order, without
duplication, it is ready to be passed to ld and ar.

By using the @file syntax, people will not be worried about the pitfall
described in the NOTE.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/Makefile.build | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3da731cf6978..f6a506318795 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -450,22 +450,18 @@ quiet_cmd_ar_lib = AR      $@
 $(obj)/lib.a: $(lib-y) FORCE
 	$(call if_changed,ar_lib)
 
-# NOTE:
-# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
-# module is turned into a multi object module, $^ will contain header file
-# dependencies recorded in the .*.cmd file.
 ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
 quiet_cmd_link_multi-m = AR [M]  $@
 cmd_link_multi-m =						\
 	$(cmd_update_lto_symversions);				\
 	rm -f $@; 						\
-	$(AR) cDPrsT $@ $(filter %.o,$^)
+	$(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
 else
 quiet_cmd_link_multi-m = LD [M]  $@
-      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
+      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@)
 endif
 
-$(multi-obj-m): FORCE
+$(multi-obj-m): %.o: %.mod FORCE
 	$(call if_changed,link_multi-m)
 $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
 
-- 
2.32.0

Re: [PATCH 7/7] kbuild: read *.mod to get objects passed to $(LD) or $(AR)

[Jeff Johnson]

On 4/6/2022 8:30 AM, Masahiro Yamada wrote:
> ld and ar support @file, which command-line options are read from.
> 
> Now that *.mod lists the member objects in the correct order, without
> duplication, it is ready to be passed to ld and ar.
> 
> By using the @file syntax, people will not be worried about the pitfall
> described in the NOTE.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>   scripts/Makefile.build | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3da731cf6978..f6a506318795 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -450,22 +450,18 @@ quiet_cmd_ar_lib = AR      $@
>   $(obj)/lib.a: $(lib-y) FORCE
>   	$(call if_changed,ar_lib)
>   
> -# NOTE:
> -# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> -# module is turned into a multi object module, $^ will contain header file
> -# dependencies recorded in the .*.cmd file.
>   ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
>   quiet_cmd_link_multi-m = AR [M]  $@
>   cmd_link_multi-m =						\
>   	$(cmd_update_lto_symversions);				\
>   	rm -f $@; 						\
> -	$(AR) cDPrsT $@ $(filter %.o,$^)
> +	$(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
>   else
>   quiet_cmd_link_multi-m = LD [M]  $@
> -      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
> +      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@)
>   endif
>   
> -$(multi-obj-m): FORCE
> +$(multi-obj-m): %.o: %.mod FORCE
>   	$(call if_changed,link_multi-m)
>   $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
>   

Looks like this also addresses the out-of-tree issue described in 
<https://lore.kernel.org/linux-kbuild/1610500731-30960-2-git-send-email-jjohnson@codeaurora.org/>

:)

/jeff

Re: [PATCH 1/7] kbuild: reuse suffix-search to refactor multi_depend

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The complicated part of multi_depend is the same as suffix-search.
>
> Reuse it.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/Makefile.lib | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 9f69ecdd7977..d56cda3c1e8a 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -238,7 +238,7 @@ endif
>  define multi_depend
>  $(foreach m, $(notdir $1), \
>         $(eval $(obj)/$m: \
> -       $(addprefix $(obj)/, $(foreach s, $3, $($(m:%$(strip $2)=%$(s)))))))
> +       $(addprefix $(obj)/, $(call suffix-search, $m, $2, $3))))
>  endef
>
>  # Copy a file
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 7/7] kbuild: read *.mod to get objects passed to $(LD) or $(AR)

[Masahiro Yamada]

On Thu, Apr 7, 2022 at 3:13 AM Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>
> On 4/6/2022 8:30 AM, Masahiro Yamada wrote:
> > ld and ar support @file, which command-line options are read from.
> >
> > Now that *.mod lists the member objects in the correct order, without
> > duplication, it is ready to be passed to ld and ar.
> >
> > By using the @file syntax, people will not be worried about the pitfall
> > described in the NOTE.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >   scripts/Makefile.build | 10 +++-------
> >   1 file changed, 3 insertions(+), 7 deletions(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 3da731cf6978..f6a506318795 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -450,22 +450,18 @@ quiet_cmd_ar_lib = AR      $@
> >   $(obj)/lib.a: $(lib-y) FORCE
> >       $(call if_changed,ar_lib)
> >
> > -# NOTE:
> > -# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> > -# module is turned into a multi object module, $^ will contain header file
> > -# dependencies recorded in the .*.cmd file.
> >   ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
> >   quiet_cmd_link_multi-m = AR [M]  $@
> >   cmd_link_multi-m =                                          \
> >       $(cmd_update_lto_symversions);                          \
> >       rm -f $@;                                               \
> > -     $(AR) cDPrsT $@ $(filter %.o,$^)
> > +     $(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
> >   else
> >   quiet_cmd_link_multi-m = LD [M]  $@
> > -      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
> > +      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@)
> >   endif
> >
> > -$(multi-obj-m): FORCE
> > +$(multi-obj-m): %.o: %.mod FORCE
> >       $(call if_changed,link_multi-m)
> >   $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
> >
>
> Looks like this also addresses the out-of-tree issue described in
> <https://lore.kernel.org/linux-kbuild/1610500731-30960-2-git-send-email-jjohnson@codeaurora.org/>
>
> :)
>
> /jeff

But, not perfectly.

This patch fixed the linker part, but the same issue is remaining in cmd_mod.

The following patch is an easy fix-up.



diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index f6a506318795..468f9e646370 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -303,8 +303,8 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
        $(call if_changed,cc_prelink_modules)
 endif

-cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o,
-objs -y -m)) | \
-       $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
+cmd_mod = echo $(call real-search, $*.o, .o, -objs -y -m) | \
+       $(AWK) -v RS='( |\n)' '!x[$$0]++ { print("$(obj)/"$$0) }' > $@

 $(obj)/%.mod: FORCE
        $(call if_changed,mod)





But, please do not submit a patch yet.


This patch series is just preparation for yet another
bigger clean-up.

One of my big goals is to clean up Clang LTO builds.

Clang LTO made Kbuild really ugly.

I am re-implementing various parts, but I have not
completed the work yet.

Meanwhile, I incrementally submit prerequisite
refactoring patches.

The issues of external module builds _might_ be fixed
as a side-effect of other refactoring, but I am more
interested in what the final code will look like.

--
Best Regards
Masahiro Yamada

Re: [PATCH 2/7] kbuild: make multi_depend work with targets in subdirectory

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Precisely speaking, when you get the stem of the path, you should use
> $(patsubst $(obj)/%,%,...) instead of $(notdir ...).
>
> I do not see this usecase, but if you create a composite object in a
> subdirectory, the Makefile should look like this:
>
>    obj-$(CONFIG_FOO) += dir/foo.o
>    dir/foo-objs      := dir/foo1.o dir/foo2.o
>
> The member objects should be assigned to dir/foo-objs instead of
> foo-objs.
>
> This syntax is more consistent with commit 54b8ae66ae1a ("kbuild:
> change *FLAGS_<basetarget>.o to take the path relative to $(obj)").
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/Makefile.lib | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index d56cda3c1e8a..0453a1904646 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -236,9 +236,9 @@ endif
>  # Usage:
>  #   $(call multi_depend, multi_used_targets, suffix_to_remove, suffix_to_add)
>  define multi_depend
> -$(foreach m, $(notdir $1), \
> -       $(eval $(obj)/$m: \
> -       $(addprefix $(obj)/, $(call suffix-search, $m, $2, $3))))
> +$(foreach m, $1, \
> +       $(eval $m: \
> +       $(addprefix $(obj)/, $(call suffix-search, $(patsubst $(obj)/%,%,$m), $2, $3))))
>  endef
>
>  # Copy a file
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/7] kbuild: reuse real-search to simplify cmd_mod

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The first command in cmd_mod is similar to the real-search macro.
> Reuse it.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/Makefile.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index f15c245dc17e..857329844789 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -306,7 +306,7 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
>  endif
>
>  cmd_mod = { \
> -       echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), $(@:.mod=.o)); \
> +       echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)); \
>         $(undefined_syms) echo; \
>         } > $@
>
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 4/7] kbuild: split the second line of *.mod into *.usyms

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The *.mod files have two lines; the first line lists the member objects
> of the module, and the second line, if CONFIG_TRIM_UNUSED_KSYMS=y, lists
> the undefined symbols.

Enabling EXPERT and TRIM_UNUSED_KSYMS, I didn't find any .mod files
containing a second line that wasn't empty. Is there an example that
has such symbol list that I can use to verify?
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 5/7] kbuild: get rid of duplication in *.mod files

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> It is allowed to add the same objects multiple times to obj-y / obj-m:
>
>   obj-y += foo.o foo.o foo.o
>   obj-m += bar.o bar.o bar.o
>
> It is also allowed to add the same objects multiple times to a composite
> module:
>
>   obj-m    += foo.o
>   foo-objs := foo1.o foo2.o foo2.o foo1.o
>
> This flexibility is useful because the same object might be selected by
> different CONFIG options, like this:
>
>   obj-m               += foo.o
>   foo-y               := foo1.o
>   foo-$(CONFIG_FOO_X) += foo2.o
>   foo-$(CONFIG_FOO_Y) += foo2.o
>
> The duplicated objects are omitted at link time. It works naturally in
> Makefiles because GNU Make removes duplication in $^ without changing
> the order.
>
> It is working well, almost...
>
> A small flaw I notice is, *.mod contains duplication in such a case.
>
> This is probably not a big deal. As far as I know, the only small
> problem is scripts/mod/sumversion.c parses the same file multiple
> times.
>
> I am fixing this because I plan to reuse *.mod for other purposes,
> where the duplication can be problematic.
>
> The code change is quite simple. We already use awk to drop duplicated
> lines in modules.order (see cmd_modules_order in the same file).

Why does the top level Makefile reassign cmd_modules_order rather than
use the original value in scripts/Makefile.build?

> I copied the code, but changed RS to use spaces as record separators.
>
> I also changed the file format to list one object per line.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/Makefile.build   | 3 ++-
>  scripts/mod/sumversion.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 6ae92d119dfa..f7a30f378e20 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -303,7 +303,8 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
>         $(call if_changed,cc_prelink_modules)
>  endif
>
> -cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) > $@
> +cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
> +       $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@

God AWK is unreadable. Any reason we can't use GNU make's sort builtin?
https://www.gnu.org/software/make/manual/html_node/Text-Functions.html

>
>  $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
>         $(call if_changed,mod)
> diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
> index 0125698f2037..79bb9eaa65ac 100644
> --- a/scripts/mod/sumversion.c
> +++ b/scripts/mod/sumversion.c
> @@ -398,7 +398,7 @@ void get_src_version(const char *modname, char sum[], unsigned sumlen)
>         buf = read_text_file(filelist);
>
>         md4_init(&md);
> -       while ((fname = strsep(&buf, " \n"))) {
> +       while ((fname = strsep(&buf, "\n"))) {
>                 if (!*fname)
>                         continue;
>                 if (!(is_static_library(fname)) &&
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 6/7] kbuild: make *.mod not depend on *.o

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The dependency
>
>     $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o
>
> ... exists because *.mod files previously contained undefined symbols,
> which are computed from *.o files when CONFIG_TRIM_UNUSED_KSYMS=y.
>
> Now that the undefined symbols are put into separate *.usyms files,
> there is no reason to make *.mod depend on *.o files.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  Makefile               | 3 ++-
>  scripts/Makefile.build | 5 ++---
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 82ee893909e9..e915aacd02b0 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1792,7 +1792,8 @@ ifdef single-build
>
>  # .ko is special because modpost is needed
>  single-ko := $(sort $(filter %.ko, $(MAKECMDGOALS)))
> -single-no-ko := $(sort $(patsubst %.ko,%.mod, $(MAKECMDGOALS)))
> +single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS)) \
> +               $(foreach x, o mod, $(patsubst %.ko, %.$x, $(single-ko)))

I'm on board with this patch, and the overall goal with the series. My
brain is having a hard time parsing `o mod` though. Can you walk me
through that? Are those targets for .o and .mod files, respectively?

>
>  $(single-ko): single_modpost
>         @:
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index f7a30f378e20..3da731cf6978 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -85,7 +85,7 @@ ifdef need-builtin
>  targets-for-builtin += $(obj)/built-in.a
>  endif
>
> -targets-for-modules := $(foreach x, mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
> +targets-for-modules := $(foreach x, o mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
>                                 $(patsubst %.o, %.$x, $(filter %.o, $(obj-m))))
>
>  ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
> @@ -306,7 +306,7 @@ endif
>  cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
>         $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
>
> -$(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
> +$(obj)/%.mod: FORCE
>         $(call if_changed,mod)
>
>  # List module undefined symbols
> @@ -469,7 +469,6 @@ $(multi-obj-m): FORCE
>         $(call if_changed,link_multi-m)
>  $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
>
> -targets += $(multi-obj-m)
>  targets := $(filter-out $(PHONY), $(targets))
>
>  # Add intermediate targets:
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 7/7] kbuild: read *.mod to get objects passed to $(LD) or $(AR)

[Nick Desaulniers]

On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> ld and ar support @file, which command-line options are read from.
>
> Now that *.mod lists the member objects in the correct order, without
> duplication, it is ready to be passed to ld and ar.
>
> By using the @file syntax, people will not be worried about the pitfall
> described in the NOTE.

Clever! Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/Makefile.build | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3da731cf6978..f6a506318795 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -450,22 +450,18 @@ quiet_cmd_ar_lib = AR      $@
>  $(obj)/lib.a: $(lib-y) FORCE
>         $(call if_changed,ar_lib)
>
> -# NOTE:
> -# Do not replace $(filter %.o,^) with $(real-prereqs). When a single object
> -# module is turned into a multi object module, $^ will contain header file
> -# dependencies recorded in the .*.cmd file.
>  ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
>  quiet_cmd_link_multi-m = AR [M]  $@
>  cmd_link_multi-m =                                             \
>         $(cmd_update_lto_symversions);                          \
>         rm -f $@;                                               \
> -       $(AR) cDPrsT $@ $(filter %.o,$^)
> +       $(AR) cDPrsT $@ @$(patsubst %.o,%.mod,$@)
>  else
>  quiet_cmd_link_multi-m = LD [M]  $@
> -      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^)
> +      cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ @$(patsubst %.o,%.mod,$@)
>  endif
>
> -$(multi-obj-m): FORCE
> +$(multi-obj-m): %.o: %.mod FORCE
>         $(call if_changed,link_multi-m)
>  $(call multi_depend, $(multi-obj-m), .o, -objs -y -m)
>
> --
> 2.32.0
>


-- 
Thanks,
~Nick Desaulniers

RE: [PATCH 6/7] kbuild: make *.mod not depend on *.o

[David Laight]

From: Nick Desaulniers
> Sent: 07 April 2022 18:59
> 
> On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > The dependency
> >
> >     $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o
> >
> > ... exists because *.mod files previously contained undefined symbols,
> > which are computed from *.o files when CONFIG_TRIM_UNUSED_KSYMS=y.
> >
> > Now that the undefined symbols are put into separate *.usyms files,
> > there is no reason to make *.mod depend on *.o files.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  Makefile               | 3 ++-
> >  scripts/Makefile.build | 5 ++---
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 82ee893909e9..e915aacd02b0 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1792,7 +1792,8 @@ ifdef single-build
> >
> >  # .ko is special because modpost is needed
> >  single-ko := $(sort $(filter %.ko, $(MAKECMDGOALS)))
> > -single-no-ko := $(sort $(patsubst %.ko,%.mod, $(MAKECMDGOALS)))
> > +single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS)) \
> > +               $(foreach x, o mod, $(patsubst %.ko, %.$x, $(single-ko)))
> 
> I'm on board with this patch, and the overall goal with the series. My
> brain is having a hard time parsing `o mod` though. Can you walk me
> through that? Are those targets for .o and .mod files, respectively?

I think I'd do:
single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS))
single-no-ko += $(patsubst %.ko, %.o, $(single-ko))
single-no-ko += $(patsubst %.ko, %.mod, $(single-ko))

Although you can use the simpler SYSV make suffix substitution syntax:
single-no-ko += $(single-ko:.ko=.o) $(single-ko:.ko=.mod)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH 4/7] kbuild: split the second line of *.mod into *.usyms

[Masahiro Yamada]

On Fri, Apr 8, 2022 at 2:47 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > The *.mod files have two lines; the first line lists the member objects
> > of the module, and the second line, if CONFIG_TRIM_UNUSED_KSYMS=y, lists
> > the undefined symbols.
>
> Enabling EXPERT and TRIM_UNUSED_KSYMS, I didn't find any .mod files
> containing a second line that wasn't empty. Is there an example that
> has such symbol list that I can use to verify?


Modules are usually symbol consumers in order to be useful.
(and some of them are symbol providers as well).

So, it is rare to see an empty unresolved symbol list.

But, it is theoretically possible to create such a module.


This is sample code.


obj-m += foo.o

----------------(foo.c begin)----------------
#include <linux/module.h>

MODULE_LICENSE("GPL");
----------------(foo.c end)----------------






> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 5/7] kbuild: get rid of duplication in *.mod files

[Masahiro Yamada]

On Fri, Apr 8, 2022 at 2:55 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > It is allowed to add the same objects multiple times to obj-y / obj-m:
> >
> >   obj-y += foo.o foo.o foo.o
> >   obj-m += bar.o bar.o bar.o
> >
> > It is also allowed to add the same objects multiple times to a composite
> > module:
> >
> >   obj-m    += foo.o
> >   foo-objs := foo1.o foo2.o foo2.o foo1.o
> >
> > This flexibility is useful because the same object might be selected by
> > different CONFIG options, like this:
> >
> >   obj-m               += foo.o
> >   foo-y               := foo1.o
> >   foo-$(CONFIG_FOO_X) += foo2.o
> >   foo-$(CONFIG_FOO_Y) += foo2.o
> >
> > The duplicated objects are omitted at link time. It works naturally in
> > Makefiles because GNU Make removes duplication in $^ without changing
> > the order.
> >
> > It is working well, almost...
> >
> > A small flaw I notice is, *.mod contains duplication in such a case.
> >
> > This is probably not a big deal. As far as I know, the only small
> > problem is scripts/mod/sumversion.c parses the same file multiple
> > times.
> >
> > I am fixing this because I plan to reuse *.mod for other purposes,
> > where the duplication can be problematic.
> >
> > The code change is quite simple. We already use awk to drop duplicated
> > lines in modules.order (see cmd_modules_order in the same file).
>
> Why does the top level Makefile reassign cmd_modules_order rather than
> use the original value in scripts/Makefile.build?


Because the top level Makefile does not include scripts/Makefile.build



> > I copied the code, but changed RS to use spaces as record separators.
> >
> > I also changed the file format to list one object per line.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/Makefile.build   | 3 ++-
> >  scripts/mod/sumversion.c | 2 +-
> >  2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 6ae92d119dfa..f7a30f378e20 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -303,7 +303,8 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
> >         $(call if_changed,cc_prelink_modules)
> >  endif
> >
> > -cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) > $@
> > +cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
> > +       $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
>
> God AWK is unreadable. Any reason we can't use GNU make's sort builtin?
> https://www.gnu.org/software/make/manual/html_node/Text-Functions.html


I did that in the previous submission.
https://lore.kernel.org/lkml/20220405113359.2880241-8-masahiroy@kernel.org/


After some thoughts, I decided to drop duplicates without sorting.

If I alphabetically sorted the object list,
7/7 of this series would be impossible.


I am not a big fan of AWK, but I do not know a cleaner way.
If you know a better idea, please tell me.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 6/7] kbuild: make *.mod not depend on *.o

[Masahiro Yamada]

On Fri, Apr 8, 2022 at 6:39 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: Nick Desaulniers
> > Sent: 07 April 2022 18:59
> >
> > On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > The dependency
> > >
> > >     $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o
> > >
> > > ... exists because *.mod files previously contained undefined symbols,
> > > which are computed from *.o files when CONFIG_TRIM_UNUSED_KSYMS=y.
> > >
> > > Now that the undefined symbols are put into separate *.usyms files,
> > > there is no reason to make *.mod depend on *.o files.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > >  Makefile               | 3 ++-
> > >  scripts/Makefile.build | 5 ++---
> > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 82ee893909e9..e915aacd02b0 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1792,7 +1792,8 @@ ifdef single-build
> > >
> > >  # .ko is special because modpost is needed
> > >  single-ko := $(sort $(filter %.ko, $(MAKECMDGOALS)))
> > > -single-no-ko := $(sort $(patsubst %.ko,%.mod, $(MAKECMDGOALS)))
> > > +single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS)) \
> > > +               $(foreach x, o mod, $(patsubst %.ko, %.$x, $(single-ko)))
> >
> > I'm on board with this patch, and the overall goal with the series. My
> > brain is having a hard time parsing `o mod` though. Can you walk me
> > through that? Are those targets for .o and .mod files, respectively?


Yes.

Kbuild can build a module individually.

    make  foo/bar/baz.ko

(but modpost check does not work well)

To do this, Kbuild needs to descend to
the directory and generate
foo/bar/baz.o  and  foo/bar/baz.mod.

Previously, foo/bar/baz.o was generated as a
prerequisite of foo/bar/baz.mod, but now we
need to request Kbuild to generate both of them.





> I think I'd do:
> single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS))
> single-no-ko += $(patsubst %.ko, %.o, $(single-ko))
> single-no-ko += $(patsubst %.ko, %.mod, $(single-ko))
>
> Although you can use the simpler SYSV make suffix substitution syntax:
> single-no-ko += $(single-ko:.ko=.o) $(single-ko:.ko=.mod)


Right.   I tend to use $(patsubst ), but
in some places, shorter SYSV syntax is used.
I admit inconsistency.





>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 6/7] kbuild: make *.mod not depend on *.o

[Masahiro Yamada]

On Fri, Apr 8, 2022 at 9:37 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Apr 8, 2022 at 6:39 AM David Laight <David.Laight@aculab.com> wrote:
> >
> > From: Nick Desaulniers
> > > Sent: 07 April 2022 18:59
> > >
> > > On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > The dependency
> > > >
> > > >     $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o
> > > >
> > > > ... exists because *.mod files previously contained undefined symbols,
> > > > which are computed from *.o files when CONFIG_TRIM_UNUSED_KSYMS=y.
> > > >
> > > > Now that the undefined symbols are put into separate *.usyms files,
> > > > there is no reason to make *.mod depend on *.o files.
> > > >
> > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > > ---
> > > >
> > > >  Makefile               | 3 ++-
> > > >  scripts/Makefile.build | 5 ++---
> > > >  2 files changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 82ee893909e9..e915aacd02b0 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1792,7 +1792,8 @@ ifdef single-build
> > > >
> > > >  # .ko is special because modpost is needed
> > > >  single-ko := $(sort $(filter %.ko, $(MAKECMDGOALS)))
> > > > -single-no-ko := $(sort $(patsubst %.ko,%.mod, $(MAKECMDGOALS)))
> > > > +single-no-ko := $(filter-out $(single-ko), $(MAKECMDGOALS)) \
> > > > +               $(foreach x, o mod, $(patsubst %.ko, %.$x, $(single-ko)))
> > >
> > > I'm on board with this patch, and the overall goal with the series. My
> > > brain is having a hard time parsing `o mod` though. Can you walk me
> > > through that? Are those targets for .o and .mod files, respectively?
>
>
> Yes.
>
> Kbuild can build a module individually.
>
>     make  foo/bar/baz.ko
>
> (but modpost check does not work well)
>
> To do this, Kbuild needs to descend to
> the directory and generate
> foo/bar/baz.o  and  foo/bar/baz.mod.
>
> Previously, foo/bar/baz.o was generated as a
> prerequisite of foo/bar/baz.mod, but now we
> need to request Kbuild to generate both of them.
>


BTW, this feature is broken for CONFIG_LTO_CLANG=y
because the ELF object is not foo/bar/baz.o
but foo/bar/baz.prelink.o
(which was renamed from foo/bar/baz.lto.o).

I will not fix it, though.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 5/7] kbuild: get rid of duplication in *.mod files

[Nick Desaulniers]

On Thu, Apr 7, 2022 at 5:08 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Apr 8, 2022 at 2:55 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > index 6ae92d119dfa..f7a30f378e20 100644
> > > --- a/scripts/Makefile.build
> > > +++ b/scripts/Makefile.build
> > > @@ -303,7 +303,8 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
> > >         $(call if_changed,cc_prelink_modules)
> > >  endif
> > >
> > > -cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) > $@
> > > +cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
> > > +       $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
> >
> > God AWK is unreadable. Any reason we can't use GNU make's sort builtin?
> > https://www.gnu.org/software/make/manual/html_node/Text-Functions.html
>
>
> I did that in the previous submission.
> https://lore.kernel.org/lkml/20220405113359.2880241-8-masahiroy@kernel.org/
>
>
> After some thoughts, I decided to drop duplicates without sorting.
>
> If I alphabetically sorted the object list,
> 7/7 of this series would be impossible.
>
>
> I am not a big fan of AWK, but I do not know a cleaner way.
> If you know a better idea, please tell me.

```
# stable_dedup.py
from sys import argv

wordset = set()
argv.pop(0)
for word in argv: wordset.add(word)
for word in wordset: print(word)
```
If that ever shows up in a profile of a kernel build, <set> in C++
looks pretty similar.  Then that script can be reused in a couple of
other places, and has a more descriptive name that hints at what it
does.

Compare that with `$(AWK) -v RS='( |\n)' '!x[$$0]++'`.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 5/7] kbuild: get rid of duplication in *.mod files

[Masahiro Yamada]

On Sat, Apr 9, 2022 at 5:43 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Apr 7, 2022 at 5:08 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Fri, Apr 8, 2022 at 2:55 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> > >
> > > On Wed, Apr 6, 2022 at 8:31 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > > > index 6ae92d119dfa..f7a30f378e20 100644
> > > > --- a/scripts/Makefile.build
> > > > +++ b/scripts/Makefile.build
> > > > @@ -303,7 +303,8 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
> > > >         $(call if_changed,cc_prelink_modules)
> > > >  endif
> > > >
> > > > -cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) > $@
> > > > +cmd_mod = echo $(addprefix $(obj)/, $(call real-search, $*.o, .o, -objs -y -m)) | \
> > > > +       $(AWK) -v RS='( |\n)' '!x[$$0]++' > $@
> > >
> > > God AWK is unreadable. Any reason we can't use GNU make's sort builtin?
> > > https://www.gnu.org/software/make/manual/html_node/Text-Functions.html
> >
> >
> > I did that in the previous submission.
> > https://lore.kernel.org/lkml/20220405113359.2880241-8-masahiroy@kernel.org/
> >
> >
> > After some thoughts, I decided to drop duplicates without sorting.
> >
> > If I alphabetically sorted the object list,
> > 7/7 of this series would be impossible.
> >
> >
> > I am not a big fan of AWK, but I do not know a cleaner way.
> > If you know a better idea, please tell me.
>
> ```
> # stable_dedup.py
> from sys import argv
>
> wordset = set()
> argv.pop(0)
> for word in argv: wordset.add(word)
> for word in wordset: print(word)
> ```
> If that ever shows up in a profile of a kernel build, <set> in C++
> looks pretty similar.  Then that script can be reused in a couple of
> other places, and has a more descriptive name that hints at what it
> does.
>
> Compare that with `$(AWK) -v RS='( |\n)' '!x[$$0]++'`.


As I said, I want to drop duplicates without changing the argument order.

Your python code shuffles the order since it adds arguments to set() first.


    $ cat stable_dedup.py
    #!/usr/bin/python3
    from sys import argv
    wordset = set()
    argv.pop(0)
    for word in argv: wordset.add(word)
    for word in wordset: print(word)

    $ ./stable_dedup.py  c b a a b
    c
    a
    b



Here, the output I expect is "c b a".


If I am allowed to change the order, I would use
Make's $(sort ...) function or "sort -u" shell command.




Of course, it is pretty easy to write a python script
that dedups arguments without changing the order.


    $ cat dedup-by-python
    #!/usr/bin/python3
    import sys
    wordset = set()

    for x in sys.argv[1:]:
        if x not in wordset:
            print(x)
        wordset.add(x)

    $ ./dedup-by-python c b a a b
    c
    b
    a


Even this script looks like a bad approach.


Please note cmd_mod is invoked as many times
as the number of modules.
So, this happens many times, especially for allmodconfig.


Python takes a lot of overhead times for initialization.


AWK implementation is much faster.
It is apparent from perf.




[1] AWK implementation

    $ cat test-data.txt
    c b a a b

    $ cat dedup-by-awk
    #!/usr/bin/awk -f
    BEGIN { RS="( |\n)" }
    !x[$0]++ { print($0) }


    # perf stat  -- ./dedup-by-awk < test-data.txt
    c
    b
    a

 Performance counter stats for './dedup-by-awk':

              1.06 msec task-clock                #    0.790 CPUs
utilized
                 0      context-switches          #    0.000 /sec
                 0      cpu-migrations            #    0.000 /sec
               201      page-faults               #  189.755 K/sec
         3,671,995      cycles                    #    3.467 GHz
         3,932,770      instructions              #    1.07  insn per
cycle
           754,811      branches                  #  712.582 M/sec
            21,154      branch-misses             #    2.80% of all
branches
        18,350,660      slots                     #   17.324 G/sec
         4,173,875      topdown-retiring          #     22.7% retiring
         2,230,864      topdown-bad-spec          #     12.2% bad
speculation
         5,757,069      topdown-fe-bound          #     31.4% frontend
bound
         6,188,850      topdown-be-bound          #     33.7% backend
bound

       0.001341605 seconds time elapsed

       0.001476000 seconds user
       0.000000000 seconds sys



[2]  Python implementation

    # perf stat  -- ./dedup-by-python   c b a a b
   c
   b
   a

 Performance counter stats for './dedup-by-python c b a a b':

              9.34 msec task-clock                #    0.967 CPUs
utilized
                 0      context-switches          #    0.000 /sec
                 0      cpu-migrations            #    0.000 /sec
               756      page-faults               #   80.947 K/sec
        31,045,653      cycles                    #    3.324 GHz
        39,175,531      instructions              #    1.26  insn per
cycle
         8,488,886      branches                  #  908.929 M/sec
           326,947      branch-misses             #    3.85% of all
branches
       152,587,445      slots                     #   16.338 G/sec
        37,698,074      topdown-retiring          #     24.7% retiring
        32,911,017      topdown-bad-spec          #     21.6% bad
speculation
        55,051,156      topdown-fe-bound          #     36.1% frontend
bound
        26,927,196      topdown-be-bound          #     17.6% backend
bound

       0.009661105 seconds time elapsed

       0.006485000 seconds user
       0.003242000 seconds sys





> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] kbuild: more misc cleanups

[Masahiro Yamada]

On Thu, Apr 7, 2022 at 12:32 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
>
> I sent the first batch of cleanups:
> https://lore.kernel.org/linux-kbuild/20220405113359.2880241-1-masahiroy@kernel.org/T/#t
>
> I took 01-06, 09-10.
> I dropped 07, 08.
>
> This is the second batch.
>

Applied to linux-kbuild.


>
>
> Masahiro Yamada (7):
>   kbuild: reuse suffix-search to refactor multi_depend
>   kbuild: make multi_depend work with targets in subdirectory
>   kbuild: reuse real-search to simplify cmd_mod
>   kbuild: split the second line of *.mod into *.usyms
>   kbuild: get rid of duplication in *.mod files
>   kbuild: make *.mod not depend on *.o
>   kbuild: read *.mod to get objects passed to $(LD) or $(AR)
>
>  .gitignore                  |  1 +
>  Makefile                    |  5 +++--
>  scripts/Makefile.build      | 31 ++++++++++++++-----------------
>  scripts/Makefile.lib        |  6 +++---
>  scripts/adjust_autoksyms.sh |  2 +-
>  scripts/gen_autoksyms.sh    | 18 +++++++++++-------
>  scripts/mod/sumversion.c    | 11 ++---------
>  7 files changed, 35 insertions(+), 39 deletions(-)
>
> --
> 2.32.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH 0/7] kbuild: more misc cleanups

[Elliot Berman]

Hi Masahiro,

On 4/15/2022 12:20 AM, Masahiro Yamada wrote:
> On Thu, Apr 7, 2022 at 12:32 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>>
>> I sent the first batch of cleanups:
>> https://lore.kernel.org/linux-kbuild/20220405113359.2880241-1-masahiroy@kernel.org/T/#t
>>
>> I took 01-06, 09-10.
>> I dropped 07, 08.
>>
>> This is the second batch.
>>
> 
> Applied to linux-kbuild.
> 
> 

I didn't see the last patch (kbuild: read *.mod to get objects passed to 
$(LD) or $(AR)) applied. Was the last patch intentionally skipped?

>>
>>
>> Masahiro Yamada (7):
>>    kbuild: reuse suffix-search to refactor multi_depend
>>    kbuild: make multi_depend work with targets in subdirectory
>>    kbuild: reuse real-search to simplify cmd_mod
>>    kbuild: split the second line of *.mod into *.usyms
>>    kbuild: get rid of duplication in *.mod files
>>    kbuild: make *.mod not depend on *.o
>>    kbuild: read *.mod to get objects passed to $(LD) or $(AR)
>>
>>   .gitignore                  |  1 +
>>   Makefile                    |  5 +++--
>>   scripts/Makefile.build      | 31 ++++++++++++++-----------------
>>   scripts/Makefile.lib        |  6 +++---
>>   scripts/adjust_autoksyms.sh |  2 +-
>>   scripts/gen_autoksyms.sh    | 18 +++++++++++-------
>>   scripts/mod/sumversion.c    | 11 ++---------
>>   7 files changed, 35 insertions(+), 39 deletions(-)
>>
>> --
>> 2.32.0
>>
> 
> 

Re: [PATCH 0/7] kbuild: more misc cleanups

[Masahiro Yamada]

On Sat, Apr 23, 2022 at 4:07 AM Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> Hi Masahiro,
>
> On 4/15/2022 12:20 AM, Masahiro Yamada wrote:
> > On Thu, Apr 7, 2022 at 12:32 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >>
> >>
> >> I sent the first batch of cleanups:
> >> https://lore.kernel.org/linux-kbuild/20220405113359.2880241-1-masahiroy@kernel.org/T/#t
> >>
> >> I took 01-06, 09-10.
> >> I dropped 07, 08.
> >>
> >> This is the second batch.
> >>
> >
> > Applied to linux-kbuild.
> >
> >
>
> I didn't see the last patch (kbuild: read *.mod to get objects passed to
> $(LD) or $(AR)) applied. Was the last patch intentionally skipped?


I do not know why but something wrong happened to my git operation.
I will apply it.
Thanks for pointing it out.







> >>
> >>
> >> Masahiro Yamada (7):
> >>    kbuild: reuse suffix-search to refactor multi_depend
> >>    kbuild: make multi_depend work with targets in subdirectory
> >>    kbuild: reuse real-search to simplify cmd_mod
> >>    kbuild: split the second line of *.mod into *.usyms
> >>    kbuild: get rid of duplication in *.mod files
> >>    kbuild: make *.mod not depend on *.o
> >>    kbuild: read *.mod to get objects passed to $(LD) or $(AR)
> >>
> >>   .gitignore                  |  1 +
> >>   Makefile                    |  5 +++--
> >>   scripts/Makefile.build      | 31 ++++++++++++++-----------------
> >>   scripts/Makefile.lib        |  6 +++---
> >>   scripts/adjust_autoksyms.sh |  2 +-
> >>   scripts/gen_autoksyms.sh    | 18 +++++++++++-------
> >>   scripts/mod/sumversion.c    | 11 ++---------
> >>   7 files changed, 35 insertions(+), 39 deletions(-)
> >>
> >> --
> >> 2.32.0
> >>
> >
> >



-- 
Best Regards
Masahiro Yamada