Re: [PATCH v2 5/5] mm: hugetlb: enable CMA allocation for PMD-sized pages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] mm: hugetlb: infinite FOLL_LONGTERM migration loop for PMD CMA folios
- [High] mm: hugetlb: alloc_buddy_frozen_folio() consumes CMA pages without bitmap bits
- [High] mm: hugetlb: folio_lock deadlock in CMA alloc for PMD hugetlb faults
--

commit 9cf077a849a76f9279657f4fa6aef60b1b9348ba
Author: Qi Zheng <qi.zheng@linux.dev>

mm: hugetlb: enable CMA allocation for PMD-sized pages

This commit enables configuring a dedicated CMA pool for PMD-sized HugeTLB
pages via hugetlb_cma=<capacity>@<page-size>, trying buddy allocation first
for non-gigantic PMD pages before falling back to CMA.

> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index c1a8d00be11ac..bad5cd4978c5e 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1835,22 +1835,20 @@ static struct folio *only_alloc_fresh_hugetlb_folio(struct hstate *h,
>  {
>  	struct folio *folio = NULL;
>  	int order = huge_page_order(h);
> +	bool cma_only = hugetlb_cma_exclusive_alloc(order);
>  
>  	if (nid == NUMA_NO_NODE)
>  		nid = numa_mem_id();
>  
> -	if (allow_cma && order_is_gigantic(order))
> +	if (!order_is_gigantic(order) && !cma_only)
> +		folio = alloc_buddy_frozen_folio(order, gfp_mask, nid, nmask,
> +						 node_alloc_noretry);

[Severity: High]
When hugetlb_cma=<size>@<pmd-size> is configured without hugetlb_cma_only=1,
can calling alloc_buddy_frozen_folio() first consume pages from the reserved
CMA pool without marking the CMA bitmap, leading to a livelock in
alloc_and_dissolve_hugetlb_folio() or premature failure in cma_range_alloc()?

For non-gigantic PMD hstates where hugepage_movable_supported(h) is true,
htlb_alloc_mask(h) includes GFP_HIGHUSER_MOVABLE. In
__alloc_frozen_pages_noprof(), prepare_alloc_pages() and
alloc_flags_slowpath() set ALLOC_CMA via alloc_flags_cma(gfp_mask), which
allows mm/page_alloc.c:__rmqueue() to allocate PMD-sized folios directly
from the MIGRATE_CMA freelists of the hugetlb_cma pool without setting bits
in cmr->bitmap:

mm/page_alloc.c:__rmqueue()
	case RMQUEUE_CMA:
		if (alloc_flags & ALLOC_CMA) {
			page = __rmqueue_cma_fallback(zone, order);
			if (page) {
				*mode = RMQUEUE_CMA;
				return page;
			}
		}

Once completely free PMD-order buddy blocks are exhausted,
only_alloc_fresh_hugetlb_folio() falls back to
hugetlb_cma_alloc_frozen_folio() -> cma_alloc_frozen_compound() ->
cma_range_alloc(). Because the bitmap bits for the buddy-allocated PMD
HugeTLB folios are still zero, cma_range_alloc() selects those ranges and
calls alloc_contig_frozen_range() -> isolate_migratepages_block() ->
isolate_or_dissolve_huge_folio() -> alloc_and_dissolve_hugetlb_folio().

During batch pool allocation in hugetlb_hstate_alloc_pages_onenode(),
hugetlb_pages_alloc_boot_node(), or set_max_huge_pages(), the earlier
buddy-allocated folios are still held on the caller's local list with
refcount 0 and without the hugetlb_freed flag set until
prep_and_add_allocated_folios() runs after the loop. When
alloc_and_dissolve_hugetlb_folio() encounters one of those folios, it spins
indefinitely waiting for the current task to enqueue it on the freelist:

mm/hugetlb.c:alloc_and_dissolve_hugetlb_folio()
	} else if (!folio_test_hugetlb_freed(old_folio)) {
		spin_unlock_irq(&hugetlb_lock);
		cond_resched();
		goto retry;
	} else {
		h = folio_hstate(old_folio);
		if (!new_folio) {
			spin_unlock_irq(&hugetlb_lock);
			gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
			new_folio = alloc_fresh_hugetlb_folio(h, gfp_mask, nid,
							      NULL, false);
			if (!new_folio)
				return -ENOMEM;

Even when old_folio is already on the HugeTLB freelist (or in use when no
free pool pages remain), alloc_and_dissolve_hugetlb_folio() or
unmap_and_move_hugetlb_folio() -> alloc_migration_target() calls
alloc_fresh_hugetlb_folio() with allow_cma set to false. That retries the
same alloc_buddy_frozen_folio() call that just failed and returns -ENOMEM,
causing mm/cma.c:cma_range_alloc() to break out of the bitmap scan loop
immediately without trying the remaining CMA pageblocks:

mm/cma.c:cma_range_alloc()
		mutex_lock(&cma->alloc_mutex);
		ret = alloc_contig_frozen_range(pfn, pfn + count, ACR_FLAGS_CMA, gfp);
		mutex_unlock(&cma->alloc_mutex);
		if (!ret)
			break;

		cma_clear_bitmap(cma, cmr, pfn, count);
		if (ret != -EBUSY)
			break;

> +
> +	if (!folio && allow_cma)
>  		folio = hugetlb_cma_alloc_frozen_folio(order, gfp_mask, nid, nmask);

[Severity: High]
Can this cause an infinite migration loop when a userspace process pins a
PMD-sized HugeTLB mapping with FOLL_LONGTERM via __gup_longterm_locked() or
memfd_pin_folios()?

Folios allocated by hugetlb_cma_alloc_frozen_folio() are placed in
MIGRATE_CMA pageblocks, so folio_is_longterm_pinnable() returns false. When
pinning with FOLL_LONGTERM, mm/gup.c:__gup_longterm_locked() (and
memfd_pin_folios()) sets PF_MEMALLOC_PIN via memalloc_pin_save() and loops
while check_and_migrate_movable_pages() returns -EAGAIN:

mm/gup.c:__gup_longterm_locked()
	flags = memalloc_pin_save();
	do {
		nr_pinned_pages = __get_user_pages_locked(mm, start, nr_pages,
							  pages, locked,
							  gup_flags);
		if (nr_pinned_pages <= 0) {
			rc = nr_pinned_pages;
			break;
		}

		/* FOLL_LONGTERM implies FOLL_PIN */
		rc = check_and_migrate_movable_pages(nr_pinned_pages, pages);
	} while (rc == -EAGAIN);

Previously, only gigantic folios were allocated from hugetlb_cma, and
alloc_migrate_hugetlb_folio() rejected gigantic hstates, terminating the
loop with -ENOMEM. With non-gigantic PMD folios now allocated from CMA,
migrate_longterm_unpinnable_folios() calls migrate_pages() with
alloc_migration_target() and MR_LONGTERM_PIN, which passes allow_cma as true
(mtc->reason != MR_CONTIG_RANGE):

mm/migrate.c:alloc_migration_target()
	if (folio_test_hugetlb(src)) {
		struct hstate *h = folio_hstate(src);

		gfp_mask = htlb_modify_alloc_mask(h, gfp_mask);
		return alloc_hugetlb_folio_nodemask(h, nid,
						mtc->nmask, gfp_mask,
						htlb_allow_alloc_fallback(mtc->reason),
						mtc->reason != MR_CONTIG_RANGE);
	}

When dequeue_hugetlb_folio_node_exact() skips free CMA folios in the pool
because PF_MEMALLOC_PIN is set and falls back to
alloc_migrate_hugetlb_folio() -> alloc_fresh_hugetlb_folio() ->
only_alloc_fresh_hugetlb_folio(), if cma_only is true or if
alloc_buddy_frozen_folio() fails to find a non-movable buddy block,
hugetlb_cma_alloc_frozen_folio() in mm/hugetlb_cma.c does not check
PF_MEMALLOC_PIN and allocates another MIGRATE_CMA folio. Migration then
succeeds into the new CMA folio, check_and_migrate_movable_pages() returns
-EAGAIN, and the pin path loops indefinitely migrating between CMA folios.

Should hugetlb_cma_alloc_frozen_folio() or only_alloc_fresh_hugetlb_folio()
skip CMA allocation when current->flags & PF_MEMALLOC_PIN is set?

[Severity: High]
Could falling back to hugetlb_cma_alloc_frozen_folio() for non-gigantic PMD
pages during a HugeTLB page fault also deadlock on folio_lock?

During a HugeTLB fault (hugetlb_fault() -> hugetlb_wp() or
hugetlb_no_page()), alloc_hugetlb_folio() is called while holding
hugetlb_fault_mutex_table[hash] and hugetlb_vma_lock_read(vma). In the
copy-on-write path in mm/hugetlb.c:hugetlb_wp(), the faulting task also
holds folio_lock(old_folio) with HPG_migratable set on old_folio:

mm/hugetlb.c:hugetlb_wp()
	spin_unlock(vmf->ptl);
	new_folio = alloc_hugetlb_folio(vma, vmf->address, cow_from_owner);

When surplus allocation in alloc_surplus_hugetlb_folio() ->
only_alloc_fresh_hugetlb_folio() falls back to
hugetlb_cma_alloc_frozen_folio() -> cma_alloc_frozen_compound() ->
cma_range_alloc() -> alloc_contig_frozen_range(), if the target CMA range
contains an in-use buddy-allocated PMD HugeTLB folio src,
mm/hugetlb.c:isolate_or_dissolve_huge_folio() only rejects gigantic folios
and isolates src onto cc.migratepages:

mm/hugetlb.c:isolate_or_dissolve_huge_folio()
	if (order_is_gigantic(folio_order(folio)))
		return -ENOMEM;

	if (folio_ref_count(folio) && folio_isolate_hugetlb(folio, list))
		ret = 0;

__alloc_contig_migrate_range() then calls migrate_pages() ->
migrate_hugetlbs() -> unmap_and_move_hugetlb_folio() with mode set to
MIGRATE_SYNC. Once alloc_migration_target() obtains a replacement folio, on
pass > 2 (force == 1) mm/migrate.c:unmap_and_move_hugetlb_folio() performs a
blocking folio_lock(src):

mm/migrate.c:unmap_and_move_hugetlb_folio()
	if (!folio_trylock(src)) {
		if (!force)
			goto out;
		switch (mode) {
		case MIGRATE_SYNC:
			break;
		default:
			goto out;
		}
		folio_lock(src);
	}

In hugetlb_wp(), if src is old_folio, the current task already holds
folio_lock(old_folio) and will self-deadlock. Even if src is a different
folio (or in hugetlb_no_page()), blocking on folio_lock(src) while holding
hugetlb_fault_mutex_table[hash] and hugetlb_vma_lock_read(vma) inverts the
hugetlb_fault_mutex -> vma_lock -> folio_lock lock ordering against a
concurrent task in hugetlb_wp() on src, which holds folio_lock(src) while
dropping and re-acquiring hugetlb_fault_mutex_table[hash] and
hugetlb_vma_lock_read(vma) around unmap_ref_private().

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1792651104.git.zhengqi.arch@bytedance.com?part=5