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

Song Hu posted 1 patch 1 week, 5 days ago
mm/hugetlb.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
[PATCH] mm/hugetlb: restore failed global reservations to subpool in alloc_hugetlb_folio
Posted by Song Hu 1 week, 5 days ago
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.

Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
Signed-off-by: Song Hu <husong@kylinos.cn>
---
 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);
 	}
 
 out_end_reservation:
-- 
2.43.0
Re: [PATCH] mm/hugetlb: restore failed global reservations to subpool in alloc_hugetlb_folio
Posted by Joshua Hahn 1 week, 5 days ago
On Mon, 13 Jul 2026 19:50:08 +0800 Song Hu <husong@kylinos.cn> 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. 

> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> Signed-off-by: Song Hu <husong@kylinos.cn>
> ---
>  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@google.com/
[2] https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@gmail.com/
[3] https://lore.kernel.org/linux-mm/20260515202902.461539-1-devnexen@gmail.com/
Re: [PATCH] mm/hugetlb: restore failed global reservations to subpool in alloc_hugetlb_folio
Posted by Song Hu 1 week, 4 days ago
在 2026/7/13 22:45, Joshua Hahn 写道:
> On Mon, 13 Jul 2026 19:50:08 +0800 Song Hu <husong@kylinos.cn> 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. 
Hi Joshua,

Thanks for the review and for bringing the authors together.

First, apologies for the duplicate submission — when I sent this I was not
aware of the other proposals on the list ([1][2][3]). I ran into this leak
while looking at the subpool accounting and simply wanted to share a fix;
I did not mean to add noise to an ongoing discussion.
>> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
>> Signed-off-by: Song Hu <husong@kylinos.cn>
>> ---
>>  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? 
Good catch — it does not belong here. unlock_or_release_subpool() is meant
for paths that may drop the subpool's last reference (e.g.
hugepage_subpool_put_pages(), after which the subpool can become free). In
this error path we only adjust used_hpages and never touch spool->count, so
the subpool cannot be released here; a plain
spin_unlock_irqrestore(&spool->lock, flags) is sufficient and clearer. I
copied it from the hugetlb_reserve_pages() sibling fix (1d3f9bb4c8af), but
it is not the right helper here either; I will switch to
spin_unlock_irqrestore() in v2.

One thing that may be worth keeping in mind for the discussion:
a833a693a490 is already in 6.x -stable, so the leak is present in those
trees as well, and a backportable fix will be needed there regardless of
which direction is chosen for mainline.

For what it is worth, I verified the behavior with a small reproducer (a
size= hugetlbfs mount with nr_hugepages churned 10 -> 2 -> 10: unpatched,
the subpool only maps 2/10 pages after the leak; patched, 10/10), in case
it is useful to whoever ends up shaping the final fix.

Happy to help with whatever direction people want to take.

Thanks,
Song
>>  	}
>>  
>>  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@google.com/
> [2] https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@gmail.com/
> [3] https://lore.kernel.org/linux-mm/20260515202902.461539-1-devnexen@gmail.com/
Re: [PATCH] mm/hugetlb: restore failed global reservations to subpool in alloc_hugetlb_folio
Posted by Ackerley Tng 1 week, 5 days ago
Joshua Hahn <joshua.hahnjy@gmail.com> writes:

> On Mon, 13 Jul 2026 19:50:08 +0800 Song Hu <husong@kylinos.cn> 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@mail.gmail.com/

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@kylinos.cn>
>> ---
>>  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@google.com/
> [2] https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@gmail.com/
> [3] https://lore.kernel.org/linux-mm/20260515202902.461539-1-devnexen@gmail.com/
Re: [PATCH] mm/hugetlb: restore failed global reservations to subpool in alloc_hugetlb_folio
Posted by Song Hu 1 week, 4 days ago
在 2026/7/13 23:49, Ackerley Tng 写道:
> Joshua Hahn <joshua.hahnjy@gmail.com> writes:
>
>> On Mon, 13 Jul 2026 19:50:08 +0800 Song Hu <husong@kylinos.cn> 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@mail.gmail.com/
>
> 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@kylinos.cn>
>>> ---
>>>  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@google.com/
>> [2] https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@gmail.com/
>> [3] https://lore.kernel.org/linux-mm/20260515202902.461539-1-devnexen@gmail.com/