----- On Sep 3, 2015, at 11:36 PM, Michael Ellerman m...@ellerman.id.au wrote:
> On Thu, 2015-09-03 at 15:47 +0000, Mathieu Desnoyers wrote: >> ----- On Sep 3, 2015, at 5:33 AM, Michael Ellerman m...@ellerman.id.au wrote: >> >> > On Tue, 2015-09-01 at 11:32 -0700, Andy Lutomirski wrote: >> >> On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers >> >> <mathieu.desnoy...@efficios.com> wrote: >> >> > Just to make sure I understand: should we expect that >> >> > everyone will issue "make headers_install" on their system >> >> > before doing a make kselftest ? >> >> > >> >> > I see that a few selftests (e.g. memfd) are adding the >> >> > source tree include paths to the compiler include paths, >> >> > which I guess is to ensure that the kselftest will >> >> > work even if the system headers are not up to date. >> >> >> >> It would be really nice if there were a clean way for selftests to >> >> include the kernel headers. >> > >> > What's wrong with make headers_install? >> > >> > Or do you mean when writing the tests? That we could fix by adding the >> > ../../../../usr/include path to CFLAGS in lib.mk. And fixing all the tests >> > that >> > overwrite CFLAGS to append to CFLAGS. >> > >> >> Perhaps make should build the exportable headers somewhere as a >> >> dependency of >> >> kselftests. >> > >> > Yeah the top-level kselftest target could do that I think. >> > >> > Folks who don't want the headers installed can just run the selftests >> > Makefile >> > directly. >> > >> > Does this work for you? >> > >> > diff --git a/Makefile b/Makefile >> > index c361593..c8841d3 100644 >> > --- a/Makefile >> > +++ b/Makefile >> > @@ -1080,7 +1080,7 @@ headers_check: headers_install >> > # Kernel selftest >> > >> > PHONY += kselftest >> > -kselftest: >> > +kselftest: headers_install >> > $(Q)$(MAKE) -C tools/testing/selftests run_tests >> >> My personal experience is that make headers_install does not necessarily play >> well with the distribution header file hierarchy, which requires some tweaks >> to be done by the users (e.g. asm vs x86_64-linux-gnu). > > OK, I've never had issues. What exactly are you doing and how is it going > wrong? After some investigation, I noticed the following: 1) I first ran make headers_install as root, which installed the headers within my build tree. I later tried it again as user, and it failed due to permission issues (my bad). This is where I tried to install it into my system rather than under my build directory, which caused a mess. 2) Since make kselftest should be run as root (according to make help), this means that all the output files generated by the build are owned by root. It leads to permissions issues when trying to rebuild the tests as user afterward. Perhaps we could introduce a distinction between make kselftest_build and make kselftest_run ? The former could be executed as user, and the latter as root. > >> Also, headers_install typically expects a INSTALL_HDR_PATH. > > You can specify it, but the default is just usr/, ie. in the kernel directory, > that is what I was proposing. (Actually it's $(objtree)/usr). OK, trying it out. > >> It would be interesting if we could install the kernel headers into a >> specific location that is then re-used by kselftest, so using it without too >> much manual configuration does not require to overwrite the distribution >> header files to run tests. > > I think we can do that now, ie: > > $ ls /usr/include/linux/membarrier.h > ls: cannot access /usr/include/linux/membarrier.h: No such file or directory > > $ cd linux-next > $ make mrproper > $ make headers_install > ... > $ ls usr/include/linux/membarrier.h > usr/include/linux/membarrier.h > $ make -C tools/testing/selftests TARGETS=membarrier > make: Entering directory > '/home/michael/work/topics/selftests/linux-next/tools/testing/selftests' > for TARGET in membarrier; do \ > make -C $TARGET; \ > done; > make[1]: Entering directory > > '/home/michael/work/topics/selftests/linux-next/tools/testing/selftests/membarrier' > gcc -g -I../../../../usr/include/ membarrier_test.c -o membarrier_test > make[1]: Leaving directory > > '/home/michael/work/topics/selftests/linux-next/tools/testing/selftests/membarrier' > make: Leaving directory > '/home/michael/work/topics/selftests/linux-next/tools/testing/selftests' > > $ ./tools/testing/selftests/membarrier/membarrier_test > membarrier MEMBARRIER_CMD_QUERY failed. Function not implemented. > $ > > > So that seems to be working for me. Are you doing some different work flow, or > am I just missing something? When doing make headers_install, it indeed installs membarrier.h where we expect it under the build output dir: $ ls usr/include/linux/membarrier.h usr/include/linux/membarrier.h However, if I issue $ make -C tools/testing/selftests TARGETS=membarrier make: Entering directory `/home/efficios/git/linux-next/tools/testing/selftests' for TARGET in membarrier; do \ make -C $TARGET; \ done; make[1]: Entering directory `/home/efficios/git/linux-next/tools/testing/selftests/membarrier' gcc membarrier_test.c -o membarrier_test membarrier_test.c:2:30: fatal error: linux/membarrier.h: No such file or directory #include <linux/membarrier.h> This is after applying the modifications you requested (see patch attached). Perhaps I did something wrong ? > > I guess it probably doesn't work if you're using O=.. ? I'm not using anything special here. My src tree is my obj output directory. Thanks, Mathieu > > cheers -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
From 352465bd42737de4b97ce2072f0b36b2499dbb9e Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> Date: Tue, 1 Sep 2015 13:42:52 -0400 Subject: [PATCH] Cleanup: membarrier selftest header inclusion The kselftest can depend on having installed kernel headers. Therefore, there is no need to point to the local kernel source for headers. Signed-off-by: Mathieu Desnoyers <mathieu.desnoy...@efficios.com> CC: Michael Ellerman <m...@ellerman.id.au> CC: Andrew Morton <a...@linux-foundation.org> CC: linux-...@vger.kernel.org CC: Pranith Kumar <bobby.pr...@gmail.com> CC: Shuah Khan <shua...@osg.samsung.com> --- tools/testing/selftests/membarrier/Makefile | 9 +++------ tools/testing/selftests/membarrier/membarrier_test.c | 5 +---- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/membarrier/Makefile b/tools/testing/selftests/membarrier/Makefile index 877a503..0e2a5b7 100644 --- a/tools/testing/selftests/membarrier/Makefile +++ b/tools/testing/selftests/membarrier/Makefile @@ -1,11 +1,8 @@ -CFLAGS += -g -I../../../../usr/include/ - -all: - $(CC) $(CFLAGS) membarrier_test.c -o membarrier_test - TEST_PROGS := membarrier_test +all: $(TEST_PROGS) + include ../lib.mk clean: - $(RM) membarrier_test + $(RM) $(TEST_PROGS) diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index dde3125..535f0fe 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -1,9 +1,6 @@ #define _GNU_SOURCE -#define __EXPORTED_HEADERS__ - #include <linux/membarrier.h> -#include <asm-generic/unistd.h> -#include <sys/syscall.h> +#include <syscall.h> #include <stdio.h> #include <errno.h> #include <string.h> -- 1.9.1