Re: [PATCH 2/3] selftests: add membarrier syscall test
- On Sep 8, 2015, at 12:19 AM, Michael Ellerman m...@ellerman.id.au wrote: > On Mon, 2015-09-07 at 16:01 +, Mathieu Desnoyers wrote: >> - On Sep 3, 2015, at 11:36 PM, Michael Ellerman m...@ellerman.id.au >> wrote: >> > On Thu, 2015-09-03 at 15:47 +, Mathieu Desnoyers wrote: >> >> >> >> 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. > > Yeah OK that's a good point about root. > > I tend to build as a regular user and then copy the installed tests to another > machine where I run them as root. > >> 2) Since make kselftest should be run as root (according to make >> help), > > Well some of the tests only work when run as root. IMHO we should support > running as many tests as possible as non-root, but some of them obviously > require root. > > So you can run them as non-root, but to get maximum coverage you need to run > them as root. Works for me. We do something similar in lttng-tools. We use "tap" (https://testanything.org/) for tests, and explicitly skip all tests that require root if we detect that we don't run as root. I notice that many selftests format their own output. The nice part about standardizing on something like tap is that it simplifies automated parsing of the test output. > >> 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. > > Right. Personally I don't use the kselftest target at all, I just cd down to > tools/testing/selftests and run make there. > > If it was up to me the kselftest target would go away, because it's only > caused > us trouble so far. > > But given it's there we should try to make it work as well as possible. So > yeah > splitting it into build and run would make sense, that way you could do: > > $ make headers_install > $ make kselftest_build > $ sudo make kselftest_run > > And that would hopefully do the right thing. > > Would that improve the workflow for you? Yes. Although I'm wondering why the kernel should be different from many other projects out there. Why not simply: - Add a kselftest_build dependency to the kernel build, so tests are always built, and warnings that arise from modifying anything related to installed headers will trigger for everyone, - Add a dependency on headers_install into the obj tree to kselftest_build, - Optionally add a "make check" alias to "make kselftest". This way, running the tests becomes as simple as: make sudo make check Documentation is key here: make sure to update Documentation/kselftest.txt to document where the self-tests are looking for their system headers (not system, but within usr/ in the obj tree). This is the missing documentation bit that confused me the most. > >> > 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 >> >> This is after applying the modifications you requested >> (see patch attached). Perhaps I did something wrong ? > > Yeah sorry, you still need the -I line: > > CFLAGS += -I../../../../usr/include/ > > > We /should/ add that to lib.mk so it's inherited by everyone, but we haven't > yet. Yep, this would be a good start. > > So I think if you put that back the instructions I gave you will work? Yes, it does, thanks! Mathieu > > cheers -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-k
Re: [PATCH 2/3] selftests: add membarrier syscall test
On Mon, 2015-09-07 at 16:01 +, Mathieu Desnoyers wrote: > - On Sep 3, 2015, at 11:36 PM, Michael Ellerman m...@ellerman.id.au wrote: > > On Thu, 2015-09-03 at 15:47 +, Mathieu Desnoyers wrote: > >> > >> 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. Yeah OK that's a good point about root. I tend to build as a regular user and then copy the installed tests to another machine where I run them as root. > 2) Since make kselftest should be run as root (according to make > help), Well some of the tests only work when run as root. IMHO we should support running as many tests as possible as non-root, but some of them obviously require root. So you can run them as non-root, but to get maximum coverage you need to run them as root. > 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. Right. Personally I don't use the kselftest target at all, I just cd down to tools/testing/selftests and run make there. If it was up to me the kselftest target would go away, because it's only caused us trouble so far. But given it's there we should try to make it work as well as possible. So yeah splitting it into build and run would make sense, that way you could do: $ make headers_install $ make kselftest_build $ sudo make kselftest_run And that would hopefully do the right thing. Would that improve the workflow for you? > > 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 > > This is after applying the modifications you requested > (see patch attached). Perhaps I did something wrong ? Yeah sorry, you still need the -I line: CFLAGS += -I../../../../usr/include/ We /should/ add that to lib.mk so it's inherited by everyone, but we haven't yet. So I think if you put that back the instructions I gave you will work? cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
- On Sep 3, 2015, at 11:36 PM, Michael Ellerman m...@ellerman.id.au wrote: > On Thu, 2015-09-03 at 15:47 +, 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 >> >> 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; \
Re: [PATCH 2/3] selftests: add membarrier syscall test
On Thu, 2015-09-03 at 15:47 +, 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 > >> 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? > 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). > 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? I guess it probably doesn't work if you're using O=.. ? cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
- 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 >> 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). Also, headers_install typically expects a INSTALL_HDR_PATH. 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. Thoughts ? Thanks, Mathieu > > # --- > > cheers -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
On Tue, 2015-09-01 at 11:32 -0700, Andy Lutomirski wrote: > On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers > 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 # --- cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
On Tue, 2015-09-01 at 17:11 +, Mathieu Desnoyers wrote: > - On Aug 31, 2015, at 2:54 AM, Michael Ellerman m...@ellerman.id.au wrote: > > On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote: > >> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c > >> b/tools/testing/selftests/membarrier/membarrier_test.c > >> new file mode 100644 > >> index 000..3c9f217 > >> --- /dev/null > >> +++ b/tools/testing/selftests/membarrier/membarrier_test.c > >> @@ -0,0 +1,71 @@ > >> +#define _GNU_SOURCE > >> +#define __EXPORTED_HEADERS__ > > > > Why are you exporting that? > > > > I suspect to try and get around the "Attempt to use kernel headers from user > > space" warning. > > > > But you're correctly building against the installed headers, not the kernel > > headers, so you don't need to do that. > > Just to make sure I understand: should we expect that > everyone will issue "make headers_install" on their system > before doing a make kselftest ? Usually yes, but not always. They might be deliberately building the selftests against their installed headers. That's their choice. In a case like this the test will fail to build, which is fine IMHO. > 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. Yeah they should be fixed to not do that. The unexported kernel headers are not designed to be used from userspace. It works sometimes on some arches, depending on the exact headers that get included etc. But it's wrong™. cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers wrote: > - On Aug 31, 2015, at 2:54 AM, Michael Ellerman m...@ellerman.id.au wrote: > >> On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote: >>> From: Pranith Kumar >>> >>> This patch adds a self test for the membarrier system call. >>> >>> CC: Michael Ellerman >> >> Sorry I only just saw this due to some over zealous filtering on my end. >> >> >>> diff --git a/tools/testing/selftests/membarrier/Makefile >>> b/tools/testing/selftests/membarrier/Makefile >>> new file mode 100644 >>> index 000..877a503 >>> --- /dev/null >>> +++ b/tools/testing/selftests/membarrier/Makefile >>> @@ -0,0 +1,11 @@ >>> +CFLAGS += -g -I../../../../usr/include/ >>> + >>> +all: >>> +$(CC) $(CFLAGS) membarrier_test.c -o membarrier_test >>> >>> +TEST_PROGS := membarrier_test >> >> You don't need to specify the rule, the implict one will do exactly the same, >> so you can just do: >> >> TEST_PROGS := membarrier_test >> >> all: $(TEST_PROGS) >> >>> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c >>> b/tools/testing/selftests/membarrier/membarrier_test.c >>> new file mode 100644 >>> index 000..3c9f217 >>> --- /dev/null >>> +++ b/tools/testing/selftests/membarrier/membarrier_test.c >>> @@ -0,0 +1,71 @@ >>> +#define _GNU_SOURCE >>> +#define __EXPORTED_HEADERS__ >> >> Why are you exporting that? >> >> I suspect to try and get around the "Attempt to use kernel headers from user >> space" warning. >> >> But you're correctly building against the installed headers, not the kernel >> headers, so you don't need to do that. > > 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. Perhaps make should build the exportable headers somewhere as a dependency of kselftests. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
- On Aug 31, 2015, at 2:54 AM, Michael Ellerman m...@ellerman.id.au wrote: > On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote: >> From: Pranith Kumar >> >> This patch adds a self test for the membarrier system call. >> >> CC: Michael Ellerman > > Sorry I only just saw this due to some over zealous filtering on my end. > > >> diff --git a/tools/testing/selftests/membarrier/Makefile >> b/tools/testing/selftests/membarrier/Makefile >> new file mode 100644 >> index 000..877a503 >> --- /dev/null >> +++ b/tools/testing/selftests/membarrier/Makefile >> @@ -0,0 +1,11 @@ >> +CFLAGS += -g -I../../../../usr/include/ >> + >> +all: >> +$(CC) $(CFLAGS) membarrier_test.c -o membarrier_test >> >> +TEST_PROGS := membarrier_test > > You don't need to specify the rule, the implict one will do exactly the same, > so you can just do: > > TEST_PROGS := membarrier_test > > all: $(TEST_PROGS) > >> diff --git a/tools/testing/selftests/membarrier/membarrier_test.c >> b/tools/testing/selftests/membarrier/membarrier_test.c >> new file mode 100644 >> index 000..3c9f217 >> --- /dev/null >> +++ b/tools/testing/selftests/membarrier/membarrier_test.c >> @@ -0,0 +1,71 @@ >> +#define _GNU_SOURCE >> +#define __EXPORTED_HEADERS__ > > Why are you exporting that? > > I suspect to try and get around the "Attempt to use kernel headers from user > space" warning. > > But you're correctly building against the installed headers, not the kernel > headers, so you don't need to do that. 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. Thanks, Mathieu > >> + >> +#include >> +#include > > This should just be > > > cheers -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] selftests: add membarrier syscall test
On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote: > From: Pranith Kumar > > This patch adds a self test for the membarrier system call. > > CC: Michael Ellerman Sorry I only just saw this due to some over zealous filtering on my end. > diff --git a/tools/testing/selftests/membarrier/Makefile > b/tools/testing/selftests/membarrier/Makefile > new file mode 100644 > index 000..877a503 > --- /dev/null > +++ b/tools/testing/selftests/membarrier/Makefile > @@ -0,0 +1,11 @@ > +CFLAGS += -g -I../../../../usr/include/ > + > +all: > + $(CC) $(CFLAGS) membarrier_test.c -o membarrier_test > > +TEST_PROGS := membarrier_test You don't need to specify the rule, the implict one will do exactly the same, so you can just do: TEST_PROGS := membarrier_test all: $(TEST_PROGS) > diff --git a/tools/testing/selftests/membarrier/membarrier_test.c > b/tools/testing/selftests/membarrier/membarrier_test.c > new file mode 100644 > index 000..3c9f217 > --- /dev/null > +++ b/tools/testing/selftests/membarrier/membarrier_test.c > @@ -0,0 +1,71 @@ > +#define _GNU_SOURCE > +#define __EXPORTED_HEADERS__ Why are you exporting that? I suspect to try and get around the "Attempt to use kernel headers from user space" warning. But you're correctly building against the installed headers, not the kernel headers, so you don't need to do that. > + > +#include > +#include This should just be cheers -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/