Re: [PATCH v4 4/4] mm: add anonymous mTHP folios to the deferred split list

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: try_to_migrate_one() triggers deferred_split_folio() on underused mTHP
--

commit 82b7b7063b986d802fe52e75e17211e58f3f420c
Author: Joanne Koong <joannelkoong@gmail.com>

mm: add anonymous mTHP folios to the deferred split list

This commit queues newly mapped anonymous mTHP folios of at least 2M onto the
deferred split list in map_anon_folio_pte_nopf() so that underused mTHP folios
can be split by the deferred split shrinker under memory pressure.

> diff --git a/mm/memory.c b/mm/memory.c
> index 0c9ed04528d88..44376321180b5 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5477,6 +5477,8 @@ void map_anon_folio_pte_nopf(struct folio *folio, pte_t *pte,
>  	folio_add_lru_vma(folio, vma);
>  	set_ptes(vma->vm_mm, addr, pte, entry, nr_pages);
>  	update_mmu_cache_range(NULL, vma, addr, pte, nr_pages);
> +	if (folio_test_large(folio))
> +		deferred_split_folio(folio, false);
>  }

[Severity: High]
When deferred_split_scan() splits an underused PTE-mapped mTHP folio queued
here with PG_partially_mapped left clear, doesn't unmap_folio() trigger a
false-positive deferred_split_folio(folio, true) transition during the split?

In deferred_split_scan(), the folio is first removed from the local dispose
list while PG_partially_mapped is still false:

mm/huge_memory.c:deferred_split_scan()
	list_del_init(&folio->_deferred_list);

	if (!folio_test_partially_mapped(folio)) {
		if (folio_test_mlocked(folio))
			goto next;
		underused = thp_underused(folio);
		if (!underused)
			goto next;
	}
	if (!folio_trylock(folio))
		goto requeue;
	if (!split_folio(folio)) {

split_folio() then calls __folio_split() -> __folio_freeze_split_anon(), which
unmaps the folio before freezing its refcount:

mm/huge_memory.c:__folio_freeze_split_anon()
	if (folio_mapped(folio)) {
		anon_vma = folio_get_anon_vma(folio);
		if (!anon_vma)
			return -EBUSY;
		anon_vma_lock_write(anon_vma);
		ret = unmap_folio(folio);
		if (ret)
			goto out_unlock;
	}

	local_irq_disable();

	if (!folio_ref_freeze(folio, folio_swapcache_ref_count(folio) + 1)) {
		ret = -EAGAIN;
		goto out_no_split;
	}

Unlike a PMD-mapped THP (where split_huge_pmd_locked() with freeze=true
removes the entire PMD rmap in one folio_remove_rmap_pmd() call), an mTHP
folio is PTE-mapped, so unmap_folio() -> try_to_migrate() ->
try_to_migrate_one() removes its PTEs one subpage at a time via
folio_remove_rmap_pte().

On the first subpage unmapped while the remaining subpages are still mapped,
__folio_remove_rmap() sees partially_mapped == true and
!folio_test_partially_mapped(folio) == true:

mm/rmap.c:__folio_remove_rmap()
	if (partially_mapped && folio_test_anon(folio) &&
	    !folio_test_partially_mapped(folio) &&
	    !folio_is_device_private(folio))
		deferred_split_folio(folio, true);

Because folio->_deferred_list was emptied by list_del_init() in
deferred_split_scan() and the folio refcount is not frozen yet,
deferred_split_folio(folio, true) sets PG_partially_mapped, increments
MTHP_STAT_SPLIT_DEFERRED and MTHP_STAT_NR_ANON_PARTIALLY_MAPPED, and re-adds
folio->_deferred_list onto deferred_split_lru.

Could this cause three problems?

1. Even when folio_ref_freeze() succeeds and __folio_freeze_split_anon()
   subsequently unqueues the folio and resets PG_partially_mapped, every
   underused mTHP split will also increment MTHP_STAT_SPLIT_DEFERRED in
   addition to MTHP_STAT_SPLIT_UNDERUSED.

2. If folio_ref_freeze() fails (for example due to a concurrent GUP pin, or
   because a second CPU running deferred_split_scan() isolated the re-queued
   folio via folio_try_get() while the remaining PTEs were still being
   unmapped), __folio_freeze_split_anon() jumps to out_no_split and remaps all
   PTEs via remap_anon_folio() without unqueueing the folio or clearing
   PG_partially_mapped. That leaves a fully mapped folio permanently marked
   PG_partially_mapped with MTHP_STAT_NR_ANON_PARTIALLY_MAPPED leaked.

3. If a second CPU running deferred_split_scan() isolates the re-queued folio
   onto its local dispose list while unmap_folio() is still in progress, that
   extra refcount causes folio_ref_freeze() on the first CPU to fail. When the
   first CPU returns to deferred_split_scan(), it sees !did_split &&
   folio_test_partially_mapped(folio) == true and calls list_lru_add_irq() on
   folio->_deferred_list while the folio is on the second CPU's unlocked
   dispose list, corrupting the list.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103212004.1356327-1-joannelkoong@gmail.com?part=4