Re: [PATCH v1 2/3] selftests/mm: gup_longterm: new functional test for FOLL_LONGTERM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---

[...]

+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@xxxxxxxxx>


Thanks for the review! My selftest patches rarely get that much attention, so highly appreciated :)

--
Thanks,

David / dhildenb




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux