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