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
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 <[email protected]>
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
Just a trivial change to prepare for the next commit.
This target is still invisible from external module building.
Signed-off-by: Masahiro Yamada <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
.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
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 <[email protected]>
> ---
>
> 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
>
>
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 <[email protected]>
You're the make master!
Acked-by: Nicolas Pitre <[email protected]>
> 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
>
>
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 <[email protected]>
Acked-by: Nicolas Pitre <[email protected]>
> ---
>
> 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
>
>
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 <[email protected]>
> ---
>
> .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
>
>
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 <[email protected]>
> ---
>
> 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
>
>
2018-03-15 2:26 GMT+09:00 Nicolas Pitre <[email protected]>:
> 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 <[email protected]>
>> ---
>>
>> 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
2018-03-15 3:32 GMT+09:00 Nicolas Pitre <[email protected]>:
> 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 <[email protected]>
>
> Acked-by: Nicolas Pitre <[email protected]>
>
>> ---
>>
>> 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
2018-03-15 4:06 GMT+09:00 Nicolas Pitre <[email protected]>:
> 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
2018-03-15 3:47 GMT+09:00 Nicolas Pitre <[email protected]>:
> 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 <[email protected]>
>> ---
>>
>> .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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Best Regards
Masahiro Yamada
2018-03-15 19:04 GMT+09:00 Masahiro Yamada <[email protected]>:
> 2018-03-15 3:47 GMT+09:00 Nicolas Pitre <[email protected]>:
>> 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 <[email protected]>
>>> ---
>>>
>>> .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
On Thu, 15 Mar 2018, Masahiro Yamada wrote:
> 2018-03-15 3:32 GMT+09:00 Nicolas Pitre <[email protected]>:
> > 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
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
On Thu, 15 Mar 2018, Masahiro Yamada wrote:
> 2018-03-15 3:47 GMT+09:00 Nicolas Pitre <[email protected]>:
> > 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
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
2018-03-16 16:13 GMT+09:00 kbuild test robot <[email protected]>:
> 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