2020-03-13 22:14:56

by Bird, Tim

[permalink] [raw]
Subject: cross-compiling vm, confusion over output directory

I was working on fixing the cross-compilation for the selftests/vm tests.
Currently, there are two issues in my testing:

1) problem: required library missing from some cross-compile environments:
tools/testing/selftests/vm/mlock-random-test.c requires libcap
be installed. The target line for mlock-random-test in
tools/testing/selftests/vm/Makefile looks like this:

$(OUTPUT)/mlock-random-test: LDLIBS += -lcap

and mlock-random-test.c has this include line:
#include <sys/capability.h>

this is confusing, since this is different from the header file
linux/capability.h. It is associated with the capability library (libcap)
and not the kernel. In any event, on some distros and in some
cross-compile SDKs the package containing these files is not installed
by default.

Once this library is installed, things progress farther. Using an Ubuntu
system, you can install the cross version of this library (for arm64) by doing:
$ sudo apt install libcap-dev:arm64

1) solution:
I would like to add some meta-data about this build dependency, by putting
something in the settings file as a hint to CI build systems. Specifically, I'd like to
create the file 'tools/testing/selftests/vm/settings', with the content:
NEED_LIB=cap

We already use settings for other meta-data about a test (right now, just a
non-default timeout value), but I don't want to create a new file or syntax
for this build dependency data.

Let me know what you think.

I may follow up with some script in the kernel source tree to check these
dependencies, independent of any CI system. I have such a script in Fuego
that I could submit, but it would need some work to fit into the kernel build
flow for kselftest. The goal would be to provide a nicely formatted warning,
with a recommendation for a package install. But that's more work than
I think is needed right now just to let developers know there's a build dependency
here.

2) problem: reference to source-relative header file
the Makefile for vm uses a relative path for include directories.
Specifically, it has the line:
CFLAGS = -Wall -I ../../../../../usr/include $(EXTRA_CFLAGS)

I believe this needs to reference kernel include files from the
output directory, not the source directory.

With the relative include directory path, the program userfaultfd.c
gets compilation error like this:

userfaultfd.c:267:21: error: 'UFFD_API_RANGE_IOCTLS_BASIC' undeclared here (not in a function)
.expected_ioctls = UFFD_API_RANGE_IOCTLS_BASIC,
^
userfaultfd.c: In function 'uffd_poll_thread':
userfaultfd.c:529:8: error: 'UFFD_EVENT_FORK' undeclared (first use in this function)
case UFFD_EVENT_FORK:
^
userfaultfd.c:529:8: note: each undeclared identifier is reported only once for each function it appears in
userfaultfd.c:531:18: error: 'union <anonymous>' has no member named 'fork'
uffd = msg.arg.fork.ufd;
^

2) incomplete solution:
I originally changed this line to read:
CFLAGS = -Wall -I $(KBUILD_OUTPUT)/usr/include $(EXTRA_CFLAGS)

This works when the output directory is specified using KBUILD_OUTPUT,
but not when the output directory is specified using O=
I'm not sure what happens when the output directory is specified
with a non-source-tree current working directory.

In any event, while researching a proper solution to this, I found
the following in tools/testing/selftests/Makefile:

If compiling with ifneq ($(O),)
BUILD := $(O)
else
ifneq ($(KBUILD_OUTPUT),)
BUILD := $(KBUILD_OUTPUT)/kselftest
else
BUILD := $(shell pwd)
DEFAULT_INSTALL_HDR_PATH := 1
endif
endif

This doesn't seem right. It looks like the selftests Makefile treats a directory
passed in using O= different from one specified using KBUILD_OUTPUT
or the current working directory.
In the KBUILD_OUTPUT case, you get an extra 'kselftest' directory layer
that you don't get for the other two.

In contrast, the kernel top-level Makefile has this:
ifeq ("$(origin O)", "command line")
KBUILD_OUTPUT := $(O)
endif
(and from then on, the top-level Makefile appears to only use KBUILD_OUTPUT)

This makes it look like the rest of the kernel build system treats O= and KBUILD_OUTPUT
identically.

Am I missing something, or is there a flaw in the O=/KBUILD_OUTPUT handling in
kselftest? Please let me know and I'll try to work out an appropriate fix for
cross-compiling the vm tests.
-- Tim


2020-03-13 22:31:04

by Shuah Khan

[permalink] [raw]
Subject: Re: cross-compiling vm, confusion over output directory

Hi Tim,

You are using an ancient email address for me.

Use either [email protected] or [email protected]

On 3/13/20 4:14 PM, Bird, Tim wrote:
> I was working on fixing the cross-compilation for the selftests/vm tests.
> Currently, there are two issues in my testing:
>
> 1) problem: required library missing from some cross-compile environments:
> tools/testing/selftests/vm/mlock-random-test.c requires libcap
> be installed. The target line for mlock-random-test in
> tools/testing/selftests/vm/Makefile looks like this:
>
> $(OUTPUT)/mlock-random-test: LDLIBS += -lcap

You can use TEST_GEN_PROGS and define LDFLAGS for lcap

>
> and mlock-random-test.c has this include line:
> #include <sys/capability.h>
>
> this is confusing, since this is different from the header file
> linux/capability.h. It is associated with the capability library (libcap)
> and not the kernel. In any event, on some distros and in some
> cross-compile SDKs the package containing these files is not installed
> by default.
>
> Once this library is installed, things progress farther. Using an Ubuntu
> system, you can install the cross version of this library (for arm64) by doing:
> $ sudo apt install libcap-dev:arm64
>
> 1) solution:
> I would like to add some meta-data about this build dependency, by putting
> something in the settings file as a hint to CI build systems. Specifically, I'd like to
> create the file 'tools/testing/selftests/vm/settings', with the content:
> NEED_LIB=cap
>
> We already use settings for other meta-data about a test (right now, just a
> non-default timeout value), but I don't want to create a new file or syntax
> for this build dependency data.
>
> Let me know what you think.
>
> I may follow up with some script in the kernel source tree to check these
> dependencies, independent of any CI system. I have such a script in Fuego
> that I could submit, but it would need some work to fit into the kernel build
> flow for kselftest. The goal would be to provide a nicely formatted warning,
> with a recommendation for a package install. But that's more work than
> I think is needed right now just to let developers know there's a build dependency
> here.
>
> 2) problem: reference to source-relative header file
> the Makefile for vm uses a relative path for include directories.
> Specifically, it has the line:
> CFLAGS = -Wall -I ../../../../../usr/include $(EXTRA_CFLAGS)
>
> I believe this needs to reference kernel include files from the
> output directory, not the source directory.
>
> With the relative include directory path, the program userfaultfd.c
> gets compilation error like this:
>
> userfaultfd.c:267:21: error: 'UFFD_API_RANGE_IOCTLS_BASIC' undeclared here (not in a function)
> .expected_ioctls = UFFD_API_RANGE_IOCTLS_BASIC,
> ^
> userfaultfd.c: In function 'uffd_poll_thread':
> userfaultfd.c:529:8: error: 'UFFD_EVENT_FORK' undeclared (first use in this function)
> case UFFD_EVENT_FORK:
> ^
> userfaultfd.c:529:8: note: each undeclared identifier is reported only once for each function it appears in
> userfaultfd.c:531:18: error: 'union <anonymous>' has no member named 'fork'
> uffd = msg.arg.fork.ufd;
> ^
>
> 2) incomplete solution:
> I originally changed this line to read:
> CFLAGS = -Wall -I $(KBUILD_OUTPUT)/usr/include $(EXTRA_CFLAGS)
>

Take a look at seccomp patch I sent out. Use the approach used
in this patch.

https://lore.kernel.org/linux-kselftest/[email protected]/T/#u

> This works when the output directory is specified using KBUILD_OUTPUT,
> but not when the output directory is specified using O=
> I'm not sure what happens when the output directory is specified
> with a non-source-tree current working directory.
>
> In any event, while researching a proper solution to this, I found
> the following in tools/testing/selftests/Makefile:
>
> If compiling with ifneq ($(O),)
> BUILD := $(O)
> else
> ifneq ($(KBUILD_OUTPUT),)
> BUILD := $(KBUILD_OUTPUT)/kselftest
> else
> BUILD := $(shell pwd)
> DEFAULT_INSTALL_HDR_PATH := 1
> endif
> endif
>
> This doesn't seem right. It looks like the selftests Makefile treats a directory
> passed in using O= different from one specified using KBUILD_OUTPUT
> or the current working directory.
> In the KBUILD_OUTPUT case, you get an extra 'kselftest' directory layer
> that you don't get for the other two.

Are you using

git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git
kernelci branch

This is fixed and that is the very first patch I sent.

I also applied the patch to

git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
branch

thanks,
-- Shuah

2020-03-17 00:28:32

by Bird, Tim

[permalink] [raw]
Subject: RE: cross-compiling vm, confusion over output directory

> -----Original Message-----
> From: shuah <[email protected]>
>
> Hi Tim,
>
> You are using an ancient email address for me.
>
> Use either [email protected] or [email protected]
Sorry about that. I have now fixed my contact info for you
in my email client.

>
> On 3/13/20 4:14 PM, Bird, Tim wrote:
> > I was working on fixing the cross-compilation for the selftests/vm tests.
> > Currently, there are two issues in my testing:
> >
> > 1) problem: required library missing from some cross-compile environments:
> > tools/testing/selftests/vm/mlock-random-test.c requires libcap
> > be installed. The target line for mlock-random-test in
> > tools/testing/selftests/vm/Makefile looks like this:
> >
> > $(OUTPUT)/mlock-random-test: LDLIBS += -lcap
>
> You can use TEST_GEN_PROGS and define LDFLAGS for lcap
The example you have for seccomp uses a single LDFLAGS before
the include of ../lib.mk.

The vm/Makefile has different libraries needed for different programs:
userfaultfd need libpthread
mlock-random-test needs libcap
(something) needs librt

Do you want me to add all these to a single LDFLAGS line, like so?
LDFLAGS= -lrt -lpthread -lcap

It won't hurt anything, but it seems like overkill.
>
> >
> > and mlock-random-test.c has this include line:
> > #include <sys/capability.h>
> >
> > this is confusing, since this is different from the header file
> > linux/capability.h. It is associated with the capability library (libcap)
> > and not the kernel. In any event, on some distros and in some
> > cross-compile SDKs the package containing these files is not installed
> > by default.
> >
> > Once this library is installed, things progress farther. Using an Ubuntu
> > system, you can install the cross version of this library (for arm64) by doing:
> > $ sudo apt install libcap-dev:arm64
> >
> > 1) solution:
> > I would like to add some meta-data about this build dependency, by putting
> > something in the settings file as a hint to CI build systems. Specifically, I'd like to
> > create the file 'tools/testing/selftests/vm/settings', with the content:
> > NEED_LIB=cap
> >
> > We already use settings for other meta-data about a test (right now, just a
> > non-default timeout value), but I don't want to create a new file or syntax
> > for this build dependency data.
> >
> > Let me know what you think.
> >
> > I may follow up with some script in the kernel source tree to check these
> > dependencies, independent of any CI system. I have such a script in Fuego
> > that I could submit, but it would need some work to fit into the kernel build
> > flow for kselftest. The goal would be to provide a nicely formatted warning,
> > with a recommendation for a package install. But that's more work than
> > I think is needed right now just to let developers know there's a build dependency
> > here.
> >
> > 2) problem: reference to source-relative header file
> > the Makefile for vm uses a relative path for include directories.
> > Specifically, it has the line:
> > CFLAGS = -Wall -I ../../../../../usr/include $(EXTRA_CFLAGS)
> >
> > I believe this needs to reference kernel include files from the
> > output directory, not the source directory.
> >
> > With the relative include directory path, the program userfaultfd.c
> > gets compilation error like this:
> >
> > userfaultfd.c:267:21: error: 'UFFD_API_RANGE_IOCTLS_BASIC' undeclared here (not in a function)
> > .expected_ioctls = UFFD_API_RANGE_IOCTLS_BASIC,
> > ^
> > userfaultfd.c: In function 'uffd_poll_thread':
> > userfaultfd.c:529:8: error: 'UFFD_EVENT_FORK' undeclared (first use in this function)
> > case UFFD_EVENT_FORK:
> > ^
> > userfaultfd.c:529:8: note: each undeclared identifier is reported only once for each function it appears in
> > userfaultfd.c:531:18: error: 'union <anonymous>' has no member named 'fork'
> > uffd = msg.arg.fork.ufd;
> > ^
> >
> > 2) incomplete solution:
> > I originally changed this line to read:
> > CFLAGS = -Wall -I $(KBUILD_OUTPUT)/usr/include $(EXTRA_CFLAGS)
> >
>
> Take a look at seccomp patch I sent out. Use the approach used
> in this patch.
>
> https://lore.kernel.org/linux-kselftest/[email protected]/T/#u

I took a look at this, and if I'm reading it correctly, the CFLAGS needs to be
set before 'include ../lib.mk'

But it can't be, because the CFLAGS needs OUTPUT to be set, and OUTPUT
is set by lib.mk.

I'm going to play around with this here and see if I can come up with something
that works.
>
> > This works when the output directory is specified using KBUILD_OUTPUT,
> > but not when the output directory is specified using O=
> > I'm not sure what happens when the output directory is specified
> > with a non-source-tree current working directory.
> >
> > In any event, while researching a proper solution to this, I found
> > the following in tools/testing/selftests/Makefile:
> >
> > If compiling with ifneq ($(O),)
> > BUILD := $(O)
> > else
> > ifneq ($(KBUILD_OUTPUT),)
> > BUILD := $(KBUILD_OUTPUT)/kselftest
> > else
> > BUILD := $(shell pwd)
> > DEFAULT_INSTALL_HDR_PATH := 1
> > endif
> > endif
> >
> > This doesn't seem right. It looks like the selftests Makefile treats a directory
> > passed in using O= different from one specified using KBUILD_OUTPUT
> > or the current working directory.
> > In the KBUILD_OUTPUT case, you get an extra 'kselftest' directory layer
> > that you don't get for the other two.
>
> Are you using
>
> git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git
> kernelci branch

I wasn't, but I am now.
>
> This is fixed and that is the very first patch I sent.
>
> I also applied the patch to
>
> git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
> branch

I'm now using linux-next master and the linux-kselftest kernelci branch
for my references.

With both of these I'm getting an error with scripts/Makefile.build. (see below)

KBUILD_OUTPUT is set to
/fuego-rw/buildzone/ren1.kernelci_one.Functional.kselftest_compile_install-poky-aarch64/kbuild
(this is a directory 'kbuild' inside the kernel source tree)

The command is:
make V=1 ARCH=arm64 TARGETS="vm" kselftest-install
Here is the make execution output:
------
make -C /fuego-rw/buildzone/ren1.next_one.Functional.kselftest_compile_install-poky-aarch64/kbuild -f /fuego-rw/buildzone/ren1.next_one.Functional.kselftest_compile_install-poky-aarch64/Makefile kselftest-install
make[1]: Entering directory '/fuego-rw/buildzone/ren1.next_one.Functional.kselftest_compile_install-poky-aarch64/kbuild'
test -e include/generated/autoconf.h -a -e include/config/auto.conf || ( \
echo >&2; \
echo >&2 " ERROR: Kernel configuration is invalid."; \
echo >&2 " include/generated/autoconf.h or include/config/auto.conf are missing.";\
echo >&2 " Run 'make oldconfig && make prepare' on kernel src to fix it."; \
echo >&2 ; \
/bin/false)
make -C ../tools/testing/selftests install
make --no-builtin-rules INSTALL_HDR_PATH=$BUILD/usr \
ARCH=arm64 -C ../../.. headers_install
make -f ../scripts/Makefile.build obj=scripts/basic
make[4]: ../scripts/Makefile.build: No such file or directory
make[4]: *** No rule to make target '../scripts/Makefile.build'. Stop.
Makefile:504: recipe for target 'scripts_basic' failed
make[3]: *** [scripts_basic] Error 2
Makefile:151: recipe for target 'khdr' failed
make[2]: *** [khdr] Error 2
/fuego-rw/buildzone/ren1.next_one.Functional.kselftest_compile_install-poky-aarch64/Makefile:1228: recipe for target 'kselftest-install' failed
make[1]: *** [kselftest-install] Error 2
make[1]: Leaving directory '/fuego-rw/buildzone/ren1.next_one.Functional.kselftest_compile_install-poky-aarch64/kbuild'
Makefile:180: recipe for target 'sub-make' failed
make: *** [sub-make] Error 2-----

There's still something wonky with using KBUILD_OUTPUT
with kselftest (at least with my configuration).

Note that I switched over to using the top-level Makefile target 'kselftest-install'.

I am investigating and will report back when I figure out what the issue is.

Thanks,
-- Tim