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

[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?

>> 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/