On 7/13/26 11:17, Kiryl Shutsemau wrote: > From: "Kiryl Shutsemau (Meta)" <[email protected]> >
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 <[email protected]> > Closes: > https://sashiko.dev/#/patchset/[email protected] > Fixes: 2bad466cc9d9 ("mm/uffd: UFFD_FEATURE_WP_UNPOPULATED") > Cc: Muhammad Usama Anjum <[email protected]> > Cc: Peter Xu <[email protected]> > Cc: [email protected] > Signed-off-by: Kiryl Shutsemau <[email protected]> > 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/[email protected]/ > 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
