| Message ID | 20230710-kselftest-fix-arm64-v1-1-48e872844f25@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 43e8832fed08438e2a27afed9bac21acd0ceffe5 |
| Headers | show |
| Series | selftests: Fix arm64 test installation | expand |
On 7/10/23 07:04, Mark Brown wrote: > The recent change fc96c7c19df ("selftests: error out if kernel header > files are not yet built") to generate an error message when building > kselftests without having installed the headers is generating spurious > failures during the install step which breaks the arm64 selftests (and > only the arm64 selftests): > > Emit Tests for arm64 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[4]: *** [Makefile:26: all] Error 2 > > Presumably the arm64 tests are doing something unusual in their build > setup which could be adjusted but I didn't immediately see it and since > this is having a serious impact on test coverage in automation let's > just revert for now. > > This is causing failures in KernelCI with the command: > > make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar > > and also when building using tuxmake. > > Full log: https://storage.kernelci.org/mainline/master/v6.5-rc1/arm64/defconfig/gcc-10/logs/kselftest.log OK, I borrowed an arm64 system and was able to reproduce the problem. There are 30 or 50 other pre-existing arm64 selftest build failures which were quite misleading at first, until I got into the "right" selftests mindset of, "massive swaths of selftests are broken, deal with it". :) Anyway, I can now see new hard failure that you reported: Emit Tests for arm64 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 Let me see if there is a quick fix for this, especially given that every other subsystem is somehow avoiding this--it's probably easy, just a sec... thanks,
On Mon, Jul 10, 2023 at 01:22:41PM -0700, John Hubbard wrote: > There are 30 or 50 other pre-existing arm64 selftest build failures which were > quite misleading at first, until I got into the "right" selftests mindset of, > "massive swaths of selftests are broken, deal with it". :) There are no such thing as far as I am aware - the arm64 selftests are *very* actively used by a range of people and CI systems, I certainly build them pretty consistently and am aware of no build failures with either GCC or clang. You do need to install the headers to get the current APIs but until your commit everything was building cleanly. If you are seeing any problems please report them.
On 7/10/23 14:20, Mark Brown wrote: > On Mon, Jul 10, 2023 at 01:22:41PM -0700, John Hubbard wrote: > >> There are 30 or 50 other pre-existing arm64 selftest build failures which were >> quite misleading at first, until I got into the "right" selftests mindset of, >> "massive swaths of selftests are broken, deal with it". :) > > There are no such thing as far as I am aware - the arm64 selftests are > *very* actively used by a range of people and CI systems, I certainly > build them pretty consistently and am aware of no build failures with > either GCC or clang. You do need to install the headers to get the > current APIs but until your commit everything was building cleanly. > > If you are seeing any problems please report them. oh wow, yes, I am! It's on a slightly older installation (gcc version 8.5.0 20210514 (Red Hat 8.5.0-18)), but there are a lot of basic build failures, I'll get them together and send out a note. Meanwhile, if you would like to try a quick fix, I have one that fixes the problem on my system. I'm inclined to dress it up with a comment that explains it (with a "TODO: stop using recursive Make here"), and send it out as an actual fix: diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile index 9460cbe81bcc..ace8b67fb22d 100644 --- a/tools/testing/selftests/arm64/Makefile +++ b/tools/testing/selftests/arm64/Makefile @@ -42,7 +42,7 @@ run_tests: all done # Avoid any output on non arm64 on emit_tests -emit_tests: all +emit_tests: @for DIR in $(ARM64_SUBTARGETS); do \ BUILD_TARGET=$(OUTPUT)/$$DIR; \ make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \ <blueforge> arm64 (master)$ thanks,
On Mon, Jul 10, 2023 at 02:31:56PM -0700, John Hubbard wrote: > On 7/10/23 14:20, Mark Brown wrote: > > There are no such thing as far as I am aware - the arm64 selftests are > > *very* actively used by a range of people and CI systems, I certainly > > build them pretty consistently and am aware of no build failures with > > either GCC or clang. You do need to install the headers to get the > > current APIs but until your commit everything was building cleanly. > > If you are seeing any problems please report them. > oh wow, yes, I am! It's on a slightly older installation (gcc version > 8.5.0 20210514 (Red Hat 8.5.0-18)), but there are a lot of basic build > failures, I'll get them together and send out a note. There is a floor on binutils version for the kselftests that's more aggressive than that for the kernel itself, though that looks like RHEL 8 which has binutils 2.30 which *should* be fine for most things - the MTE tests won't build but they do have version detection so should skip, I guess you might have trouble with PAC support which doesn't have detection in the tests? It's certainly old enough that I'm surprised to hear someone doing development for anything current with it. I just tried a Debian based GCC 8 container which seems pretty happy for arm64, the command was: make -j16 O=/tmp/out INSTALL_PATH=/tmp/kselftest \ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install (the compat toolchain isn't used here IIRC). It does skip the MTE tests but otherwise isn't showing any obvious issues in the arm64 tests. > Meanwhile, if you would like to try a quick fix, I have one that fixes > the problem on my system. I'm inclined to dress it up with a comment > that explains it (with a "TODO: stop using recursive Make here"), and > send it out as an actual fix: > > diff --git a/tools/testing/selftests/arm64/Makefile b/tools/testing/selftests/arm64/Makefile > index 9460cbe81bcc..ace8b67fb22d 100644 > --- a/tools/testing/selftests/arm64/Makefile > +++ b/tools/testing/selftests/arm64/Makefile > @@ -42,7 +42,7 @@ run_tests: all > done > # Avoid any output on non arm64 on emit_tests > -emit_tests: all > +emit_tests: > @for DIR in $(ARM64_SUBTARGETS); do \ > BUILD_TARGET=$(OUTPUT)/$$DIR; \ > make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \ That does seem to work around the issue at least with a quick out of tree build, including with GCC 8.
On 7/10/23 15:30, Mark Brown wrote: ... > There is a floor on binutils version for the kselftests that's more > aggressive than that for the kernel itself, though that looks like RHEL > 8 which has binutils 2.30 which *should* be fine for most things - the > MTE tests won't build but they do have version detection so should skip, > I guess you might have trouble with PAC support which doesn't have > detection in the tests? It's certainly old enough that I'm surprised to > hear someone doing development for anything current with it. This used to be a development machine, but now it is sufficiently old that it is lightly used--that would explain how I could reserve it on short notice for this. Maybe I'll adopt it and upgrade to a modern distro, now that I seem to need an arm64 box. > > I just tried a Debian based GCC 8 container which seems pretty happy > for arm64, the command was: > > make -j16 O=/tmp/out INSTALL_PATH=/tmp/kselftest \ > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ > CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install > > (the compat toolchain isn't used here IIRC). It does skip the MTE tests > but otherwise isn't showing any obvious issues in the arm64 tests. > Thank you for providing a snapshot of what it looks like on gcc 8 over there. OK, so actually, many of the failures were due to the "all" target getting run too early (recursive make, again, uggh). With the fix below, there are still a dozen failures in selftests, but only one in the arm64 tree, after all. ... > That does seem to work around the issue at least with a quick out of > tree build, including with GCC 8. Great news! That's really helpful. And in fact, I have discovered two more things: 1) The "emit_tests" target is there apparently because commit 313a4db7f3387 ("kselftest: arm64: extend toplevel skeleton Makefile") believed that it was necessary to skip emitting tests if not on the right native platform. I'm tempted to delete the entire emit_tests target in both arm64 and riscv selftests (and that also seems to work just fine) in order to simplify things, perhaps as a follow up step. For now I'll just post the simpler fix, though. 2) riscv has copied this Makefile subtest technique to its (very small so far) set of selftests. I have no native system to test any fixes on, but I'm probably going to post a "blind" fix for that one, too. thanks,
On Mon, Jul 10, 2023 at 04:10:14PM -0700, John Hubbard wrote: > On 7/10/23 15:30, Mark Brown wrote: > > There is a floor on binutils version for the kselftests that's more > > aggressive than that for the kernel itself, though that looks like RHEL > > 8 which has binutils 2.30 which *should* be fine for most things - the > > MTE tests won't build but they do have version detection so should skip, > > I guess you might have trouble with PAC support which doesn't have > > detection in the tests? It's certainly old enough that I'm surprised to > > hear someone doing development for anything current with it. > This used to be a development machine, but now it is sufficiently old > that it is lightly used--that would explain how I could reserve it on > short notice for this. Maybe I'll adopt it and upgrade to a modern > distro, now that I seem to need an arm64 box. > > That does seem to work around the issue at least with a quick out of > > tree build, including with GCC 8. > Great news! That's really helpful. And in fact, I have discovered two > more things: > 1) The "emit_tests" target is there apparently because commit > 313a4db7f3387 ("kselftest: arm64: extend toplevel skeleton Makefile") > believed that it was necessary to skip emitting tests if not on the > right native platform. I'm tempted to delete the entire emit_tests > target in both arm64 and riscv selftests (and that also seems to work > just fine) in order to simplify things, perhaps as a follow up step. > For now I'll just post the simpler fix, though. I suspect it might've been needed at the time the patch was written but subsequent changes in the kselftest Makefile stuff have obsoleted it.
On 7/10/23 08:04, Mark Brown wrote: > The recent change fc96c7c19df ("selftests: error out if kernel header > files are not yet built") to generate an error message when building > kselftests without having installed the headers is generating spurious > failures during the install step which breaks the arm64 selftests (and > only the arm64 selftests): > > Emit Tests for arm64 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 > make[4]: *** [Makefile:26: all] Error 2 > > Presumably the arm64 tests are doing something unusual in their build > setup which could be adjusted but I didn't immediately see it and since > this is having a serious impact on test coverage in automation let's > just revert for now. > > This is causing failures in KernelCI with the command: > > make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar > > and also when building using tuxmake. > > Full log: https://storage.kernelci.org/mainline/master/v6.5-rc1/arm64/defconfig/gcc-10/logs/kselftest.log > > Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built") > Signed-off-by: Mark Brown <broonie@kernel.org> > --- > tools/testing/selftests/Makefile | 21 +-------------------- > tools/testing/selftests/lib.mk | 40 +++------------------------------------- > 2 files changed, 4 insertions(+), 57 deletions(-) > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 666b56f22a41..405683b8cb39 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -146,12 +146,10 @@ ifneq ($(KBUILD_OUTPUT),) > abs_objtree := $(realpath $(abs_objtree)) > BUILD := $(abs_objtree)/kselftest > KHDR_INCLUDES := -isystem ${abs_objtree}/usr/include > - KHDR_DIR := ${abs_objtree}/usr/include > else > BUILD := $(CURDIR) > abs_srctree := $(shell cd $(top_srcdir) && pwd) > KHDR_INCLUDES := -isystem ${abs_srctree}/usr/include > - KHDR_DIR := ${abs_srctree}/usr/include > DEFAULT_INSTALL_HDR_PATH := 1 > endif > > @@ -165,7 +163,7 @@ export KHDR_INCLUDES > # all isn't the first target in the file. > .DEFAULT_GOAL := all > > -all: kernel_header_files > +all: > @ret=1; \ > for TARGET in $(TARGETS); do \ > BUILD_TARGET=$$BUILD/$$TARGET; \ > @@ -176,23 +174,6 @@ all: kernel_header_files > ret=$$((ret * $$?)); \ > done; exit $$ret; > > -kernel_header_files: > - @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ > - if [ $$? -ne 0 ]; then \ > - RED='\033[1;31m'; \ > - NOCOLOR='\033[0m'; \ > - echo; \ > - echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ > - echo "Please run this and try again:"; \ > - echo; \ > - echo " cd $(top_srcdir)"; \ > - echo " make headers"; \ > - echo; \ > - exit 1; \ > - fi > - > -.PHONY: kernel_header_files > - > run_tests: all > @for TARGET in $(TARGETS); do \ > BUILD_TARGET=$$BUILD/$$TARGET; \ > diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk > index d17854285f2b..05400462c779 100644 > --- a/tools/testing/selftests/lib.mk > +++ b/tools/testing/selftests/lib.mk > @@ -44,26 +44,10 @@ endif > selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST)))) > top_srcdir = $(selfdir)/../../.. > > -ifeq ("$(origin O)", "command line") > - KBUILD_OUTPUT := $(O) > +ifeq ($(KHDR_INCLUDES),) > +KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include > endif > > -ifneq ($(KBUILD_OUTPUT),) > - # Make's built-in functions such as $(abspath ...), $(realpath ...) cannot > - # expand a shell special character '~'. We use a somewhat tedious way here. > - abs_objtree := $(shell cd $(top_srcdir) && mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd) > - $(if $(abs_objtree),, \ > - $(error failed to create output directory "$(KBUILD_OUTPUT)")) > - # $(realpath ...) resolves symlinks > - abs_objtree := $(realpath $(abs_objtree)) > - KHDR_DIR := ${abs_objtree}/usr/include > -else > - abs_srctree := $(shell cd $(top_srcdir) && pwd) > - KHDR_DIR := ${abs_srctree}/usr/include > -endif > - > -KHDR_INCLUDES := -isystem $(KHDR_DIR) > - > # The following are built by lib.mk common compile rules. > # TEST_CUSTOM_PROGS should be used by tests that require > # custom build rule and prevent common build rule use. > @@ -74,25 +58,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)) > > -all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \ > - $(TEST_GEN_FILES) > - > -kernel_header_files: > - @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ > - if [ $$? -ne 0 ]; then \ > - RED='\033[1;31m'; \ > - NOCOLOR='\033[0m'; \ > - echo; \ > - echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ > - echo "Please run this and try again:"; \ > - echo; \ > - echo " cd $(top_srcdir)"; \ > - echo " make headers"; \ > - echo; \ > - exit 1; \ > - fi > - > -.PHONY: kernel_header_files > +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) > > define RUN_TESTS > BASE_DIR="$(selfdir)"; \ > > --- > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 > change-id: 20230710-kselftest-fix-arm64-c023160018d7 > > Best regards, Thank you. Will apply the patch for the next rc thanks, -- Shuah
On 7/13/23 13:02, Shuah Khan wrote: > On 7/10/23 08:04, Mark Brown wrote: ... >> -kernel_header_files: >> - @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ >> - if [ $$? -ne 0 ]; then \ >> - RED='\033[1;31m'; \ >> - NOCOLOR='\033[0m'; \ >> - echo; \ >> - echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ >> - echo "Please run this and try again:"; \ >> - echo; \ >> - echo " cd $(top_srcdir)"; \ >> - echo " make headers"; \ >> - echo; \ >> - exit 1; \ >> - fi >> - >> -.PHONY: kernel_header_files >> +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) >> define RUN_TESTS >> BASE_DIR="$(selfdir)"; \ >> >> --- >> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 >> change-id: 20230710-kselftest-fix-arm64-c023160018d7 >> >> Best regards, > > Thank you. Will apply the patch for the next rc > > thanks, > -- Shuah Hi Shuah, Actually, I was hoping that my two fixes [1], [2] could be used, instead of reverting the feature. [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ thanks,
On 7/13/23 14:16, John Hubbard wrote: > On 7/13/23 13:02, Shuah Khan wrote: >> On 7/10/23 08:04, Mark Brown wrote: > ... >>> -kernel_header_files: >>> - @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ >>> - if [ $$? -ne 0 ]; then \ >>> - RED='\033[1;31m'; \ >>> - NOCOLOR='\033[0m'; \ >>> - echo; \ >>> - echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ >>> - echo "Please run this and try again:"; \ >>> - echo; \ >>> - echo " cd $(top_srcdir)"; \ >>> - echo " make headers"; \ >>> - echo; \ >>> - exit 1; \ >>> - fi >>> - >>> -.PHONY: kernel_header_files >>> +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) >>> define RUN_TESTS >>> BASE_DIR="$(selfdir)"; \ >>> >>> --- >>> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 >>> change-id: 20230710-kselftest-fix-arm64-c023160018d7 >>> >>> Best regards, >> >> Thank you. Will apply the patch for the next rc >> >> thanks, >> -- Shuah > > Hi Shuah, > > Actually, I was hoping that my two fixes [1], [2] could be used, instead > of reverting the feature. > > > [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ > > [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ > > Thanks - I will go do that now. Mark! Are you good with taking these two - do these fix the problems you are seeing? thanks, -- Shuah
On Fri, Jul 14, 2023 at 11:48:51AM -0600, Shuah Khan wrote: > On 7/13/23 14:16, John Hubbard wrote: > > Actually, I was hoping that my two fixes [1], [2] could be used, instead > > of reverting the feature. > Mark! Are you good with taking these two - do these fix the > problems you are seeing? I reviewed the one that's relevant to me already, the arm64 one, I'd not seen or tested the RISC-V one but that looks fine too. I'm pretty sure Andrew queued it already though ICBW. Either way it'd be good to get this into -rc2, this is seriously disrupting arm64 CI and I'm guessing the RISC-V CI too.
On 7/14/23 11:09, Mark Brown wrote: > On Fri, Jul 14, 2023 at 11:48:51AM -0600, Shuah Khan wrote: >> On 7/13/23 14:16, John Hubbard wrote: > >>> Actually, I was hoping that my two fixes [1], [2] could be used, instead >>> of reverting the feature. > >> Mark! Are you good with taking these two - do these fix the >> problems you are seeing? > > I reviewed the one that's relevant to me already, the arm64 one, I'd not > seen or tested the RISC-V one but that looks fine too. I'm pretty sure That riscv patch already has a Tested-by from Alexandre Ghiti: https://lore.kernel.org/f903634d-851f-af64-8d9a-6b13d813587c@ghiti.fr > Andrew queued it already though ICBW. Either way it'd be good to get > this into -rc2, this is seriously disrupting arm64 CI and I'm guessing > the RISC-V CI too. thanks,
On Fri, 14 Jul 2023 11:19:11 -0700 John Hubbard <jhubbard@nvidia.com> wrote: > On 7/14/23 11:09, Mark Brown wrote: > > On Fri, Jul 14, 2023 at 11:48:51AM -0600, Shuah Khan wrote: > >> On 7/13/23 14:16, John Hubbard wrote: > > > >>> Actually, I was hoping that my two fixes [1], [2] could be used, instead > >>> of reverting the feature. > > > >> Mark! Are you good with taking these two - do these fix the > >> problems you are seeing? > > > > I reviewed the one that's relevant to me already, the arm64 one, I'd not > > seen or tested the RISC-V one but that looks fine too. I'm pretty sure > > That riscv patch already has a Tested-by from Alexandre Ghiti: > > https://lore.kernel.org/f903634d-851f-af64-8d9a-6b13d813587c@ghiti.fr > > > > Andrew queued it already though ICBW. Either way it'd be good to get > > this into -rc2, this is seriously disrupting arm64 CI and I'm guessing > > the RISC-V CI too. > I just dropped selftests-arm64-fix-build-failure-during-the-emit_tests-step.patch and selftests-fix-arm64-test-installation.patch, as Shuah is merging them. This is all rather confusing. Perhaps a full resend of everything will help. I'll assume that Shuah will be handling them.
On 7/14/23 12:26, Andrew Morton wrote: > On Fri, 14 Jul 2023 11:19:11 -0700 John Hubbard <jhubbard@nvidia.com> wrote: > >> On 7/14/23 11:09, Mark Brown wrote: >>> On Fri, Jul 14, 2023 at 11:48:51AM -0600, Shuah Khan wrote: >>>> On 7/13/23 14:16, John Hubbard wrote: >>> >>>>> Actually, I was hoping that my two fixes [1], [2] could be used, instead >>>>> of reverting the feature. >>> >>>> Mark! Are you good with taking these two - do these fix the >>>> problems you are seeing? >>> >>> I reviewed the one that's relevant to me already, the arm64 one, I'd not >>> seen or tested the RISC-V one but that looks fine too. I'm pretty sure >> >> That riscv patch already has a Tested-by from Alexandre Ghiti: >> >> https://lore.kernel.org/f903634d-851f-af64-8d9a-6b13d813587c@ghiti.fr >> >> >>> Andrew queued it already though ICBW. Either way it'd be good to get >>> this into -rc2, this is seriously disrupting arm64 CI and I'm guessing >>> the RISC-V CI too. >> > > I just dropped > selftests-arm64-fix-build-failure-during-the-emit_tests-step.patch and > selftests-fix-arm64-test-installation.patch, as Shuah is merging them. > > This is all rather confusing. Perhaps a full resend of everything will > help. I'll assume that Shuah will be handling them. Yes. Andrew - I am applying both as we speak. I found the right versions with Tested-by tags. These will appear very soon in linux-kselftest fixes and I will send pull request for rc2. thanks, -- Shuah
On 7/14/23 11:32, Shuah Khan wrote: > On 7/14/23 12:26, Andrew Morton wrote: >> On Fri, 14 Jul 2023 11:19:11 -0700 John Hubbard <jhubbard@nvidia.com> wrote: >> >>> On 7/14/23 11:09, Mark Brown wrote: >>>> On Fri, Jul 14, 2023 at 11:48:51AM -0600, Shuah Khan wrote: >>>>> On 7/13/23 14:16, John Hubbard wrote: >>>> >>>>>> Actually, I was hoping that my two fixes [1], [2] could be used, instead >>>>>> of reverting the feature. >>>> >>>>> Mark! Are you good with taking these two - do these fix the >>>>> problems you are seeing? >>>> >>>> I reviewed the one that's relevant to me already, the arm64 one, I'd not >>>> seen or tested the RISC-V one but that looks fine too. I'm pretty sure >>> >>> That riscv patch already has a Tested-by from Alexandre Ghiti: >>> >>> https://lore.kernel.org/f903634d-851f-af64-8d9a-6b13d813587c@ghiti.fr >>> >>> >>>> Andrew queued it already though ICBW. Either way it'd be good to get >>>> this into -rc2, this is seriously disrupting arm64 CI and I'm guessing >>>> the RISC-V CI too. >>> >> >> I just dropped >> selftests-arm64-fix-build-failure-during-the-emit_tests-step.patch and >> selftests-fix-arm64-test-installation.patch, as Shuah is merging them. >> >> This is all rather confusing. Perhaps a full resend of everything will >> help. I'll assume that Shuah will be handling them. > > Yes. Andrew - I am applying both as we speak. I found the right versions > with Tested-by tags. Thanks, Shuah. Just to be clear, when you say you're applying "both", I'm hoping you mean both of these: [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ ? (As opposed to both of the ones that Andrew lists above.) thanks,
On 7/14/23 12:36, John Hubbard wrote: > On 7/14/23 11:32, Shuah Khan wrote: >> On 7/14/23 12:26, Andrew Morton wrote: >>> On Fri, 14 Jul 2023 11:19:11 -0700 John Hubbard <jhubbard@nvidia.com> wrote: >>> >>>> On 7/14/23 11:09, Mark Brown wrote: >>>>> On Fri, Jul 14, 2023 at 11:48:51AM -0600, Shuah Khan wrote: >>>>>> On 7/13/23 14:16, John Hubbard wrote: >>>>> >>>>>>> Actually, I was hoping that my two fixes [1], [2] could be used, instead >>>>>>> of reverting the feature. >>>>> >>>>>> Mark! Are you good with taking these two - do these fix the >>>>>> problems you are seeing? >>>>> >>>>> I reviewed the one that's relevant to me already, the arm64 one, I'd not >>>>> seen or tested the RISC-V one but that looks fine too. I'm pretty sure >>>> >>>> That riscv patch already has a Tested-by from Alexandre Ghiti: >>>> >>>> https://lore.kernel.org/f903634d-851f-af64-8d9a-6b13d813587c@ghiti.fr >>>> >>>> >>>>> Andrew queued it already though ICBW. Either way it'd be good to get >>>>> this into -rc2, this is seriously disrupting arm64 CI and I'm guessing >>>>> the RISC-V CI too. >>>> >>> >>> I just dropped >>> selftests-arm64-fix-build-failure-during-the-emit_tests-step.patch and >>> selftests-fix-arm64-test-installation.patch, as Shuah is merging them. >>> >>> This is all rather confusing. Perhaps a full resend of everything will >>> help. I'll assume that Shuah will be handling them. >> >> Yes. Andrew - I am applying both as we speak. I found the right versions >> with Tested-by tags. > > Thanks, Shuah. > > Just to be clear, when you say you're applying "both", I'm hoping you mean > both of these: > > > [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ > [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ > Right. The ones you have links to: Please check the latest fixes to see if we are all squared away: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=fixes thanks, -- Shuah
On 7/14/23 12:11, Shuah Khan wrote: ... >> Just to be clear, when you say you're applying "both", I'm hoping you mean >> both of these: >> >> >> [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ >> [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ >> > > Right. The ones you have links to: > > Please check the latest fixes to see if we are all squared away: > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=fixes > Yes, the patches have made it into your tree safely, looks good! thanks,
On Fri, Jul 14, 2023 at 01:11:10PM -0600, Shuah Khan wrote: > On 7/14/23 12:36, John Hubbard wrote: > > Just to be clear, when you say you're applying "both", I'm hoping you mean > > both of these: > > [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ > > [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ > Right. The ones you have links to: > Please check the latest fixes to see if we are all squared away: > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=fixes These didn't seem to make it to -rc2 - it'd be *really* good to get them for -rc3, not having the selftests there would be very disruptive to the standard arm64 workflow.
On 7/18/23 08:54, Mark Brown wrote: > On Fri, Jul 14, 2023 at 01:11:10PM -0600, Shuah Khan wrote: >> On 7/14/23 12:36, John Hubbard wrote: > >>> Just to be clear, when you say you're applying "both", I'm hoping you mean >>> both of these: > >>> [1] https://lore.kernel.org/all/20230711005629.2547838-1-jhubbard@nvidia.com/ >>> [2] https://lore.kernel.org/all/20230712193514.740033-1-jhubbard@nvidia.com/ > >> Right. The ones you have links to: > >> Please check the latest fixes to see if we are all squared away: >> https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=fixes > > These didn't seem to make it to -rc2 - it'd be *really* good to get them > for -rc3, not having the selftests there would be very disruptive to the > standard arm64 workflow. Just about to send the pull request for rc3 thanks, -- Shuah
On Tue, Jul 18, 2023 at 08:56:02AM -0600, Shuah Khan wrote: > On 7/18/23 08:54, Mark Brown wrote: > > These didn't seem to make it to -rc2 - it'd be *really* good to get them > > for -rc3, not having the selftests there would be very disruptive to the > > standard arm64 workflow. > Just about to send the pull request for rc3 Great, thanks!
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 666b56f22a41..405683b8cb39 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -146,12 +146,10 @@ ifneq ($(KBUILD_OUTPUT),) abs_objtree := $(realpath $(abs_objtree)) BUILD := $(abs_objtree)/kselftest KHDR_INCLUDES := -isystem ${abs_objtree}/usr/include - KHDR_DIR := ${abs_objtree}/usr/include else BUILD := $(CURDIR) abs_srctree := $(shell cd $(top_srcdir) && pwd) KHDR_INCLUDES := -isystem ${abs_srctree}/usr/include - KHDR_DIR := ${abs_srctree}/usr/include DEFAULT_INSTALL_HDR_PATH := 1 endif @@ -165,7 +163,7 @@ export KHDR_INCLUDES # all isn't the first target in the file. .DEFAULT_GOAL := all -all: kernel_header_files +all: @ret=1; \ for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ @@ -176,23 +174,6 @@ all: kernel_header_files ret=$$((ret * $$?)); \ done; exit $$ret; -kernel_header_files: - @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ - if [ $$? -ne 0 ]; then \ - RED='\033[1;31m'; \ - NOCOLOR='\033[0m'; \ - echo; \ - echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ - echo "Please run this and try again:"; \ - echo; \ - echo " cd $(top_srcdir)"; \ - echo " make headers"; \ - echo; \ - exit 1; \ - fi - -.PHONY: kernel_header_files - run_tests: all @for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index d17854285f2b..05400462c779 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -44,26 +44,10 @@ endif selfdir = $(realpath $(dir $(filter %/lib.mk,$(MAKEFILE_LIST)))) top_srcdir = $(selfdir)/../../.. -ifeq ("$(origin O)", "command line") - KBUILD_OUTPUT := $(O) +ifeq ($(KHDR_INCLUDES),) +KHDR_INCLUDES := -isystem $(top_srcdir)/usr/include endif -ifneq ($(KBUILD_OUTPUT),) - # Make's built-in functions such as $(abspath ...), $(realpath ...) cannot - # expand a shell special character '~'. We use a somewhat tedious way here. - abs_objtree := $(shell cd $(top_srcdir) && mkdir -p $(KBUILD_OUTPUT) && cd $(KBUILD_OUTPUT) && pwd) - $(if $(abs_objtree),, \ - $(error failed to create output directory "$(KBUILD_OUTPUT)")) - # $(realpath ...) resolves symlinks - abs_objtree := $(realpath $(abs_objtree)) - KHDR_DIR := ${abs_objtree}/usr/include -else - abs_srctree := $(shell cd $(top_srcdir) && pwd) - KHDR_DIR := ${abs_srctree}/usr/include -endif - -KHDR_INCLUDES := -isystem $(KHDR_DIR) - # The following are built by lib.mk common compile rules. # TEST_CUSTOM_PROGS should be used by tests that require # custom build rule and prevent common build rule use. @@ -74,25 +58,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)) -all: kernel_header_files $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) \ - $(TEST_GEN_FILES) - -kernel_header_files: - @ls $(KHDR_DIR)/linux/*.h >/dev/null 2>/dev/null; \ - if [ $$? -ne 0 ]; then \ - RED='\033[1;31m'; \ - NOCOLOR='\033[0m'; \ - echo; \ - echo -e "$${RED}error$${NOCOLOR}: missing kernel header files."; \ - echo "Please run this and try again:"; \ - echo; \ - echo " cd $(top_srcdir)"; \ - echo " make headers"; \ - echo; \ - exit 1; \ - fi - -.PHONY: kernel_header_files +all: $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) define RUN_TESTS BASE_DIR="$(selfdir)"; \
The recent change fc96c7c19df ("selftests: error out if kernel header files are not yet built") to generate an error message when building kselftests without having installed the headers is generating spurious failures during the install step which breaks the arm64 selftests (and only the arm64 selftests): Emit Tests for arm64 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[5]: *** [../../lib.mk:81: kernel_header_files] Error 1 make[4]: *** [Makefile:26: all] Error 2 Presumably the arm64 tests are doing something unusual in their build setup which could be adjusted but I didn't immediately see it and since this is having a serious impact on test coverage in automation let's just revert for now. This is causing failures in KernelCI with the command: make KBUILD_BUILD_USER=KernelCI FORMAT=.xz ARCH=arm64 HOSTCC=gcc CROSS_COMPILE=aarch64-linux-gnu- CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- CC="ccache aarch64-linux-gnu-gcc" O=/tmp/kci/linux/build -C/tmp/kci/linux -j10 kselftest-gen_tar and also when building using tuxmake. Full log: https://storage.kernelci.org/mainline/master/v6.5-rc1/arm64/defconfig/gcc-10/logs/kselftest.log Fixes: 9fc96c7c19df ("selftests: error out if kernel header files are not yet built") Signed-off-by: Mark Brown <broonie@kernel.org> --- tools/testing/selftests/Makefile | 21 +-------------------- tools/testing/selftests/lib.mk | 40 +++------------------------------------- 2 files changed, 4 insertions(+), 57 deletions(-) --- base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 change-id: 20230710-kselftest-fix-arm64-c023160018d7 Best regards,