Re: [PATCH 6.6] mm: replace free hugepage folios after migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] mm: missing replace_free_hugepage_folios() breaks
  alloc_contig_range()
- [High] mm: hugetlb: skipping isolated folios in dequeue breaks resv
  accounting
--

commit bc6da6c8bf105f6ed6558b1313cb2fed9d8b04a8
Author: yangge <yangge1116@126.com>

mm: replace free hugepage folios after migration

This backport updates dequeue_hugetlb_folio_node_exact() in mm/hugetlb.c to
skip free hugetlb folios located in MIGRATE_ISOLATE pageblocks so migration
does not reuse isolated hugepages. However, it omits the
replace_free_hugepage_folios() implementation and call site from the upstream
commit while retaining the upstream commit title and changelog.

Link: https://lkml.kernel.org/r/1734503588-16254-1-git-send-email-yangge1116@126.com
Link: https://lkml.kernel.org/r/1736582300-11364-1-git-send-email-yangge1116@126.com

[Severity: High]
In mm/hugetlb.c at dequeue_hugetlb_folio_node_exact() (line 1348), this
backport only takes the check for is_migrate_isolate_page(&folio->page) and
omits replace_free_hugepage_folios() along with its call after
__alloc_contig_migrate_range() in alloc_contig_range() (mm/page_alloc.c, line
6267). Does omitting replace_free_hugepage_folios() leave
alloc_contig_range() and cma_alloc() broken while also shrinking the
persistent hugetlb pool during memory offlining?

When alloc_contig_range() calls start_isolate_page_range() and
__alloc_contig_migrate_range() on a range containing in-use hugetlb folios,
alloc_migration_target() -> alloc_hugetlb_folio_nodemask() (mm/hugetlb.c,
line 2488) checks available_huge_pages(h) and dequeues a free folio from
h->hugepage_freelists[nid] instead of allocating a temporary folio via
alloc_migrate_hugetlb_folio(). Because HPG_temporary is not set on the source
folio, when migration finishes and free_huge_folio() (mm/hugetlb.c, line
1945) releases the old folio in the isolated range, enqueue_hugetlb_folio()
places it back onto h->hugepage_freelists[nid] rather than freeing it to the
buddy allocator.

Because isolate_migratepages_block() in mm/compaction.c already advanced past
the migrated folio and replace_free_hugepage_folios() is never called (even
though alloc_and_dissolve_hugetlb_folio() is already available in
mm/hugetlb.c at line 3050), the freed source folio remains on
h->hugepage_freelists when test_pages_isolated() runs in alloc_contig_range()
at line 6297, causing alloc_contig_range() and cma_alloc() to still fail with
-EBUSY. Moreover, when multiple in-use hugetlb folios are migrated, skipping
MIGRATE_ISOLATE folios in dequeue_hugetlb_folio_node_exact() without
replacing the freed source folios causes each migration to pull another free
pool folio from outside the range into use and leave multiple freed folios on
hugepage_freelists inside the isolated range before test_pages_isolated()
fails.

Similarly, during offline_pages() in mm/memory_hotplug.c, when an in-use
hugetlb folio in the offlined range is migrated into a free persistent folio
outside the range, the old source folio is returned to h->hugepage_freelists
inside the offlined range and then dissolved by dissolve_free_huge_pages() ->
dissolve_free_huge_page() (mm/hugetlb.c, line 2305), which decrements
h->max_huge_pages and shrinks the persistent hugetlb pool. Finally, the
commit subject and changelog still state that replace_free_hugepage_folios()
is introduced and called after migration, which no longer matches the patch.

[Severity: High]
In mm/hugetlb.c at dequeue_hugetlb_folio_node_exact() (line 1348),
is_migrate_isolate_page(&folio->page) skips isolated folios on
h->hugepage_freelists[nid], but those folios remain counted in
h->free_huge_pages. Can this mismatch with available_huge_pages()
(mm/hugetlb.c, line 1398) cause migration to consume reserved free folios,
breaking offline_pages() and causing SIGBUS on reserved hugetlb faults?

Because available_huge_pages(h) returns h->free_huge_pages -
h->resv_huge_pages, any free hugetlb folio in a MIGRATE_ISOLATE pageblock
(either isolated beforehand by start_isolate_page_range() or placed on
h->hugepage_freelists[nid] by free_huge_folio() after migrating an in-use
folio) still inflates available_huge_pages(h) even though
dequeue_hugetlb_folio_node_exact() refuses to dequeue it.

Suppose h->resv_huge_pages is R (backed by R non-isolated free folios) and
1 unreserved free folio is in a MIGRATE_ISOLATE pageblock, so
h->free_huge_pages is R + 1 and available_huge_pages(h) is 1. When
alloc_hugetlb_folio_nodemask() (mm/hugetlb.c, line 2490) runs to migrate the
next in-use hugetlb folio, available_huge_pages(h) is non-zero, so it calls
dequeue_hugetlb_folio_nodemask() -> dequeue_hugetlb_folio_node_exact(), which
skips the MIGRATE_ISOLATE folio and dequeues one of the R non-isolated folios
backing h->resv_huge_pages instead of falling back to
alloc_migrate_hugetlb_folio(). Once the migrated source folio is freed into
the MIGRATE_ISOLATE range, available_huge_pages(h) remains 1, allowing
subsequent migrations to move all R reserved folios into the MIGRATE_ISOLATE
range.

When offline_pages() later calls dissolve_free_huge_pages(),
dissolve_free_huge_page() (mm/hugetlb.c, line 2305) dissolves the first
unreserved folio (reducing available_huge_pages(h) to 0 and decrementing
h->max_huge_pages) and then fails with -EBUSY on the remaining isolated free
folios because available_huge_pages(h) is now 0, aborting memory offlining.

In addition, while the free folios backing h->resv_huge_pages sit in
MIGRATE_ISOLATE pageblocks, a page fault on a VMA with a valid reservation
calls alloc_hugetlb_folio() -> dequeue_hugetlb_folio_vma() (mm/hugetlb.c,
line 3113), which returns NULL because dequeue_hugetlb_folio_node_exact()
skips the isolated folios. alloc_hugetlb_folio() then falls back to
alloc_buddy_hugetlb_folio_with_mpol() -> alloc_surplus_hugetlb_folio(), which
fails when h->surplus_huge_pages >= h->nr_overcommit_huge_pages (0 by
default, and always 0 for gigantic pages), returning -ENOSPC and delivering
SIGBUS to a process faulting on validly reserved hugepages.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105070431.3100950-1-mkbund@amazon.com?part=1