lkml.org 
[lkml]   [2023]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2023-06-01 10:18    [W:0.066 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site