Re: [PATCH v2 3/4] mm: hugetlb: Fix subpool usage leak on allocation failure
From: Joshua Hahn
Date: Fri Sep 11 2026 - 10:56:35 EST
On Wed, 09 Sep 2026 14:49:28 -0700 Ackerley Tng via B4 Relay <devnull+ackerleytng.google.com@xxxxxxxxxx> wrote:
> From: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>
> When folio allocation fails early (e.g. buddy allocation failure or
> cgroup charging failure) and a reservation was not used (meaning an
> unreserved global page was needed), the subpool page acquired during the
> allocation attempt must still be returned.
>
> Currently, the subpool cleanup error path only returns the page to the
> subpool if a reservation was used. If no reservation was used, it skips
> releasing the page back to the subpool, permanently leaking the subpool's
> used pages counter.
>
> With subpools now always tracking used pages, always release the page
> back to the subpool whenever a subpool page was acquired.
>
> Opportunistically rename the local variables tracking global reservations
> needed and global reservations returned. This clarifies the accounting:
> a value of zero for needed global reservations indicates an existing
> reservation satisfies the allocation, while a non-zero value indicates
> new global pages are required.
>
> Adjust global reservations using the difference between reservations
> needed and reservations returned to properly handle races where concurrent
> threads interact with the same subpool.
>
> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
LGTM,
Reviewed-by: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
I think that we get parity in the accounting logic between
alloc_hugetlb_folio and hugetlb_reserve_pages is a nice plus as a result
of these two fixes.
> ---
> mm/hugetlb.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 9eb9f3442574c..652cfb55c6e6e 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2957,7 +2957,7 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> struct hugepage_subpool *spool = subpool_vma(vma);
> struct hstate *h = hstate_vma(vma);
> struct folio *folio;
> - long retval, gbl_chg, gbl_reserve;
> + long retval, gbl_resv_get;
> map_chg_state map_chg;
> struct mempolicy_interpreted mpoli;
> gfp_t gfp = htlb_alloc_mask(h);
> @@ -2996,8 +2996,8 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> * Or if it can get one from the pool reservation directly.
> */
> if (map_chg) {
> - gbl_chg = hugepage_subpool_get_pages(spool, 1);
> - if (gbl_chg < 0) {
> + gbl_resv_get = hugepage_subpool_get_pages(spool, 1);
> + if (gbl_resv_get < 0) {
> ret = -ENOSPC;
> goto out_end_reservation;
> }
> @@ -3006,7 +3006,7 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> * If we have the vma reservation ready, no need for extra
> * global reservation.
> */
> - gbl_chg = 0;
> + gbl_resv_get = 0;
> }
>
> /*
> @@ -3017,10 +3017,10 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> alloc_flags |= HUGETLB_ALLOC_CHARG_CGROUP_RSVD;
>
> /*
> - * gbl_chg == 0 indicates a reservation exists for this
> + * gbl_resv_get == 0 indicates a reservation exists for this
> * allocation, so try to use it.
> */
> - if (gbl_chg == 0)
> + if (gbl_resv_get == 0)
> alloc_flags |= HUGETLB_ALLOC_USE_GLOBAL_RESERVATIONS;
>
> /* Takes reference on mpol. */
> @@ -3074,13 +3074,10 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> return folio;
>
> out_subpool_put:
> - /*
> - * put page to subpool iff the quota of subpool's rsv_hpages is used
> - * during hugepage_subpool_get_pages.
> - */
> - if (map_chg && !gbl_chg) {
> - gbl_reserve = hugepage_subpool_put_pages(spool, 1);
> - hugetlb_acct_memory(h, -gbl_reserve);
> + if (map_chg) {
> + long gbl_resv_put = hugepage_subpool_put_pages(spool, 1);
> +
> + hugetlb_acct_memory(h, gbl_resv_get - gbl_resv_put);
> }
>
> out_end_reservation:
>
> --
> 2.55.0.1007.g17ff1f9808-goog