[PATCH 2/2] tools: do not include scripts/Kbuild.include

1 view
Skip to first unread message

Masahiro Yamada

unread,
Apr 15, 2021, 3:28:03 AM4/15/21
to linux-...@vger.kernel.org, Janosch Frank, Christian Borntraeger, Masahiro Yamada, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler"), some kselftests fail to build.

The tools/ directory opted out Kbuild, and went in a different
direction. They copy any kind of files to the tools/ directory
in order to do whatever they want to do in their world.

tools/build/Build.include mimics scripts/Kbuild.include, but some
tool Makefiles included the Kbuild one to import a feature that is
missing in tools/build/Build.include:

- Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
only if supported") included scripts/Kbuild.include from
tools/thermal/tmon/Makefile to import the cc-option macro.

- Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
not support -no-pie") included scripts/Kbuild.include from
tools/testing/selftests/kvm/Makefile to import the try-run macro.

- Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
failures") included scripts/Kbuild.include from
tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
target.

- Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
unrecognized option") included scripts/Kbuild.include from
tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
try-run macro.

Copy what they want there, and stop including scripts/Kbuild.include
from the tool Makefiles.

Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac...@linux.ibm.com/
Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
Reported-by: Janosch Frank <fra...@linux.ibm.com>
Reported-by: Christian Borntraeger <bornt...@de.ibm.com>
Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

tools/testing/selftests/bpf/Makefile | 3 ++-
tools/testing/selftests/kvm/Makefile | 12 +++++++++++-
.../selftests/powerpc/pmu/ebb/Makefile | 11 ++++++++++-
tools/thermal/tmon/Makefile | 19 +++++++++++++++++--
4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d872b9f41543 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,5 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../scripts/Kbuild.include
include ../../../scripts/Makefile.arch
include ../../../scripts/Makefile.include

@@ -476,3 +475,5 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR) \
prog_tests/tests.h map_tests/tests.h verifier/tests.h \
feature \
$(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc bpf_testmod.ko)
+
+.DELETE_ON_ERROR:
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..8b45bc417d83 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,15 @@
# SPDX-License-Identifier: GPL-2.0-only
-include ../../../../scripts/Kbuild.include
+
+TMPOUT = .tmp_$$$$
+
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)

all:

diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
index af3df79d8163..d5d3e869df93 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
@@ -1,5 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../../../scripts/Kbuild.include

noarg:
$(MAKE) -C ../../
@@ -8,6 +7,16 @@ noarg:
CFLAGS += -m64

TMPOUT = $(OUTPUT)/TMPDIR/
+
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
+
# Toolchains may build PIE by default which breaks the assembly
no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
$(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index 59e417ec3e13..92a683e4866c 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -1,6 +1,21 @@
# SPDX-License-Identifier: GPL-2.0
-# We need this for the "cc-option" macro.
-include ../../../scripts/Kbuild.include
+
+TMPOUT = .tmp_$$$$
+
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
+
+__cc-option = $(call try-run,\
+ $(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
+
+cc-option = $(call __cc-option, $(CC),\
+ $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))

VERSION = 1.0

--
2.27.0

Paolo Bonzini

unread,
Apr 15, 2021, 3:40:30 AM4/15/21
to Masahiro Yamada, linux-...@vger.kernel.org, Janosch Frank, Christian Borntraeger, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
I think it would make sense to add try-run, cc-option and
.DELETE_ON_ERROR to tools/build/Build.include?

Paolo

Masahiro Yamada

unread,
Apr 15, 2021, 4:05:32 AM4/15/21
to Paolo Bonzini, Linux Kbuild mailing list, Janosch Frank, Christian Borntraeger, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, bpf, clang-built-linux, k...@vger.kernel.org, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, linuxppc-dev, Networking
To be safe, I just copy-pasted what the makefiles need.
If someone wants to refactor the tool build system, that is fine,
but, to me, I do not see consistent rules or policy under tools/.

--
Best Regards
Masahiro Yamada

Christian Borntraeger

unread,
Apr 15, 2021, 4:07:01 AM4/15/21
to Masahiro Yamada, linux-...@vger.kernel.org, Janosch Frank, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
When applying this on top of d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")

I still do get

# ==== Test Assertion Failure ====
# lib/kvm_util.c:142: vm->fd >= 0
# pid=315635 tid=315635 - Invalid argument
# 1 0x0000000001002f4b: vm_open at kvm_util.c:142
# 2 (inlined by) vm_create at kvm_util.c:258
# 3 0x00000000010015ef: test_add_max_memory_regions at set_memory_region_test.c:351
# 4 (inlined by) main at set_memory_region_test.c:397
# 5 0x000003ff971abb89: ?? ??:0
# 6 0x00000000010017ad: .annobin_abi_note.c.hot at crt1.o:?
# KVM_CREATE_VM ioctl failed, rc: -1 errno: 22
not ok 7 selftests: kvm: set_memory_region_test # exit=254

and the testcase compilation does not pickup the pgste option.

Paolo Bonzini

unread,
Apr 15, 2021, 4:25:17 AM4/15/21
to Masahiro Yamada, Linux Kbuild mailing list, Janosch Frank, Christian Borntraeger, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, bpf, clang-built-linux, k...@vger.kernel.org, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, linuxppc-dev, Networking
"Please put this in a common file instead of introducing duplication" is
not asking for wholesale refactoring.

Paolo

Christian Borntraeger

unread,
Apr 16, 2021, 1:56:35 AM4/16/21
to Masahiro Yamada, linux-...@vger.kernel.org, Janosch Frank, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
What does work is the following:
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..d9c6d9c2069e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0-only
include ../../../../scripts/Kbuild.include
+include ../../../../scripts/Makefile.compiler

all:


as it does pickup the linker option handling.


Masahiro Yamada

unread,
Apr 16, 2021, 8:54:17 AM4/16/21
to Christian Borntraeger, Linux Kbuild mailing list, Janosch Frank, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, bpf, clang-built-linux, k...@vger.kernel.org, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, linuxppc-dev, Networking
Kbuild and the tools are divorced.

They cannot be married unless the tools/
build system is largely refactored.
That will be a tons of works (and
I am not sure if it is welcome).

The Kbuild refactoring should not be bothered by
the tools.
For now, I want them separated from each other.

Masahiro Yamada

unread,
Apr 16, 2021, 9:01:46 AM4/16/21
to linux-...@vger.kernel.org, Janosch Frank, Christian Borntraeger, Masahiro Yamada, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler"), some kselftests fail to build.

The tools/ directory opted out Kbuild, and went in a different
direction. They copy any kind of files to the tools/ directory
in order to do whatever they want in their world.

tools/build/Build.include mimics scripts/Kbuild.include, but some
tool Makefiles included the Kbuild one to import a feature that is
missing in tools/build/Build.include:

- Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
only if supported") included scripts/Kbuild.include from
tools/thermal/tmon/Makefile to import the cc-option macro.

- Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
not support -no-pie") included scripts/Kbuild.include from
tools/testing/selftests/kvm/Makefile to import the try-run macro.

- Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
failures") included scripts/Kbuild.include from
tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
target.

- Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
unrecognized option") included scripts/Kbuild.include from
tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
try-run macro.

Copy what they need into tools/build/Build.include, and make them
include it instead of scripts/Kbuild.include.

Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac...@linux.ibm.com/
Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
Reported-by: Janosch Frank <fra...@linux.ibm.com>
Reported-by: Christian Borntraeger <bornt...@de.ibm.com>
Signed-off-by: Masahiro Yamada <masa...@kernel.org>
---

Changes in v2:
- copy macros to tools/build/BUild.include

tools/build/Build.include | 24 +++++++++++++++++++
tools/testing/selftests/bpf/Makefile | 2 +-
tools/testing/selftests/kvm/Makefile | 2 +-
.../selftests/powerpc/pmu/ebb/Makefile | 2 +-
tools/thermal/tmon/Makefile | 2 +-
5 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/tools/build/Build.include b/tools/build/Build.include
index 585486e40995..2cf3b1bde86e 100644
--- a/tools/build/Build.include
+++ b/tools/build/Build.include
@@ -100,3 +100,27 @@ cxx_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(CXXFLAGS) -D"BUILD_STR(s)=\#s" $(CXX
## HOSTCC C flags

host_c_flags = -Wp,-MD,$(depfile) -Wp,-MT,$@ $(KBUILD_HOSTCFLAGS) -D"BUILD_STR(s)=\#s" $(HOSTCFLAGS_$(basetarget).o) $(HOSTCFLAGS_$(obj))
+
+# output directory for tests below
+TMPOUT = .tmp_$$$$
+
+# try-run
+# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
+# Exit code chooses option. "$$TMP" serves as a temporary file and is
+# automatically cleaned up.
+try-run = $(shell set -e; \
+ TMP=$(TMPOUT)/tmp; \
+ mkdir -p $(TMPOUT); \
+ trap "rm -rf $(TMPOUT)" EXIT; \
+ if ($(1)) >/dev/null 2>&1; \
+ then echo "$(2)"; \
+ else echo "$(3)"; \
+ fi)
+
+# cc-option
+# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586)
+cc-option = $(call try-run, \
+ $(CC) -Werror $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
+
+# delete partially updated (i.e. corrupted) files on error
+.DELETE_ON_ERROR:
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..17a5cdf48d37 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../scripts/Kbuild.include
+include ../../../build/Build.include
include ../../../scripts/Makefile.arch
include ../../../scripts/Makefile.include

diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..5ef141f265bd 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
-include ../../../../scripts/Kbuild.include
+include ../../../build/Build.include

all:

diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
index af3df79d8163..c5ecb4634094 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
@@ -1,5 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
-include ../../../../../../scripts/Kbuild.include
+include ../../../../../build/Build.include

noarg:
$(MAKE) -C ../../
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index 59e417ec3e13..9db867df7679 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
# We need this for the "cc-option" macro.
-include ../../../scripts/Kbuild.include
+include ../../build/Build.include

Christian Borntraeger

unread,
Apr 16, 2021, 9:26:19 AM4/16/21
to Masahiro Yamada, linux-...@vger.kernel.org, Janosch Frank, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
looks better.
Tested-by: Christian Borntraeger <bornt...@de.ibm.com>

Paolo Bonzini

unread,
Apr 16, 2021, 9:27:48 AM4/16/21
to Christian Borntraeger, Masahiro Yamada, linux-...@vger.kernel.org, Janosch Frank, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
Thank you very much Masahiro, this look great.

Paolo

Yonghong Song

unread,
Apr 16, 2021, 11:39:41 AM4/16/21
to Masahiro Yamada, linux-...@vger.kernel.org, Janosch Frank, Christian Borntraeger, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, b...@vger.kernel.org, clang-bu...@googlegroups.com, k...@vger.kernel.org, linux-...@vger.kernel.org, linux-k...@vger.kernel.org, linuxp...@lists.ozlabs.org, net...@vger.kernel.org
LGTM although I see some tools Makefile directly added
".DELETE_ON_ERROR:" in their Makefile.

Acked-by: Yonghong Song <y...@fb.com>

Masahiro Yamada

unread,
Apr 17, 2021, 10:58:16 AM4/17/21
to Linux Kbuild mailing list, Janosch Frank, Christian Borntraeger, Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor, Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, bpf, clang-built-linux, k...@vger.kernel.org, Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK, linuxppc-dev, Networking
Applied to linux-kbuild.
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210416130051.239782-1-masahiroy%40kernel.org.
Reply all
Reply to author
Forward
0 new messages