Messages in this thread Patch in this message | | | Subject | Re: selftests/vm madv_populate.c test | From | Shuah Khan <> | Date | Fri, 15 Oct 2021 10:40:48 -0600 |
| |
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
| |