Messages in this thread | | | Date | Thu, 1 Jun 2023 10:16:41 +0200 | From | David Hildenbrand <> | Subject | Re: [PATCH v1 2/3] selftests/mm: gup_longterm: new functional test for FOLL_LONGTERM |
| |
On 28.05.23 17:03, Lorenzo Stoakes wrote: > On Fri, May 19, 2023 at 12:27:22PM +0200, David Hildenbrand wrote: >> Let's add a new test for checking whether GUP long-term page pinning >> works as expected (R/O vs. R/W, MAP_PRIVATE vs. MAP_SHARED, GUP vs. >> GUP-fast). Note that COW handling with long-term R/O pinning in private >> mappings, and pinning of anonymous memory in general, is tested by the >> COW selftest. This test, therefore, focuses on page pinning in >> file mappings. >> >> The most interesting case is probably the "local tmpfile" case, as that >> will likely end up on a "real" filesystem such as ext4 or xfs, not on a >> virtual one like tmpfs or hugetlb where any long-term page pinning is >> always expected to succeed. >> >> For now, only add tests that use the "/sys/kernel/debug/gup_test" >> interface. We'll add tests based on liburing separately next. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> ---
[...]
>> +static void do_test(int fd, size_t size, enum test_type type, bool shared) >> +{ >> + __fsword_t fs_type = get_fs_type(fd); >> + bool should_work; >> + char *mem; >> + int ret; >> + >> + if (ftruncate(fd, size)) { >> + ksft_test_result_fail("ftruncate() failed\n"); >> + return; >> + } >> + >> + if (fallocate(fd, 0, 0, size)) { >> + if (size == pagesize) >> + ksft_test_result_fail("fallocate() failed\n"); >> + else >> + ksft_test_result_skip("need more free huge pages\n"); >> + return; >> + } >> + >> + mem = mmap(NULL, size, PROT_READ | PROT_WRITE, >> + shared ? MAP_SHARED : MAP_PRIVATE, fd, 0); >> + if (mem == MAP_FAILED) { >> + if (size == pagesize || shared) >> + ksft_test_result_fail("mmap() failed\n"); >> + else >> + ksft_test_result_skip("need more free huge pages\n"); >> + return; >> + } >> + >> + /* >> + * Fault in the page writable such that GUP-fast can eventually pin >> + * it immediately. >> + */ >> + memset(mem, 0, size); >
For shared mappings, MAP_POPULATE will not fault-in the pages writable. See mm/gup.c:populate_vma_page_range().
[There is also the case that mmap() doesn't fail if populate fails, but that's only a side note regarding weird semantics of MAP_POPULATE]
[...]
>> + int flags = MFD_HUGETLB; >> + int fd; >> + >> + ksft_print_msg("[RUN] %s ... with memfd hugetlb (%zu kB)\n", desc, >> + hugetlbsize / 1024); >> + >> + flags |= __builtin_ctzll(hugetlbsize) << MFD_HUGE_SHIFT; > > Hm this feels a little cute :)
It's a weird interfacing, having to specify the desired size via flags ... see the man page of memfd_create, which links to the man page of mmap: "the desired huge page size can be configured by encoding the base-2 logarithm of the desired page size in the six bits at the offset MAP_HUGE_SHIFT".
FWIW, we're using the same approach in cow.c already [and other memfd users like QEMU do it just like that, using ctz].
[...]
>> diff --git a/tools/testing/selftests/mm/run_vmtests.sh b/tools/testing/selftests/mm/run_vmtests.sh >> index 4893eb60d96d..b6b1eb6a8a6b 100644 >> --- a/tools/testing/selftests/mm/run_vmtests.sh >> +++ b/tools/testing/selftests/mm/run_vmtests.sh >> @@ -24,7 +24,7 @@ separated by spaces: >> - mmap >> tests for mmap(2) >> - gup_test >> - tests for gup using gup_test interface >> + tests for gup > > Super nitty again, but I'm guessing this means the CONFIG_GUP_TEST > interface, perhaps worth keeping?
With this patch, agreed. But not longer with the next patch -- guess I simplified when splitting it up. If there are no strong feelings I'll leave it in this patch.
[...]
>> > > OK this patch is really nice + well implemented, I can only point out a > couple EXTREMELY nitty comments :) Thanks very much for adding a test for > this, it's super useful! > > Therefore, > > Reviewed-by: Lorenzo Stoakes <lstoakes@gmail.com> >
Thanks for the review! My selftest patches rarely get that much attention, so highly appreciated :)
-- Thanks,
David / dhildenb
| |