All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS
@ 2018-03-14 16:44 Masahiro Yamada
  2018-03-14 16:44 ` [PATCH 1/7] kbuild: clear LDFLAGS in the top Makefile Masahiro Yamada
                   ` (6 more replies)
  0 siblings, 7 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel




Masahiro Yamada (7):
  kbuild: clear LDFLAGS in the top Makefile
  kbuild: touch autoksyms.h when it is really missing
  kbuild: move 'scripts' target below
  kbuild: restore touching autoksyms.h to the top Makefile
  kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module
    building
  kbuild: move include/config/ksym/* to include/ksym/*
  kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS

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

-- 
2.7.4

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

* [PATCH 1/7] kbuild: clear LDFLAGS in the top Makefile
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2018-03-14 16:44 ` Masahiro Yamada
  2018-03-14 16:44   ` Masahiro Yamada
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

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

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

diff --git a/Makefile b/Makefile
index e02d092..5eb5d9d 100644
--- a/Makefile
+++ b/Makefile
@@ -426,6 +426,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

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

* [PATCH 2/7] kbuild: touch autoksyms.h when it is really missing
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2018-03-14 16:44   ` Masahiro Yamada
  2018-03-14 16:44   ` Masahiro Yamada
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	linux-kernel

>From the comment, I expect this code creates autoksyms.h in case it
is missing, but it actually touches it when it does exists.

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

I do not know when this code is useful...

If autoksyms.h were missing, build should have already failed because
include/{linux,asm-generic}/export.h need it.

Maybe for standalone test?
Or, in order to make this script self-contained?


 scripts/adjust_autoksyms.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 513da1a..a52210b 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -49,7 +49,7 @@ case "${KCONFIG_CONFIG}" in
 esac
 
 # In case it doesn't exist yet...
-if [ -e "$cur_ksyms_file" ]; then touch "$cur_ksyms_file"; fi
+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.
-- 
2.7.4

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

* [PATCH 2/7] kbuild: touch autoksyms.h when it is really missing
@ 2018-03-14 16:44   ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	linux-kernel

From the comment, I expect this code creates autoksyms.h in case it
is missing, but it actually touches it when it does exists.

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

I do not know when this code is useful...

If autoksyms.h were missing, build should have already failed because
include/{linux,asm-generic}/export.h need it.

Maybe for standalone test?
Or, in order to make this script self-contained?


 scripts/adjust_autoksyms.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 513da1a..a52210b 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -49,7 +49,7 @@ case "${KCONFIG_CONFIG}" in
 esac
 
 # In case it doesn't exist yet...
-if [ -e "$cur_ksyms_file" ]; then touch "$cur_ksyms_file"; fi
+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.
-- 
2.7.4


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

* [PATCH 3/7] kbuild: move 'scripts' target below
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
  2018-03-14 16:44 ` [PATCH 1/7] kbuild: clear LDFLAGS in the top Makefile Masahiro Yamada
  2018-03-14 16:44   ` Masahiro Yamada
@ 2018-03-14 16:44 ` Masahiro Yamada
  2018-03-16  7:13     ` kbuild test robot
  2018-03-14 16:44   ` Masahiro Yamada
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

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

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

diff --git a/Makefile b/Makefile
index 5eb5d9d..fab0e19 100644
--- a/Makefile
+++ b/Makefile
@@ -556,14 +556,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/
@@ -1059,6 +1051,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

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

* [PATCH 4/7] kbuild: restore touching autoksyms.h to the top Makefile
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2018-03-14 16:44   ` Masahiro Yamada
  2018-03-14 16:44   ` Masahiro Yamada
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

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 'script' 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 needed
<generated/autoksyms.h> if CONFIG_TRIM_UNUSED_KSYMS.  This was the
reason of race.

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

I remove 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>
Commit:

The discussion happened in Nov. 2016.

I was not involved in it.
I was not the Kbuild maintainer at that time.

END

---

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

diff --git a/Makefile b/Makefile
index fab0e19..decc870 100644
--- a/Makefile
+++ b/Makefile
@@ -1010,9 +1010,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)
 
@@ -1056,7 +1058,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
@@ -1086,7 +1088,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 cb3ec53..eb139a1 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -38,8 +38,6 @@ nconfig: $(obj)/nconf
 # for external use.
 silentoldconfig: $(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)/streamline_config.pl $(obj)/conf
-- 
2.7.4

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

* [PATCH 4/7] kbuild: restore touching autoksyms.h to the top Makefile
@ 2018-03-14 16:44   ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

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 'script' 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 needed
<generated/autoksyms.h> if CONFIG_TRIM_UNUSED_KSYMS.  This was the
reason of race.

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

I remove 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>
Commit:

The discussion happened in Nov. 2016.

I was not involved in it.
I was not the Kbuild maintainer at that time.

END

---

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

diff --git a/Makefile b/Makefile
index fab0e19..decc870 100644
--- a/Makefile
+++ b/Makefile
@@ -1010,9 +1010,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)
 
@@ -1056,7 +1058,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
@@ -1086,7 +1088,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 cb3ec53..eb139a1 100644
--- a/scripts/kconfig/Makefile
+++ b/scripts/kconfig/Makefile
@@ -38,8 +38,6 @@ nconfig: $(obj)/nconf
 # for external use.
 silentoldconfig: $(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)/streamline_config.pl $(obj)/conf
-- 
2.7.4


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

* [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (3 preceding siblings ...)
  2018-03-14 16:44   ` Masahiro Yamada
@ 2018-03-14 16:44 ` Masahiro Yamada
  2018-03-14 18:32   ` Nicolas Pitre
  2018-03-14 16:44 ` [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/* Masahiro Yamada
  2018-03-14 16:44 ` [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
  6 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

If CONFIG_TRIM_UNUSED_KYMS is enabled, KBUILD_MODULES is set.  This
code is unneeded for external module building because KBUILD_MODULES
is always set.  Move this code inside "ifeq ($(KBUILD_EXTMOD),)"
conditional.

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

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

diff --git a/Makefile b/Makefile
index decc870..e60b16f 100644
--- a/Makefile
+++ b/Makefile
@@ -603,13 +603,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
@@ -1010,6 +1003,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

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

* [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/*
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (4 preceding siblings ...)
  2018-03-14 16:44 ` [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building Masahiro Yamada
@ 2018-03-14 16:44 ` Masahiro Yamada
  2018-03-14 18:47   ` Nicolas Pitre
  2018-03-14 16:44 ` [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
  6 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

The idea of using fixdep was inspired by Kconfig, but autoksyms
is unrelated to Kconfig.  So, I want to get those touched files
out of include/config/.  The directory include/ksym/ is removed
by "make clean".  We do not need to keep it for external module
building.

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

 .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 e60b16f..1dab647 100644
--- a/Makefile
+++ b/Makefile
@@ -1327,7 +1327,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 065324a..045971e 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -368,7 +368,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 -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
+	esac | tr ";" "\n" | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/\1/p'
 
 cmd_and_fixdep =                                                             \
 	$(echo-cmd) $(cmd_$(1));                                             \
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index a52210b..7bb3618 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -81,7 +81,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

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

* [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS
  2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
                   ` (5 preceding siblings ...)
  2018-03-14 16:44 ` [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/* Masahiro Yamada
@ 2018-03-14 16:44 ` Masahiro Yamada
  2018-03-14 19:06   ` Nicolas Pitre
  6 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-14 16:44 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Nicolas Pitre, Jarod Wilson, Prarit Bhargava, Masahiro Yamada,
	Michal Marek, linux-kernel

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
generated the dependency list.

link-vmlinus.sh is costly, so it is better to not run it when unneeded.
Split CONFIG_TRIM_UNUSED_KSYMS handling 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").

I also changed adjust_autoksyms.sh to simply exit with 1 or 0 to make
it look straightforward.

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

 Makefile                    | 28 ++++++++++++----------------
 scripts/adjust_autoksyms.sh |  3 +--
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index 1dab647..0a7bab6 100644
--- a/Makefile
+++ b/Makefile
@@ -987,21 +987,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 \
-	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
-endif
+# 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 autoksyms_recursive
 
 # For the kernel to actually contain only the needed exported symbols,
 # we have to build modules as well to determine what those symbols are.
@@ -1023,7 +1013,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
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
index 7bb3618..1377cf8 100755
--- a/scripts/adjust_autoksyms.sh
+++ b/scripts/adjust_autoksyms.sh
@@ -95,8 +95,7 @@ if [ $changed -gt 0 ]; then
 	info "KSYMS" "symbols: before=$old, after=$new, changed=$changed"
 	info "UPD" "$cur_ksyms_file"
 	mv -f "$new_ksyms_file" "$cur_ksyms_file"
-	# Then trigger a rebuild of affected source files
-	exec $@
+	exit 1
 else
 	rm -f "$new_ksyms_file"
 fi
-- 
2.7.4

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

* Re: [PATCH 2/7] kbuild: touch autoksyms.h when it is really missing
  2018-03-14 16:44   ` Masahiro Yamada
  (?)
@ 2018-03-14 17:26   ` Nicolas Pitre
  2018-03-15  6:26     ` Masahiro Yamada
  -1 siblings, 1 reply; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-14 17:26 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: linux-kbuild, Jarod Wilson, Prarit Bhargava, linux-kernel

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> From the comment, I expect this code creates autoksyms.h in case it
> is missing, but it actually touches it when it does exists.

Oops, indeed.

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
> I do not know when this code is useful...
> 
> If autoksyms.h were missing, build should have already failed because
> include/{linux,asm-generic}/export.h need it.
> 
> Maybe for standalone test?
> Or, in order to make this script self-contained?

I agree it isn't very useful. Proof: it was wrong and wouldn't have 
worked as intended. So the best fix might be to simply remove that line.



> 
> 
>  scripts/adjust_autoksyms.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index 513da1a..a52210b 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -49,7 +49,7 @@ case "${KCONFIG_CONFIG}" in
>  esac
>  
>  # In case it doesn't exist yet...
> -if [ -e "$cur_ksyms_file" ]; then touch "$cur_ksyms_file"; fi
> +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.
> -- 
> 2.7.4
> 
> 

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

* Re: [PATCH 4/7] kbuild: restore touching autoksyms.h to the top Makefile
  2018-03-14 16:44   ` Masahiro Yamada
  (?)
@ 2018-03-14 17:52   ` Nicolas Pitre
  -1 siblings, 0 replies; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-14 17:52 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jarod Wilson, Prarit Bhargava, Michal Marek, linux-kernel

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> 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 'script' 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 needed
> <generated/autoksyms.h> if CONFIG_TRIM_UNUSED_KSYMS.  This was the
> reason of race.
> 
> I am not happy to have unrelated code in the Kconfig Makefile, so
> getting it back to the top Makefile.
> 
> I remove 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>

You're the make master!

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



> Commit:
> 
> The discussion happened in Nov. 2016.
> 
> I was not involved in it.
> I was not the Kbuild maintainer at that time.
> 
> END
> 
> ---
> 
>  Makefile                 | 12 +++++++-----
>  scripts/kconfig/Makefile |  2 --
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index fab0e19..decc870 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1010,9 +1010,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)
>  
> @@ -1056,7 +1058,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
> @@ -1086,7 +1088,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 cb3ec53..eb139a1 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -38,8 +38,6 @@ nconfig: $(obj)/nconf
>  # for external use.
>  silentoldconfig: $(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)/streamline_config.pl $(obj)/conf
> -- 
> 2.7.4
> 
> 

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

* Re: [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building
  2018-03-14 16:44 ` [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building Masahiro Yamada
@ 2018-03-14 18:32   ` Nicolas Pitre
  2018-03-15  6:36     ` Masahiro Yamada
  0 siblings, 1 reply; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-14 18:32 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jarod Wilson, Prarit Bhargava, Michal Marek, linux-kernel

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> If CONFIG_TRIM_UNUSED_KYMS is enabled, KBUILD_MODULES is set.  

Not when you do "make vmlinux" though.

> This code is unneeded for external module building because 
> KBUILD_MODULES is always set.  Move this code inside "ifeq 
> ($(KBUILD_EXTMOD),)" conditional.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

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

> ---
> 
>  Makefile | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index decc870..e60b16f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -603,13 +603,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
> @@ -1010,6 +1003,13 @@ ifdef CONFIG_TRIM_UNUSED_KSYMS
>  	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
>  endif
>  
> +# For the kernel to actually contain only the needed exported symbols,
> A+# 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
> 
> 

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

* Re: [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/*
  2018-03-14 16:44 ` [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/* Masahiro Yamada
@ 2018-03-14 18:47   ` Nicolas Pitre
  2018-03-15 10:04     ` Masahiro Yamada
  0 siblings, 1 reply; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-14 18:47 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jarod Wilson, Prarit Bhargava, Michal Marek, linux-kernel

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> The idea of using fixdep was inspired by Kconfig, but autoksyms
> is unrelated to Kconfig.  So, I want to get those touched files
> out of include/config/.  The directory include/ksym/ is removed
> by "make clean".  We do not need to keep it for external module
> building.

It could be argued that include/config/ is not strictly containing 
configuration data either and is slightly misleading.

What about moving include/config/ and include/ksym/ under 
include/depfiles/ ? In fact that could even be 
include/generated/depfiles/config/ and include/generated/depfiles/ksym/ 
to trim down the top include directory.

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  .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 e60b16f..1dab647 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1327,7 +1327,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 065324a..045971e 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -368,7 +368,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 -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
> +	esac | tr ";" "\n" | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/\1/p'
>  
>  cmd_and_fixdep =                                                             \
>  	$(echo-cmd) $(cmd_$(1));                                             \
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index a52210b..7bb3618 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -81,7 +81,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
> 
> 

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

* Re: [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS
  2018-03-14 16:44 ` [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
@ 2018-03-14 19:06   ` Nicolas Pitre
  2018-03-15  8:00     ` Masahiro Yamada
  0 siblings, 1 reply; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-14 19:06 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, Jarod Wilson, Prarit Bhargava, Michal Marek, linux-kernel

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> 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
> generated the dependency list.

But the dependency list for vmlinux contains FORCE so the target is 
always remade in (2) anyway. The decision to actually invoke 
link-vmlinux.sh comes from "if_changed". Why is $(call if_changed,...) 
in (6) not noticing that vmlinux is up to date?

> 
> link-vmlinus.sh is costly, so it is better to not run it when unneeded.
> Split CONFIG_TRIM_UNUSED_KSYMS handling to a dedicated target.
> 
> The reason of commit 2441e78b1919 ("krbuild: 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").
> 
> I also changed adjust_autoksyms.sh to simply exit with 1 or 0 to make
> it look straightforward.

That is wrong. If the script fails for some reason (it runs with -e set) 
then this will trigger an endless recursive make instead of failing the 
build.

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
> 
>  Makefile                    | 28 ++++++++++++----------------
>  scripts/adjust_autoksyms.sh |  3 +--
>  2 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 1dab647..0a7bab6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -987,21 +987,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 \
> -	  "$(MAKE) -f $(srctree)/Makefile vmlinux"
> -endif
> +# 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 autoksyms_recursive
>  
>  # For the kernel to actually contain only the needed exported symbols,
>  # we have to build modules as well to determine what those symbols are.
> @@ -1023,7 +1013,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
> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
> index 7bb3618..1377cf8 100755
> --- a/scripts/adjust_autoksyms.sh
> +++ b/scripts/adjust_autoksyms.sh
> @@ -95,8 +95,7 @@ if [ $changed -gt 0 ]; then
>  	info "KSYMS" "symbols: before=$old, after=$new, changed=$changed"
>  	info "UPD" "$cur_ksyms_file"
>  	mv -f "$new_ksyms_file" "$cur_ksyms_file"
> -	# Then trigger a rebuild of affected source files
> -	exec $@
> +	exit 1
>  else
>  	rm -f "$new_ksyms_file"
>  fi
> -- 
> 2.7.4
> 
> 

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

* Re: [PATCH 2/7] kbuild: touch autoksyms.h when it is really missing
  2018-03-14 17:26   ` Nicolas Pitre
@ 2018-03-15  6:26     ` Masahiro Yamada
  0 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-15  6:26 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Linux Kernel Mailing List

2018-03-15 2:26 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Thu, 15 Mar 2018, Masahiro Yamada wrote:
>
>> From the comment, I expect this code creates autoksyms.h in case it
>> is missing, but it actually touches it when it does exists.
>
> Oops, indeed.
>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>> I do not know when this code is useful...
>>
>> If autoksyms.h were missing, build should have already failed because
>> include/{linux,asm-generic}/export.h need it.
>>
>> Maybe for standalone test?
>> Or, in order to make this script self-contained?
>
> I agree it isn't very useful. Proof: it was wrong and wouldn't have
> worked as intended. So the best fix might be to simply remove that line.
>

OK, I will remove the code.




-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building
  2018-03-14 18:32   ` Nicolas Pitre
@ 2018-03-15  6:36     ` Masahiro Yamada
  2018-03-15 18:30       ` Nicolas Pitre
  0 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-15  6:36 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

2018-03-15 3:32 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Thu, 15 Mar 2018, Masahiro Yamada wrote:
>
>> If CONFIG_TRIM_UNUSED_KYMS is enabled, KBUILD_MODULES is set.
>
> Not when you do "make vmlinux" though.

I could not understand this.

Unless I am missing something,
I think this code is always parsed.



>> This code is unneeded for external module building because
>> KBUILD_MODULES is always set.  Move this code inside "ifeq
>> ($(KBUILD_EXTMOD),)" conditional.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>
> Acked-by: Nicolas Pitre <nico@linaro.org>
>
>> ---
>>
>>  Makefile | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index decc870..e60b16f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -603,13 +603,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
>> @@ -1010,6 +1003,13 @@ ifdef CONFIG_TRIM_UNUSED_KSYMS
>>         "$(MAKE) -f $(srctree)/Makefile vmlinux"
>>  endif
>>
>> +# For the kernel to actually contain only the needed exported symbols,
>> A+# 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
>>
>>



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS
  2018-03-14 19:06   ` Nicolas Pitre
@ 2018-03-15  8:00     ` Masahiro Yamada
  2018-03-15 18:50       ` Nicolas Pitre
  0 siblings, 1 reply; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-15  8:00 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

2018-03-15 4:06 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Thu, 15 Mar 2018, Masahiro Yamada wrote:
>
>> 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
>> generated the dependency list.
>
> But the dependency list for vmlinux contains FORCE so the target is
> always remade in (2) anyway. The decision to actually invoke
> link-vmlinux.sh comes from "if_changed". Why is $(call if_changed,...)
> in (6) not noticing that vmlinux is up to date?

if_changed (more specifically 'any-prereq')
is implemented based on '$?'.

So, this problem can be narrowed down to
how Make decides '$?'.


If you apply the first 6 patches, and put the following
debug code, you will see what has happened to $?


@@ -1024,6 +1026,7 @@ cmd_link-vmlinux =
                  \
        $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)

 vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
+       @echo newer deps: $?
        +$(call if_changed,link-vmlinux)

 # Build samples along the rest of the kernel



I am not familiar with Make internal, but
I can test it with simple code.


[Test Makefile]
A: B
        cp B A
        echo $?

B: C
        cp C B
        touch A

[Result]
$ rm -rf A B C
$ touch C
$ make
cp C B
touch A
cp B A
echo B
B


In the recipe of 'B', 'A' is touched,
so the dependency 'A: B' has already been met
before the recipe of 'A' is executed,
but Make does not notice that fact,
then tries to update 'A'.


The situation is the same.
vmlinux has been updated in the recursed sub-make,
but it is not noticed.




>>
>> link-vmlinus.sh is costly, so it is better to not run it when unneeded.
>> Split CONFIG_TRIM_UNUSED_KSYMS handling to a dedicated target.
>>
>> The reason of commit 2441e78b1919 ("krbuild: 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").
>>
>> I also changed adjust_autoksyms.sh to simply exit with 1 or 0 to make
>> it look straightforward.
>
> That is wrong. If the script fails for some reason (it runs with -e set)
> then this will trigger an endless recursive make instead of failing the
> build.

Sorry, I missed this case.  You are right.

I will restore the original code.



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/*
  2018-03-14 18:47   ` Nicolas Pitre
@ 2018-03-15 10:04     ` Masahiro Yamada
  2018-03-15 11:01       ` Masahiro Yamada
  2018-03-15 18:59       ` Nicolas Pitre
  0 siblings, 2 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-15 10:04 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

2018-03-15 3:47 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> On Thu, 15 Mar 2018, Masahiro Yamada wrote:
>
>> The idea of using fixdep was inspired by Kconfig, but autoksyms
>> is unrelated to Kconfig.  So, I want to get those touched files
>> out of include/config/.  The directory include/ksym/ is removed
>> by "make clean".  We do not need to keep it for external module
>> building.
>
> It could be argued that include/config/ is not strictly containing
> configuration data either and is slightly misleading.

But, slightly related to configuration, IMHO.
At least they carry timestamps that are updated
when kernel configuration is changed.

The difference between include/config/ and include/ksym/ is that
files under include/config/ are necessary for building
external modules (so should be cleaned away by mrproper)
whereas include/ksym/ is unnecessary for external modules
since vmlinux and in-kernel modules do not depend on
external modules.


I wonder if trimming symbols makes sense for external modules.

EXPORT_SYMBOL(_GPL) in external modules are always trimmed
since vmlinux and in-kernel modules never rely on them.

If an external module exports symbols,
it expects they will be used by other external modules.

So, the patch like follows make sense?




kbuild: do not trim symbols in external modules

diff --git a/Makefile b/Makefile
index 0a7bab6..bdf565e 100644
--- a/Makefile
+++ b/Makefile
@@ -998,6 +998,7 @@ autoksyms_recursive: $(vmlinux-deps)
 # (this can be evaluated only once include/config/auto.conf has been included)
 ifdef CONFIG_TRIM_UNUSED_KSYMS
   KBUILD_MODULES := 1
+  export KBUILD_TRIM_UNUSED_KSYMS := 1
 endif

 autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 045971e..3249d5f 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -345,7 +345,7 @@ if_changed_dep = $(if $(strip $(any-prereq)
$(arg-check) ),                  \
        @set -e;                                                             \
        $(cmd_and_fixdep), @:)

-ifndef CONFIG_TRIM_UNUSED_KSYMS
+ifndef KBUILD_TRIM_UNUSED_KSYMS

 cmd_and_fixdep =                                                             \
        $(echo-cmd) $(cmd_$(1));                                             \




> What about moving include/config/ and include/ksym/ under
> include/depfiles/ ? In fact that could even be
> include/generated/depfiles/config/ and include/generated/depfiles/ksym/
> to trim down the top include directory.
>
>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> ---
>>
>>  .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 e60b16f..1dab647 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1327,7 +1327,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 065324a..045971e 100644
>> --- a/scripts/Kbuild.include
>> +++ b/scripts/Kbuild.include
>> @@ -368,7 +368,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 -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
>> +     esac | tr ";" "\n" | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/\1/p'
>>
>>  cmd_and_fixdep =                                                             \
>>       $(echo-cmd) $(cmd_$(1));                                             \
>> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>> index a52210b..7bb3618 100755
>> --- a/scripts/adjust_autoksyms.sh
>> +++ b/scripts/adjust_autoksyms.sh
>> @@ -81,7 +81,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
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/*
  2018-03-15 10:04     ` Masahiro Yamada
@ 2018-03-15 11:01       ` Masahiro Yamada
  2018-03-15 18:59       ` Nicolas Pitre
  1 sibling, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-15 11:01 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

2018-03-15 19:04 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>:
> 2018-03-15 3:47 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
>> On Thu, 15 Mar 2018, Masahiro Yamada wrote:
>>
>>> The idea of using fixdep was inspired by Kconfig, but autoksyms
>>> is unrelated to Kconfig.  So, I want to get those touched files
>>> out of include/config/.  The directory include/ksym/ is removed
>>> by "make clean".  We do not need to keep it for external module
>>> building.
>>
>> It could be argued that include/config/ is not strictly containing
>> configuration data either and is slightly misleading.
>
> But, slightly related to configuration, IMHO.
> At least they carry timestamps that are updated
> when kernel configuration is changed.
>
> The difference between include/config/ and include/ksym/ is that
> files under include/config/ are necessary for building
> external modules (so should be cleaned away by mrproper)
> whereas include/ksym/ is unnecessary for external modules
> since vmlinux and in-kernel modules do not depend on
> external modules.
>
>
> I wonder if trimming symbols makes sense for external modules.
>
> EXPORT_SYMBOL(_GPL) in external modules are always trimmed
> since vmlinux and in-kernel modules never rely on them.
>
> If an external module exports symbols,
> it expects they will be used by other external modules.
>
> So, the patch like follows make sense?
>
>
>
>
> kbuild: do not trim symbols in external modules
>
> diff --git a/Makefile b/Makefile
> index 0a7bab6..bdf565e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -998,6 +998,7 @@ autoksyms_recursive: $(vmlinux-deps)
>  # (this can be evaluated only once include/config/auto.conf has been included)
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>    KBUILD_MODULES := 1
> +  export KBUILD_TRIM_UNUSED_KSYMS := 1
>  endif
>
>  autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 045971e..3249d5f 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -345,7 +345,7 @@ if_changed_dep = $(if $(strip $(any-prereq)
> $(arg-check) ),                  \
>         @set -e;                                                             \
>         $(cmd_and_fixdep), @:)
>
> -ifndef CONFIG_TRIM_UNUSED_KSYMS
> +ifndef KBUILD_TRIM_UNUSED_KSYMS
>
>  cmd_and_fixdep =                                                             \
>         $(echo-cmd) $(cmd_$(1));                                             \
>
>
>
>
>> What about moving include/config/ and include/ksym/ under
>> include/depfiles/ ? In fact that could even be
>> include/generated/depfiles/config/ and include/generated/depfiles/ksym/
>> to trim down the top include directory.
>>
>>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> ---
>>>
>>>  .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 e60b16f..1dab647 100644
>>> --- a/Makefile
>>> +++ b/Makefile
>>> @@ -1327,7 +1327,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 065324a..045971e 100644
>>> --- a/scripts/Kbuild.include
>>> +++ b/scripts/Kbuild.include
>>> @@ -368,7 +368,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 -rn 's/^.*=== __KSYM_(.*) ===.*$$/KSYM_\1/p'
>>> +     esac | tr ";" "\n" | sed -rn 's/^.*=== __KSYM_(.*) ===.*$$/\1/p'
>>>
>>>  cmd_and_fixdep =                                                             \
>>>       $(echo-cmd) $(cmd_$(1));                                             \
>>> diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
>>> index a52210b..7bb3618 100755
>>> --- a/scripts/adjust_autoksyms.sh
>>> +++ b/scripts/adjust_autoksyms.sh
>>> @@ -81,7 +81,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 */
>>> --


No.  All exported symbols in external modules are
still trimmed.

Hmm.  Probably, this is a "Do not do it" thing.



-- 
Best Regards
Masahiro Yamada

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

* Re: [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building
  2018-03-15  6:36     ` Masahiro Yamada
@ 2018-03-15 18:30       ` Nicolas Pitre
  0 siblings, 0 replies; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-15 18:30 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> 2018-03-15 3:32 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> > On Thu, 15 Mar 2018, Masahiro Yamada wrote:
> >
> >> If CONFIG_TRIM_UNUSED_KYMS is enabled, KBUILD_MODULES is set.
> >
> > Not when you do "make vmlinux" though.
> 
> I could not understand this.
> 
> Unless I am missing something,
> I think this code is always parsed.

Sorry, you're right. I had misread your sentence when I replied.


Nicolas

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

* Re: [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS
  2018-03-15  8:00     ` Masahiro Yamada
@ 2018-03-15 18:50       ` Nicolas Pitre
  0 siblings, 0 replies; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-15 18:50 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> I am not familiar with Make internal, but
> I can test it with simple code.
> 
> 
> [Test Makefile]
> A: B
>         cp B A
>         echo $?
> 
> B: C
>         cp C B
>         touch A
> 
> [Result]
> $ rm -rf A B C
> $ touch C
> $ make
> cp C B
> touch A
> cp B A
> echo B
> B
> 
> 
> In the recipe of 'B', 'A' is touched,
> so the dependency 'A: B' has already been met
> before the recipe of 'A' is executed,
> but Make does not notice that fact,
> then tries to update 'A'.
> 
> 
> The situation is the same.
> vmlinux has been updated in the recursed sub-make,
> but it is not noticed.

OK, I agree.


Nicolas

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

* Re: [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/*
  2018-03-15 10:04     ` Masahiro Yamada
  2018-03-15 11:01       ` Masahiro Yamada
@ 2018-03-15 18:59       ` Nicolas Pitre
  1 sibling, 0 replies; 26+ messages in thread
From: Nicolas Pitre @ 2018-03-15 18:59 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Jarod Wilson, Prarit Bhargava,
	Michal Marek, Linux Kernel Mailing List

On Thu, 15 Mar 2018, Masahiro Yamada wrote:

> 2018-03-15 3:47 GMT+09:00 Nicolas Pitre <nicolas.pitre@linaro.org>:
> > On Thu, 15 Mar 2018, Masahiro Yamada wrote:
> >
> >> The idea of using fixdep was inspired by Kconfig, but autoksyms
> >> is unrelated to Kconfig.  So, I want to get those touched files
> >> out of include/config/.  The directory include/ksym/ is removed
> >> by "make clean".  We do not need to keep it for external module
> >> building.
> >
> > It could be argued that include/config/ is not strictly containing
> > configuration data either and is slightly misleading.
> 
> But, slightly related to configuration, IMHO.
> At least they carry timestamps that are updated
> when kernel configuration is changed.

Yes. But for the sake of argument, the ksym timestamps are updated only 
when configuration is changed too. Fundamentally they're both about 
dependencies, hence my naming suggestion of deps/config/ and deps/ksym/ 
so not to clutter the top include directory too much.

> The difference between include/config/ and include/ksym/ is that
> files under include/config/ are necessary for building
> external modules (so should be cleaned away by mrproper)
> whereas include/ksym/ is unnecessary for external modules
> since vmlinux and in-kernel modules do not depend on
> external modules.

Agreed. 

> I wonder if trimming symbols makes sense for external modules.

Probably not.


Nicolas

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

* Re: [PATCH 3/7] kbuild: move 'scripts' target below
  2018-03-14 16:44 ` [PATCH 3/7] kbuild: move 'scripts' target below Masahiro Yamada
@ 2018-03-16  7:13     ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2018-03-16  7:13 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: kbuild-all, linux-kbuild, Nicolas Pitre, Jarod Wilson,
	Prarit Bhargava, Masahiro Yamada, Michal Marek, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180315]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-various-fix-clean-up-improvements-of-CONFIG_TRIM_UNUSED_KSYMS/20180316-123605
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Masahiro-Yamada/kbuild-various-fix-clean-up-improvements-of-CONFIG_TRIM_UNUSED_KSYMS/20180316-123605 HEAD 4825fb9eaae4e9ae97e48daeaa1bdcad6ea8b12f builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> make[1]: *** No rule to make target 'autoksyms', needed by 'scripts'.
   make[1]: Target '_all' not remade because of errors.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6733 bytes --]

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

* Re: [PATCH 3/7] kbuild: move 'scripts' target below
@ 2018-03-16  7:13     ` kbuild test robot
  0 siblings, 0 replies; 26+ messages in thread
From: kbuild test robot @ 2018-03-16  7:13 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: kbuild-all, linux-kbuild, Nicolas Pitre, Jarod Wilson,
	Prarit Bhargava, Michal Marek, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

Hi Masahiro,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.16-rc5 next-20180315]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-various-fix-clean-up-improvements-of-CONFIG_TRIM_UNUSED_KSYMS/20180316-123605
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Masahiro-Yamada/kbuild-various-fix-clean-up-improvements-of-CONFIG_TRIM_UNUSED_KSYMS/20180316-123605 HEAD 4825fb9eaae4e9ae97e48daeaa1bdcad6ea8b12f builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

>> make[1]: *** No rule to make target 'autoksyms', needed by 'scripts'.
   make[1]: Target '_all' not remade because of errors.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6733 bytes --]

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

* Re: [PATCH 3/7] kbuild: move 'scripts' target below
  2018-03-16  7:13     ` kbuild test robot
  (?)
@ 2018-03-16  7:20     ` Masahiro Yamada
  -1 siblings, 0 replies; 26+ messages in thread
From: Masahiro Yamada @ 2018-03-16  7:20 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, Linux Kbuild mailing list, Nicolas Pitre,
	Jarod Wilson, Prarit Bhargava, Michal Marek,
	Linux Kernel Mailing List

2018-03-16 16:13 GMT+09:00 kbuild test robot <lkp@intel.com>:
> Hi Masahiro,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.16-rc5 next-20180315]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Masahiro-Yamada/kbuild-various-fix-clean-up-improvements-of-CONFIG_TRIM_UNUSED_KSYMS/20180316-123605
> config: i386-tinyconfig (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> Note: the linux-review/Masahiro-Yamada/kbuild-various-fix-clean-up-improvements-of-CONFIG_TRIM_UNUSED_KSYMS/20180316-123605 HEAD 4825fb9eaae4e9ae97e48daeaa1bdcad6ea8b12f builds fine.
>       It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
>>> make[1]: *** No rule to make target 'autoksyms', needed by 'scripts'.
>    make[1]: Target '_all' not remade because of errors.
>

Seems a left-over garbage.

I will fix it.



-- 
Best Regards
Masahiro Yamada

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

end of thread, other threads:[~2018-03-16  7:21 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 16:44 [PATCH 0/7] kbuild: various fix, clean-up, improvements of CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
2018-03-14 16:44 ` [PATCH 1/7] kbuild: clear LDFLAGS in the top Makefile Masahiro Yamada
2018-03-14 16:44 ` [PATCH 2/7] kbuild: touch autoksyms.h when it is really missing Masahiro Yamada
2018-03-14 16:44   ` Masahiro Yamada
2018-03-14 17:26   ` Nicolas Pitre
2018-03-15  6:26     ` Masahiro Yamada
2018-03-14 16:44 ` [PATCH 3/7] kbuild: move 'scripts' target below Masahiro Yamada
2018-03-16  7:13   ` kbuild test robot
2018-03-16  7:13     ` kbuild test robot
2018-03-16  7:20     ` Masahiro Yamada
2018-03-14 16:44 ` [PATCH 4/7] kbuild: restore touching autoksyms.h to the top Makefile Masahiro Yamada
2018-03-14 16:44   ` Masahiro Yamada
2018-03-14 17:52   ` Nicolas Pitre
2018-03-14 16:44 ` [PATCH 5/7] kbuild: hide CONFIG_TRIM_UNUSED_KSYMS code from external module building Masahiro Yamada
2018-03-14 18:32   ` Nicolas Pitre
2018-03-15  6:36     ` Masahiro Yamada
2018-03-15 18:30       ` Nicolas Pitre
2018-03-14 16:44 ` [PATCH 6/7] kbuild: move include/config/ksym/* to include/ksym/* Masahiro Yamada
2018-03-14 18:47   ` Nicolas Pitre
2018-03-15 10:04     ` Masahiro Yamada
2018-03-15 11:01       ` Masahiro Yamada
2018-03-15 18:59       ` Nicolas Pitre
2018-03-14 16:44 ` [PATCH 7/7] kbuild: link vmlinux just once for CONFIG_TRIM_UNUSED_KSYMS Masahiro Yamada
2018-03-14 19:06   ` Nicolas Pitre
2018-03-15  8:00     ` Masahiro Yamada
2018-03-15 18:50       ` Nicolas Pitre

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.