linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] kbuild: misc cleanups
@ 2022-04-05 14:02 Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Alexander Lobakin, Michal Marek,
	Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
	Rasmus Villemoes, Sami Tolvanen, llvm


This is a series of prerequisite cleanups of my next work.


Masahiro Yamada (10):
  kbuild: factor out genksyms command from cmd_gensymtypes_{c,S}
  kbuild: do not remove empty *.symtypes explicitly
  modpost: remove useless export_from_sec()
  modpost: move export_from_secname() call to more relevant place
  modpost: remove redundant initializes for static variables
  modpost: remove annoying namespace_from_kstrtabns()
  kbuild: get rid of duplication in the first line of *.mod files
  kbuild: split the second line of *.mod into *.usyms
  kbuild: refactor cmd_modversions_c
  kbuild: refactor cmd_modversions_S

 .gitignore                  |  1 +
 Makefile                    |  2 +-
 scripts/Makefile.build      | 86 ++++++++++++++++---------------------
 scripts/adjust_autoksyms.sh |  2 +-
 scripts/gen_autoksyms.sh    | 18 +++++---
 scripts/mod/modpost.c       | 49 ++++++---------------
 scripts/mod/modpost.h       |  4 --
 scripts/mod/sumversion.c    | 11 +----
 8 files changed, 64 insertions(+), 109 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S}
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers

The genksyms command part in cmd_gensymtypes_{c,S} is duplicated.
Factor it out into the 'genksyms' macro.

For the readability, I slightly refactor the arguments to genksyms.

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

Changes in v2:
  - Fix the location of the closing parenthesis

 scripts/Makefile.build | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 9717e6f6fb31..31e0e33dfe5d 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -125,13 +125,14 @@ cmd_cpp_i_c       = $(CPP) $(c_flags) -o $@ $<
 $(obj)/%.i: $(src)/%.c FORCE
 	$(call if_changed_dep,cpp_i_c)
 
+genksyms = scripts/genksyms/genksyms		\
+	$(if $(1), -T $(2))			\
+	$(if $(CONFIG_MODULE_REL_CRCS), -R)	\
+	$(if $(KBUILD_PRESERVE), -p)		\
+	-r $(or $(wildcard $(2:.symtypes=.symref)), /dev/null)
+
 # These mirror gensymtypes_S and co below, keep them in synch.
-cmd_gensymtypes_c =                                                         \
-    $(CPP) -D__GENKSYMS__ $(c_flags) $< |                                   \
-    scripts/genksyms/genksyms $(if $(1), -T $(2))                           \
-     $(patsubst y,-R,$(CONFIG_MODULE_REL_CRCS))                             \
-     $(if $(KBUILD_PRESERVE),-p)                                            \
-     -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))
+cmd_gensymtypes_c = $(CPP) -D__GENKSYMS__ $(c_flags) $< | $(genksyms)
 
 quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@
 cmd_cc_symtypes_c =                                                         \
@@ -344,11 +345,7 @@ cmd_gensymtypes_S =                                                         \
     $(CPP) $(a_flags) $< |                                                  \
      grep "\<___EXPORT_SYMBOL\>" |                                          \
      sed 's/.*___EXPORT_SYMBOL[[:space:]]*\([a-zA-Z0-9_]*\)[[:space:]]*,.*/EXPORT_SYMBOL(\1);/' ; } | \
-    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - |                                \
-    scripts/genksyms/genksyms $(if $(1), -T $(2))                           \
-     $(patsubst y,-R,$(CONFIG_MODULE_REL_CRCS))                             \
-     $(if $(KBUILD_PRESERVE),-p)                                            \
-     -r $(firstword $(wildcard $(2:.symtypes=.symref) /dev/null))
+    $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
 
 quiet_cmd_cc_symtypes_S = SYM $(quiet_modtag) $@
 cmd_cc_symtypes_S =                                                         \
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 03/10] modpost: remove useless export_from_sec() Masahiro Yamada
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers

Presumably, 'test -s $@ || rm -f $@' intends to remove the output when
the genksyms command fails.

It is unneeded because .DELETE_ON_ERROR automatically removes the output
on failure.

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

Changes in v2:
  - Fix accidental drop of '> /dev/null'

 scripts/Makefile.build | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 31e0e33dfe5d..3ef2373f0a57 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -135,9 +135,7 @@ genksyms = scripts/genksyms/genksyms		\
 cmd_gensymtypes_c = $(CPP) -D__GENKSYMS__ $(c_flags) $< | $(genksyms)
 
 quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@
-cmd_cc_symtypes_c =                                                         \
-    $(call cmd_gensymtypes_c,true,$@) >/dev/null;                           \
-    test -s $@ || rm -f $@
+      cmd_cc_symtypes_c = $(call cmd_gensymtypes_c,true,$@) >/dev/null
 
 $(obj)/%.symtypes : $(src)/%.c FORCE
 	$(call cmd,cc_symtypes_c)
@@ -348,9 +346,7 @@ cmd_gensymtypes_S =                                                         \
     $(CPP) -D__GENKSYMS__ $(c_flags) -xc - | $(genksyms)
 
 quiet_cmd_cc_symtypes_S = SYM $(quiet_modtag) $@
-cmd_cc_symtypes_S =                                                         \
-    $(call cmd_gensymtypes_S,true,$@) >/dev/null;                           \
-    test -s $@ || rm -f $@
+      cmd_cc_symtypes_S = $(call cmd_gensymtypes_S,true,$@) >/dev/null
 
 $(obj)/%.symtypes : $(src)/%.S FORCE
 	$(call cmd,cc_symtypes_S)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 03/10] modpost: remove useless export_from_sec()
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place Masahiro Yamada
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Nick Desaulniers, Michal Marek

With commit 1743694eb235 ("modpost: stop symbol preloading for
modversion CRC") applied, now export_from_sec() is useless.

handle_symbol() is called for every symbol in the ELF.

When 'symname' does not start with "__ksymtab", export_from_sec() is
called, and the returned value is stored in 'export'.

It is used in the last part of handle_symbol():

    if (strstarts(symname, "__ksymtab_")) {
            name = symname + strlen("__ksymtab_");
            sym_add_exported(name, mod, export);
    }

'export' is used only when 'symname' starts with "__ksymtab_".

So, the value returned by export_from_sec() is never used.

Remove useless export_from_sec(). This makes further cleanups possible.

I put the temporary code:

    export = export_unknown;

Otherwise, I would get the compiler warning:

    warning: 'export' may be used uninitialized in this function [-Wmaybe-uninitialized]

This is apparently false positive because

    if (strstarts(symname, "__ksymtab_")

... is a stronger condition than:

    if (strstarts(symname, "__ksymtab")

Anyway, this part will be cleaned up by the next commit.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

Changes in v2:
  - Fix compiler warning

 scripts/mod/modpost.c | 17 ++---------------
 scripts/mod/modpost.h |  4 ----
 2 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index ed9d056d2108..eebb32689816 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -369,16 +369,6 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
 		return export_unknown;
 }
 
-static enum export export_from_sec(struct elf_info *elf, unsigned int sec)
-{
-	if (sec == elf->export_sec)
-		return export_plain;
-	else if (sec == elf->export_gpl_sec)
-		return export_gpl;
-	else
-		return export_unknown;
-}
-
 static const char *namespace_from_kstrtabns(const struct elf_info *info,
 					    const Elf_Sym *sym)
 {
@@ -576,10 +566,7 @@ static int parse_elf(struct elf_info *info, const char *filename)
 				fatal("%s has NOBITS .modinfo\n", filename);
 			info->modinfo = (void *)hdr + sechdrs[i].sh_offset;
 			info->modinfo_len = sechdrs[i].sh_size;
-		} else if (strcmp(secname, "__ksymtab") == 0)
-			info->export_sec = i;
-		else if (strcmp(secname, "__ksymtab_gpl") == 0)
-			info->export_gpl_sec = i;
+		}
 
 		if (sechdrs[i].sh_type == SHT_SYMTAB) {
 			unsigned int sh_link_idx;
@@ -703,7 +690,7 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
 	if (strstarts(symname, "__ksymtab"))
 		export = export_from_secname(info, get_secindex(info, sym));
 	else
-		export = export_from_sec(info, get_secindex(info, sym));
+		export = export_unknown;
 
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 0c47ff95c0e2..a85dcec3669a 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -25,7 +25,6 @@
 #define Elf_Sym     Elf32_Sym
 #define Elf_Addr    Elf32_Addr
 #define Elf_Sword   Elf64_Sword
-#define Elf_Section Elf32_Half
 #define ELF_ST_BIND ELF32_ST_BIND
 #define ELF_ST_TYPE ELF32_ST_TYPE
 
@@ -40,7 +39,6 @@
 #define Elf_Sym     Elf64_Sym
 #define Elf_Addr    Elf64_Addr
 #define Elf_Sword   Elf64_Sxword
-#define Elf_Section Elf64_Half
 #define ELF_ST_BIND ELF64_ST_BIND
 #define ELF_ST_TYPE ELF64_ST_TYPE
 
@@ -138,8 +136,6 @@ struct elf_info {
 	Elf_Shdr     *sechdrs;
 	Elf_Sym      *symtab_start;
 	Elf_Sym      *symtab_stop;
-	Elf_Section  export_sec;
-	Elf_Section  export_gpl_sec;
 	char         *strtab;
 	char	     *modinfo;
 	unsigned int modinfo_len;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (2 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 03/10] modpost: remove useless export_from_sec() Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 05/10] modpost: remove redundant initializes for static variables Masahiro Yamada
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Nick Desaulniers, Michal Marek

The assigned 'export' is only used when

    if (strstarts(symname, "__ksymtab_"))

is met. The else-part of the assignment is the dead code.

Move the export_from_secname() call to where it is used.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index eebb32689816..f9e54247ae1d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -684,14 +684,8 @@ static void handle_modversion(const struct module *mod,
 static void handle_symbol(struct module *mod, struct elf_info *info,
 			  const Elf_Sym *sym, const char *symname)
 {
-	enum export export;
 	const char *name;
 
-	if (strstarts(symname, "__ksymtab"))
-		export = export_from_secname(info, get_secindex(info, sym));
-	else
-		export = export_unknown;
-
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
 		if (strstarts(symname, "__gnu_lto_")) {
@@ -726,7 +720,11 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
 	default:
 		/* All exported symbols */
 		if (strstarts(symname, "__ksymtab_")) {
+			enum export export;
+
 			name = symname + strlen("__ksymtab_");
+			export = export_from_secname(info,
+						     get_secindex(info, sym));
 			sym_add_exported(name, mod, export);
 		}
 		if (strcmp(symname, "init_module") == 0)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 05/10] modpost: remove redundant initializes for static variables
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (3 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns() Masahiro Yamada
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers

These are cleared without explicit initializes.

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

Changes in v2:
  - New

 scripts/mod/modpost.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index f9e54247ae1d..2a202764ff48 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -23,15 +23,15 @@
 #include "../../include/linux/license.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
-static int modversions = 0;
+static int modversions;
 /* Is CONFIG_MODULE_SRCVERSION_ALL set? */
-static int all_versions = 0;
+static int all_versions;
 /* If we are modposting external module set to 1 */
-static int external_module = 0;
+static int external_module;
 /* Only warn about unresolved symbols */
-static int warn_unresolved = 0;
+static int warn_unresolved;
 /* How a symbol is exported */
-static int sec_mismatch_count = 0;
+static int sec_mismatch_count;
 static int sec_mismatch_warn_only = true;
 /* ignore missing files */
 static int ignore_missing_files;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns()
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (4 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 05/10] modpost: remove redundant initializes for static variables Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files Masahiro Yamada
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers

There are two call sites for sym_update_namespace().

When the symbol has no namespace, s->namespace is set to NULL,
but the conversion from "" to NULL is done in two different places.

[1] read_symbols()

  This gets the namespace from __kstrtabns_<symbol>. If the symbol has
  no namespace, sym_get_data(info, sym) returns the empty string "".
  namespace_from_kstrtabns() converts it to NULL before it is passed to
  sym_update_namespace().

[2] read_dump()

  This gets the namespace from the dump file, *.symvers. If the symbol
  has no namespace, the 'namespace' is the empty string "", which is
  directly passed into sym_update_namespace(). The conversion from
  "" to NULL is done in sym_update_namespace().

namespace_from_kstrtabns() exists only for creating this inconsistency.

By removing it, sym_update_namespace() is consistently passed with ""
when the symbol has no namespace.

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

Changes in v2:
  - new

 scripts/mod/modpost.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2a202764ff48..522d5249d196 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -369,13 +369,6 @@ static enum export export_from_secname(struct elf_info *elf, unsigned int sec)
 		return export_unknown;
 }
 
-static const char *namespace_from_kstrtabns(const struct elf_info *info,
-					    const Elf_Sym *sym)
-{
-	const char *value = sym_get_data(info, sym);
-	return value[0] ? value : NULL;
-}
-
 static void sym_update_namespace(const char *symname, const char *namespace)
 {
 	struct symbol *s = find_symbol(symname);
@@ -391,8 +384,7 @@ static void sym_update_namespace(const char *symname, const char *namespace)
 	}
 
 	free(s->namespace);
-	s->namespace =
-		namespace && namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
+	s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
 }
 
 /**
@@ -2049,9 +2041,7 @@ static void read_symbols(const char *modname)
 		/* Apply symbol namespaces from __kstrtabns_<symbol> entries. */
 		if (strstarts(symname, "__kstrtabns_"))
 			sym_update_namespace(symname + strlen("__kstrtabns_"),
-					     namespace_from_kstrtabns(&info,
-								      sym));
-
+					     sym_get_data(&info, sym));
 		if (strstarts(symname, "__crc_"))
 			handle_modversion(mod, &info, sym,
 					  symname + strlen("__crc_"));
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (5 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns() Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms Masahiro Yamada
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers

The first line of *.mod lists the object files that consist of the
module. This may contain duplication if the same object is added
multiple times, like this:

  obj-m := foo.o
  foo-$(CONFIG_FOO1_X) += foo1.o
  foo-$(CONFIG_FOO1_Y) += foo1.o
  foo-$(CONFIG_FOO2_X) += foo2.o
  foo-$(CONFIG_FOO2_Y) += foo2.o

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 over again.
This can be avoided by adding $(sort ...). It has a side-effect that
sorts the objects alphabetically, but it is not a big deal, either.

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

Changes in v2:
  - new

 scripts/Makefile.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3ef2373f0a57..63625877aeae 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -307,8 +307,10 @@ $(obj)/%.prelink.o: $(obj)/%.o FORCE
 	$(call if_changed,cc_prelink_modules)
 endif
 
+multi-m-prereqs = $(sort $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)))
+
 cmd_mod = { \
-	echo $(if $($*-objs)$($*-y)$($*-m), $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)), $(@:.mod=.o)); \
+	echo $(if $(multi-m-prereqs), $(multi-m-prereqs), $(@:.mod=.o)); \
 	$(undefined_syms) echo; \
 	} > $@
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (6 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 09/10] kbuild: refactor cmd_modversions_c Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 10/10] kbuild: refactor cmd_modversions_S Masahiro Yamada
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Alexander Lobakin, Michal Marek,
	Nick Desaulniers, Nicolas Schier, Rasmus Villemoes,
	Sami Tolvanen

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.

These two are orthogonal. For further cleanups, let's split the second
line out to separate *.usyms files, which are generated only when
CONFIG_TRIM_UNUSED_KSYMS=y.

Previously, the list of undefined symbols ended up with a very long
line, but now symbols are split by new lines.

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

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

Changes in v2:
  - new

 .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 63625877aeae..d934bdf84de4 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 suffix, mod $(if $(CONFIG_TRIM_UNUSED_KSYMS), usyms), \
+				$(patsubst %.o, %.$(suffix), $(filter %.o, $(obj-m))))
 
 ifneq ($(CONFIG_LTO_CLANG)$(CONFIG_X86_KERNEL_IBT),)
 targets-for-modules += $(patsubst %.o, %.prelink.o, $(filter %.o, $(obj-m)))
@@ -260,9 +261,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
@@ -309,14 +307,17 @@ endif
 
 multi-m-prereqs = $(sort $(addprefix $(obj)/, $($*-objs) $($*-y) $($*-m)))
 
-cmd_mod = { \
-	echo $(if $(multi-m-prereqs), $(multi-m-prereqs), $(@:.mod=.o)); \
-	$(undefined_syms) echo; \
-	} > $@
+cmd_mod = echo $(if $(multi-m-prereqs), $(multi-m-prereqs), $(@:.mod=.o)) > $@
 
 $(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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 09/10] kbuild: refactor cmd_modversions_c
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (7 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  2022-04-05 14:02 ` [PATCH v2 10/10] kbuild: refactor cmd_modversions_S Masahiro Yamada
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nathan Chancellor,
	Nick Desaulniers, llvm

cmd_modversions_c implements two parts; run genksyms to calculate CRCs
of exported symbols, run $(LD) to update the object with the CRCs. The
latter is not executed for CONFIG_LTO_CLANG=y since the object is not
ELF but LLVM bit code at this point.

The first part can be unified because we can always use $(NM) instead
of "$(OBJDUMP) -h" to dump the symbols.

Split the code into the two macros, cmd_gen_symversions_c and
cmd_modversions.

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

Changes in v2:
 - new

 scripts/Makefile.build | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index d934bdf84de4..ba2be555f942 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -169,29 +169,25 @@ ifdef CONFIG_MODVERSIONS
 #   the actual value of the checksum generated by genksyms
 # o remove .tmp_<file>.o to <file>.o
 
-ifdef CONFIG_LTO_CLANG
 # Generate .o.symversions files for each .o with exported symbols, and link these
 # to the kernel and/or modules at the end.
-cmd_modversions_c =								\
+gen_symversions =								\
 	if $(NM) $@ 2>/dev/null | grep -q __ksymtab; then			\
-		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
+		$(call cmd_gensymtypes_$(1),$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
 		    > $@.symversions;						\
 	else									\
 		rm -f $@.symversions;						\
-	fi;
-else
-cmd_modversions_c =								\
-	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
-		$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
-		    > $(@D)/.tmp_$(@F:.o=.ver);					\
-										\
+	fi
+
+cmd_gen_symversions_c =	$(call gen_symversions,c)
+
+cmd_modversions =								\
+	if [ -r $@.symversions ]; then						\
 		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
-			-T $(@D)/.tmp_$(@F:.o=.ver);				\
+			-T $@.symversions;					\
 		mv -f $(@D)/.tmp_$(@F) $@;					\
-		rm -f $(@D)/.tmp_$(@F:.o=.ver);					\
 	fi
 endif
-endif
 
 ifdef CONFIG_FTRACE_MCOUNT_USE_RECORDMCOUNT
 # compiler will not generate __mcount_loc use recordmcount or recordmcount.pl
@@ -269,7 +265,8 @@ define rule_cc_o_c
 	$(call cmd,checksrc)
 	$(call cmd,checkdoc)
 	$(call cmd,gen_objtooldep)
-	$(call cmd,modversions_c)
+	$(call cmd,gen_symversions_c)
+	$(if $(CONFIG_LTO_CLANG),,$(call cmd,modversions))
 	$(call cmd,record_mcount)
 endef
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 10/10] kbuild: refactor cmd_modversions_S
  2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
                   ` (8 preceding siblings ...)
  2022-04-05 14:02 ` [PATCH v2 09/10] kbuild: refactor cmd_modversions_c Masahiro Yamada
@ 2022-04-05 14:02 ` Masahiro Yamada
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 14:02 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers

Split the code into two macros, cmd_gen_symversions_S for running
genksyms, and cmd_modversions for running $(LD) to update the object
with CRCs.

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

Changes in v2:
  - new

 scripts/Makefile.build | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index ba2be555f942..58be0997c5dd 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -274,7 +274,8 @@ define rule_as_o_S
 	$(call cmd_and_fixdep,as_o_S)
 	$(call cmd,gen_ksymdeps)
 	$(call cmd,gen_objtooldep)
-	$(call cmd,modversions_S)
+	$(call cmd,gen_symversions_S)
+	$(call cmd,modversions)
 endef
 
 # Built-in and composite module parts
@@ -366,16 +367,8 @@ ifdef CONFIG_ASM_MODVERSIONS
 # versioning matches the C process described above, with difference that
 # we parse asm-prototypes.h C header to get function definitions.
 
-cmd_modversions_S =								\
-	if $(OBJDUMP) -h $@ | grep -q __ksymtab; then				\
-		$(call cmd_gensymtypes_S,$(KBUILD_SYMTYPES),$(@:.o=.symtypes))	\
-		    > $(@D)/.tmp_$(@F:.o=.ver);					\
-										\
-		$(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ 		\
-			-T $(@D)/.tmp_$(@F:.o=.ver);				\
-		mv -f $(@D)/.tmp_$(@F) $@;					\
-		rm -f $(@D)/.tmp_$(@F:.o=.ver);					\
-	fi
+cmd_gen_symversions_S = $(call gen_symversions,S)
+
 endif
 
 $(obj)/%.o: $(src)/%.S FORCE
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place
  2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
  0 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Nick Desaulniers, Michal Marek

The assigned 'export' is only used when

    if (strstarts(symname, "__ksymtab_"))

is met. The else-part of the assignment is the dead code.

Move the export_from_secname() call to where it is used.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
---

(no changes since v1)

 scripts/mod/modpost.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index eebb32689816..f9e54247ae1d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -684,14 +684,8 @@ static void handle_modversion(const struct module *mod,
 static void handle_symbol(struct module *mod, struct elf_info *info,
 			  const Elf_Sym *sym, const char *symname)
 {
-	enum export export;
 	const char *name;
 
-	if (strstarts(symname, "__ksymtab"))
-		export = export_from_secname(info, get_secindex(info, sym));
-	else
-		export = export_unknown;
-
 	switch (sym->st_shndx) {
 	case SHN_COMMON:
 		if (strstarts(symname, "__gnu_lto_")) {
@@ -726,7 +720,11 @@ static void handle_symbol(struct module *mod, struct elf_info *info,
 	default:
 		/* All exported symbols */
 		if (strstarts(symname, "__ksymtab_")) {
+			enum export export;
+
 			name = symname + strlen("__ksymtab_");
+			export = export_from_secname(info,
+						     get_secindex(info, sym));
 			sym_add_exported(name, mod, export);
 		}
 		if (strcmp(symname, "init_module") == 0)
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-04-06  1:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-05 14:02 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 03/10] modpost: remove useless export_from_sec() Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 05/10] modpost: remove redundant initializes for static variables Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns() Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 09/10] kbuild: refactor cmd_modversions_c Masahiro Yamada
2022-04-05 14:02 ` [PATCH v2 10/10] kbuild: refactor cmd_modversions_S Masahiro Yamada
  -- strict thread matches above, loose matches on Subject: below --
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).