Re: [PATCH 5.15] 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/hugetlb: isolated free pages in free_huge_pages break
  reservations and offline
- [Medium] mm: missing free hugepage replacement after migration in
  alloc_contig_range()
--

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

mm: replace free hugepage folios after migration

This backport takes the dequeue_huge_page_node_exact() hunk from upstream
commit 04f13d241b8b146b23038bffd907cb8278391d07 so that free hugetlb pages
in MIGRATE_ISOLATE pageblocks are skipped when allocating from the hugetlb
pool.

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: Medium]
In mm/hugetlb.c, dequeue_huge_page_node_exact() is updated to skip pages
where is_migrate_isolate_page(page) is true, while the commit subject and
changelog state that replace_free_hugepage_folios() is introduced and
invoked after migrating in-use hugetlb pages so cma_alloc() and
test_pages_isolated() succeed.

In 5.15, hugetlb pages are allocated with GFP_HIGHUSER_MOVABLE via
htlb_alloc_mask() and can reside in MIGRATE_CMA and MIGRATE_MOVABLE
pageblocks. During cma_alloc() -> alloc_contig_range() ->
__alloc_contig_migrate_range() in mm/page_alloc.c,
isolate_or_dissolve_huge_page() in mm/hugetlb.c dissolves free hugepages in
the target range via alloc_and_dissolve_huge_page(), but for in-use
hugepages (page_count(head) > 0) it isolates them onto cc->migratepages via
isolate_hugetlb() and advances cc->migrate_pfn past them.

When migrate_pages() migrates those in-use hugepages via
alloc_migration_target() -> alloc_huge_page_nodemask(), if unreserved free
hugepages exist in the pool (h->free_huge_pages - h->resv_huge_pages > 0),
dequeue_huge_page_nodemask() returns a free pool hugepage without
HPageTemporary set. Because HPageTemporary(newpage) is false,
move_hugetlb_state() does not set HPageTemporary on the old source page, so
free_huge_page() enqueues the old page back onto h->hugepage_freelists[nid]
inside the MIGRATE_ISOLATE range instead of returning it to the buddy
allocator.

Because this backport omits the post-migration replacement step after
__alloc_contig_migrate_range() (even though alloc_and_dissolve_huge_page()
already exists in mm/hugetlb.c), won't migrating in-use persistent hugetlb
pages still leave the freed source hugepages on h->hugepage_freelists
inside [start, end) and cause test_pages_isolated() in alloc_contig_range()
to fail with -EBUSY? Also, should the commit subject and message be updated
if only the dequeue_huge_page_node_exact() change is intended?

[Severity: High]
In mm/hugetlb.c, dequeue_huge_page_node_exact() skips pages where
is_migrate_isolate_page(page) is true, but those isolated pages remain on
h->hugepage_freelists[nid] and remain counted in h->free_huge_pages. Can
this cause both memory offlining (offline_pages()) and reserved hugetlb
page faults (alloc_huge_page()) to fail when free hugepages sit in a
MIGRATE_ISOLATE range?

Consider a memory block [start_pfn, end_pfn) being offlined via
offline_pages() that contains two in-use hugepages H0 and H1 (or one free
hugepage H0 and one in-use hugepage H1), while outside the block there is
one unreserved free hugepage F0 and one reserved free hugepage F1 backing
an existing reservation (h->free_huge_pages == 2, h->resv_huge_pages == 1):

1. offline_pages() calls start_isolate_page_range(), marking
   [start_pfn, end_pfn) as MIGRATE_ISOLATE, and then calls
   do_migrate_range().

2. Migrating H0 dequeues F0. When H0 is released via
   putback_active_hugepage() -> free_huge_page() -> enqueue_huge_page(), H0
   is placed onto h->hugepage_freelists[nid] inside the MIGRATE_ISOLATE
   range and increments h->free_huge_pages back to 2.

3. When migrating H1, alloc_huge_page_nodemask() in mm/hugetlb.c checks:

       if (h->free_huge_pages - h->resv_huge_pages > 0)

   Because the isolated page H0 is still counted in h->free_huge_pages,
   2 - 1 > 0 succeeds even though the only non-isolated free page (F1) is
   needed to back h->resv_huge_pages.

4. dequeue_huge_page_node_exact() skips H0 because
   is_migrate_isolate_page(H0) is true and dequeues the reserved page F1
   for H1. H1 is then freed onto h->hugepage_freelists[nid] inside the
   MIGRATE_ISOLATE range, leaving both free hugepages (H0 and H1) inside
   the MIGRATE_ISOLATE range while h->resv_huge_pages == 1.

5. In offline_pages(), dissolve_free_huge_pages(start_pfn, end_pfn)
   dissolves H0 (reducing h->free_huge_pages to 1) and then calls
   dissolve_free_huge_page() on H1, which hits:

       if (h->free_huge_pages - h->resv_huge_pages == 0)
           goto out;

   and fails with -EBUSY, aborting offline_pages().

In addition, gather_surplus_pages() in mm/hugetlb.c computes:

    needed = (h->resv_huge_pages + delta) - h->free_huge_pages;
    if (needed <= 0) {
        h->resv_huge_pages += delta;
        return 0;
    }

which counts MIGRATE_ISOLATE free pages in h->free_huge_pages as usable to
satisfy reservations. Whenever the free pages backing h->resv_huge_pages
are in MIGRATE_ISOLATE pageblocks (either from migration swapping F1 with
H1 above, or from gather_surplus_pages()), a page fault on a VMA with a
valid reservation calls alloc_huge_page() -> dequeue_huge_page_vma(), which
skips the isolated pages in dequeue_huge_page_node_exact() and returns
NULL.

alloc_huge_page() then falls back to alloc_buddy_huge_page_with_mpol() ->
alloc_surplus_huge_page(), which fails when h->surplus_huge_pages >=
h->nr_overcommit_huge_pages (0 by default) or hstate_is_gigantic(h),
returning ERR_PTR(-ENOSPC) and delivering SIGBUS to a process faulting on a
reserved mapping.

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