Re: [PATCH v9 32/32] selftests/mm: add PMD swap entry tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] selftests/mm: check_huge_anon() in swapoff test passes on
  swapped PMDs
- [Medium] selftests/mm: uffdio_move test fails UFFDIO_API when RWP is
  unsupported
--

commit 19ef341b36a063e4b63f435180149741746c0988
Author: Usama Arif <usama.arif@linux.dev>

selftests/mm: add PMD swap entry tests

This commit adds a selftest suite for PMD-level swap entries covering basic
fault-in, swapin_sync, fork, COW, mprotect, munmap, userfaultfd move/RWP,
mremap, pagemap, mincore, madvise, and swapoff operations. It also adds a
read_vmstat() helper to vm_util.c and wires the test into run_vmtests.sh.

> diff --git a/tools/testing/selftests/mm/pmd_swap.c b/tools/testing/selftests/mm/pmd_swap.c
> new file mode 100644
> index 0000000000000..8af94bd7e105d
> --- /dev/null
> +++ b/tools/testing/selftests/mm/pmd_swap.c
> @@ -0,0 +1,989 @@

[ ... ]

> +TEST_F(pmd_swap, uffdio_move)
> +{
> +	struct uffdio_register reg = {};
> +	struct uffdio_move move = {};
> +	struct uffdio_api api = {};
> +	bool rwp;
> +
> +	self->aux = mmap_pmd_aligned(self->pmd_size);
> +	if (self->aux == MAP_FAILED)
> +		SKIP(return, "Could not mmap aligned dst\n");
> +	ASSERT_EQ(madvise(self->aux, self->pmd_size, MADV_HUGEPAGE), 0);
> +
> +	self->uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> +	if (self->uffd < 0)
> +		SKIP(return, "userfaultfd unavailable\n");
> +
> +	api.api = UFFD_API;
> +	api.features = UFFD_FEATURE_MOVE | UFFD_FEATURE_RWP;
> +	if (ioctl(self->uffd, UFFDIO_API, &api) ||
> +	    !(api.features & UFFD_FEATURE_MOVE))
> +		SKIP(return, "UFFD_FEATURE_MOVE unsupported\n");
> +	rwp = api.features & UFFD_FEATURE_RWP;

[Severity: Medium]
In TEST_F(pmd_swap, uffdio_move) in tools/testing/selftests/mm/pmd_swap.c,
will requesting UFFD_FEATURE_MOVE | UFFD_FEATURE_RWP in a single UFFDIO_API
ioctl cause the test to skip whenever UFFD_FEATURE_RWP is unsupported?

In mm/userfaultfd.c, userfaultfd_api() rejects the call with -EINVAL if any
requested feature bit is unavailable rather than masking off unsupported bits:

mm/userfaultfd.c:userfaultfd_api() {
	...
	uffdio_api.features = uffd_api_available_features(ctx, features);
	ret = -EINVAL;
	if (features & ~uffdio_api.features)
		goto err_out;
	...
}

On architectures or configurations where UFFD_FEATURE_MOVE is supported but
UFFD_FEATURE_RWP is cleared by uffd_api_available_features() (because
VM_UFFD_RWP == VM_NONE || !pgtable_supports_uffd()), ioctl(self->uffd,
UFFDIO_API, &api) fails with -EINVAL, skipping the test with "UFFD_FEATURE_MOVE
unsupported" and leaving the !rwp fallback path unreachable.

[ ... ]

> +TEST_F(pmd_swap, swapoff)
> +{
> +	int ret, err;
> +
> +	self->swap_dev = getenv("PMD_SWAP_DEVICE");
> +	if (!self->swap_dev)
> +		SKIP(return, "PMD_SWAP_DEVICE env var not set\n");
> +	/*
> +	 * Otherwise a higher-priority device may have taken the PMD swap
> +	 * entry and swapoff() would operate on the wrong backend.
> +	 */
> +	if (!swap_device_is_only_active(self->swap_dev, &self->swap_prio))
> +		SKIP(return, "PMD_SWAP_DEVICE must be the only active swap device\n");
> +
> +	self->uffd = register_rwp(self->mem, self->pmd_size, true);
> +
> +	ret = swapoff(self->swap_dev);
> +	err = errno;
> +	if (!ret)
> +		self->swap_disabled = true;
> +	ASSERT_EQ(ret, 0) {
> +		TH_LOG("swapoff(%s) failed: %s", self->swap_dev, strerror(err));
> +	}
> +
> +	/*
> +	 * Check residency before touching the memory. If we read
> +	 * first, a bug that left a PMD swap entry in place after swapoff
> +	 * would silently trigger do_huge_pmd_swap_page() and reinstall a
> +	 * PMD mapping, masking the regression.
> +	 */
> +	if (self->zswap_enabled)
> +		ksft_print_msg("zswap enabled: PMD restoration not checked\n");
> +	else
> +		ASSERT_TRUE(check_huge_anon(self->mem, self->pmd_size, 1,
> +					    self->pmd_size));

[Severity: Medium]
Does check_huge_anon() in TEST_F(pmd_swap, swapoff) in
tools/testing/selftests/mm/pmd_swap.c actually verify that the PMD swap entry
is no longer swapped out before the test reads self->mem?

In tools/testing/selftests/mm/vm_util.c, check_huge_anon() calls
__check_huge() followed by __check_type():

tools/testing/selftests/mm/vm_util.c:__check_huge() {
	...
	for (; start < end; start += hpage_size) {
		categories = pagemap_scan_get_categories(pagemap_fd, start);
		if (categories & PAGE_IS_HUGE)
			nr_pmd_mappings++;
	}
	...
}

tools/testing/selftests/mm/vm_util.c:__check_type() {
	...
	for (; start < end; start += page_size) {
		categories = pagemap_scan_get_categories(pagemap_fd, start);
		if ((categories & PAGE_IS_PRESENT) != PAGE_IS_PRESENT)
			continue;

		if ((type == CHECK_TYPE_FILE) != !!(categories & PAGE_IS_FILE))
			goto out;
	}

	ret = true;
	...
}

In fs/proc/task_mmu.c, pagemap_scan_thp_entry()->pagemap_thp_category() sets
PAGE_IS_HUGE | PAGE_IS_SWAPPED on a non-present PMD swap entry. Because
__check_huge() only tests PAGE_IS_HUGE without PAGE_IS_PRESENT, and
__check_type() skips non-present entries via continue, check_huge_anon()
returns true even if swapoff() left the non-present PMD swap entry in place.

Additionally, when self->zswap_enabled is true, no residency or swap check is
performed at all before expect_rwp_fault() and verify_pattern() read
self->mem. Would checking that none of the pages in the range remain swapped
in pagemap (or checking residency directly) before reading self->mem catch a
left-behind swap entry in both cases?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105183940.2236717-1-usama.arif@linux.dev?part=32