Re: selftests/vm madv_populate.c test

[David Hildenbrand]

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.

-- 
Thanks,

David / dhildenb