* [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S}
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 16:16 ` Nick Desaulniers
2022-04-05 20:00 ` Nicolas Schier
2022-04-05 11:33 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
` (8 subsequent siblings)
9 siblings, 2 replies; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 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] 24+ messages in thread* Re: [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S}
2022-04-05 11:33 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
@ 2022-04-05 16:16 ` Nick Desaulniers
2022-04-05 20:00 ` Nicolas Schier
1 sibling, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2022-04-05 16:16 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek
On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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>
It looks like you may have sent v2 twice; apologies if I comment on
BOTH threads rather than 1.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S}
2022-04-05 11:33 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
2022-04-05 16:16 ` Nick Desaulniers
@ 2022-04-05 20:00 ` Nicolas Schier
1 sibling, 0 replies; 24+ messages in thread
From: Nicolas Schier @ 2022-04-05 20:00 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Michal Marek, Nick Desaulniers
On Tue, Apr 05, 2022 at 08:33:49PM +0900 Masahiro Yamada wrote:
> 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>
> ---
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
>
> 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
>
>
--
epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 16:18 ` Nick Desaulniers
2022-04-05 20:01 ` Nicolas Schier
2022-04-05 11:33 ` [PATCH v2 03/10] modpost: remove useless export_from_sec() Masahiro Yamada
` (7 subsequent siblings)
9 siblings, 2 replies; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 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] 24+ messages in thread* Re: [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly
2022-04-05 11:33 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
@ 2022-04-05 16:18 ` Nick Desaulniers
2022-04-05 20:01 ` Nicolas Schier
1 sibling, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2022-04-05 16:18 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek
On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly
2022-04-05 11:33 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
2022-04-05 16:18 ` Nick Desaulniers
@ 2022-04-05 20:01 ` Nicolas Schier
1 sibling, 0 replies; 24+ messages in thread
From: Nicolas Schier @ 2022-04-05 20:01 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Michal Marek, Nick Desaulniers
On Tue, Apr 05, 2022 at 08:33:50PM +0900 Masahiro Yamada wrote:
> 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>
> ---
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
>
> 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
>
>
--
epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 03/10] modpost: remove useless export_from_sec()
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 01/10] kbuild: factor out genksyms command from cmd_gensymtypes_{c,S} Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 02/10] kbuild: do not remove empty *.symtypes explicitly Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place Masahiro Yamada
` (6 subsequent siblings)
9 siblings, 0 replies; 24+ 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
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] 24+ 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
` (2 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 03/10] modpost: remove useless export_from_sec() Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 05/10] modpost: remove redundant initializes for static variables Masahiro Yamada
` (5 subsequent siblings)
9 siblings, 0 replies; 24+ 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] 24+ messages in thread* [PATCH v2 05/10] modpost: remove redundant initializes for static variables
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
` (3 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 04/10] modpost: move export_from_secname() call to more relevant place Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 16:21 ` Nick Desaulniers
2022-04-05 11:33 ` [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns() Masahiro Yamada
` (4 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers
These are initialized with zeros 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] 24+ messages in thread* Re: [PATCH v2 05/10] modpost: remove redundant initializes for static variables
2022-04-05 11:33 ` [PATCH v2 05/10] modpost: remove redundant initializes for static variables Masahiro Yamada
@ 2022-04-05 16:21 ` Nick Desaulniers
0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2022-04-05 16:21 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek
On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> These are initialized with zeros without explicit initializes.
s/initializes/initializers/
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns()
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
` (4 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 05/10] modpost: remove redundant initializes for static variables Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 16:26 ` Nick Desaulniers
2022-04-05 11:33 ` [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files Masahiro Yamada
` (3 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 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] 24+ messages in thread* Re: [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns()
2022-04-05 11:33 ` [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns() Masahiro Yamada
@ 2022-04-05 16:26 ` Nick Desaulniers
0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2022-04-05 16:26 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel, Michal Marek
On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
` (5 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 06/10] modpost: remove annoying namespace_from_kstrtabns() Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 20:16 ` Nicolas Schier
2022-04-05 11:33 ` [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms Masahiro Yamada
` (2 subsequent siblings)
9 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 UTC (permalink / raw)
To: linux-kbuild
Cc: linux-kernel, Masahiro Yamada, Michal Marek, Nick Desaulniers
The first line of *.mod lists the member objects of the module.
This list 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] 24+ messages in thread* Re: [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files
2022-04-05 11:33 ` [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files Masahiro Yamada
@ 2022-04-05 20:16 ` Nicolas Schier
2022-04-06 5:28 ` Masahiro Yamada
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Schier @ 2022-04-05 20:16 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Michal Marek, Nick Desaulniers
On Tue, Apr 05, 2022 at 08:33:55PM +0900 Masahiro Yamada wrote:
> The first line of *.mod lists the member objects of the module.
> This list 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)); \
I'd rather expected to see $(or) here, too, as in commit 5c8166419acf ("kbuild:
replace $(if A,A,B) with $(or A,B)").
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
> $(undefined_syms) echo; \
> } > $@
>
> --
> 2.32.0
>
>
--
epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
-- frykten for herren er opphav til kunnskap --
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files
2022-04-05 20:16 ` Nicolas Schier
@ 2022-04-06 5:28 ` Masahiro Yamada
2022-04-06 14:49 ` Masahiro Yamada
0 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-06 5:28 UTC (permalink / raw)
To: Nicolas Schier
Cc: Linux Kbuild mailing list, Linux Kernel Mailing List,
Michal Marek, Nick Desaulniers
On Wed, Apr 6, 2022 at 5:16 AM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Tue, Apr 05, 2022 at 08:33:55PM +0900 Masahiro Yamada wrote:
> > The first line of *.mod lists the member objects of the module.
> > This list 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)); \
>
> I'd rather expected to see $(or) here, too, as in commit 5c8166419acf ("kbuild:
> replace $(if A,A,B) with $(or A,B)").
Ah, good catch.
I fixed it up locally.
Thanks for the review.
>
> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
>
> > $(undefined_syms) echo; \
> > } > $@
> >
> > --
> > 2.32.0
> >
> >
>
> --
> epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc
> ↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f
> -- frykten for herren er opphav til kunnskap --
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files
2022-04-06 5:28 ` Masahiro Yamada
@ 2022-04-06 14:49 ` Masahiro Yamada
0 siblings, 0 replies; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-06 14:49 UTC (permalink / raw)
To: Nicolas Schier
Cc: Linux Kbuild mailing list, Linux Kernel Mailing List,
Michal Marek, Nick Desaulniers
On Wed, Apr 6, 2022 at 2:28 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Apr 6, 2022 at 5:16 AM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > On Tue, Apr 05, 2022 at 08:33:55PM +0900 Masahiro Yamada wrote:
> > > The first line of *.mod lists the member objects of the module.
> > > This list 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)); \
> >
> > I'd rather expected to see $(or) here, too, as in commit 5c8166419acf ("kbuild:
> > replace $(if A,A,B) with $(or A,B)").
>
> Ah, good catch.
>
> I fixed it up locally.
I changed my mind.
I will throw away 07 and 08.
I will send different patches later.
>
> Thanks for the review.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
` (6 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 07/10] kbuild: get rid of duplication in the first line of *.mod files Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 20:30 ` Nicolas Schier
2022-04-05 11:33 ` [PATCH v2 09/10] kbuild: refactor cmd_modversions_c Masahiro Yamada
2022-04-05 11:33 ` [PATCH v2 10/10] kbuild: refactor cmd_modversions_S Masahiro Yamada
9 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 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, lets' 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] 24+ messages in thread* Re: [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms
2022-04-05 11:33 ` [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms Masahiro Yamada
@ 2022-04-05 20:30 ` Nicolas Schier
0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Schier @ 2022-04-05 20:30 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Alexander Lobakin, Michal Marek,
Nick Desaulniers, Nicolas Schier, Rasmus Villemoes,
Sami Tolvanen
On Tue, Apr 05, 2022 at 08:33:56PM +0900 Masahiro Yamada 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.
>
> These two are orthogonal. For further cleanups, lets' split the second
s/lets'/let's/ ?
> 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
>
>
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 09/10] kbuild: refactor cmd_modversions_c
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
` (7 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 08/10] kbuild: split the second line of *.mod into *.usyms Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 16:42 ` Nick Desaulniers
2022-04-05 11:33 ` [PATCH v2 10/10] kbuild: refactor cmd_modversions_S Masahiro Yamada
9 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 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] 24+ messages in thread* Re: [PATCH v2 09/10] kbuild: refactor cmd_modversions_c
2022-04-05 11:33 ` [PATCH v2 09/10] kbuild: refactor cmd_modversions_c Masahiro Yamada
@ 2022-04-05 16:42 ` Nick Desaulniers
0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2022-04-05 16:42 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Michal Marek, Nathan Chancellor,
llvm, Sami Tolvanen, Kees Cook
On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 10/10] kbuild: refactor cmd_modversions_S
2022-04-05 11:33 [PATCH v2 00/10] kbuild: misc cleanups Masahiro Yamada
` (8 preceding siblings ...)
2022-04-05 11:33 ` [PATCH v2 09/10] kbuild: refactor cmd_modversions_c Masahiro Yamada
@ 2022-04-05 11:33 ` Masahiro Yamada
2022-04-05 16:47 ` Nick Desaulniers
9 siblings, 1 reply; 24+ messages in thread
From: Masahiro Yamada @ 2022-04-05 11:33 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] 24+ messages in thread* Re: [PATCH v2 10/10] kbuild: refactor cmd_modversions_S
2022-04-05 11:33 ` [PATCH v2 10/10] kbuild: refactor cmd_modversions_S Masahiro Yamada
@ 2022-04-05 16:47 ` Nick Desaulniers
0 siblings, 0 replies; 24+ messages in thread
From: Nick Desaulniers @ 2022-04-05 16:47 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, Michal Marek, Sami Tolvanen,
Kees Cook, Nathan Chancellor
On Tue, Apr 5, 2022 at 4:34 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> 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>
Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>
> 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 24+ messages in thread