Re: [PATCH 2/3] selftests: add membarrier syscall test

----- On Sep 8, 2015, at 12:19 AM, Michael Ellerman mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org wrote:

On Mon, 2015-09-07 at 16:01 +0000, Mathieu Desnoyers wrote:

quoted

----- On Sep 3, 2015, at 11:36 PM, Michael Ellerman mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org wrote:

quoted

On Thu, 2015-09-03 at 15:47 +0000, Mathieu Desnoyers wrote:

quoted

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.

quoted

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.

quoted

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.

quoted

quoted

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 ?

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