Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1216043 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/3] selftests: add membarrier syscall test Michael Ellerman <mpe@ellerman.id.au> - 2015-08-31 09:00 +0200
    Re: [PATCH 2/3] selftests: add membarrier syscall test Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2015-09-01 19:20 +0200
      Re: [PATCH 2/3] selftests: add membarrier syscall test Andy Lutomirski <luto@amacapital.net> - 2015-09-01 20:40 +0200
        Re: [PATCH 2/3] selftests: add membarrier syscall test Michael Ellerman <mpe@ellerman.id.au> - 2015-09-03 11:40 +0200
          Re: [PATCH 2/3] selftests: add membarrier syscall test Mathieu Desnoyers <mathieu.desnoyers@efficios.com> - 2015-09-03 18:00 +0200
            Re: [PATCH 2/3] selftests: add membarrier syscall test Michael Ellerman <mpe@ellerman.id.au> - 2015-09-04 05:40 +0200
      Re: [PATCH 2/3] selftests: add membarrier syscall test Michael Ellerman <mpe@ellerman.id.au> - 2015-09-03 11:30 +0200

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

On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote:
> From: Pranith Kumar <bobby.prani@gmail.com>
> 
> This patch adds a self test for the membarrier system call.
> 
> CC: Michael Ellerman <mpe@ellerman.id.au>

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 0000000..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 0000000..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 <linux/membarrier.h>
> +#include <asm-generic/unistd.h>

This should just be <unistd.h>


cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1216978

----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe@ellerman.id.au wrote:

> On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote:
>> From: Pranith Kumar <bobby.prani@gmail.com>
>> 
>> This patch adds a self test for the membarrier system call.
>> 
>> CC: Michael Ellerman <mpe@ellerman.id.au>
> 
> 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 0000000..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 0000000..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 <linux/membarrier.h>
>> +#include <asm-generic/unistd.h>
> 
> This should just be <unistd.h>
> 
> 
> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217016

On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> ----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe@ellerman.id.au wrote:
>
>> On Fri, 2015-07-10 at 16:58 -0400, Mathieu Desnoyers wrote:
>>> From: Pranith Kumar <bobby.prani@gmail.com>
>>>
>>> This patch adds a self test for the membarrier system call.
>>>
>>> CC: Michael Ellerman <mpe@ellerman.id.au>
>>
>> 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 0000000..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 0000000..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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218126

On Tue, 2015-09-01 at 11:32 -0700, Andy Lutomirski wrote:
> On Tue, Sep 1, 2015 at 10:11 AM, Mathieu Desnoyers
> <mathieu.desnoyers@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
 
 # ---------------------------------------------------------------------------

cheers


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218385

----- On Sep 3, 2015, at 5:33 AM, Michael Ellerman mpe@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.desnoyers@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).
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218666

On Thu, 2015-09-03 at 15:47 +0000, Mathieu Desnoyers wrote:
> ----- On Sep 3, 2015, at 5:33 AM, Michael Ellerman mpe@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.desnoyers@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?

> 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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1218123

On Tue, 2015-09-01 at 17:11 +0000, Mathieu Desnoyers wrote:
> ----- On Aug 31, 2015, at 2:54 AM, Michael Ellerman mpe@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 0000000..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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web