Re: selftests/vm madv_populate.c test

From: David Hildenbrand
Date: Fri Oct 15 2021 - 12:34:23 EST

Next message: Rob Herring: "Re: [PATCH 0/4] Update mpc5200 dts files to fix warnings"
Previous message: Andrei Vagin: "[PATCH] KVM: x86/mmu: kvm_faultin_pfn has to return false if pfh is returned"
In reply to: David Hildenbrand: "Re: selftests/vm madv_populate.c test"
Next in thread: Shuah Khan: "Re: selftests/vm madv_populate.c test"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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) */

--
Thanks,

David / dhildenb

Next message: Rob Herring: "Re: [PATCH 0/4] Update mpc5200 dts files to fix warnings"
Previous message: Andrei Vagin: "[PATCH] KVM: x86/mmu: kvm_faultin_pfn has to return false if pfh is returned"
In reply to: David Hildenbrand: "Re: selftests/vm madv_populate.c test"
Next in thread: Shuah Khan: "Re: selftests/vm madv_populate.c test"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]