Re: selftests/vm madv_populate.c test

From: Shuah Khan
Date: Fri Oct 15 2021 - 12:40:52 EST


On 10/15/21 10:34 AM, David Hildenbrand wrote:
On 15.10.21 18:28, David Hildenbrand wrote:
On 15.10.21 18:25, Shuah Khan wrote:
On 10/15/21 10:19 AM, David Hildenbrand wrote:
On 15.10.21 18:15, David Hildenbrand wrote:
On 15.10.21 18:06, David Hildenbrand wrote:
On 15.10.21 17:47, David Hildenbrand wrote:
On 15.10.21 17:45, Shuah Khan wrote:
On 9/18/21 1:41 AM, David Hildenbrand wrote:
On 18.09.21 00:45, Shuah Khan wrote:
Hi David,

I am running into the following warning when try to build this test:

madv_populate.c:334:2: warning: #warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition" [-Wcpp]
    334 | #warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"
        |  ^~~~~~~


I see that the following handling is in place. However there is no
other information to explain why the check is necessary.

#if defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE)

#else /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */

#warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"

I do see these defined in:

include/uapi/asm-generic/mman-common.h:#define MADV_POPULATE_READ       22
include/uapi/asm-generic/mman-common.h:#define MADV_POPULATE_WRITE      23

Is this the case of missing include from madv_populate.c?

Hi Shuan,

note that we're including "#include <sys/mman.h>", which in my
understanding maps to the version installed on your system instead
of the one in our build environment.ing.

So as soon as you have a proper kernel + the proper headers installed
and try to build, it would pick up MADV_POPULATE_READ and
MADV_POPULATE_WRITE from the updated headers. That makes sense: you
annot run any MADV_POPULATE_READ/MADV_POPULATE_WRITE tests on a kernel
that doesn't support it.

See vm/userfaultfd.c where we do something similar.


Kselftest is for testing the kernel with kernel headers. That is the
reason why there is the dependency on header install.


As soon as we have a proper environment, it seems to work just fine:

Linux vm-0 5.15.0-0.rc1.20210915git3ca706c189db.13.fc36.x86_64 #1 SMP Thu Sep 16 11:32:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
[root@vm-0 linux]# cat /etc/redhat-release
Fedora release 36 (Rawhide)

This is a distro release. We don't want to have dependency on headers
from the distro to run selftests. Hope this makes sense.

I still see this on my test system running Linux 5.15-rc5.

Did you also install Linux headers? I assume no, correct?


What happens in your environment when compiling and running the
memfd_secret test?

If assume you'll see a "skip" when executing, because it might also
refer to the local version of linux headers and although it builds, it
really cannot build something "functional". It just doesn't add a
"#warning" to make that obvious.


The following works but looks extremely hackish.

diff --git a/tools/testing/selftests/vm/madv_populate.c
b/tools/testing/selftests/vm/madv_populate.c
index b959e4ebdad4..ab26163db540 100644
--- a/tools/testing/selftests/vm/madv_populate.c
+++ b/tools/testing/selftests/vm/madv_populate.c
@@ -14,12 +14,11 @@
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
+#include "../../../../usr/include/linux/mman.h"
#include <sys/mman.h>

#include "../kselftest.h"

-#if defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE)
-
/*
* For now, we're using 2 MiB of private anonymous memory for all tests.
*/
@@ -328,15 +327,3 @@ int main(int argc, char **argv)
err, ksft_test_num());
return ksft_exit_pass();
}
-
-#else /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */
-
-#warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"
-
-int main(int argc, char **argv)
-{
- ksft_print_header();
- ksft_exit_skip("MADV_POPULATE_READ or MADV_POPULATE_WRITE not
defined\n");
-}
-
-#endif /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */


There has to be some clean way to achieve the same.


Sorry for the spam,

diff --git a/tools/testing/selftests/vm/Makefile
b/tools/testing/selftests/vm/Makefile
index d9605bd10f2d..ce198b329ff5 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -23,7 +23,7 @@ MACHINE ?= $(shell echo $(uname_M) | sed -e
's/aarch64.*/arm64/' -e 's/ppc64.*/p
# LDLIBS.
MAKEFLAGS += --no-builtin-rules

-CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS)
+CFLAGS = -Wall -idirafter ../../../../usr/include $(EXTRA_CFLAGS)
LDLIBS = -lrt -lpthread
TEST_GEN_FILES = compaction_test
TEST_GEN_FILES += gup_test


Seems to set the right include path priority.



Yes. It works on linux-next-20211012

Do you mind sending a me patch for this?

I just double-checked (after make clean) and there is still something
wrong :( the only think that seems to work is the

+#include "../../../../usr/include/linux/mman.h"
#include <sys/mman.h>

hack.

Using "-nostdinc" won't work because we need other headers :(


And ... I think I know the problem.

In ../../../../usr/include, there is no "sys" directory. It's called
"linux".

But including <linux/mman.h> instead of <sys/mman.h> doesn't work
either. The only thing that seems to work is


diff --git a/tools/testing/selftests/vm/madv_populate.c
b/tools/testing/selftests/vm/madv_populate.c
index b959e4ebdad4..3ee0e8275600 100644
--- a/tools/testing/selftests/vm/madv_populate.c
+++ b/tools/testing/selftests/vm/madv_populate.c
@@ -14,12 +14,11 @@
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
+#include <linux/mman.h>
#include <sys/mman.h>

#include "../kselftest.h"

-#if defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE)
-
/*
* For now, we're using 2 MiB of private anonymous memory for all tests.
*/
@@ -328,15 +327,3 @@ int main(int argc, char **argv)
err, ksft_test_num());
return ksft_exit_pass();
}
-
-#else /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */
-
-#warning "missing MADV_POPULATE_READ or MADV_POPULATE_WRITE definition"
-
-int main(int argc, char **argv)
-{
- ksft_print_header();
- ksft_exit_skip("MADV_POPULATE_READ or MADV_POPULATE_WRITE not
defined\n");
-}
-
-#endif /* defined(MADV_POPULATE_READ) && defined(MADV_POPULATE_WRITE) */



I tried with just the following and it worked after kselftest-clean
as well.

diff --git a/tools/testing/selftests/vm/madv_populate.c b/tools/testing/selftests/vm/madv_populate.c
index b959e4ebdad4..f9e4b8e1b28c 100644
--- a/tools/testing/selftests/vm/madv_populate.c
+++ b/tools/testing/selftests/vm/madv_populate.c
@@ -14,6 +14,7 @@
#include <unistd.h>
#include <errno.h>
#include <fcntl.h>
+#include <linux/mman.h>
#include <sys/mman.h>
#include "../kselftest.h"

thanks,
-- Shuah