[PATCH v2 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS

[PATCH v2 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS

[Masahiro Yamada]

Masahiro Yamada (7):
  kbuild: clear LDFLAGS in the top Makefile
  kbuild: remove wrong 'touch' in adjust_autoksyms.sh
  kbuild: move 'scripts' target below
  kbuild: restore touching autoksyms.h to the top Makefile
  kbuild: move CONFIG_TRIM_UNUSED_KSYMS code unneeded for external
    module
  kbuild: move include/config/ksym/* to include/ksym/*
  kbuild: link vmlinux only once for CONFIG_TRIM_UNUSED_KSYMS

 .gitignore                  |  1 +
 Makefile                    | 66 ++++++++++++++++++++++-----------------------
 scripts/Kbuild.include      |  2 +-
 scripts/adjust_autoksyms.sh |  5 +---
 scripts/basic/fixdep.c      |  8 +++---
 scripts/kconfig/Makefile    |  2 --
 6 files changed, 39 insertions(+), 45 deletions(-)

-- 
2.7.4

[PATCH v2 1/7] kbuild: clear LDFLAGS in the top Makefile

[Masahiro Yamada]

Currently LDFLAGS is not cleared, so same flags are accumulated in
LDFLAGS when the top Makefile is recursively invoked.

I found unneeded rebuild for ARCH=arm64 when CONFIG_TRIM_UNUSED_KSYMS
is enabled.  If include/generated/autoksyms.h is updated, the top
Makefile is recursively invoked, then arch/arm64/Makefile adds one
more '-maarch64linux'.  Due to the command line change, modules are
rebuilt needlessly.

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

Changes in v2: None

 Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile b/Makefile
index d9bb6dd6..ac8755d 100644
--- a/Makefile
+++ b/Makefile
@@ -437,6 +437,7 @@ KBUILD_CFLAGS_KERNEL :=
 KBUILD_AFLAGS_MODULE  := -DMODULE
 KBUILD_CFLAGS_MODULE  := -DMODULE
 KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
+LDFLAGS :=
 GCC_PLUGINS_CFLAGS :=
 
 export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
-- 
2.7.4

Re: [PATCH v2 1/7] kbuild: clear LDFLAGS in the top Makefile

[Nicolas Pitre]

On Fri, 16 Mar 2018, Masahiro Yamada wrote:

> Currently LDFLAGS is not cleared, so same flags are accumulated in
> LDFLAGS when the top Makefile is recursively invoked.
> 
> I found unneeded rebuild for ARCH=arm64 when CONFIG_TRIM_UNUSED_KSYMS
> is enabled.  If include/generated/autoksyms.h is updated, the top
> Makefile is recursively invoked, then arch/arm64/Makefile adds one
> more '-maarch64linux'.  Due to the command line change, modules are
> rebuilt needlessly.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

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


> ---
> 
> Changes in v2: None
> 
>  Makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Makefile b/Makefile
> index d9bb6dd6..ac8755d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -437,6 +437,7 @@ KBUILD_CFLAGS_KERNEL :=
>  KBUILD_AFLAGS_MODULE  := -DMODULE
>  KBUILD_CFLAGS_MODULE  := -DMODULE
>  KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
> +LDFLAGS :=
>  GCC_PLUGINS_CFLAGS :=
>  
>  export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC
> -- 
> 2.7.4
> 
> 

[PATCH v2 2/7] kbuild: remove wrong 'touch' in adjust_autoksyms.sh

[Masahiro Yamada]

The comment mentions it creates autoksyms.h in case it is missing,
but the actual code touches it when it does exists.

The build system creates it anyway because <linux/export.h> and
<asm-generic/export.h> need it.

The code would not have worked as intended, and people have not
noticed it.  This is a proof that we can simply remove it.

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

Changes in v2:
  - Remove the code instead of fixing it

 scripts/adjust_autoksyms.sh | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index a162258..e0dd0d5 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -48,9 +48,6 @@ case "${KCONFIG_CONFIG}" in
 	. "./${KCONFIG_CONFIG}"
 esac
 
-# In case it doesn't exist yet...
-if [ -e "$cur_ksyms_file" ]; then touch "$cur_ksyms_file"; fi
-
 # Generate a new ksym list file with symbols needed by the current
 # set of modules.
 cat > "$new_ksyms_file" << EOT
-- 
2.7.4

Re: [PATCH v2 2/7] kbuild: remove wrong 'touch' in adjust_autoksyms.sh

[Nicolas Pitre]

On Fri, 16 Mar 2018, Masahiro Yamada wrote:

> The comment mentions it creates autoksyms.h in case it is missing,
> but the actual code touches it when it does exists.
> 
> The build system creates it anyway because <linux/export.h> and
> <asm-generic/export.h> need it.
> 
> The code would not have worked as intended, and people have not
> noticed it.  This is a proof that we can simply remove it.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

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

> ---
> 
> Changes in v2:
>   - Remove the code instead of fixing it
> 
>  scripts/adjust_autoksyms.sh | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index a162258..e0dd0d5 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -48,9 +48,6 @@ case "${KCONFIG_CONFIG}" in
>  	. "./${KCONFIG_CONFIG}"
>  esac
>  
> -# In case it doesn't exist yet...
> -if [ -e "$cur_ksyms_file" ]; then touch "$cur_ksyms_file"; fi
> -
>  # Generate a new ksym list file with symbols needed by the current
>  # set of modules.
>  cat > "$new_ksyms_file" << EOT
> -- 
> 2.7.4
> 
> 

[PATCH v2 3/7] kbuild: move 'scripts' target below

[Masahiro Yamada]

Just a trivial change to prepare for the next commit.
This target is still invisible from external module building.

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

Changes in v2: None

 Makefile | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index ac8755d..f8e9e12 100644
--- a/Makefile
+++ b/Makefile
@@ -567,14 +567,6 @@ endif
 export KBUILD_MODULES KBUILD_BUILTIN
 
 ifeq ($(KBUILD_EXTMOD),)
-# Additional helpers built in scripts/
-# Carefully list dependencies so we do not try to build scripts twice
-# in parallel
-PHONY += scripts
-scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
-	 asm-generic gcc-plugins
-	$(Q)$(MAKE) $(build)=$(@)
-
 # Objects we will link into vmlinux / subdirs we need to visit
 init-y		:= init/
 drivers-y	:= drivers/ sound/ firmware/
@@ -1070,6 +1062,13 @@ endef
 include/config/kernel.release: include/config/auto.conf FORCE
 	$(call filechk,kernel.release)
 
+# Additional helpers built in scripts/
+# Carefully list dependencies so we do not try to build scripts twice
+# in parallel
+PHONY += scripts
+scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
+	 asm-generic gcc-plugins autoksyms
+	$(Q)$(MAKE) $(build)=$(@)
 
 # Things we need to do before we recursively start building the kernel
 # or the modules are listed in "prepare".
-- 
2.7.4

[PATCH v2 4/7] kbuild: restore touching autoksyms.h to the top Makefile

[Masahiro Yamada]

Commit d3fc425e819b ("kbuild: make sure autoksyms.h exists early")
moved the code that touches autoksyms.h to scripts/kconfig/Makefile
with obscure reason.

>From Nicolas' comment [1], he did not seem to be sure about the root
cause.

I guess I figured it out, so here is a fix-up I think is more correct.
According to the error log in the original post [2], the build failed
in scripts/mod/devicetable-offsets.c

scripts/mod/Makefile is descended from scripts/Makefile, which is
invoked from the top-level Makefile by the 'scripts' target.

To build vmlinux and/or modules, Kbuild descend into $(vmlinux-dirs).
This depends on 'prepare' and 'scripts' as follows:

  $(vmlinux-dirs): prepare scripts

Because there is no dependency between 'prepare' and 'scripts', the
parallel building can run them simultaneously.

'prepare' depends on 'prepare1', which touched autoksyms.h, whereas
'scripts' descends into script/, then scripts/mod/, which needs
<generated/autoksyms.h> if CONFIG_TRIM_UNUSED_KSYMS.  It was the
reason of the race.

I am not happy to have unrelated code in the Kconfig Makefile, so
getting it back to the top Makefile.

I removed the standalone test target because I want to use it to
create an empty autoksyms.h file.  Here is a little improvement;
unnecessary autoksyms.h is not created when CONFIG_TRIM_UNUSED_KSYMS
is disabled.

[1] https://lkml.org/lkml/2016/11/30/734
[2] https://lkml.org/lkml/2016/11/30/531

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Nicolas Pitre <nico@linaro.org>
---

Changes in v2: None

 Makefile                 | 12 +++++++-----
 scripts/kconfig/Makefile |  2 --
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index f8e9e12..0a3895c 100644
--- a/Makefile
+++ b/Makefile
@@ -1021,9 +1021,11 @@ ifdef CONFIG_TRIM_UNUSED_KSYMS
 	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
 endif
 
-# standalone target for easier testing
-include/generated/autoksyms.h: FORCE
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh true
+autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
+
+$(autoksyms_h):
+	$(Q)mkdir -p $(dir $@)
+	$(Q)touch $@
 
 ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
 
@@ -1067,7 +1069,7 @@ include/config/kernel.release: include/config/auto.conf FORCE
 # in parallel
 PHONY += scripts
 scripts: scripts_basic include/config/auto.conf include/config/tristate.conf \
-	 asm-generic gcc-plugins autoksyms
+	 asm-generic gcc-plugins $(autoksyms_h)
 	$(Q)$(MAKE) $(build)=$(@)
 
 # Things we need to do before we recursively start building the kernel
@@ -1097,7 +1099,7 @@ endif
 # that need to depend on updated CONFIG_* values can be checked here.
 prepare2: prepare3 prepare-compiler-check outputmakefile asm-generic
 
-prepare1: prepare2 $(version_h) include/generated/utsrelease.h \
+prepare1: prepare2 $(version_h) $(autoksyms_h) include/generated/utsrelease.h \
                    include/config/auto.conf
 	$(cmd_crmodverdir)
 
diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
index 78c96aa..f9bdd02 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -38,8 +38,6 @@ nconfig: $(obj)/nconf
 # for external use.
 syncconfig: $(obj)/conf
 	$(Q)mkdir -p include/config include/generated
-	$(Q)test -e include/generated/autoksyms.h || \
-	    touch   include/generated/autoksyms.h
 	$< $(silent) --$@ $(Kconfig)
 
 localyesconfig localmodconfig: $(obj)/conf
-- 
2.7.4

[PATCH v2 5/7] kbuild: move CONFIG_TRIM_UNUSED_KSYMS code unneeded for external module

[Masahiro Yamada]

The external module building does not need to parse this code because
KBUILD_MODULES is always set anyway.

Move this code inside the "ifeq ($(KBUILD_EXTMOD),) ... endif" block.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Acked-by: Nicolas Pitre <nico@linaro.org>
---

Changes in v2: None

 Makefile | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 0a3895c..ef42adb 100644
--- a/Makefile
+++ b/Makefile
@@ -614,13 +614,6 @@ else
 include/config/auto.conf: ;
 endif # $(dot-config)
 
-# For the kernel to actually contain only the needed exported symbols,
-# we have to build modules as well to determine what those symbols are.
-# (this can be evaluated only once include/config/auto.conf has been included)
-ifdef CONFIG_TRIM_UNUSED_KSYMS
-  KBUILD_MODULES := 1
-endif
-
 # The all: target is the default when no target is given on the
 # command line.
 # This allow a user to issue only 'make' to build a kernel including modules
@@ -1021,6 +1014,13 @@ ifdef CONFIG_TRIM_UNUSED_KSYMS
 	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
 endif
 
+# For the kernel to actually contain only the needed exported symbols,
+# we have to build modules as well to determine what those symbols are.
+# (this can be evaluated only once include/config/auto.conf has been included)
+ifdef CONFIG_TRIM_UNUSED_KSYMS
+  KBUILD_MODULES := 1
+endif
+
 autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
 
 $(autoksyms_h):
-- 
2.7.4

[PATCH v2 6/7] kbuild: move include/config/ksym/* to include/ksym/*

[Masahiro Yamada]

The idea of using fixdep was inspired by Kconfig, but autoksyms
belongs to a different group.  So, I want move those touched files
under include/config/ksym/ to include/ksym/.

The directory include/ksym/ can be removed by "make clean" because
it is meaningless for the external module building.

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

Changes in v2:
 - Rebase on linux-next
   (resolve a conflict to https://patchwork.kernel.org/patch/10204653/)

 .gitignore                  | 1 +
 Makefile                    | 2 +-
 scripts/Kbuild.include      | 2 +-
 scripts/adjust_autoksyms.sh | 2 +-
 scripts/basic/fixdep.c      | 8 ++++----
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/.gitignore b/.gitignore
index 1be78fd..85bcc26 100644
--- a/.gitignore
+++ b/.gitignore
@@ -87,6 +87,7 @@ modules.builtin
 #
 include/config
 include/generated
+include/ksym
 arch/*/include/generated
 
 # stgit generated dirs
diff --git a/Makefile b/Makefile
index ef42adb..5fee703 100644
--- a/Makefile
+++ b/Makefile
@@ -1338,7 +1338,7 @@ endif # CONFIG_MODULES
 # make distclean Remove editor backup files, patch leftover files and the like
 
 # Directories & files removed with 'make clean'
-CLEAN_DIRS  += $(MODVERDIR)
+CLEAN_DIRS  += $(MODVERDIR) include/ksym
 
 # Directories & files removed with 'make mrproper'
 MRPROPER_DIRS  += include/config usr/include include/generated          \
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index f9c2f07..cce31ee 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -366,7 +366,7 @@ ksym_dep_filter =                                                            \
 	    $(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_\(.*\) ===.*$$/KSYM_\1/p'
+	esac | tr ";" "\n" | sed -n 's/^.*=== __KSYM_\(.*\) ===.*$$/_\1/p'
 
 cmd_and_fixdep =                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index e0dd0d5..f11cae6 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -80,7 +80,7 @@ sort "$cur_ksyms_file" "$new_ksyms_file" | uniq -u |
 sed -n 's/^#define __KSYM_\(.*\) 1/\1/p' | tr "A-Z_" "a-z/" |
 while read sympath; do
 	if [ -z "$sympath" ]; then continue; fi
-	depfile="include/config/ksym/${sympath}.h"
+	depfile="include/ksym/${sympath}.h"
 	mkdir -p "$(dirname "$depfile")"
 	touch "$depfile"
 	echo $((count += 1))
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 449b68c..f387538 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -113,11 +113,11 @@ static void usage(void)
 /*
  * Print out a dependency path from a symbol name
  */
-static void print_config(const char *m, int slen)
+static void print_dep(const char *m, int slen, const char *dir)
 {
 	int c, i;
 
-	printf("    $(wildcard include/config/");
+	printf("    $(wildcard %s/", dir);
 	for (i = 0; i < slen; i++) {
 		c = m[i];
 		if (c == '_')
@@ -140,7 +140,7 @@ static void do_extra_deps(void)
 			fprintf(stderr, "fixdep: bad data on stdin\n");
 			exit(1);
 		}
-		print_config(buf, len - 1);
+		print_dep(buf, len - 1, "include/ksym");
 	}
 }
 
@@ -208,7 +208,7 @@ static void use_config(const char *m, int slen)
 	    return;
 
 	define_config(m, slen, hash);
-	print_config(m, slen);
+	print_dep(m, slen, "include/config");
 }
 
 /* test if s ends in sub */
-- 
2.7.4

Re: [PATCH v2 6/7] kbuild: move include/config/ksym/* to include/ksym/*

[Nicolas Pitre]

On Fri, 16 Mar 2018, Masahiro Yamada wrote:

> The idea of using fixdep was inspired by Kconfig, but autoksyms
> belongs to a different group.  So, I want move those touched files
> under include/config/ksym/ to include/ksym/.
> 
> The directory include/ksym/ can be removed by "make clean" because
> it is meaningless for the external module building.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

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


> ---
> 
> Changes in v2:
>  - Rebase on linux-next
>    (resolve a conflict to https://patchwork.kernel.org/patch/10204653/)
> 
>  .gitignore                  | 1 +
>  Makefile                    | 2 +-
>  scripts/Kbuild.include      | 2 +-
>  scripts/adjust_autoksyms.sh | 2 +-
>  scripts/basic/fixdep.c      | 8 ++++----
>  5 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 1be78fd..85bcc26 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -87,6 +87,7 @@ modules.builtin
>  #
>  include/config
>  include/generated
> +include/ksym
>  arch/*/include/generated
>  
>  # stgit generated dirs
> diff --git a/Makefile b/Makefile
> index ef42adb..5fee703 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1338,7 +1338,7 @@ endif # CONFIG_MODULES
>  # make distclean Remove editor backup files, patch leftover files and the like
>  
>  # Directories & files removed with 'make clean'
> -CLEAN_DIRS  += $(MODVERDIR)
> +CLEAN_DIRS  += $(MODVERDIR) include/ksym
>  
>  # Directories & files removed with 'make mrproper'
>  MRPROPER_DIRS  += include/config usr/include include/generated          \
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index f9c2f07..cce31ee 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -366,7 +366,7 @@ ksym_dep_filter =                                                            \
>  	    $(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_\(.*\) ===.*$$/KSYM_\1/p'
> +	esac | tr ";" "\n" | sed -n 's/^.*=== __KSYM_\(.*\) ===.*$$/_\1/p'
>  
>  cmd_and_fixdep =                                                             \
>  	$(echo-cmd) $(cmd_$(1));                                             \
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index e0dd0d5..f11cae6 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -80,7 +80,7 @@ sort "$cur_ksyms_file" "$new_ksyms_file" | uniq -u |
>  sed -n 's/^#define __KSYM_\(.*\) 1/\1/p' | tr "A-Z_" "a-z/" |
>  while read sympath; do
>  	if [ -z "$sympath" ]; then continue; fi
> -	depfile="include/config/ksym/${sympath}.h"
> +	depfile="include/ksym/${sympath}.h"
>  	mkdir -p "$(dirname "$depfile")"
>  	touch "$depfile"
>  	echo $((count += 1))
> diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
> index 449b68c..f387538 100644
> --- a/scripts/basic/fixdep.c
> +++ b/scripts/basic/fixdep.c
> @@ -113,11 +113,11 @@ static void usage(void)
>  /*
>   * Print out a dependency path from a symbol name
>   */
> -static void print_config(const char *m, int slen)
> +static void print_dep(const char *m, int slen, const char *dir)
>  {
>  	int c, i;
>  
> -	printf("    $(wildcard include/config/");
> +	printf("    $(wildcard %s/", dir);
>  	for (i = 0; i < slen; i++) {
>  		c = m[i];
>  		if (c == '_')
> @@ -140,7 +140,7 @@ static void do_extra_deps(void)
>  			fprintf(stderr, "fixdep: bad data on stdin\n");
>  			exit(1);
>  		}
> -		print_config(buf, len - 1);
> +		print_dep(buf, len - 1, "include/ksym");
>  	}
>  }
>  
> @@ -208,7 +208,7 @@ static void use_config(const char *m, int slen)
>  	    return;
>  
>  	define_config(m, slen, hash);
> -	print_config(m, slen);
> +	print_dep(m, slen, "include/config");
>  }
>  
>  /* test if s ends in sub */
> -- 
> 2.7.4
> 
> 

[PATCH v2 7/7] kbuild: link vmlinux only once for CONFIG_TRIM_UNUSED_KSYMS

[Masahiro Yamada]

If CONFIG_TRIM_UNUSED_KSYMS is enabled and the kernel is built from
a pristine state, the vmlinux is linked twice.

[1] A user runs "make"

[2] First build with empty autoksyms.h

[3] adjust_autoksyms.sh updates autoksyms.h and recurses "make vmlinux"

  --------(begin sub-make)--------
  [4] Second build with new autoksyms.h

  [5] link-vmlinux.sh is invoked because "vmlinux" is missing
  ---------(end sub-make)---------

[6] link-vmlinux.sh is invoked again despite "vmlinux" is up-to-date.

The reason of [6] is probably because Make already decided to update
"vmlinux" at the time of [2] because "vmlinux" was missing when Make
built up the dependency graph.

Because 'if_changed' is implemented based on '$?', this issue can be
narrowed down to how Make handles '$?'.

You can test it with the following simple code:

[Test Makefile]
  A: B
          cp B A
          @echo newer prerequisite: $?

  B: C
          cp C B
          touch A

[Result]
  $ rm -f A B
  $ touch C
  $ make
  cp C B
  touch A
  cp B A
  newer prerequisite: B

Here, 'A' has been touched in the recipe of 'B'.  So, the dependency
'A: B' has already been met before the recipe of 'A' is executed.
However, Make does not notice the fact that the recipe of 'B' also
updates 'A' as a side-effect.

The situation is similar in this case; 'vmlinux' has actually been
updated in the 'vmlinux_prereq' target.  Make cannot predict this, so
judges 'vmlinux' is old.

link-vmlinus.sh is costly, so it is better to not run it when unneeded.
Split CONFIG_TRIM_UNUSED_KSYMS recursion to a dedicated target.

The reason of commit 2441e78b1919 ("kbuild: better abstract vmlinux
sequential prerequisites") was to cater to CONFIG_BUILD_DOCSRC, but
it was later removed by commit 184892925118 ("samples: move blackfin
gptimers-example from Documentation").

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

Changes in v2:
  - Discard my wrong change to adjust_autoksyms.sh
  - Add more commit log to explain how Make is working

 Makefile | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/Makefile b/Makefile
index 5fee703..3e871d2 100644
--- a/Makefile
+++ b/Makefile
@@ -998,21 +998,11 @@ export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Doc
 
 vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_INIT) $(KBUILD_VMLINUX_MAIN) $(KBUILD_VMLINUX_LIBS)
 
-# Include targets which we want to execute sequentially if the rest of the
-# kernel build went well. If CONFIG_TRIM_UNUSED_KSYMS is set, this might be
-# evaluated more than once.
-PHONY += vmlinux_prereq
-vmlinux_prereq: $(vmlinux-deps) FORCE
-ifdef CONFIG_HEADERS_CHECK
-	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
-endif
-ifdef CONFIG_GDB_SCRIPTS
-	$(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
-endif
-ifdef CONFIG_TRIM_UNUSED_KSYMS
-	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh \
+# Recurse until adjust_autoksyms.sh is satisfied
+PHONY += autoksyms_recursive
+autoksyms_recursive: $(vmlinux-deps)
+	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh  \
 	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
-endif
 
 # For the kernel to actually contain only the needed exported symbols,
 # we have to build modules as well to determine what those symbols are.
@@ -1034,7 +1024,13 @@ cmd_link-vmlinux =                                                 \
 	$(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;    \
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
-vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
+vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
+ifdef CONFIG_HEADERS_CHECK
+	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
+endif
+ifdef CONFIG_GDB_SCRIPTS
+	$(Q)ln -fsn $(abspath $(srctree)/scripts/gdb/vmlinux-gdb.py)
+endif
 	+$(call if_changed,link-vmlinux)
 
 # Build samples along the rest of the kernel
-- 
2.7.4

Re: [PATCH v2 7/7] kbuild: link vmlinux only once for CONFIG_TRIM_UNUSED_KSYMS

[Nicolas Pitre]

On Fri, 16 Mar 2018, Masahiro Yamada wrote:

> +# Recurse until adjust_autoksyms.sh is satisfied
> +PHONY += autoksyms_recursive
> +autoksyms_recursive: $(vmlinux-deps)
> +	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh  \
>  	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
> -endif
>  
>  # For the kernel to actually contain only the needed exported symbols,
>  # we have to build modules as well to determine what those symbols are.
> @@ -1034,7 +1024,13 @@ cmd_link-vmlinux =                                                 \
>  	$(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;    \
>  	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>  
> -vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
> +vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE

Don't you have to make the autoksyms_recursive dependency conditional on 
CONFIG_TRIM_UNUSED_KSYMS?


Nicolas

Re: [PATCH v2 7/7] kbuild: link vmlinux only once for CONFIG_TRIM_UNUSED_KSYMS

[Masahiro Yamada]

2018-03-16 11:32 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Fri, 16 Mar 2018, Masahiro Yamada wrote:
>
>> +# Recurse until adjust_autoksyms.sh is satisfied
>> +PHONY += autoksyms_recursive
>> +autoksyms_recursive: $(vmlinux-deps)
>> +     $(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh  \
>>         "$(MAKE) -f $(srctree)/Makefile vmlinux"
>> -endif
>>
>>  # For the kernel to actually contain only the needed exported symbols,
>>  # we have to build modules as well to determine what those symbols are.
>> @@ -1034,7 +1024,13 @@ cmd_link-vmlinux =                                                 \
>>       $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;    \
>>       $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>>
>> -vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
>> +vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
>
> Don't you have to make the autoksyms_recursive dependency conditional on
> CONFIG_TRIM_UNUSED_KSYMS?
>

Oops, I accidentally removed that conditional.

I will fix it shortly.  Thanks!


-- 
Best Regards
Masahiro Yamada