2018-11-20 01:10:13

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc.

As a Kbuild maintainer, I always struggle to keep the core makefiles
clean because people tend to squeeze more and more clutter code into
the kbuild core in order to do what they want to do.

The biggest step forward in this series is to re-implement
the build trick of CONFIG_TRIM_UNUSED_KSYMS in a cleaner way.
scripts/Kbuild.include now looks nice again.
Also, in my rough estimation, building with CONFIG_TRIM_UNUSED_KSYMS
became 40-50 % faster.

Besides those, nice cleanups are here and there.

Masahiro Yamada (9):
kbuild: let fixdep directly write to .*.cmd files
kbuild: remove redundant 'set -e' from filechk_* defines
kbuild: remove redundant 'set -e' from sub_cmd_record_mcount
kbuild: refactor modversions build rules
kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
kbuild: change if_changed_rule for multi-line recipe
kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule
kbuild: refactor if_changed and if_changed_dep
kbuild: remove redundant 'set -e' from cmd_* defines

Makefile | 13 +++---
arch/um/Makefile | 2 +-
include/asm-generic/export.h | 13 +++---
include/linux/export.h | 18 ++++----
scripts/Kbuild.include | 49 +++-----------------
scripts/Makefile.build | 105 ++++++++++++++++++-------------------------
scripts/Makefile.lib | 2 +-
scripts/basic/fixdep.c | 31 ++-----------
scripts/gen_ksymdeps.sh | 25 +++++++++++
scripts/package/Makefile | 1 -
10 files changed, 106 insertions(+), 153 deletions(-)
create mode 100755 scripts/gen_ksymdeps.sh

--
2.7.4



2018-11-20 01:08:24

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 1/9] kbuild: let fixdep directly write to .*.cmd files

Currently, fixdep writes dependencies to .*.tmp, which is renamed to
.*.cmd after everything succeeds. This is a very safe way to avoid
corrupted .*.cmd files. The if_changed_dep has carried this safety
mechanism since it was added in 2002.

If fixdep fails for some reasons or a user terminates the build while
fixdep is running, the incomplete output from the fixdep could be
troublesome.

This is my insight about some bad scenarios:

[1] If the compiler succeeds to generate *.o file, but fixdep fails
to write necessary dependencies to .*.cmd file, Make will miss
to rebuild the object when headers or CONFIG options are changed.
In this case, fixdep should not generate .*.cmd file at all so
that 'arg-check' will surely trigger the rebuild of the object.

[2] A partially constructed .*.cmd file may not be a syntactically
correct makefile. The next time Make runs, it would include it,
then fail to parse it. Once this happens, 'make clean' is be the
only way to fix it.

In fact, [1] is no longer a problem since 9c2af1c7377a ("kbuild: add
.DELETE_ON_ERROR special target"). Make deletes a target file on any
failure in its recipe. Because fixdep is a part of the recipe of *.o
target, if it fails, the *.o is deleted anyway. However, I am a bit
worried about the slight possibility of [2].

So, here is a solution. Let fixdep directly write to a .*.cmd file,
but allow makefiles to include it only when its corresponding target
exists.

This effectively reverts commit 2982c953570b ("kbuild: remove redundant
$(wildcard ...) for cmd_files calculation"), and commit 00d78ab2ba75
("kbuild: remove dead code in cmd_files calculation in top Makefile")
because now we must check the presence of targets.

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

Changes in v2:
- New patch

Makefile | 13 +++++++------
scripts/Kbuild.include | 10 ++++------
scripts/Makefile.build | 10 ++++------
3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index ddbf627..b78cc97 100644
--- a/Makefile
+++ b/Makefile
@@ -1039,6 +1039,8 @@ ifdef CONFIG_GDB_SCRIPTS
endif
+$(call if_changed,link-vmlinux)

+targets := vmlinux
+
# Build samples along the rest of the kernel. This needs headers_install.
ifdef CONFIG_SAMPLES
vmlinux-dirs += samples
@@ -1760,13 +1762,12 @@ quiet_cmd_depmod = DEPMOD $(KERNELRELEASE)
cmd_crmodverdir = $(Q)mkdir -p $(MODVERDIR) \
$(if $(KBUILD_MODULES),; rm -f $(MODVERDIR)/*)

-# read all saved command lines
-cmd_files := $(wildcard .*.cmd)
+# read saved command lines for existing targets
+exist-targets := $(wildcard $(sort $(targets)))

-ifneq ($(cmd_files),)
- $(cmd_files): ; # Do not try to update included dependency files
- include $(cmd_files)
-endif
+cmd_files := $(foreach f,$(exist-targets),$(dir $(f)).$(notdir $(f)).cmd)
+$(cmd_files): ; # Do not try to update included dependency files
+-include $(cmd_files)

endif # ifeq ($(config-targets),1)
endif # ifeq ($(mixed-targets),1)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index bb01555..6cf6a8b 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -264,9 +264,8 @@ ifndef CONFIG_TRIM_UNUSED_KSYMS

cmd_and_fixdep = \
$(echo-cmd) $(cmd_$(1)); \
- scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).tmp;\
- rm -f $(depfile); \
- mv -f $(dot-target).tmp $(dot-target).cmd;
+ scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
+ rm -f $(depfile);

else

@@ -289,9 +288,8 @@ cmd_and_fixdep = \
$(echo-cmd) $(cmd_$(1)); \
$(ksym_dep_filter) | \
scripts/basic/fixdep -e $(depfile) $@ '$(make-cmd)' \
- > $(dot-target).tmp; \
- rm -f $(depfile); \
- mv -f $(dot-target).tmp $(dot-target).cmd;
+ > $(dot-target).cmd; \
+ rm -f $(depfile);

endif

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index a8e7ba9..c909588 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -529,17 +529,15 @@ FORCE:
# optimization, we don't need to read them if the target does not
# exist, we will rebuild anyway in that case.

-cmd_files := $(wildcard $(foreach f,$(sort $(targets)),$(dir $(f)).$(notdir $(f)).cmd))
+exist-targets := $(wildcard $(sort $(targets)))

-ifneq ($(cmd_files),)
- include $(cmd_files)
-endif
+-include $(foreach f,$(exist-targets),$(dir $(f)).$(notdir $(f)).cmd)

ifneq ($(KBUILD_SRC),)
# Create directories for object files if they do not exist
obj-dirs := $(sort $(obj) $(patsubst %/,%, $(dir $(targets))))
-# If cmd_files exist, their directories apparently exist. Skip mkdir.
-exist-dirs := $(sort $(patsubst %/,%, $(dir $(cmd_files))))
+# If targets exist, their directories apparently exist. Skip mkdir.
+exist-dirs := $(sort $(patsubst %/,%, $(dir $(exist-targets))))
obj-dirs := $(strip $(filter-out $(exist-dirs), $(obj-dirs)))
ifneq ($(obj-dirs),)
$(shell mkdir -p $(obj-dirs))
--
2.7.4


2018-11-20 01:08:31

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 9/9] kbuild: remove redundant 'set -e' from cmd_* defines

These three cmd_* are invoked in the $(call cmd,*) form.

Now that 'set -e' moved to the 'cmd' macro, they do not need to
explicitly give 'set -e'.

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

Changes in v2: None

scripts/Makefile.build | 2 --
scripts/package/Makefile | 1 -
2 files changed, 3 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3bad2aa..d589d57 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -134,7 +134,6 @@ cmd_gensymtypes_c = \

quiet_cmd_cc_symtypes_c = SYM $(quiet_modtag) $@
cmd_cc_symtypes_c = \
- set -e; \
$(call cmd_gensymtypes_c,true,$@) >/dev/null; \
test -s $@ || rm -f $@

@@ -337,7 +336,6 @@ cmd_gensymtypes_S = \

quiet_cmd_cc_symtypes_S = SYM $(quiet_modtag) $@
cmd_cc_symtypes_S = \
- set -e; \
$(call cmd_gensymtypes_S,true,$@) >/dev/null; \
test -s $@ || rm -f $@

diff --git a/scripts/package/Makefile b/scripts/package/Makefile
index 73503eb..453fece 100644
--- a/scripts/package/Makefile
+++ b/scripts/package/Makefile
@@ -33,7 +33,6 @@ MKSPEC := $(srctree)/scripts/package/mkspec

quiet_cmd_src_tar = TAR $(2).tar.gz
cmd_src_tar = \
-set -e; \
if test "$(objtree)" != "$(srctree)"; then \
echo >&2; \
echo >&2 " ERROR:"; \
--
2.7.4


2018-11-20 01:08:54

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS

My main motivation of this commit is to clean up scripts/Kbuild.include
and scripts/Makefile.build.

Currently, CONFIG_TRIM_UNUSED_KSYMS works with a tricky gimmick;
possibly exported symbols are detected by letting $(CPP) replace
EXPORT_SYMBOL* with a special string '=== __KSYM_*===', which is
post-processed by sed, and passed to fixdep. The extra preprocessing
is costly, and hacking cmd_and_fixdep is ugly.

I came up with a new way to find exported symbols; insert a dummy
symbol __ksym_marker_* to each potentially exported symbol. Those
dummy symbols are picked up by $(NM), post-processed by sed, then
appended to .*.cmd files. I collected the post-process part to a
new shell script scripts/gen_ksymdeps.sh for readability. The dummy
symbols are put into the .discard.* section so that the linker
script rips them off the final vmlinux or modules.

A nice side-effect is building with CONFIG_TRIM_UNUSED_KSYMS will
be much faster.

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

Changes in v2:
- Let gen_ksymdeps.sh add dependencies to .*.cmd file directly
- Fix typos

include/asm-generic/export.h | 13 ++++++++-----
include/linux/export.h | 18 +++++++++---------
scripts/Kbuild.include | 28 ----------------------------
scripts/Makefile.build | 7 +++++++
scripts/basic/fixdep.c | 31 ++++---------------------------
scripts/gen_ksymdeps.sh | 25 +++++++++++++++++++++++++
6 files changed, 53 insertions(+), 69 deletions(-)
create mode 100755 scripts/gen_ksymdeps.sh

diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 4d73e6e..294d6ae 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -59,16 +59,19 @@ __kcrctab_\name:
.endm
#undef __put

-#if defined(__KSYM_DEPS__)
-
-#define __EXPORT_SYMBOL(sym, val, sec) === __KSYM_##sym ===
-
-#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
+#if defined(CONFIG_TRIM_UNUSED_KSYMS)

#include <linux/kconfig.h>
#include <generated/autoksyms.h>

+.macro __ksym_marker sym
+ .section ".discard.ksym","a"
+__ksym_marker_\sym:
+ .previous
+.endm
+
#define __EXPORT_SYMBOL(sym, val, sec) \
+ __ksym_marker sym; \
__cond_export_sym(sym, val, sec, __is_defined(__KSYM_##sym))
#define __cond_export_sym(sym, val, sec, conf) \
___cond_export_sym(sym, val, sec, conf)
diff --git a/include/linux/export.h b/include/linux/export.h
index ce764a5..fd8711e 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -92,22 +92,22 @@ struct kernel_symbol {
*/
#define __EXPORT_SYMBOL(sym, sec)

-#elif defined(__KSYM_DEPS__)
+#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
+
+#include <generated/autoksyms.h>

/*
* For fine grained build dependencies, we want to tell the build system
* about each possible exported symbol even if they're not actually exported.
- * We use a string pattern that is unlikely to be valid code that the build
- * system filters out from the preprocessor output (see ksym_dep_filter
- * in scripts/Kbuild.include).
+ * We use a symbol pattern __ksym_marker_<symbol> that the build system filters
+ * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
+ * discarded in the final link stage.
*/
-#define __EXPORT_SYMBOL(sym, sec) === __KSYM_##sym ===
-
-#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
-
-#include <generated/autoksyms.h>
+#define __ksym_marker(sym) \
+ static int __ksym_marker_##sym[0] __section(".discard.ksym") __used

#define __EXPORT_SYMBOL(sym, sec) \
+ __ksym_marker(sym); \
__cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
#define __cond_export_sym(sym, sec, conf) \
___cond_export_sym(sym, sec, conf)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 6cf6a8b..4b943f4 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -260,39 +260,11 @@ if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ), \
@set -e; \
$(cmd_and_fixdep), @:)

-ifndef CONFIG_TRIM_UNUSED_KSYMS
-
cmd_and_fixdep = \
$(echo-cmd) $(cmd_$(1)); \
scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
rm -f $(depfile);

-else
-
-# Filter out exported kernel symbol names from the preprocessor output.
-# See also __KSYM_DEPS__ in include/linux/export.h.
-# We disable the depfile generation here, so as not to overwrite the existing
-# depfile while fixdep is parsing it.
-flags_nodeps = $(filter-out -Wp$(comma)-M%, $($(1)))
-ksym_dep_filter = \
- case "$(1)" in \
- cc_*_c|cpp_i_c) \
- $(CPP) $(call flags_nodeps,c_flags) -D__KSYM_DEPS__ $< ;; \
- as_*_S|cpp_s_S) \
- $(CPP) $(call flags_nodeps,a_flags) -D__KSYM_DEPS__ $< ;; \
- boot*|build*|cpp_its_S|*cpp_lds_S|dtc|host*|vdso*) : ;; \
- *) echo "Don't know how to preprocess $(1)" >&2; false ;; \
- esac | tr ";" "\n" | sed -n 's/^.*=== __KSYM_\(.*\) ===.*$$/_\1/p'
-
-cmd_and_fixdep = \
- $(echo-cmd) $(cmd_$(1)); \
- $(ksym_dep_filter) | \
- scripts/basic/fixdep -e $(depfile) $@ '$(make-cmd)' \
- > $(dot-target).cmd; \
- rm -f $(depfile);
-
-endif
-
# Usage: $(call if_changed_rule,foo)
# Will check if $(cmd_foo) or any of the prerequisites changed,
# and if so will execute $(rule_foo).
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4f94579..c23ee45 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -254,9 +254,15 @@ objtool_dep = $(objtool_obj) \
$(wildcard include/config/orc/unwinder.h \
include/config/stack/validation.h)

+ifdef CONFIG_TRIM_UNUSED_KSYMS
+cmd_gen_ksymdeps = \
+ $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd;
+endif
+
define rule_cc_o_c
$(call echo-cmd,checksrc) $(cmd_checksrc) \
$(call cmd_and_fixdep,cc_o_c) \
+ $(cmd_gen_ksymdeps) \
$(cmd_checkdoc) \
$(call echo-cmd,objtool) $(cmd_objtool) \
$(cmd_modversions_c) \
@@ -265,6 +271,7 @@ endef

define rule_as_o_S
$(call cmd_and_fixdep,as_o_S) \
+ $(cmd_gen_ksymdeps) \
$(call echo-cmd,objtool) $(cmd_objtool) \
$(cmd_modversions_S)
endef
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 850966f3..facbd60 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -105,8 +105,7 @@

static void usage(void)
{
- fprintf(stderr, "Usage: fixdep [-e] <depfile> <target> <cmdline>\n");
- fprintf(stderr, " -e insert extra dependencies given on stdin\n");
+ fprintf(stderr, "Usage: fixdep <depfile> <target> <cmdline>\n");
exit(1);
}

@@ -131,21 +130,6 @@ static void print_dep(const char *m, int slen, const char *dir)
printf(".h) \\\n");
}

-static void do_extra_deps(void)
-{
- char buf[80];
-
- while (fgets(buf, sizeof(buf), stdin)) {
- int len = strlen(buf);
-
- if (len < 2 || buf[len - 1] != '\n') {
- fprintf(stderr, "fixdep: bad data on stdin\n");
- exit(1);
- }
- print_dep(buf, len - 1, "include/ksym");
- }
-}
-
struct item {
struct item *next;
unsigned int len;
@@ -293,7 +277,7 @@ static int is_ignored_file(const char *s, int len)
* assignments are parsed not only by make, but also by the rather simple
* parser in scripts/mod/sumversion.c.
*/
-static void parse_dep_file(char *m, const char *target, int insert_extra_deps)
+static void parse_dep_file(char *m, const char *target)
{
char *p;
int is_last, is_target;
@@ -369,9 +353,6 @@ static void parse_dep_file(char *m, const char *target, int insert_extra_deps)
exit(1);
}

- if (insert_extra_deps)
- do_extra_deps();
-
printf("\n%s: $(deps_%s)\n\n", target, target);
printf("$(deps_%s):\n", target);
}
@@ -379,13 +360,9 @@ static void parse_dep_file(char *m, const char *target, int insert_extra_deps)
int main(int argc, char *argv[])
{
const char *depfile, *target, *cmdline;
- int insert_extra_deps = 0;
void *buf;

- if (argc == 5 && !strcmp(argv[1], "-e")) {
- insert_extra_deps = 1;
- argv++;
- } else if (argc != 4)
+ if (argc != 4)
usage();

depfile = argv[1];
@@ -395,7 +372,7 @@ int main(int argc, char *argv[])
printf("cmd_%s := %s\n\n", target, cmdline);

buf = read_file(depfile);
- parse_dep_file(buf, target, insert_extra_deps);
+ parse_dep_file(buf, target);
free(buf);

return 0;
diff --git a/scripts/gen_ksymdeps.sh b/scripts/gen_ksymdeps.sh
new file mode 100755
index 0000000..1324986
--- /dev/null
+++ b/scripts/gen_ksymdeps.sh
@@ -0,0 +1,25 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+set -e
+
+# List of exported symbols
+ksyms=$($NM $1 | sed -n 's/.*__ksym_marker_\(.*\)/\1/p' | tr A-Z a-z)
+
+if [ -z "$ksyms" ]; then
+ exit 0
+fi
+
+echo
+echo "ksymdeps_$1 := \\"
+
+for s in $ksyms
+do
+ echo $s | sed -e 's:^_*: $(wildcard include/ksym/:' \
+ -e 's:__*:/:g' -e 's/$/.h) \\/'
+done
+
+echo
+echo "$1: \$(ksymdeps_$1)"
+echo
+echo "\$(ksymdeps_$1):"
--
2.7.4


2018-11-20 01:08:55

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 7/9] kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule

With the change of rule_cc_o_c / rule_as_o_S in the last commit, each
command is executed in a separate subshell. Rip off unneeded semicolons.

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

Changes in v2:
- Clean up cmd_and_fixdep as well

scripts/Kbuild.include | 2 +-
scripts/Makefile.build | 16 ++++++++--------
2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 978d72b..79c4875 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -263,7 +263,7 @@ if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ), \
cmd_and_fixdep = \
$(echo-cmd) $(cmd_$(1)); \
scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
- rm -f $(depfile);
+ rm -f $(depfile)

# Usage: $(call if_changed_rule,foo)
# Will check if $(cmd_foo) or any of the prerequisites changed,
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index e445b3e..ffd2fe4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -75,14 +75,14 @@ __build: $(if $(KBUILD_BUILTIN),$(builtin-target) $(lib-target) $(extra-y)) \
# Linus' kernel sanity checking tool
ifeq ($(KBUILD_CHECKSRC),1)
quiet_cmd_checksrc = CHECK $<
- cmd_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
+ cmd_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
else ifeq ($(KBUILD_CHECKSRC),2)
quiet_cmd_force_checksrc = CHECK $<
- cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $< ;
+ cmd_force_checksrc = $(CHECK) $(CHECKFLAGS) $(c_flags) $<
endif

ifneq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
- cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $< ;
+ cmd_checkdoc = $(srctree)/scripts/kernel-doc -none $<
endif

# Do section mismatch analysis for each module/built-in.a
@@ -178,7 +178,7 @@ cmd_modversions_c = \
-T $(@D)/.tmp_$(@F:.o=.ver); \
mv -f $(@D)/.tmp_$(@F) $@; \
rm -f $(@D)/.tmp_$(@F:.o=.ver); \
- fi;
+ fi
endif

ifdef CONFIG_FTRACE_MCOUNT_RECORD
@@ -211,7 +211,7 @@ cmd_record_mcount = \
if [ "$(findstring $(CC_FLAGS_FTRACE),$(_c_flags))" = \
"$(CC_FLAGS_FTRACE)" ]; then \
$(sub_cmd_record_mcount) \
- fi;
+ fi
endif # CC_USING_RECORD_MCOUNT
endif # CONFIG_FTRACE_MCOUNT_RECORD

@@ -241,7 +241,7 @@ endif
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file
cmd_objtool = $(if $(patsubst y%,, \
$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
- $(__objtool_obj) $(objtool_args) $@;)
+ $(__objtool_obj) $(objtool_args) $@)
objtool_obj = $(if $(patsubst y%,, \
$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
$(__objtool_obj))
@@ -256,7 +256,7 @@ objtool_dep = $(objtool_obj) \

ifdef CONFIG_TRIM_UNUSED_KSYMS
cmd_gen_ksymdeps = \
- $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd;
+ $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
endif

define rule_cc_o_c
@@ -372,7 +372,7 @@ cmd_modversions_S = \
-T $(@D)/.tmp_$(@F:.o=.ver); \
mv -f $(@D)/.tmp_$(@F) $@; \
rm -f $(@D)/.tmp_$(@F:.o=.ver); \
- fi;
+ fi
endif
endif

--
2.7.4


2018-11-20 01:09:57

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 8/9] kbuild: refactor if_changed and if_changed_dep

'@set -e; $(echo-cmd) $(cmd_$(1)' can be replaced with '$(cmd)'.

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

Changes in v2: None

scripts/Kbuild.include | 9 +++------
scripts/Makefile.build | 4 ++--
2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 79c4875..33aacca 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -251,17 +251,14 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)

# Execute command if command has changed or prerequisite(s) are updated.
if_changed = $(if $(strip $(any-prereq) $(arg-check)), \
- @set -e; \
- $(echo-cmd) $(cmd_$(1)); \
+ $(cmd); \
printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)

# Execute the command and also postprocess generated .d dependencies file.
-if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ), \
- @set -e; \
- $(cmd_and_fixdep), @:)
+if_changed_dep = $(if $(strip $(any-prereq) $(arg-check)), $(cmd_and_fixdep), @:)

cmd_and_fixdep = \
- $(echo-cmd) $(cmd_$(1)); \
+ $(cmd); \
scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
rm -f $(depfile)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index ffd2fe4..3bad2aa 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -261,7 +261,7 @@ endif

define rule_cc_o_c
$(call cmd,checksrc)
- @$(call cmd_and_fixdep,cc_o_c)
+ $(call cmd_and_fixdep,cc_o_c)
$(call cmd,gen_ksymdeps)
$(call cmd,checkdoc)
$(call cmd,objtool)
@@ -270,7 +270,7 @@ define rule_cc_o_c
endef

define rule_as_o_S
- @$(call cmd_and_fixdep,as_o_S)
+ $(call cmd_and_fixdep,as_o_S)
$(call cmd,gen_ksymdeps)
$(call cmd,objtool)
$(call cmd,modversions_S)
--
2.7.4


2018-11-20 02:17:18

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 4/9] kbuild: refactor modversions build rules

Let $(CC) compile objects into normal files *.o instead of .tmp_*.o
whether CONFIG_MODVERSIONS is enabled or not. With this, the input
file for objtool is always *.o so objtool_o can go away.

I guess the reason of using .tmp_*.o for intermediate objects was
to avoid leaving incomplete *.o file (, whose timestamp says it is
up-to-date) when the genksyms tool failed for some reasons.

It no longer matters because any targets are deleted on errors since
commit 9c2af1c7377a ("kbuild: add .DELETE_ON_ERROR special target").

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

Changes in v2:
- Clarify the benefit of this patch
It is nice to delete objtool_o and ease the following commits

scripts/Makefile.build | 54 ++++++++++++++++----------------------------------
1 file changed, 17 insertions(+), 37 deletions(-)

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 032ca24..4f94579 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -154,35 +154,30 @@ $(obj)/%.ll: $(src)/%.c FORCE
# (See cmd_cc_o_c + relevant part of rule_cc_o_c)

quiet_cmd_cc_o_c = CC $(quiet_modtag) $@
+ cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<

-ifndef CONFIG_MODVERSIONS
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<
-
-else
+ifdef CONFIG_MODVERSIONS
# When module versioning is enabled the following steps are executed:
-# o compile a .tmp_<file>.o from <file>.c
-# o if .tmp_<file>.o doesn't contain a __ksymtab version, i.e. does
-# not export symbols, we just rename .tmp_<file>.o to <file>.o and
-# are done.
+# o compile a <file>.o from <file>.c
+# o if <file>.o doesn't contain a __ksymtab version, i.e. does
+# not export symbols, it's done.
# o otherwise, we calculate symbol versions using the good old
# genksyms on the preprocessed source and postprocess them in a way
# that they are usable as a linker script
-# o generate <file>.o from .tmp_<file>.o using the linker to
+# o generate .tmp_<file>.o from <file>.o using the linker to
# replace the unresolved symbols __crc_exported_symbol with
# the actual value of the checksum generated by genksyms
-
-cmd_cc_o_c = $(CC) $(c_flags) -c -o $(@D)/.tmp_$(@F) $<
+# o remove .tmp_<file>.o to <file>.o

cmd_modversions_c = \
- if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then \
+ if $(OBJDUMP) -h $@ | grep -q __ksymtab; then \
$(call cmd_gensymtypes_c,$(KBUILD_SYMTYPES),$(@:.o=.symtypes)) \
> $(@D)/.tmp_$(@F:.o=.ver); \
\
- $(LD) $(KBUILD_LDFLAGS) -r -o $@ $(@D)/.tmp_$(@F) \
+ $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ \
-T $(@D)/.tmp_$(@F:.o=.ver); \
- rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver); \
- else \
mv -f $(@D)/.tmp_$(@F) $@; \
+ rm -f $(@D)/.tmp_$(@F:.o=.ver); \
fi;
endif

@@ -241,19 +236,12 @@ ifneq ($(RETPOLINE_CFLAGS),)
endif
endif

-
-ifdef CONFIG_MODVERSIONS
-objtool_o = $(@D)/.tmp_$(@F)
-else
-objtool_o = $(@)
-endif
-
# 'OBJECT_FILES_NON_STANDARD := y': skip objtool checking for a directory
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'y': skip objtool checking for a file
# 'OBJECT_FILES_NON_STANDARD_foo.o := 'n': override directory skip for a file
cmd_objtool = $(if $(patsubst y%,, \
$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
- $(__objtool_obj) $(objtool_args) "$(objtool_o)";)
+ $(__objtool_obj) $(objtool_args) $@;)
objtool_obj = $(if $(patsubst y%,, \
$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
$(__objtool_obj))
@@ -357,34 +345,26 @@ $(obj)/%.s: $(src)/%.S FORCE
$(call if_changed_dep,cpp_s_S)

quiet_cmd_as_o_S = AS $(quiet_modtag) $@
+ cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<

-ifndef CONFIG_MODVERSIONS
-cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
-
-else
+ifdef CONFIG_MODVERSIONS

ASM_PROTOTYPES := $(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/asm-prototypes.h)

-ifeq ($(ASM_PROTOTYPES),)
-cmd_as_o_S = $(CC) $(a_flags) -c -o $@ $<
-
-else
+ifneq ($(ASM_PROTOTYPES),)

# versioning matches the C process described above, with difference that
# we parse asm-prototypes.h C header to get function definitions.

-cmd_as_o_S = $(CC) $(a_flags) -c -o $(@D)/.tmp_$(@F) $<
-
cmd_modversions_S = \
- if $(OBJDUMP) -h $(@D)/.tmp_$(@F) | grep -q __ksymtab; then \
+ 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) \
+ $(LD) $(KBUILD_LDFLAGS) -r -o $(@D)/.tmp_$(@F) $@ \
-T $(@D)/.tmp_$(@F:.o=.ver); \
- rm -f $(@D)/.tmp_$(@F) $(@D)/.tmp_$(@F:.o=.ver); \
- else \
mv -f $(@D)/.tmp_$(@F) $@; \
+ rm -f $(@D)/.tmp_$(@F:.o=.ver); \
fi;
endif
endif
--
2.7.4


2018-11-20 02:17:18

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 6/9] kbuild: change if_changed_rule for multi-line recipe

The 'define' ... 'endef' directive can describe a recipe that consists
of multiple lines. For example,

all:
@echo hello
@echo world

... can be written as:

define greeting
@echo hello
@echo world
endif

all:
$(greeting)

This is useful to confine a series of shell commands into a single
macro without losing readability.

However, rule_cc_o_c and rule_as_o_S in scripts/Makefile.build are
written like this (with a trailing semicolon in each cmd_*):

define rule_cc_o_c
[action1] ; \
[action2] ; \
[action3] ;
endef

All shell commands are concatenated with '; \' so that it looks like
a single command from the Makefile point of view. This does not
exploit the benefits of 'define' ... 'endef' form because a single
shell command can be simply written, like this:

rule_cc_o_c = \
[action1] ; \
[action2] ; \
[action3] ;

I guess the intention for the command concatenation was to let the
'@set -e' in if_changed_rule cover all the commands.

We can improve the readability by moving '@set -e' to the 'cmd' macro.
The combo of $(call echo-cmd,*) $(cmd_*) in rule_cc_o_c and rule_as_o_S
have been replaced with $(call cmd,*). The trailing back-slashes have
been removed.

Here is a note about the performance: the commands in rule_cc_o_c and
rule_as_o_S were previously executed all together in a single subshell,
but now each line in a separate subshell. This means Make will spawn
extra subshells [1]. I measured the build performance for
x86_64_defconfig + CONFIG_MODVERSIONS + CONFIG_TRIM_UNUSED_KSYMS
and I saw slight performance regression, but I believe code readability
and maintainability wins.

[1] Precisely, GNU Make may optimize this by executing the command
directly instead of forking a subshell, if no shell special
characters are found in the command line and omitting the subshell
will not change the behavior.

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

Changes in v2:
- Rewrite commit message more precisely, and mention about the
build performance

scripts/Kbuild.include | 6 ++----
scripts/Makefile.build | 22 +++++++++++-----------
2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 4b943f4..978d72b 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -215,7 +215,7 @@ echo-cmd = $(if $($(quiet)cmd_$(1)),\
echo ' $(call escsq,$($(quiet)cmd_$(1)))$(echo-why)';)

# printing commands
-cmd = @$(echo-cmd) $(cmd_$(1))
+cmd = @set -e; $(echo-cmd) $(cmd_$(1))

# Add $(obj)/ for paths that are not absolute
objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o)))
@@ -268,9 +268,7 @@ cmd_and_fixdep = \
# Usage: $(call if_changed_rule,foo)
# Will check if $(cmd_foo) or any of the prerequisites changed,
# and if so will execute $(rule_foo).
-if_changed_rule = $(if $(strip $(any-prereq) $(arg-check) ), \
- @set -e; \
- $(rule_$(1)), @:)
+if_changed_rule = $(if $(strip $(any-prereq) $(arg-check)), $(rule_$(1)), @:)

###
# why - tell why a target got built
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index c23ee45..e445b3e 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -260,20 +260,20 @@ cmd_gen_ksymdeps = \
endif

define rule_cc_o_c
- $(call echo-cmd,checksrc) $(cmd_checksrc) \
- $(call cmd_and_fixdep,cc_o_c) \
- $(cmd_gen_ksymdeps) \
- $(cmd_checkdoc) \
- $(call echo-cmd,objtool) $(cmd_objtool) \
- $(cmd_modversions_c) \
- $(call echo-cmd,record_mcount) $(cmd_record_mcount)
+ $(call cmd,checksrc)
+ @$(call cmd_and_fixdep,cc_o_c)
+ $(call cmd,gen_ksymdeps)
+ $(call cmd,checkdoc)
+ $(call cmd,objtool)
+ $(call cmd,modversions_c)
+ $(call cmd,record_mcount)
endef

define rule_as_o_S
- $(call cmd_and_fixdep,as_o_S) \
- $(cmd_gen_ksymdeps) \
- $(call echo-cmd,objtool) $(cmd_objtool) \
- $(cmd_modversions_S)
+ @$(call cmd_and_fixdep,as_o_S)
+ $(call cmd,gen_ksymdeps)
+ $(call cmd,objtool)
+ $(call cmd,modversions_S)
endef

# List module undefined symbols (or empty line if not enabled)
--
2.7.4


2018-11-20 02:21:24

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 3/9] kbuild: remove redundant 'set -e' from sub_cmd_record_mcount

This is executed inside the if_changed_rule, which already sets
'set -e'.

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

Changes in v2: None

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

diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index c909588..032ca24 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -204,7 +204,7 @@ sub_cmd_record_mcount = \
recordmcount_source := $(srctree)/scripts/recordmcount.c \
$(srctree)/scripts/recordmcount.h
else
-sub_cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
+sub_cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
"$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
"$(if $(CONFIG_64BIT),64,32)" \
"$(OBJDUMP)" "$(OBJCOPY)" "$(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS)" \
--
2.7.4


2018-11-20 02:21:32

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 2/9] kbuild: remove redundant 'set -e' from filechk_* defines

The filechk macro in scripts/Kbuild.include already sets 'set -e'.

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

Changes in v2: None

arch/um/Makefile | 2 +-
scripts/Makefile.lib | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/um/Makefile b/arch/um/Makefile
index ab1066c..71ff3d0 100644
--- a/arch/um/Makefile
+++ b/arch/um/Makefile
@@ -152,7 +152,7 @@ $(HOST_DIR)/um/user-offsets.s: __headers FORCE
$(Q)$(MAKE) $(build)=$(HOST_DIR)/um $@

define filechk_gen-asm-offsets
- (set -e; \
+ ( \
echo "/*"; \
echo " * DO NOT MODIFY."; \
echo " *"; \
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 8fe4468..ae3ae97 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -406,7 +406,7 @@ endef
# Use filechk to avoid rebuilds when a header changes, but the resulting file
# does not
define filechk_offsets
- (set -e; \
+ ( \
echo "#ifndef $2"; \
echo "#define $2"; \
echo "/*"; \
--
2.7.4


2018-11-20 03:48:16

by Nicolas Pitre

[permalink] [raw]
Subject: Re: [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS

On Tue, 20 Nov 2018, Masahiro Yamada wrote:

> My main motivation of this commit is to clean up scripts/Kbuild.include
> and scripts/Makefile.build.
>
> Currently, CONFIG_TRIM_UNUSED_KSYMS works with a tricky gimmick;
> possibly exported symbols are detected by letting $(CPP) replace
> EXPORT_SYMBOL* with a special string '=== __KSYM_*===', which is
> post-processed by sed, and passed to fixdep. The extra preprocessing
> is costly, and hacking cmd_and_fixdep is ugly.
>
> I came up with a new way to find exported symbols; insert a dummy
> symbol __ksym_marker_* to each potentially exported symbol. Those
> dummy symbols are picked up by $(NM), post-processed by sed, then
> appended to .*.cmd files. I collected the post-process part to a
> new shell script scripts/gen_ksymdeps.sh for readability. The dummy
> symbols are put into the .discard.* section so that the linker
> script rips them off the final vmlinux or modules.
>
> A nice side-effect is building with CONFIG_TRIM_UNUSED_KSYMS will
> be much faster.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

Nice work.

Reviewed-by: Nicolas Pitre <[email protected]>

> ---
>
> Changes in v2:
> - Let gen_ksymdeps.sh add dependencies to .*.cmd file directly
> - Fix typos
>
> include/asm-generic/export.h | 13 ++++++++-----
> include/linux/export.h | 18 +++++++++---------
> scripts/Kbuild.include | 28 ----------------------------
> scripts/Makefile.build | 7 +++++++
> scripts/basic/fixdep.c | 31 ++++---------------------------
> scripts/gen_ksymdeps.sh | 25 +++++++++++++++++++++++++
> 6 files changed, 53 insertions(+), 69 deletions(-)
> create mode 100755 scripts/gen_ksymdeps.sh
>
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index 4d73e6e..294d6ae 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -59,16 +59,19 @@ __kcrctab_\name:
> .endm
> #undef __put
>
> -#if defined(__KSYM_DEPS__)
> -
> -#define __EXPORT_SYMBOL(sym, val, sec) === __KSYM_##sym ===
> -
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +#if defined(CONFIG_TRIM_UNUSED_KSYMS)
>
> #include <linux/kconfig.h>
> #include <generated/autoksyms.h>
>
> +.macro __ksym_marker sym
> + .section ".discard.ksym","a"
> +__ksym_marker_\sym:
> + .previous
> +.endm
> +
> #define __EXPORT_SYMBOL(sym, val, sec) \
> + __ksym_marker sym; \
> __cond_export_sym(sym, val, sec, __is_defined(__KSYM_##sym))
> #define __cond_export_sym(sym, val, sec, conf) \
> ___cond_export_sym(sym, val, sec, conf)
> diff --git a/include/linux/export.h b/include/linux/export.h
> index ce764a5..fd8711e 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -92,22 +92,22 @@ struct kernel_symbol {
> */
> #define __EXPORT_SYMBOL(sym, sec)
>
> -#elif defined(__KSYM_DEPS__)
> +#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +
> +#include <generated/autoksyms.h>
>
> /*
> * For fine grained build dependencies, we want to tell the build system
> * about each possible exported symbol even if they're not actually exported.
> - * We use a string pattern that is unlikely to be valid code that the build
> - * system filters out from the preprocessor output (see ksym_dep_filter
> - * in scripts/Kbuild.include).
> + * We use a symbol pattern __ksym_marker_<symbol> that the build system filters
> + * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
> + * discarded in the final link stage.
> */
> -#define __EXPORT_SYMBOL(sym, sec) === __KSYM_##sym ===
> -
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> -
> -#include <generated/autoksyms.h>
> +#define __ksym_marker(sym) \
> + static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
>
> #define __EXPORT_SYMBOL(sym, sec) \
> + __ksym_marker(sym); \
> __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
> #define __cond_export_sym(sym, sec, conf) \
> ___cond_export_sym(sym, sec, conf)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 6cf6a8b..4b943f4 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -260,39 +260,11 @@ if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ), \
> @set -e; \
> $(cmd_and_fixdep), @:)
>
> -ifndef CONFIG_TRIM_UNUSED_KSYMS
> -
> cmd_and_fixdep = \
> $(echo-cmd) $(cmd_$(1)); \
> scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
> rm -f $(depfile);
>
> -else
> -
> -# Filter out exported kernel symbol names from the preprocessor output.
> -# See also __KSYM_DEPS__ in include/linux/export.h.
> -# We disable the depfile generation here, so as not to overwrite the existing
> -# depfile while fixdep is parsing it.
> -flags_nodeps = $(filter-out -Wp$(comma)-M%, $($(1)))
> -ksym_dep_filter = \
> - case "$(1)" in \
> - cc_*_c|cpp_i_c) \
> - $(CPP) $(call flags_nodeps,c_flags) -D__KSYM_DEPS__ $< ;; \
> - as_*_S|cpp_s_S) \
> - $(CPP) $(call flags_nodeps,a_flags) -D__KSYM_DEPS__ $< ;; \
> - boot*|build*|cpp_its_S|*cpp_lds_S|dtc|host*|vdso*) : ;; \
> - *) echo "Don't know how to preprocess $(1)" >&2; false ;; \
> - esac | tr ";" "\n" | sed -n 's/^.*=== __KSYM_\(.*\) ===.*$$/_\1/p'
> -
> -cmd_and_fixdep = \
> - $(echo-cmd) $(cmd_$(1)); \
> - $(ksym_dep_filter) | \
> - scripts/basic/fixdep -e $(depfile) $@ '$(make-cmd)' \
> - > $(dot-target).cmd; \
> - rm -f $(depfile);
> -
> -endif
> -
> # Usage: $(call if_changed_rule,foo)
> # Will check if $(cmd_foo) or any of the prerequisites changed,
> # and if so will execute $(rule_foo).
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 4f94579..c23ee45 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -254,9 +254,15 @@ objtool_dep = $(objtool_obj) \
> $(wildcard include/config/orc/unwinder.h \
> include/config/stack/validation.h)
>
> +ifdef CONFIG_TRIM_UNUSED_KSYMS
> +cmd_gen_ksymdeps = \
> + $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd;
> +endif
> +
> define rule_cc_o_c
> $(call echo-cmd,checksrc) $(cmd_checksrc) \
> $(call cmd_and_fixdep,cc_o_c) \
> + $(cmd_gen_ksymdeps) \
> $(cmd_checkdoc) \
> $(call echo-cmd,objtool) $(cmd_objtool) \
> $(cmd_modversions_c) \
> @@ -265,6 +271,7 @@ endef
>
> define rule_as_o_S
> $(call cmd_and_fixdep,as_o_S) \
> + $(cmd_gen_ksymdeps) \
> $(call echo-cmd,objtool) $(cmd_objtool) \
> $(cmd_modversions_S)
> endef
> diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
> index 850966f3..facbd60 100644
> --- a/scripts/basic/fixdep.c
> +++ b/scripts/basic/fixdep.c
> @@ -105,8 +105,7 @@
>
> static void usage(void)
> {
> - fprintf(stderr, "Usage: fixdep [-e] <depfile> <target> <cmdline>\n");
> - fprintf(stderr, " -e insert extra dependencies given on stdin\n");
> + fprintf(stderr, "Usage: fixdep <depfile> <target> <cmdline>\n");
> exit(1);
> }
>
> @@ -131,21 +130,6 @@ static void print_dep(const char *m, int slen, const char *dir)
> printf(".h) \\\n");
> }
>
> -static void do_extra_deps(void)
> -{
> - char buf[80];
> -
> - while (fgets(buf, sizeof(buf), stdin)) {
> - int len = strlen(buf);
> -
> - if (len < 2 || buf[len - 1] != '\n') {
> - fprintf(stderr, "fixdep: bad data on stdin\n");
> - exit(1);
> - }
> - print_dep(buf, len - 1, "include/ksym");
> - }
> -}
> -
> struct item {
> struct item *next;
> unsigned int len;
> @@ -293,7 +277,7 @@ static int is_ignored_file(const char *s, int len)
> * assignments are parsed not only by make, but also by the rather simple
> * parser in scripts/mod/sumversion.c.
> */
> -static void parse_dep_file(char *m, const char *target, int insert_extra_deps)
> +static void parse_dep_file(char *m, const char *target)
> {
> char *p;
> int is_last, is_target;
> @@ -369,9 +353,6 @@ static void parse_dep_file(char *m, const char *target, int insert_extra_deps)
> exit(1);
> }
>
> - if (insert_extra_deps)
> - do_extra_deps();
> -
> printf("\n%s: $(deps_%s)\n\n", target, target);
> printf("$(deps_%s):\n", target);
> }
> @@ -379,13 +360,9 @@ static void parse_dep_file(char *m, const char *target, int insert_extra_deps)
> int main(int argc, char *argv[])
> {
> const char *depfile, *target, *cmdline;
> - int insert_extra_deps = 0;
> void *buf;
>
> - if (argc == 5 && !strcmp(argv[1], "-e")) {
> - insert_extra_deps = 1;
> - argv++;
> - } else if (argc != 4)
> + if (argc != 4)
> usage();
>
> depfile = argv[1];
> @@ -395,7 +372,7 @@ int main(int argc, char *argv[])
> printf("cmd_%s := %s\n\n", target, cmdline);
>
> buf = read_file(depfile);
> - parse_dep_file(buf, target, insert_extra_deps);
> + parse_dep_file(buf, target);
> free(buf);
>
> return 0;
> diff --git a/scripts/gen_ksymdeps.sh b/scripts/gen_ksymdeps.sh
> new file mode 100755
> index 0000000..1324986
> --- /dev/null
> +++ b/scripts/gen_ksymdeps.sh
> @@ -0,0 +1,25 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +set -e
> +
> +# List of exported symbols
> +ksyms=$($NM $1 | sed -n 's/.*__ksym_marker_\(.*\)/\1/p' | tr A-Z a-z)
> +
> +if [ -z "$ksyms" ]; then
> + exit 0
> +fi
> +
> +echo
> +echo "ksymdeps_$1 := \\"
> +
> +for s in $ksyms
> +do
> + echo $s | sed -e 's:^_*: $(wildcard include/ksym/:' \
> + -e 's:__*:/:g' -e 's/$/.h) \\/'
> +done
> +
> +echo
> +echo "$1: \$(ksymdeps_$1)"
> +echo
> +echo "\$(ksymdeps_$1):"
> --
> 2.7.4
>
>

2018-11-21 17:48:12

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc.

On Tue, Nov 20, 2018 at 10:11 AM Masahiro Yamada
<[email protected]> wrote:
>
> As a Kbuild maintainer, I always struggle to keep the core makefiles
> clean because people tend to squeeze more and more clutter code into
> the kbuild core in order to do what they want to do.
>
> The biggest step forward in this series is to re-implement
> the build trick of CONFIG_TRIM_UNUSED_KSYMS in a cleaner way.
> scripts/Kbuild.include now looks nice again.
> Also, in my rough estimation, building with CONFIG_TRIM_UNUSED_KSYMS
> became 40-50 % faster.
>
> Besides those, nice cleanups are here and there.
>
> Masahiro Yamada (9):
> kbuild: let fixdep directly write to .*.cmd files
> kbuild: remove redundant 'set -e' from filechk_* defines
> kbuild: remove redundant 'set -e' from sub_cmd_record_mcount
> kbuild: refactor modversions build rules
> kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
> kbuild: change if_changed_rule for multi-line recipe
> kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule
> kbuild: refactor if_changed and if_changed_dep
> kbuild: remove redundant 'set -e' from cmd_* defines

Series, applied to linux-kbuild.





> Makefile | 13 +++---
> arch/um/Makefile | 2 +-
> include/asm-generic/export.h | 13 +++---
> include/linux/export.h | 18 ++++----
> scripts/Kbuild.include | 49 +++-----------------
> scripts/Makefile.build | 105 ++++++++++++++++++-------------------------
> scripts/Makefile.lib | 2 +-
> scripts/basic/fixdep.c | 31 ++-----------
> scripts/gen_ksymdeps.sh | 25 +++++++++++
> scripts/package/Makefile | 1 -
> 10 files changed, 106 insertions(+), 153 deletions(-)
> create mode 100755 scripts/gen_ksymdeps.sh
>
> --
> 2.7.4
>


--
Best Regards
Masahiro Yamada