linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc.
@ 2018-11-20  1:05 Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 1/9] kbuild: let fixdep directly write to .*.cmd files Masahiro Yamada
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	linux-arch, Arnd Bergmann, Michael Ellerman, linux-um,
	linux-kernel, Michal Marek, Richard Weinberger, Andrew Morton,
	Will Deacon, Ard Biesheuvel, Jeff Dike

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


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

* [PATCH v2 1/9] kbuild: let fixdep directly write to .*.cmd files
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 2/9] kbuild: remove redundant 'set -e' from filechk_* defines Masahiro Yamada
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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 <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 2/9] kbuild: remove redundant 'set -e' from filechk_* defines
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 1/9] kbuild: let fixdep directly write to .*.cmd files Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 3/9] kbuild: remove redundant 'set -e' from sub_cmd_record_mcount Masahiro Yamada
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	linux-um, linux-kernel, Michal Marek, Richard Weinberger,
	Jeff Dike

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

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 3/9] kbuild: remove redundant 'set -e' from sub_cmd_record_mcount
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 1/9] kbuild: let fixdep directly write to .*.cmd files Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 2/9] kbuild: remove redundant 'set -e' from filechk_* defines Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 4/9] kbuild: refactor modversions build rules Masahiro Yamada
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 4/9] kbuild: refactor modversions build rules
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (2 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 3/9] kbuild: remove redundant 'set -e' from sub_cmd_record_mcount Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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 <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (3 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 4/9] kbuild: refactor modversions build rules Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  3:47   ` Nicolas Pitre
  2018-11-20  1:05 ` [PATCH v2 6/9] kbuild: change if_changed_rule for multi-line recipe Masahiro Yamada
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	linux-arch, Arnd Bergmann, Michael Ellerman, linux-kernel,
	Michal Marek, Will Deacon, Ard Biesheuvel, Andrew Morton

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 <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 6/9] kbuild: change if_changed_rule for multi-line recipe
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (4 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 7/9] kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule Masahiro Yamada
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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 <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 7/9] kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (5 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 6/9] kbuild: change if_changed_rule for multi-line recipe Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 8/9] kbuild: refactor if_changed and if_changed_dep Masahiro Yamada
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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 <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 8/9] kbuild: refactor if_changed and if_changed_dep
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (6 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 7/9] kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-20  1:05 ` [PATCH v2 9/9] kbuild: remove redundant 'set -e' from cmd_* defines Masahiro Yamada
  2018-11-21 14:39 ` [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

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


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

* [PATCH v2 9/9] kbuild: remove redundant 'set -e' from cmd_* defines
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (7 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 8/9] kbuild: refactor if_changed and if_changed_dep Masahiro Yamada
@ 2018-11-20  1:05 ` Masahiro Yamada
  2018-11-21 14:39 ` [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-20  1:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, Masahiro Yamada,
	Michal Marek, linux-kernel

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 <yamada.masahiro@socionext.com>
---

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


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

* Re: [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
  2018-11-20  1:05 ` [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2018-11-20  3:47   ` Nicolas Pitre
  0 siblings, 0 replies; 12+ messages in thread
From: Nicolas Pitre @ 2018-11-20  3:47 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Sam Ravnborg, Rasmus Villemoes, linux-arch,
	Arnd Bergmann, Michael Ellerman, linux-kernel, Michal Marek,
	Will Deacon, Ard Biesheuvel, Andrew Morton

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 <yamada.masahiro@socionext.com>

Nice work.

Reviewed-by: Nicolas Pitre <nico@linaro.org>

> ---
> 
> 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
> 
> 

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

* Re: [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc.
  2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
                   ` (8 preceding siblings ...)
  2018-11-20  1:05 ` [PATCH v2 9/9] kbuild: remove redundant 'set -e' from cmd_* defines Masahiro Yamada
@ 2018-11-21 14:39 ` Masahiro Yamada
  9 siblings, 0 replies; 12+ messages in thread
From: Masahiro Yamada @ 2018-11-21 14:39 UTC (permalink / raw)
  To: Linux Kbuild mailing list
  Cc: Sam Ravnborg, Nicolas Pitre, Rasmus Villemoes, linux-arch,
	Arnd Bergmann, Michael Ellerman, linux-um,
	Linux Kernel Mailing List, Michal Marek, Richard Weinberger,
	Andrew Morton, Will Deacon, Ard Biesheuvel, Jeff Dike

On Tue, Nov 20, 2018 at 10:11 AM Masahiro Yamada
<yamada.masahiro@socionext.com> 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

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

end of thread, other threads:[~2018-11-21 14:39 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-20  1:05 [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 1/9] kbuild: let fixdep directly write to .*.cmd files Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 2/9] kbuild: remove redundant 'set -e' from filechk_* defines Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 3/9] kbuild: remove redundant 'set -e' from sub_cmd_record_mcount Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 4/9] kbuild: refactor modversions build rules Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
2018-11-20  3:47   ` Nicolas Pitre
2018-11-20  1:05 ` [PATCH v2 6/9] kbuild: change if_changed_rule for multi-line recipe Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 7/9] kbuild: remove trailing semicolon from cmd_* passed to if_changed_rule Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 8/9] kbuild: refactor if_changed and if_changed_dep Masahiro Yamada
2018-11-20  1:05 ` [PATCH v2 9/9] kbuild: remove redundant 'set -e' from cmd_* defines Masahiro Yamada
2018-11-21 14:39 ` [PATCH v2 0/9] kbuild: clean-up modversion, TRIM_UNUSED_KSYMS, if_changed_rule, etc Masahiro Yamada

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