Re: [PATCH 4/4] selftests/mm: add a THP compaction selftest

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] selftests/mm: unwritten last_page freed via zero page on
  split in main()
- [Low] selftests/mm: missing MADV_NOHUGEPAGE before split_huge_pages()
  in main()
--

commit 8fc76b939a1f641b9678f19aaded037c1f80ae5b
Author: Sarthak Sharma <sarthak.sharma@arm.com>

selftests/mm: add a THP compaction selftest

This commit adds a selftest that allocates PMD-sized THPs, splits them to
order 0 while retaining one base page per range, and verifies that a
subsequent mapping can recover THPs via memory compaction.

> diff --git a/tools/testing/selftests/mm/thp-compaction.c b/tools/testing/selftests/mm/thp-compaction.c
> new file mode 100644
> index 0000000000000..8935f1968a6da
> --- /dev/null
> +++ b/tools/testing/selftests/mm/thp-compaction.c
> @@ -0,0 +1,215 @@

[ ... ]

> +int main(void)
> +{

[ ... ]

> +	/*
> +	 * Split all allocated THPs to order 0, keeping only the final base page
> +	 * from each THP
> +	 */
> +	for (size_t i = 0; i < nr_initial; i++) {
> +		p = initial + i * pmd_size;
> +		last_page = p + pmd_size - psize();
> +
> +		if (!is_backed_by_folio(p, pmd_order, pagemap_fd, kpageflags_fd)) {
> +			ksft_test_result_skip(
> +				"Initial range is not backed by a resident PMD sized folio\n");
> +			ksft_finished();
> +		}
> +
> +		initial_pfns[i] = pagemap_get_pfn(pagemap_fd, p);
> +
> +		split_huge_pages(p, psize(), 0);
> +		if (!is_backed_by_folio(last_page, 0, pagemap_fd, kpageflags_fd)) {
> +			ksft_test_result_fail("Failed to split THP to order 0\n");
> +			ksft_finished();
> +		}
> +		if (madvise(p, pmd_size - psize(), MADV_DONTNEED))
> +			ksft_exit_fail_perror("MADV_DONTNEED");
> +	}

[Severity: Medium]
Does split_huge_pages() replace last_page with the shared zero page and
free its physical page?

allocate_transhuge() only writes to the first word at p, leaving last_page
zero-filled.  When splitting an anonymous THP to order 0,
__folio_freeze_split_anon() passes TTU_USE_SHARED_ZEROPAGE to
remap_anon_folio(), which remaps zero-filled tail pages to the shared zero
page and frees their physical folios.  madvise(MADV_DONTNEED) then frees
subpage 0, leaving the entire PMD range free in the buddy allocator rather
than fragmented.  Because the shared zero page is present in pagemap and
has no compound flags in kpageflags, both is_backed_by_folio() and
PAGEMAP_PRESENT() succeed, so the retry phase allocates from free buddy
pages without exercising compaction.

Writing non-zero data to last_page before split_huge_pages() will keep the
physical tail page mapped.

[Severity: Low]
Could khugepaged collapse ranges in initial back into THPs during the split
and retry loops?

allocate_transhuge() calls madvise(MADV_HUGEPAGE) on each range in initial,
registering the VMA with khugepaged.  Without clearing VM_HUGEPAGE before
splitting, khugepaged can collapse p before is_backed_by_folio() checks the
order-0 split, or collapse the single-page-retained ranges (which have
HPAGE_PMD_NR - 1 pte_none entries, matching the default max_ptes_none) and
consume compacted THPs meant for the retry mapping.  Calling
madvise(MADV_NOHUGEPAGE) on initial before the split loop would avoid this.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112090250.334344-1-sarthak.sharma@arm.com?part=4