Re: [PATCH v4] fs/proc/task_mmu: fix PAGEMAP_SCAN written state for PMD holes

["David Hildenbrand (Arm)" <david@kernel.org>]

On 7/13/26 11:17, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> 

Reading this, some of the details how this fits together are missing. You
capture some of that in the comment.

> PAGEMAP_SCAN reports an unpopulated PTE in a uffd-wp VMA as written, but
> a range with no page table at all -- a PMD hole -- is skipped:
> pagemap_scan_pte_hole() tests p->cur_vma_category, which never carries
> PAGE_IS_WRITTEN, so the hole is neither reported nor (under
> PM_SCAN_WP_MATCHING) armed.

Okay, the reason is that UFFD_FEATURE_WP_UNPOPULATED will make use of uffd
markers when protecting a range.

Seeing that marker gone translates to "MADV_DONTNEED was used". At least on
anonymous memory, looking at zap_install_uffd_wp_if_needed().

> 
> MADV_DONTNEED has fill-with-zeros semantics: it changes the contents of
> the range to zeroes (a subsequent read maps the zero page), which write

Only in MAP_PRIVATE | MAP_ANON mappings.

For e.g., MAP_PRIVATE file/shmem it will fallback to the original pagecache page
and there are no such guarantees.

> tracking must report as written. An anonymous THP is write-protected in
> place as a huge PMD, so a full-PMD MADV_DONTNEED clears it to pmd_none --
> a hole -- and the zeroing goes unreported. A write-tracking
> checkpoint/migration tool (e.g. CRIU) then treats the range as unchanged
> and keeps its previous contents, so after restore or live migration the
> process reads stale data instead of zeroes -- data corruption.

With UFFD_FEATURE_WP_UNPOPULATED, uffd-write-protecting a range without a PMD
table will end up allocating a page table (pgtable_populate_needed) that will be
filled with uffd-wp markers.

So what needs to happen is getting a THP collapsed there, to then zap the THP.

Or, of course, zapping a THP that was uffd-wp'ed. (which is what your test case
does IIUC)

> 
> Report a hole in a non-hugetlb uffd-wp VMA as written, matching the
> pte_none handling in pagemap_page_category(); the existing
> PM_SCAN_WP_MATCHING path then arms it via uffd_wp_range().

About which memory backing are walking about? Anon? Shmem? Something else?

> 
> hugetlb is excluded: pagemap_hugetlb_category() reports an empty hugetlb
> entry (huge_pte_none) as not-written, unlike pagemap_page_category(),
> which reports pte_none as written. pagemap_scan_pte_hole() fires for a
> hugetlb slot only when it has no page table; keeping that not-written
> matches how an allocated-but-empty hugetlb entry reads, so the hole and
> the empty-entry cases agree within the VMA.
> 
> Add a pagemap_ioctl selftest covering the anon-THP PMD-hole case.
> 


Do we really want to backport a test case? Usually we split them from the actual
fix.

> Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260707151349.92143-1-kirill@shutemov.name
> Fixes: 2bad466cc9d9 ("mm/uffd: UFFD_FEATURE_WP_UNPOPULATED")
> Cc: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
> Assisted-by: Claude:claude-fable-5
> ---
> 
> Changes since v3 [1]:
>   - Include <linux/mman.h> for MADV_COLLAPSE; <sys/mman.h> lacks it on
>     older glibc (e.g. 2.34), breaking the selftest build. Same approach
>     as fd5295afae91 ("selftests/mm: hmm-tests: include linux/mman.h to
>     access MADV_COLLAPSE"). Reported by Zenghui Yu.
> 
> [1] https://lore.kernel.org/all/20260709121629.205562-1-kirill@shutemov.name/
>  fs/proc/task_mmu.c                         | 27 +++++++++-
>  tools/testing/selftests/mm/pagemap_ioctl.c | 57 +++++++++++++++++++++-
>  2 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index d45c729ab6bb..03ead4184546 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -3049,12 +3049,35 @@ static int pagemap_scan_pte_hole(unsigned long addr, unsigned long end,
>  {
>  	struct pagemap_scan_private *p = walk->private;
>  	struct vm_area_struct *vma = walk->vma;
> +	unsigned long categories;
>  	int ret, err;
>  
> -	if (!vma || !pagemap_scan_is_interesting_page(p->cur_vma_category, p))
> +	if (!vma)
>  		return 0;
>  
> -	ret = pagemap_scan_output(p->cur_vma_category, p, addr, &end);
> +	/*
> +	 * An unpopulated range with no page table -- e.g. a 2MB anon THP
> +	 * dropped via MADV_DONTNEED, which pagemap_page_category() never sees
> +	 * -- reads as written on a uffd-wp VMA, matching the pte_none case
> +	 * there. Reporting it also lets the PM_SCAN_WP_MATCHING arming below
> +	 * install markers (uffd_wp_range() allocates the page table under
> +	 * WP_UNPOPULATED), so the next scan sees it clean until re-written.
> +	 *
> +	 * hugetlb is excluded: pagemap_hugetlb_category() reports an empty
> +	 * hugetlb entry (huge_pte_none) as not-written, unlike
> +	 * pagemap_page_category(), which reports pte_none as written. This
> +	 * path fires for a hugetlb slot only when it has no page table;
> +	 * keeping that not-written matches how an allocated-but-empty
> +	 * hugetlb entry reads, so the two agree within the VMA.

Can that all be shortened?

"In a uffd-wp VMA, any unpopulated range is treated as written, as uffd-wp
registration populates page tables and installs markers with WP_UNPOPULATED. See
pte_none() handling in pagemap_page_category().

hugetlb handling differs, see pagemap_hugetlb_category().
"

> +	 */
> +	categories = p->cur_vma_category;
> +	if (userfaultfd_wp(vma) && !is_vm_hugetlb_page(vma))
> +		categories |= PAGE_IS_WRITTEN;


[...]

> +/*
> + * A 2MB anon THP dropped with MADV_DONTNEED leaves a pmd_none hole with no
> + * page table, which pagemap_page_category() never sees. PAGEMAP_SCAN must
> + * still report it as written on a uffd-wp VMA, via pagemap_scan_pte_hole().
> + */
> +static void unpopulated_thp_hole_test(void)
> +{
> +	long npages, written = 0, ret, i;
> +	struct page_region regions[16];
> +	char *area, *mem;
> +
> +	if (!hpage_size) {
> +		ksft_test_result_skip("%s THP not supported\n", __func__);
> +		return;
> +	}
> +	npages = hpage_size / page_size;
> +
> +	/* Get a PMD-aligned range so the range can be a single THP. */
> +	area = mmap(NULL, 2 * hpage_size, PROT_READ | PROT_WRITE,
> +		    MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +	if (area == MAP_FAILED)
> +		ksft_exit_fail_msg("%s mmap failed\n", __func__);

Why exit the test? unpopulated_scan_test() seems to do that, but that is also
rather suboptimal. We can easily recover and continue executing tests.


> +	mem = (char *)(((unsigned long)area + hpage_size - 1) & ~(hpage_size - 1));
> +
> +	memset(mem, 1, hpage_size);
> +	if (madvise(mem, hpage_size, MADV_COLLAPSE) ||
> +	    !check_huge_anon(mem, 1, hpage_size)) {
> +		ksft_test_result_skip("%s could not form a THP\n", __func__);
> +		munmap(area, 2 * hpage_size);
> +		return;
> +	}
> +
> +	wp_init(mem, hpage_size);
> +
> +	/* Drop the whole PMD: it is cleared to a pmd_none hole. */
> +	if (madvise(mem, hpage_size, MADV_DONTNEED))
> +		ksft_exit_fail_msg("%s MADV_DONTNEED failed\n", __func__);
> +
> +	ret = pagemap_ioctl(mem, hpage_size, regions, 16, 0, 0,
> +			    PAGE_IS_WRITTEN, 0, 0, PAGE_IS_WRITTEN);
> +	if (ret < 0)
> +		ksft_exit_fail_msg("%s scan failed\n", __func__);
> +	for (i = 0; i < ret; i++)
> +		written += LEN(regions[i]);
> +
> +	ksft_test_result(written == npages,
> +			 "%s pmd-hole reported written (%ld of %ld)\n",
> +			 __func__, written, npages);
> +
> +	wp_free(mem, hpage_size);
> +	munmap(area, 2 * hpage_size);

There is quite some overlap with unpopulated_scan_test. Primarily the THP
allocation differs.

Couldn't we make the sequence similar by

(1) mmap
(2) wp_init(mem, mem_size); -> Populates page table
(3) memset(mem, 1, hpage_size); -> Allocates all entries
(4) MADV_COLLAPSE -> Get a THP
(5) MADV_DONTNEED -> Drop the THP

So couldn't we reuse most of unpopulated_scan_test in a reworked way?

> +}
> +
>  int sanity_tests(void)
>  {
>  	unsigned long long mem_size, vec_size;
> @@ -1610,7 +1664,7 @@ int main(int __attribute__((unused)) argc, char *argv[])
>  	if (!hugetlb_setup_default(4))
>  		ksft_print_msg("HugeTLB test will be skipped\n");
>  
> -	ksft_set_plan(118);
> +	ksft_set_plan(119);
>  
>  	page_size = getpagesize();
>  	hpage_size = read_pmd_pagesize();
> @@ -1790,6 +1844,7 @@ int main(int __attribute__((unused)) argc, char *argv[])
>  
>  	/* 18. Unpopulated pte scan-path consistency */
>  	unpopulated_scan_test();
> +	unpopulated_thp_hole_test();

Any reason this is not a unpopulated_thp_scan_test ?

-- 
Cheers,

David