[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4341bedc-81b9-b8ed-fcaa-b282c976d203@linuxfoundation.org>
Date: Fri, 15 Oct 2021 10:40:48 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: David Hildenbrand <david@...hat.com>
Cc: linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org,
Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: selftests/vm madv_populate.c test
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@...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
Powered by blists - more mailing lists