Re: [PATCH v2 6.1 1/2] 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: skipping isolated free pages in
  dequeue_huge_page_node_exact() breaks reservation accounting
- [Medium] mm/hugetlb: repeated hugepage replacement across tail PFNs in
  replace_free_hugepage_folios()
--

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

mm: replace free hugepage folios after migration

This commit adds replace_free_hugepage_folios() to replace free hugetlb
folios in an isolated PFN range after migration and skips MIGRATE_ISOLATE
pages when dequeuing free hugetlb pages.

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

> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f549adc0b6705..46031ca5a7fb1 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c

[ ... ]

> @@ -1343,6 +1344,9 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid)
>  		if (PageHWPoison(page))
>  			continue;
>  
> +		if (is_migrate_isolate_page(page))
> +			continue;
> +
>  		list_move(&page->lru, &h->hugepage_activelist);
>  		set_page_refcounted(page);
>  		ClearHPageFreed(page);

[Severity: High]
Does skipping MIGRATE_ISOLATE pages in dequeue_huge_page_node_exact()
break hugetlb reservation accounting while those pages remain counted in
h->free_huge_pages?

When a free hugepage backing h->resv_huge_pages lies in a MIGRATE_ISOLATE
pageblock during alloc_contig_range() or offline_pages(), a fault on a VMA
with a valid reservation sees dequeue_huge_page_vma() return NULL and
falls back to alloc_buddy_huge_page_with_mpol() ->
alloc_surplus_huge_page().  With overcommit at its default of 0 (or for
gigantic pages), alloc_huge_page() returns ERR_PTR(-ENOSPC) and delivers
SIGBUS to the faulting task.

Similarly, an isolated unreserved page on h->hugepage_freelists still
inflates available_huge_pages(h) (h->free_huge_pages -
h->resv_huge_pages), allowing alloc_huge_page_nodemask() or an unreserved
fault in dequeue_huge_page_vma() to pass the available_huge_pages(h) check
and consume a non-isolated reserved page instead.  Once only isolated free
pages remain to back h->resv_huge_pages, dissolve_free_huge_page() fails
with -EBUSY on !available_huge_pages(h), aborting offline_pages().

> @@ -3073,6 +3077,44 @@ int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list)
>  	return ret;
>  }
>  
> +/*
> + *  replace_free_hugepage_folios - Replace free hugepage folios in a given pfn
> + *  range with new folios.
> + *  @start_pfn: start pfn of the given pfn range
> + *  @end_pfn: end pfn of the given pfn range
> + *  Returns 0 on success, otherwise negated error.
> + */
> +int replace_free_hugepage_folios(unsigned long start_pfn, unsigned long end_pfn)
> +{
> +	struct hstate *h;
> +	struct folio *folio;
> +	int ret = 0;
> +
> +	LIST_HEAD(isolate_list);
> +
> +	while (start_pfn < end_pfn) {
> +		folio = pfn_folio(start_pfn);
> +		if (folio_test_hugetlb(folio)) {
> +			h = folio_hstate(folio);
> +		} else {
> +			start_pfn++;
> +			continue;
> +		}
> +
> +		if (!folio_ref_count(folio)) {
> +			ret = alloc_and_dissolve_huge_page(h, &folio->page,
> +							   &isolate_list);
> +			if (ret)
> +				break;
> +
> +			putback_movable_pages(&isolate_list);
> +		}
> +		start_pfn++;
> +	}
> +
> +	return ret;
> +}

[Severity: Medium]
Can advancing start_pfn by 1 cause replace_free_hugepage_folios() to
repeatedly replace the same hugepage across all of its tail PFNs?

alloc_and_dissolve_huge_page() calls update_and_free_page() ->
__update_and_free_page() and returns 0:

mm/hugetlb.c:__update_and_free_page() {
    ...
	if (hugetlb_vmemmap_restore(h, page)) {
		spin_lock_irq(&hugetlb_lock);
		add_hugetlb_page(h, page, true);
		spin_unlock_irq(&hugetlb_lock);
		return;
	}
    ...
}

If hugetlb_vmemmap_restore() fails to allocate vmemmap pages,
add_hugetlb_page() puts the undissolved compound page back on the freelist
with refcount 0 as a surplus page.  Because the loop only increments
start_pfn by 1 and pfn_folio(start_pfn) resolves each tail page back to
the head folio, the remaining pages_per_huge_page(h) - 1 iterations call
alloc_and_dissolve_huge_page() on the same folio again, allocating extra
hugepages and inflating h->nr_huge_pages and h->surplus_huge_pages.
Advancing start_pfn by pages_per_huge_page(h) (or the next hugepage
boundary) avoids revisiting the tail pages.

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