Re: [PATCH v9 17/32] mm: swap in PMD swap entries as whole THPs during swapoff

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Critical] mm/swapfile: race between unuse_pmd_entry() and
  do_swap_page() corrupts rmap
--

commit 2a3a8bc71439dccac7a6dee3671d26f7c3dba876
Author: Usama Arif <usama.arif@linux.dev>

mm: swap in PMD swap entries as whole THPs during swapoff

This commit adds swap_pmd_cache_lookup(), unuse_pmd(), and unuse_pmd_entry()
to swap in PMD swap entries as whole THPs during swapoff when possible,
falling back to splitting the PMD into PTEs otherwise.

> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 6b5149b88fa43..7e91dc50a5131 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c

[ ... ]

> @@ -2695,6 +2696,182 @@ static int unuse_pte_range(struct vm_area_struct *vma, pmd_t *pmd,

[ ... ]

> +static int unuse_pmd_entry(struct vm_area_struct *vma, pmd_t *pmd,
> +			   unsigned long addr, softleaf_t entry)
> +{
> +	enum swap_pmd_cache cache_state;
> +	struct folio *folio;
> +	pmd_t pmdval;
> +	int ret;
> +
> +	cache_state = swap_pmd_cache_lookup(entry, &folio);
> +	if (cache_state == SWAP_PMD_CACHE_SPLIT) {
> +		ret = -EAGAIN;
> +		goto split_fallback;
> +	}
> +	if (!folio) {
> +		struct vm_fault vmf = {
> +			.vma = vma,
> +			.address = addr,
> +			.real_address = addr,
> +			.pmd = pmd,
> +		};
> +
> +		if (zswap_is_present(entry, HPAGE_PMD_NR)) {
> +			ret = -EAGAIN;
> +			goto split_fallback;
> +		}
> +
> +		folio = swapin_sync(entry, GFP_HIGHUSER_MOVABLE,
> +				    BIT(HPAGE_PMD_ORDER), &vmf, NULL, 0);
> +		if (IS_ERR_OR_NULL(folio)) {
> +			ret = folio ? PTR_ERR(folio) : -ENOMEM;
> +			goto split_fallback;
> +		}
> +	}
> +
> +	folio_lock(folio);
> +	folio_wait_writeback(folio);
> +	/*
> +	 * If the cached folio is no longer PMD-sized (e.g. split in the
> +	 * swap cache by deferred_split_scan() or memory_failure() while
> +	 * the PMD swap entry was installed), the PMD swap entry no longer
> +	 * maps a single contiguous folio.  Split the PMD swap entry so
> +	 * unuse_pte_range() can swap the per-slot folios in individually.
> +	 */
> +	if (folio_nr_pages(folio) != HPAGE_PMD_NR) {
> +		folio_unlock(folio);
> +		folio_put(folio);
> +		ret = -EAGAIN;
> +		goto split_fallback;
> +	}
> +	ret = unuse_pmd(vma, pmd, addr, entry, folio);
> +	/*
> +	 * PTE fallback cannot add a single-page rmap to a newly allocated,
> +	 * PMD-sized !anon folio, and it cannot do anything useful with a folio
> +	 * that failed to read. Remove either from the swap cache so each slot
> +	 * is read back into an order-0 folio. An uptodate anon swap-cache folio
> +	 * can be mapped one PTE at a time and must stay cached so poisoned
> +	 * subpages remain visible.
> +	 */
> +	if (ret && folio_matches_swap_entry(folio, entry) &&
> +	    (!folio_test_uptodate(folio) || !folio_test_anon(folio)))
> +		swap_cache_del_folio(folio);

[Severity: Critical]
Can a concurrent page fault in do_swap_page() race with unuse_pmd_entry() and
map this newly allocated PMD-sized !anon folio via PTEs before
swap_cache_del_folio() runs, corrupting the folio's rmap state?

In mm/swapfile.c, try_to_unuse() -> unuse_mm() -> unuse_pmd_range() ->
unuse_pmd_entry() calls swapin_sync() with BIT(HPAGE_PMD_ORDER) without
holding pmd_lock (and with only mmap_read_lock(mm) held). swapin_sync()
allocates an order HPAGE_PMD_ORDER folio with !folio_test_anon(folio) and
folio_test_pmd_mappable(folio) == true, inserts it into the swap cache
covering [entry, entry + HPAGE_PMD_NR), and initiates swap_read_folio().

When the swap read completes, the folio is unlocked before unuse_pmd_entry()
acquires folio_lock(folio). During that window, a concurrent page fault on
split PTEs covering the same swap range (either in the same mm after a
concurrent PMD split such as madvise or mprotect, or in a forked mm whose PMD
swap entry was split into HPAGE_PMD_NR contiguous PTE swap entries) can enter
do_swap_page(), find the folio via swap_cache_get_folio(entry), and lock it
first.

In mm/memory.c:do_swap_page(), when all HPAGE_PMD_NR PTEs in the split page
table are still contiguous swap entries, folio_pte_range() and
swap_pte_batch() compute nr_pages == HPAGE_PMD_NR == folio_nr_pages(folio), so
the size mismatch check does not trigger:

	if (!folio_test_anon(folio) && folio_test_large(folio) &&
	    nr_pages != folio_nr_pages(folio)) {
		if (!WARN_ON_ONCE(folio_test_dirty(folio)))
			swap_cache_del_folio(folio);
		goto out_nomap;
	}

do_swap_page() then calls folio_add_new_anon_rmap(folio, vma, address,
rmap_flags) and maps the folio across HPAGE_PMD_NR PTEs with set_ptes().
However, mm/rmap.c:folio_add_new_anon_rmap() assumes any
folio_test_pmd_mappable(folio) folio is being mapped at the PMD level:

	} else if (!folio_test_pmd_mappable(folio)) {
		...
	} else {
		nr = folio_large_nr_pages(folio);
		/* increment count (starts at -1) */
		atomic_set(&folio->_entire_mapcount, 0);
		folio_set_large_mapcount(folio, 1, vma);
		if (IS_ENABLED(CONFIG_PAGE_MAPCOUNT))
			atomic_set(&folio->_nr_pages_mapped, ENTIRELY_MAPPED);
		if (exclusive)
			SetPageAnonExclusive(&folio->page);
		nr_pmdmapped = nr;
	}

This initializes PMD-mapped rmap accounting (_entire_mapcount = 0,
_large_mapcount = 1, per-page _mapcount left at -1, and PageAnonExclusive set
only on page 0) on a folio that is actually mapped by HPAGE_PMD_NR PTEs, which
will corrupt mapcounts and trigger BUG checks when those PTEs are later
unmapped via folio_remove_rmap_ptes().

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