* [PATCH 0/4] Fix kselftest build with sub-directory
@ 2022-07-08 16:23 Guillaume Tucker
2022-07-08 16:23 ` [PATCH 1/4] selftests: drop khdr make target Guillaume Tucker
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-08 16:23 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
Earlier attempts to get "make O=build kselftest-all" to work were
not successful as they made undesirable changes to some functions
in the top-level Makefile. This series takes a different
approach by removing the root cause of the problem within
kselftest, which is when the sub-Makefile tries to install kernel
headers "backwards" by calling make with the top-level Makefile.
The actual issue comes from the fact that $(srctree) is ".." when
building in a sub-directory with "O=build" which then obviously
makes "-C $(top_srcdir)" point outside of the real source tree.
With this series, the generic kselftest targets work as expected
from the top level with or without a build directory e.g.:
$ make kselftest-all
$ make O=build kselftest-all
Then in order to build using the sub-Makefile explicitly, the
headers have to be installed first. This is arguably a valid
requirement to have when building a tool from a sub-Makefile.
For example, "make -C tools/testing/nvdimm/" fails in a similar
way until <asm/rwonce.h> has been generated by a kernel build.
Guillaume Tucker (4):
selftests: drop khdr make target
selftests: stop using KSFT_KHDR_INSTALL
selftests: drop KSFT_KHDR_INSTALL make target
Makefile: add headers_install to kselftest targets
Makefile | 4 +-
tools/testing/selftests/Makefile | 28 +-------------
tools/testing/selftests/arm64/mte/Makefile | 1 -
tools/testing/selftests/arm64/signal/Makefile | 1 -
.../selftests/arm64/signal/test_signals.h | 4 +-
.../selftests/drivers/s390x/uvdevice/Makefile | 1 -
.../selftests/futex/functional/Makefile | 1 -
tools/testing/selftests/kvm/Makefile | 1 -
tools/testing/selftests/landlock/Makefile | 1 -
tools/testing/selftests/lib.mk | 38 -------------------
tools/testing/selftests/net/Makefile | 1 -
tools/testing/selftests/net/mptcp/Makefile | 1 -
tools/testing/selftests/tc-testing/Makefile | 1 -
tools/testing/selftests/vm/Makefile | 1 -
14 files changed, 5 insertions(+), 79 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] selftests: drop khdr make target
2022-07-08 16:23 [PATCH 0/4] Fix kselftest build with sub-directory Guillaume Tucker
@ 2022-07-08 16:23 ` Guillaume Tucker
2022-07-08 16:23 ` [PATCH 2/4] selftests: stop using KSFT_KHDR_INSTALL Guillaume Tucker
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-08 16:23 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
Drop the "khdr" make target as it fails when the build directory is a
sub-directory of the source tree. Rely on the "headers_install"
target to have been run first instead.
For example, here's a typical error this patch is addressing:
$ make O=build -j32 kselftest-gen_tar
make[1]: Entering directory '/home/kernelci/linux/build'
make --no-builtin-rules INSTALL_HDR_PATH=/home/kernelci/linux/build/usr \
ARCH=x86 -C ../../.. headers_install
make[3]: Entering directory '/home/kernelci/linux'
Makefile:1022: ../scripts/Makefile.extrawarn: No such file or directory
The source directory is determined in the top-level Makefile as ".."
relatively to the "build" directory, but then the kselftest Makefile
switches to "-C ../../.." so "../scripts" then points one level higher
than the source tree e.g. "linux/../scripts" - which fails obviously.
There is no other use-case in the kernel tree where a sub-directory
Makefile tries to call a top-level make target, and it appears this
isn't really a valid thing to do.
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
tools/testing/selftests/Makefile | 27 ++-------------------------
1 file changed, 2 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index de11992dc577..619451e82863 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -151,30 +151,7 @@ export KHDR_INCLUDES
# all isn't the first target in the file.
.DEFAULT_GOAL := all
-# Install headers here once for all tests. KSFT_KHDR_INSTALL_DONE
-# is used to avoid running headers_install from lib.mk.
-# Invoke headers install with --no-builtin-rules to avoid circular
-# dependency in "make kselftest" case. In this case, second level
-# make inherits builtin-rules which will use the rule generate
-# Makefile.o and runs into
-# "Circular Makefile.o <- prepare dependency dropped."
-# and headers_install fails and test compile fails.
-#
-# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile
-# invokes them as sub-makes and --no-builtin-rules is not necessary,
-# but doesn't cause any failures. Keep it simple and use the same
-# flags in both cases.
-# Local build cases: "make kselftest", "make -C" - headers are installed
-# in the default INSTALL_HDR_PATH usr/include.
-khdr:
-ifeq (1,$(DEFAULT_INSTALL_HDR_PATH))
- $(MAKE) --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install
-else
- $(MAKE) --no-builtin-rules INSTALL_HDR_PATH=$(abs_objtree)/usr \
- ARCH=$(ARCH) -C $(top_srcdir) headers_install
-endif
-
-all: khdr
+all:
@ret=1; \
for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
@@ -274,4 +251,4 @@ clean:
$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET clean;\
done;
-.PHONY: khdr all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean gen_tar
+.PHONY: all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean gen_tar
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] selftests: stop using KSFT_KHDR_INSTALL
2022-07-08 16:23 [PATCH 0/4] Fix kselftest build with sub-directory Guillaume Tucker
2022-07-08 16:23 ` [PATCH 1/4] selftests: drop khdr make target Guillaume Tucker
@ 2022-07-08 16:23 ` Guillaume Tucker
2022-07-08 16:23 ` [PATCH 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-08 16:23 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
Stop using the KSFT_KHDR_INSTALL flag as installing the kernel headers
from the kselftest Makefile is causing some issues. Instead, rely on
the headers to be installed directly by the top-level Makefile
"headers_install" make target prior to building kselftest.
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
tools/testing/selftests/arm64/mte/Makefile | 1 -
tools/testing/selftests/arm64/signal/Makefile | 1 -
tools/testing/selftests/arm64/signal/test_signals.h | 4 +---
tools/testing/selftests/drivers/s390x/uvdevice/Makefile | 1 -
tools/testing/selftests/futex/functional/Makefile | 1 -
tools/testing/selftests/kvm/Makefile | 1 -
tools/testing/selftests/landlock/Makefile | 1 -
tools/testing/selftests/net/Makefile | 1 -
tools/testing/selftests/net/mptcp/Makefile | 1 -
tools/testing/selftests/tc-testing/Makefile | 1 -
tools/testing/selftests/vm/Makefile | 1 -
11 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/tools/testing/selftests/arm64/mte/Makefile b/tools/testing/selftests/arm64/mte/Makefile
index 409e3e53d00a..a5a0744423d8 100644
--- a/tools/testing/selftests/arm64/mte/Makefile
+++ b/tools/testing/selftests/arm64/mte/Makefile
@@ -22,7 +22,6 @@ ifeq ($(mte_cc_support),1)
TEST_GEN_PROGS := $(PROGS)
# Get Kernel headers installed and use them.
-KSFT_KHDR_INSTALL := 1
else
$(warning compiler "$(CC)" does not support the ARMv8.5 MTE extension.)
$(warning test program "mte" will not be created.)
diff --git a/tools/testing/selftests/arm64/signal/Makefile b/tools/testing/selftests/arm64/signal/Makefile
index ac4ad0005715..be7520a863b0 100644
--- a/tools/testing/selftests/arm64/signal/Makefile
+++ b/tools/testing/selftests/arm64/signal/Makefile
@@ -11,7 +11,6 @@ PROGS := $(patsubst %.c,%,$(SRCS))
TEST_GEN_PROGS := $(notdir $(PROGS))
# Get Kernel headers installed and use them.
-KSFT_KHDR_INSTALL := 1
# Including KSFT lib.mk here will also mangle the TEST_GEN_PROGS list
# to account for any OUTPUT target-dirs optionally provided by
diff --git a/tools/testing/selftests/arm64/signal/test_signals.h b/tools/testing/selftests/arm64/signal/test_signals.h
index c70fdec7d7c4..0c645834ddc3 100644
--- a/tools/testing/selftests/arm64/signal/test_signals.h
+++ b/tools/testing/selftests/arm64/signal/test_signals.h
@@ -9,9 +9,7 @@
#include <ucontext.h>
/*
- * Using ARCH specific and sanitized Kernel headers installed by KSFT
- * framework since we asked for it by setting flag KSFT_KHDR_INSTALL
- * in our Makefile.
+ * Using ARCH specific and sanitized Kernel headers from the tree.
*/
#include <asm/ptrace.h>
#include <asm/hwcap.h>
diff --git a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
index 5e701d2708d4..891215a7dc8a 100644
--- a/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
+++ b/tools/testing/selftests/drivers/s390x/uvdevice/Makefile
@@ -11,7 +11,6 @@ else
TEST_GEN_PROGS := test_uvdevice
top_srcdir ?= ../../../../../..
-KSFT_KHDR_INSTALL := 1
khdr_dir = $(top_srcdir)/usr/include
LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include
diff --git a/tools/testing/selftests/futex/functional/Makefile b/tools/testing/selftests/futex/functional/Makefile
index b8152c573e8a..732149011692 100644
--- a/tools/testing/selftests/futex/functional/Makefile
+++ b/tools/testing/selftests/futex/functional/Makefile
@@ -22,7 +22,6 @@ TEST_GEN_FILES := \
TEST_PROGS := run.sh
top_srcdir = ../../../../..
-KSFT_KHDR_INSTALL := 1
DEFAULT_INSTALL_HDR_PATH := 1
include ../../lib.mk
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 81470a99ed1c..e15bb9693922 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -4,7 +4,6 @@ include ../../../build/Build.include
all:
top_srcdir = ../../../..
-KSFT_KHDR_INSTALL := 1
# For cross-builds to work, UNAME_M has to map to ARCH and arch specific
# directories and targets in this Makefile. "uname -m" doesn't map to
diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index 0b0049e133bb..1313e44e8fb9 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -8,7 +8,6 @@ TEST_GEN_PROGS := $(src_test:.c=)
TEST_GEN_PROGS_EXTENDED := true
-KSFT_KHDR_INSTALL := 1
OVERRIDE_TARGETS := 1
include ../lib.mk
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 464df13831f2..eaf3bb457576 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -63,7 +63,6 @@ TEST_GEN_FILES += bind_bhash_test
TEST_FILES := settings
-KSFT_KHDR_INSTALL := 1
include ../lib.mk
include bpf/Makefile
diff --git a/tools/testing/selftests/net/mptcp/Makefile b/tools/testing/selftests/net/mptcp/Makefile
index f905d5358e68..1af2f66fb59a 100644
--- a/tools/testing/selftests/net/mptcp/Makefile
+++ b/tools/testing/selftests/net/mptcp/Makefile
@@ -1,7 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
top_srcdir = ../../../../..
-KSFT_KHDR_INSTALL := 1
CFLAGS = -Wall -Wl,--no-as-needed -O2 -g -I$(top_srcdir)/usr/include $(KHDR_INCLUDES)
diff --git a/tools/testing/selftests/tc-testing/Makefile b/tools/testing/selftests/tc-testing/Makefile
index 4d639279f41e..cb553eac9f41 100644
--- a/tools/testing/selftests/tc-testing/Makefile
+++ b/tools/testing/selftests/tc-testing/Makefile
@@ -5,7 +5,6 @@ top_srcdir = $(abspath ../../../..)
APIDIR := $(top_scrdir)/include/uapi
TEST_GEN_FILES = action.o
-KSFT_KHDR_INSTALL := 1
include ../lib.mk
PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1)
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index 44f25acfbeca..108587cb327a 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -94,7 +94,6 @@ TEST_PROGS := run_vmtests.sh
TEST_FILES := test_vmalloc.sh
TEST_FILES += test_hmm.sh
-KSFT_KHDR_INSTALL := 1
include ../lib.mk
$(OUTPUT)/madv_populate: vm_util.c
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] selftests: drop KSFT_KHDR_INSTALL make target
2022-07-08 16:23 [PATCH 0/4] Fix kselftest build with sub-directory Guillaume Tucker
2022-07-08 16:23 ` [PATCH 1/4] selftests: drop khdr make target Guillaume Tucker
2022-07-08 16:23 ` [PATCH 2/4] selftests: stop using KSFT_KHDR_INSTALL Guillaume Tucker
@ 2022-07-08 16:23 ` Guillaume Tucker
2022-07-08 16:23 ` [PATCH 4/4] Makefile: add headers_install to kselftest targets Guillaume Tucker
2022-07-08 17:14 ` [PATCH 0/4] Fix kselftest build with sub-directory Shuah Khan
4 siblings, 0 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-08 16:23 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
Drop the KSFT_KHDR_INSTALL make target now that all use-cases have
been removed from the other kselftest Makefiles.
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
tools/testing/selftests/Makefile | 1 -
tools/testing/selftests/lib.mk | 38 --------------------------------
2 files changed, 39 deletions(-)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 619451e82863..e060777239a4 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -143,7 +143,6 @@ endif
# Prepare for headers install
include $(top_srcdir)/scripts/subarch.include
ARCH ?= $(SUBARCH)
-export KSFT_KHDR_INSTALL_DONE := 1
export BUILD
export KHDR_INCLUDES
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 2a2d240cdc1b..df5f853951f2 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -30,45 +30,7 @@ TEST_GEN_PROGS := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS))
TEST_GEN_PROGS_EXTENDED := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_PROGS_EXTENDED))
TEST_GEN_FILES := $(patsubst %,$(OUTPUT)/%,$(TEST_GEN_FILES))
-ifdef KSFT_KHDR_INSTALL
-top_srcdir ?= ../../../..
-include $(top_srcdir)/scripts/subarch.include
-ARCH ?= $(SUBARCH)
-
-# set default goal to all, so make without a target runs all, even when
-# all isn't the first target in the file.
-.DEFAULT_GOAL := all
-
-# Invoke headers install with --no-builtin-rules to avoid circular
-# dependency in "make kselftest" case. In this case, second level
-# make inherits builtin-rules which will use the rule generate
-# Makefile.o and runs into
-# "Circular Makefile.o <- prepare dependency dropped."
-# and headers_install fails and test compile fails.
-# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile
-# invokes them as sub-makes and --no-builtin-rules is not necessary,
-# but doesn't cause any failures. Keep it simple and use the same
-# flags in both cases.
-# Note that the support to install headers from lib.mk is necessary
-# when test Makefile is run directly with "make -C".
-# When local build is done, headers are installed in the default
-# INSTALL_HDR_PATH usr/include.
-.PHONY: khdr
-.NOTPARALLEL:
-khdr:
-ifndef KSFT_KHDR_INSTALL_DONE
-ifeq (1,$(DEFAULT_INSTALL_HDR_PATH))
- $(MAKE) --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install
-else
- $(MAKE) --no-builtin-rules INSTALL_HDR_PATH=$$OUTPUT/usr \
- ARCH=$(ARCH) -C $(top_srcdir) headers_install
-endif
-endif
-
-all: khdr $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
-else
all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES)
-endif
define RUN_TESTS
BASE_DIR="$(selfdir)"; \
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] Makefile: add headers_install to kselftest targets
2022-07-08 16:23 [PATCH 0/4] Fix kselftest build with sub-directory Guillaume Tucker
` (2 preceding siblings ...)
2022-07-08 16:23 ` [PATCH 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
@ 2022-07-08 16:23 ` Guillaume Tucker
2022-07-12 1:35 ` Masahiro Yamada
2022-07-13 6:38 ` Guillaume Tucker
2022-07-08 17:14 ` [PATCH 0/4] Fix kselftest build with sub-directory Shuah Khan
4 siblings, 2 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-08 16:23 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
Add headers_install as a dependency to kselftest targets so that they
can be run directly from the top of the tree. The kselftest Makefile
used to try to call headers_install "backwards" but failed due to the
relative path not being consistent.
Now we can either run this directly:
$ make O=build kselftest-all
or this:
$ make O=build headers_install
$ make O=build -C tools/testing/selftest all
The same commands work as well when building directly in the source
tree (no O=) or any arbitrary path (relative or absolute).
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
Makefile | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 1a6678d817bd..afc9d739ba44 100644
--- a/Makefile
+++ b/Makefile
@@ -1347,10 +1347,10 @@ tools/%: FORCE
# Kernel selftest
PHONY += kselftest
-kselftest:
+kselftest: headers_install
$(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
-kselftest-%: FORCE
+kselftest-%: headers_install FORCE
$(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
PHONY += kselftest-merge
--
2.30.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Fix kselftest build with sub-directory
2022-07-08 16:23 [PATCH 0/4] Fix kselftest build with sub-directory Guillaume Tucker
` (3 preceding siblings ...)
2022-07-08 16:23 ` [PATCH 4/4] Makefile: add headers_install to kselftest targets Guillaume Tucker
@ 2022-07-08 17:14 ` Shuah Khan
2022-07-08 17:37 ` Bird, Tim
2022-07-11 12:13 ` Anders Roxell
4 siblings, 2 replies; 12+ messages in thread
From: Shuah Khan @ 2022-07-08 17:14 UTC (permalink / raw)
To: Guillaume Tucker, Masahiro Yamada, Michal Marek,
Nick Desaulniers, Shuah Khan, Kees Cook, Naresh Kamboju,
Anders Roxell
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest, Shuah Khan
On 7/8/22 10:23 AM, Guillaume Tucker wrote:
> Earlier attempts to get "make O=build kselftest-all" to work were
> not successful as they made undesirable changes to some functions
> in the top-level Makefile. This series takes a different
> approach by removing the root cause of the problem within
> kselftest, which is when the sub-Makefile tries to install kernel
> headers "backwards" by calling make with the top-level Makefile.
> The actual issue comes from the fact that $(srctree) is ".." when
> building in a sub-directory with "O=build" which then obviously
> makes "-C $(top_srcdir)" point outside of the real source tree.
>
> With this series, the generic kselftest targets work as expected
> from the top level with or without a build directory e.g.:
>
> $ make kselftest-all
>
> $ make O=build kselftest-all
>
> Then in order to build using the sub-Makefile explicitly, the
> headers have to be installed first. This is arguably a valid
> requirement to have when building a tool from a sub-Makefile.
> For example, "make -C tools/testing/nvdimm/" fails in a similar
> way until <asm/rwonce.h> has been generated by a kernel build.
>
> Guillaume Tucker (4):
> selftests: drop khdr make target
> selftests: stop using KSFT_KHDR_INSTALL
> selftests: drop KSFT_KHDR_INSTALL make target
> Makefile: add headers_install to kselftest targets
>
This takes us to back to the state before b2d35fa5fc80 added
khdr support. I reluctantly agreed to the change and it has
proven to be a problematic change. I would rather have had the
dependency stated that headers should be installed prior to
building tests - test build depends on kernel build anyway and
having dependency on headers having build isn't a huge deal.
So I am in favor of getting rid of khdr support. However, this
khdr support was a change originated from Linaro test ring. Undoing
this might have implication on their workflow.
I will pull them into the discussion so they are aware of it and
be prepared for this change.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 0/4] Fix kselftest build with sub-directory
2022-07-08 17:14 ` [PATCH 0/4] Fix kselftest build with sub-directory Shuah Khan
@ 2022-07-08 17:37 ` Bird, Tim
2022-07-11 12:13 ` Anders Roxell
1 sibling, 0 replies; 12+ messages in thread
From: Bird, Tim @ 2022-07-08 17:37 UTC (permalink / raw)
To: Shuah Khan, Guillaume Tucker, Masahiro Yamada, Michal Marek,
Nick Desaulniers, Shuah Khan, Kees Cook, Naresh Kamboju,
Anders Roxell
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
> -----Original Message-----
> From: Shuah Khan <skhan@linuxfoundation.org>
>
> On 7/8/22 10:23 AM, Guillaume Tucker wrote:
> > Earlier attempts to get "make O=build kselftest-all" to work were
> > not successful as they made undesirable changes to some functions
> > in the top-level Makefile. This series takes a different
> > approach by removing the root cause of the problem within
> > kselftest, which is when the sub-Makefile tries to install kernel
> > headers "backwards" by calling make with the top-level Makefile.
> > The actual issue comes from the fact that $(srctree) is ".." when
> > building in a sub-directory with "O=build" which then obviously
> > makes "-C $(top_srcdir)" point outside of the real source tree.
> >
> > With this series, the generic kselftest targets work as expected
> > from the top level with or without a build directory e.g.:
> >
> > $ make kselftest-all
> >
> > $ make O=build kselftest-all
> >
> > Then in order to build using the sub-Makefile explicitly, the
> > headers have to be installed first. This is arguably a valid
> > requirement to have when building a tool from a sub-Makefile.
> > For example, "make -C tools/testing/nvdimm/" fails in a similar
> > way until <asm/rwonce.h> has been generated by a kernel build.
> >
> > Guillaume Tucker (4):
> > selftests: drop khdr make target
> > selftests: stop using KSFT_KHDR_INSTALL
> > selftests: drop KSFT_KHDR_INSTALL make target
> > Makefile: add headers_install to kselftest targets
> >
>
> This takes us to back to the state before b2d35fa5fc80 added
> khdr support. I reluctantly agreed to the change and it has
> proven to be a problematic change. I would rather have had the
> dependency stated that headers should be installed prior to
> building tests - test build depends on kernel build anyway and
> having dependency on headers having build isn't a huge deal.
>
> So I am in favor of getting rid of khdr support. However, this
> khdr support was a change originated from Linaro test ring. Undoing
> this might have implication on their workflow.
>
> I will pull them into the discussion so they are aware of it and
> be prepared for this change.
I ran into this bug quite a while ago. I reported it here:
https://lore.kernel.org/all/ECADFF3FD767C149AD96A924E7EA6EAF977BD214@USCULXMSG01.am.sony.com/
it was fixed here:
https://lore.kernel.org/all/20191015014505.14259-1-skhan@linuxfoundation.org/
but apparently reverting it was discussed, based on this:
https://lore.kernel.org/all/8d34a9b9-f8f3-0e37-00bf-c342cf3d4074@arm.com/
I'm not sure what happened after that.
I was able to work around it by avoiding
putting the build directory inside the source tree.
I strongly support getting rid of the khdr stuff, as it's quite hard
to follow. I think my workflow would not be affected (but I should
run off and test it.)
Thanks for working on this Guillaume!
-- Tim
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Fix kselftest build with sub-directory
2022-07-08 17:14 ` [PATCH 0/4] Fix kselftest build with sub-directory Shuah Khan
2022-07-08 17:37 ` Bird, Tim
@ 2022-07-11 12:13 ` Anders Roxell
2022-07-11 23:08 ` Shuah Khan
1 sibling, 1 reply; 12+ messages in thread
From: Anders Roxell @ 2022-07-11 12:13 UTC (permalink / raw)
To: Shuah Khan
Cc: Guillaume Tucker, Masahiro Yamada, Michal Marek,
Nick Desaulniers, Shuah Khan, Kees Cook, Naresh Kamboju, kernel,
linux-kbuild, linux-kernel, linux-kselftest
On Fri, 8 Jul 2022 at 19:14, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 7/8/22 10:23 AM, Guillaume Tucker wrote:
> > Earlier attempts to get "make O=build kselftest-all" to work were
> > not successful as they made undesirable changes to some functions
> > in the top-level Makefile. This series takes a different
> > approach by removing the root cause of the problem within
> > kselftest, which is when the sub-Makefile tries to install kernel
> > headers "backwards" by calling make with the top-level Makefile.
> > The actual issue comes from the fact that $(srctree) is ".." when
> > building in a sub-directory with "O=build" which then obviously
> > makes "-C $(top_srcdir)" point outside of the real source tree.
> >
> > With this series, the generic kselftest targets work as expected
> > from the top level with or without a build directory e.g.:
> >
> > $ make kselftest-all
> >
> > $ make O=build kselftest-all
> >
> > Then in order to build using the sub-Makefile explicitly, the
> > headers have to be installed first. This is arguably a valid
> > requirement to have when building a tool from a sub-Makefile.
> > For example, "make -C tools/testing/nvdimm/" fails in a similar
> > way until <asm/rwonce.h> has been generated by a kernel build.
> >
> > Guillaume Tucker (4):
> > selftests: drop khdr make target
> > selftests: stop using KSFT_KHDR_INSTALL
> > selftests: drop KSFT_KHDR_INSTALL make target
> > Makefile: add headers_install to kselftest targets
> >
>
> This takes us to back to the state before b2d35fa5fc80 added
> khdr support. I reluctantly agreed to the change and it has
> proven to be a problematic change. I would rather have had the
> dependency stated that headers should be installed prior to
> building tests - test build depends on kernel build anyway and
> having dependency on headers having build isn't a huge deal.
I agree that it's not a huge deal.
>
> So I am in favor of getting rid of khdr support. However, this
> khdr support was a change originated from Linaro test ring. Undoing
> this might have implication on their workflow.
It shouldn't be a problem.
I've been running these patches through a smoke test and it looks
good.
Tested-by: Anders Roxell <anders.roxell@linaro.org>
Cheers,
Anders
>
> I will pull them into the discussion so they are aware of it and
> be prepared for this change.
>
> thanks,
> -- Shuah
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Fix kselftest build with sub-directory
2022-07-11 12:13 ` Anders Roxell
@ 2022-07-11 23:08 ` Shuah Khan
0 siblings, 0 replies; 12+ messages in thread
From: Shuah Khan @ 2022-07-11 23:08 UTC (permalink / raw)
To: Anders Roxell
Cc: Guillaume Tucker, Masahiro Yamada, Michal Marek,
Nick Desaulniers, Shuah Khan, Kees Cook, Naresh Kamboju, kernel,
linux-kbuild, linux-kernel, linux-kselftest, Shuah Khan
On 7/11/22 6:13 AM, Anders Roxell wrote:
> On Fri, 8 Jul 2022 at 19:14, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 7/8/22 10:23 AM, Guillaume Tucker wrote:
>>> Earlier attempts to get "make O=build kselftest-all" to work were
>>> not successful as they made undesirable changes to some functions
>>> in the top-level Makefile. This series takes a different
>>> approach by removing the root cause of the problem within
>>> kselftest, which is when the sub-Makefile tries to install kernel
>>> headers "backwards" by calling make with the top-level Makefile.
>>> The actual issue comes from the fact that $(srctree) is ".." when
>>> building in a sub-directory with "O=build" which then obviously
>>> makes "-C $(top_srcdir)" point outside of the real source tree.
>>>
>>> With this series, the generic kselftest targets work as expected
>>> from the top level with or without a build directory e.g.:
>>>
>>> $ make kselftest-all
>>>
>>> $ make O=build kselftest-all
>>>
>>> Then in order to build using the sub-Makefile explicitly, the
>>> headers have to be installed first. This is arguably a valid
>>> requirement to have when building a tool from a sub-Makefile.
>>> For example, "make -C tools/testing/nvdimm/" fails in a similar
>>> way until <asm/rwonce.h> has been generated by a kernel build.
>>>
>>> Guillaume Tucker (4):
>>> selftests: drop khdr make target
>>> selftests: stop using KSFT_KHDR_INSTALL
>>> selftests: drop KSFT_KHDR_INSTALL make target
>>> Makefile: add headers_install to kselftest targets
>>>
>>
>> This takes us to back to the state before b2d35fa5fc80 added
>> khdr support. I reluctantly agreed to the change and it has
>> proven to be a problematic change. I would rather have had the
>> dependency stated that headers should be installed prior to
>> building tests - test build depends on kernel build anyway and
>> having dependency on headers having build isn't a huge deal.
>
> I agree that it's not a huge deal.
>
>>
>> So I am in favor of getting rid of khdr support. However, this
>> khdr support was a change originated from Linaro test ring. Undoing
>> this might have implication on their workflow.
>
> It shouldn't be a problem.
> I've been running these patches through a smoke test and it looks
> good.
>
> Tested-by: Anders Roxell <anders.roxell@linaro.org>
>
>
Thank you Anders for confirming this isn't a problem for Linaro workflow
and testing.
Than you Guillaume for fixing the problem. I will apply these for 5.20-rc1.
thanks,
-- Shuah
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Makefile: add headers_install to kselftest targets
2022-07-08 16:23 ` [PATCH 4/4] Makefile: add headers_install to kselftest targets Guillaume Tucker
@ 2022-07-12 1:35 ` Masahiro Yamada
2022-07-12 8:32 ` Guillaume Tucker
2022-07-13 6:38 ` Guillaume Tucker
1 sibling, 1 reply; 12+ messages in thread
From: Masahiro Yamada @ 2022-07-12 1:35 UTC (permalink / raw)
To: Guillaume Tucker
Cc: Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook, kernel,
Linux Kbuild mailing list, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK
On Sat, Jul 9, 2022 at 1:23 AM Guillaume Tucker
<guillaume.tucker@collabora.com> wrote:
>
> Add headers_install as a dependency to kselftest targets so that they
> can be run directly from the top of the tree. The kselftest Makefile
> used to try to call headers_install "backwards" but failed due to the
> relative path not being consistent.
>
> Now we can either run this directly:
>
> $ make O=build kselftest-all
>
> or this:
>
> $ make O=build headers_install
> $ make O=build -C tools/testing/selftest all
>
> The same commands work as well when building directly in the source
> tree (no O=) or any arbitrary path (relative or absolute).
>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
> Makefile | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1a6678d817bd..afc9d739ba44 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1347,10 +1347,10 @@ tools/%: FORCE
> # Kernel selftest
>
> PHONY += kselftest
> -kselftest:
> +kselftest: headers_install
> $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
Nit.
Please use 'headers' for in-kernel use of exportedI headers.
kselftest: headers
>
> -kselftest-%: FORCE
> +kselftest-%: headers_install FORCE
> $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
Ditto.
kselftest-%: headers FORCE
>
> PHONY += kselftest-merge
> --
> 2.30.2
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Makefile: add headers_install to kselftest targets
2022-07-12 1:35 ` Masahiro Yamada
@ 2022-07-12 8:32 ` Guillaume Tucker
0 siblings, 0 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-12 8:32 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook, kernel,
Linux Kbuild mailing list, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK
On 12/07/2022 02:35, Masahiro Yamada wrote:
> On Sat, Jul 9, 2022 at 1:23 AM Guillaume Tucker
> <guillaume.tucker@collabora.com> wrote:
>>
>> Add headers_install as a dependency to kselftest targets so that they
>> can be run directly from the top of the tree. The kselftest Makefile
>> used to try to call headers_install "backwards" but failed due to the
>> relative path not being consistent.
>>
>> Now we can either run this directly:
>>
>> $ make O=build kselftest-all
>>
>> or this:
>>
>> $ make O=build headers_install
>> $ make O=build -C tools/testing/selftest all
>>
>> The same commands work as well when building directly in the source
>> tree (no O=) or any arbitrary path (relative or absolute).
>>
>> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
>> ---
>> Makefile | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 1a6678d817bd..afc9d739ba44 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1347,10 +1347,10 @@ tools/%: FORCE
>> # Kernel selftest
>>
>> PHONY += kselftest
>> -kselftest:
>> +kselftest: headers_install
>> $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests run_tests
>
> Nit.
> Please use 'headers' for in-kernel use of exportedI headers.
>
> kselftest: headers
>
>
>>
>> -kselftest-%: FORCE
>> +kselftest-%: headers_install FORCE
>> $(Q)$(MAKE) -C $(srctree)/tools/testing/selftests $*
>
> Ditto.
>
> kselftest-%: headers FORCE
Thank you all for the reviews.
I've sent a v2 with this tweak and double-checked that the
kselftest build produced exactly the same result.
Best wishes,
Guillaume
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/4] Makefile: add headers_install to kselftest targets
2022-07-08 16:23 ` [PATCH 4/4] Makefile: add headers_install to kselftest targets Guillaume Tucker
2022-07-12 1:35 ` Masahiro Yamada
@ 2022-07-13 6:38 ` Guillaume Tucker
1 sibling, 0 replies; 12+ messages in thread
From: Guillaume Tucker @ 2022-07-13 6:38 UTC (permalink / raw)
To: Masahiro Yamada, Michal Marek, Nick Desaulniers, Shuah Khan, Kees Cook
Cc: kernel, linux-kbuild, linux-kernel, linux-kselftest
On 08/07/2022 18:23, Guillaume Tucker wrote:
>
> $ make O=build headers_install
> $ make O=build -C tools/testing/selftest all
Typo, it should be selftests:
$ make O=build -C tools/testing/selftests all
Thanks,
Guillaume
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-07-13 6:38 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-08 16:23 [PATCH 0/4] Fix kselftest build with sub-directory Guillaume Tucker
2022-07-08 16:23 ` [PATCH 1/4] selftests: drop khdr make target Guillaume Tucker
2022-07-08 16:23 ` [PATCH 2/4] selftests: stop using KSFT_KHDR_INSTALL Guillaume Tucker
2022-07-08 16:23 ` [PATCH 3/4] selftests: drop KSFT_KHDR_INSTALL make target Guillaume Tucker
2022-07-08 16:23 ` [PATCH 4/4] Makefile: add headers_install to kselftest targets Guillaume Tucker
2022-07-12 1:35 ` Masahiro Yamada
2022-07-12 8:32 ` Guillaume Tucker
2022-07-13 6:38 ` Guillaume Tucker
2022-07-08 17:14 ` [PATCH 0/4] Fix kselftest build with sub-directory Shuah Khan
2022-07-08 17:37 ` Bird, Tim
2022-07-11 12:13 ` Anders Roxell
2022-07-11 23:08 ` Shuah Khan
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.