Re: [PATCH] mm/hugetlb: restore failed global reservations to subpool in alloc_hugetlb_folio

[Song Hu]

在 2026/7/13 23:49, Ackerley Tng 写道:
> Joshua Hahn <joshua.hahnjy@xxxxxxxxx> writes:
>
>> On Mon, 13 Jul 2026 19:50:08 +0800 Song Hu <husong@xxxxxxxxxx> wrote:
>>
>> Hi Song, thank you for the patch.
>>
>>> When hugetlb_alloc_folio() fails, alloc_hugetlb_folio() only rolls back
>>> spool->used_hpages in the out_subpool_put path when gbl_chg == 0. For
>>> gbl_chg > 0 (e.g. a size= hugetlbfs mount), hugepage_subpool_get_pages()
>>> has already incremented used_hpages, but the error path skips the
>>> rollback, so each failed fault permanently leaks one used_hpage until
>>> the subpool is exhausted and hugepage_subpool_get_pages() itself fails.
>>>
>>> Decrement used_hpages for the gbl_chg > 0 case too, mirroring the
>>> hugetlb_reserve_pages() fix.
>> So something is clearly wrong with this codepath here; there are now 4
>> competing fixes in the mailing list currently being discussed [1] [2] [3]
>> including this one and they all do things slightly differently.
>> Let's please agree on what the correct solution is,
>> I've CC-ed the authors of those 3 other solutions to discuss here.
>>
> Thanks for connecting us!
>
> I'd like to make a pitch for centralizing the fix into the subpool [1]
> :) I think it will also let us clean up the existing codepaths where
> each path does its own open-coding and reaching into the subpool. Also,
> if that cleanup is centralized into the subpool, we might be able to
> track availability instead of reservations [4], which I think simplifies
> HugeTLB reservations and removes some possible races (for keeping
> rsv_hpages and resv_huge_pages in sync) completely.
>
> [4] https://lore.kernel.org/all/CAEvNRgGN0HSJ2iLSDD2haSKOxifa-uhkO9Hwossh0+Q_d9fzOw@xxxxxxxxxxxxxx/
>
> I understand the above solution is a deeper change, does anyone have any
> thoughts on the approach, or existing tests other than libhugetlbfs and
> tools/testing/selftests/mm/ksft_hugetlb.sh (which pass) that I could use
> to prove this works?
Hi Ackerley,

Thanks for the write-up. The centralization reads cleanly, and tracking
availability instead of reservations does look like it would remove a real
class of the rsv_hpages / resv_huge_pages sync races, so I don't have an
objection to that direction for mainline.

On tests — I put together a small reproducer while debugging this, which
might be useful regardless of which fix lands: a size= hugetlbfs mount (so
the subpool has a max_hpages), with nr_hugepages churned 10 -> 2 -> 10
around a MAP_NORESERVE fault loop. Without a fix the subpool permanently
loses capacity (it maps 2/10 pages even with 10 global pages free); with a
fix it recovers to 10/10.

One thing worth flagging for -stable: a833a693a490 is already in 6.x
-stable, so the leak is live there too, and a larger refactor probably is
not backportable — so a minimal fix may still be wanted on the stable
branches even if centralization goes in for mainline.

Thanks,
Song
>>> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
>>> Signed-off-by: Song Hu <husong@xxxxxxxxxx>
>>> ---
>>>  mm/hugetlb.c | 13 +++++++++++++
>>>  1 file changed, 13 insertions(+)
>>>
>>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>>> index d6c812d1857b..8413ec92d836 100644
>>> --- a/mm/hugetlb.c
>>> +++ b/mm/hugetlb.c
>>> @@ -3073,6 +3073,19 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>>>  	if (map_chg && !gbl_chg) {
>>>  		gbl_reserve = hugepage_subpool_put_pages(spool, 1);
>>>  		hugetlb_acct_memory(h, -gbl_reserve);
>>> +	} else if (map_chg && gbl_chg > 0 && spool) {
>>> +		/*
>>> +		 * Restore used_hpages for the globally-requested page that
>>> +		 * hugepage_subpool_get_pages() counted against the subpool's
>>> +		 * maximum, but which we failed to back from the global pool.
>>> +		 * Mirrors the fix in hugetlb_reserve_pages() (1d3f9bb4c8af).
>>> +		 */
>>> +		unsigned long flags;
>>> +
>>> +		spin_lock_irqsave(&spool->lock, flags);
>>> +		if (spool->max_hpages != -1)
>>> +			spool->used_hpages -= gbl_chg;
>>> +		unlock_or_release_subpool(spool, flags);
>> Why are we unlocking or releasing the subpool here?
>>
>>>  	}
>>>
>>>  out_end_reservation:
>>> --
>>> 2.43.0
>> Thanks again for the patch,
>> Joshua
>>
>> [1] https://lore.kernel.org/all/20260708-hugetlb-alloc-failure-fixes-v2-2-c7f27cbb462b@xxxxxxxxxx/
>> [2] https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@xxxxxxxxx/
>> [3] https://lore.kernel.org/linux-mm/20260515202902.461539-1-devnexen@xxxxxxxxx/