I included it in tools/testing/selftests/mm/madv_populate.c as well BUT that was
not sufficient to make it compile in all environments.
Hi, David,
Hi,
It's unobvious why, for (as you can witness from the source)
#include <linux/mman.h> includes
#include <asm/mman.h> includes
#include <asm-generic/mman.h> which in turn includes
#include <asm-generic/mman-common.h>
that defines all of the
#define MADV_PAGEOUT 21 /* reclaim these pages */
#define MADV_POPULATE_READ 22 /* populate (prefault) page tables readable */
#define MADV_POPULATE_WRITE 23 /* populate (prefault) page tables writable */
So, everything is within the in-tree includes, so I can't tell what went wrong.
I see that "#include <linux/magic.h>" is already used.
BTW, did userfaultfd.c compile on that Debian 11.5 environment?
I don't recall, unfortunately. I think yes, everything else compiled,
but I might be wrong, as I focused on running some specific tests on
sparc64 (below).
Maybe __NR_userfaultfd was missing completely and simply turned
userfaultfd.c into a simple "skip".
commit d88825f22b8f1cad8acb429b6bf0a54c85d7b93f
Author: David Hildenbrand <david@xxxxxxxxxx>
Date: Mon Dec 5 20:37:14 2022 +0100
selftests/vm: madv_populate: fix missing MADV_POPULATE_(READ|WRITE) definitions
The tests fail to compile in some environments (e.g., Debian 11.5 on x86).
On Debian 11 on x86_64 I had no issues (just checked). (Your fix applied.)
Only MADV_PAGEOUT is defined in /usr/include:
mtodorov@magrf:~$ grep -E '(MADV_PAGEOUT|MADV_POPULATE_READ|MADV_POPULATE_WRITE)' -r /usr/include
/usr/include/x86_64-linux-gnu/bits/mman-linux.h:# define MADV_PAGEOUT 21 /* Reclaim these pages. */
/usr/include/asm-generic/mman-common.h:#define MADV_PAGEOUT 21 /* reclaim these pages */
mtodorov@magrf:~$
However, "memfd-secret.c" and "mlock-random-test.c" fail to include <sys/capability.h>
despite being in both /usr/include/linux/capability.h and ../../../../usr/include/sys/capability.h:
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$ make -k
gcc -Wall -I /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../.. -I
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -isystem
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -no-pie memfd_secret.c -lrt
-lpthread -lcap -o /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/vm/memfd_secret
memfd_secret.c:16:10: fatal error: sys/capability.h: No such file or directory
16 | #include <sys/capability.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [../lib.mk:145: /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/vm/migration] Error 1
gcc -Wall -I /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../.. -I
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -isystem
/home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/../../../usr/include -no-pie mlock-random-test.c -lrt
-lpthread -lcap -o /home/admin/mtodorov/linux/kernel/linux_torvalds/tools/testing/selftests/vm/mlock-random-test
mlock-random-test.c:8:10: fatal error: sys/capability.h: No such file or directory
8 | #include <sys/capability.h>
| ^~~~~~~~~~~~~~~~~~
compilation terminated.
usr/include/linux/capability.h is existing, but the compiler somehow fails to find it:
And I think that's the root cause.
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$ grep sys/capability.h *.c
memfd_secret.c:#include <sys/capability.h>
mlock-random-test.c:#include <sys/capability.h>
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$ find /usr/include -name capability.h
/usr/include/linux/capability.h
mtodorov@magrf:~/linux/kernel/linux_torvalds/tools/testing/selftests/vm$
Let's simply conditionally define MADV_POPULATE_(READ|WRITE) if not
already defined, similar to how the khugepaged.c test handles it.
Link: https://lkml.kernel.org/r/20221205193716.276024-3-david@xxxxxxxxxx
Fixes: 39b2e5cae43d ("selftests/vm: make MADV_POPULATE_(READ|WRITE) use in-tree headers")
Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
Cc: Shuah Khan <shuah@xxxxxxxxxx>
Cc: Yang Li <yang.lee@xxxxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
IMHO, I don't have your experience and knowledge, but from the fresh perspective,
kernel will bring in its own selftest suite all the defines in <linux/mman.h>
that are supported in the particular version.
AFAIK, testing with different version of kernel is not recommended.
I recall that the following was my test setup: debian 11 is the last
debian version one can possibly (with quite some effort) get running on
sparc64 in QEMU. I installed a custom kernel to test some sparc64
changes and also ran some selftests (including e.g., softdirty,
madv_populate and cow). So I was running a kernel that matched the
source tree, just the installed header files were outdated.
Of course, we can easily test with "gcc -E" what exactly gets included.
We don't seem to have a uniform way of handling MADV_PAGEOUT and
MADV_POPULATE(READ|WRITE) even within different C sources within
tools/testing/selftests/vm itself, so it means that we spread the spectrum
of systems and compilations that we will break. :-/
To repeat what is obvious, I am not an authority on the Linux kernel,
but as I said, I have the benefit of a fresh perspective ...
There was an open source proverb that "more eyes see more" :)
The thing is: I don't lose sleeps about defines that will never change
as long as it compiles. That's the most important part (for me) with
these tests. Even if it would change, and I don't see how, we'd get test
failures and could fix it up.
... and I raised that I ran into issues with what you propose. That's
really all I am saying :)
And to repeat what I said: If we can find a cleanup for all these
relevant "ifndef" in selftests/mm I'd be very happy. As long as it
compiles I'm all for cleanups. It would also be acceptable to say "what
David tested was wrong" -- but that needs clear justification.
--
Thanks,
David / dhildenb
