Linux kernel -stable discussions
 help / color / mirror / Atom feed
* [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
       [not found] ` <20260428030712.66256-2-enderaoelyther@gmail.com>
@ 2026-04-28 11:30   ` Zhao Li
  2026-09-06  2:31     ` Andrew Morton
                       ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Zhao Li @ 2026-04-28 11:30 UTC (permalink / raw)
  To: Andrew Morton
  Cc: mawupeng1, Zhao Li, Muchun Song, Oscar Salvador,
	David Hildenbrand, linux-mm, linux-kernel, stable

alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
is set.  For a subpool with max_hpages != -1, that bumps used_hpages
regardless of whether it returns gbl_chg = 0 (rsv slot consumed) or
gbl_chg > 0 (used_hpages slot only).  If the allocation later fails
before a folio is returned, the unwind must undo the used_hpages
bump.  The old cleanup only ran for !gbl_chg, leaking used_hpages on
the gbl_chg > 0 path.

For gbl_chg > 0 on max-only subpools (max_hpages != -1, min_hpages
== -1), hugepage_subpool_get_pages() took only a speculative
used_hpages slot.  Drop that slot directly under spool->lock.  In
that configuration hugepage_subpool_put_pages() cannot restore
rsv_hpages, so the direct decrement is the exact inverse and is
race-free against concurrent puts.  This matches the used_hpages-only
part of hugetlb_reserve_pages()'s out_put_pages cleanup, but
restricts it to the max-only case where no rsv_hpages restoration is
possible.

Mounts with min_hpages != -1 are left unchanged for now.  v2's
approach (hugepage_subpool_put_pages() + h->resv_huge_pages++ to
back a restored rsv_hpages slot) double-counts global backing under
concurrent free_huge_folio() and creates phantom reservations under
concurrent hugetlb_unreserve_pages().  Safe cleanup of that quadrant
needs a coordinated fix across multiple call sites.

Reproduced on size=20M hugetlbfs with the faulting task in a hugetlb
cgroup whose limit is exceeded.  Vanilla leaks 6/8 hugepages of
subpool quota; this patch leaks 0/8.  Verified under QEMU.

Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
Cc: stable@vger.kernel.org # v6.15+
Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
---
Changes in v3:
- Replace v2's hugepage_subpool_put_pages() + h->resv_huge_pages++ on
  the gbl_chg > 0 branch with a direct used_hpages-- under spool->lock.
- Restrict the cleanup to (max_hpages != -1, min_hpages == -1) where
  the direct decrement is the exact inverse of the speculative bump.

Changes in v2:
- Skip the gbl_chg > 0 cleanup when max_hpages is unset.
- Add hugepage_subpool_put_pages() + h->resv_huge_pages++ on the
  gbl_chg > 0 branch.

 mm/hugetlb.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f24bf49be047e..cfdeaf6394c5b 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3025,13 +3025,24 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 		hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h),
 						    h_cg);
 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) {
+		if (!gbl_chg) {
+			/* Full inverse when subpool_get_pages() consumed rsv_hpages. */
+			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
+			hugetlb_acct_memory(h, -gbl_reserve);
+		} else if (gbl_chg > 0 && spool && spool->min_hpages == -1 &&
+			   spool->max_hpages != -1) {
+			unsigned long flags;
+
+			/*
+			 * For max-only subpools, subpool_get_pages() took only a
+			 * speculative used_hpages slot. Drop that slot directly.
+			 */
+			spin_lock_irqsave(&spool->lock, flags);
+			if (spool->used_hpages > 0)
+				spool->used_hpages--;
+			unlock_or_release_subpool(spool, flags);
+		}
 	}


--
2.50.1 (Apple Git-155)

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
@ 2026-09-06  2:31     ` Andrew Morton
  2026-09-08  7:12       ` Zhao Li
  2026-09-28  5:26     ` Ackerley Tng
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2026-09-06  2:31 UTC (permalink / raw)
  To: Zhao Li
  Cc: mawupeng1, Muchun Song, Oscar Salvador, David Hildenbrand,
	linux-mm, linux-kernel, stable

On Tue, 28 Apr 2026 19:30:38 +0800 Zhao Li <enderaoelyther@gmail.com> wrote:

> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
> is set.  For a subpool with max_hpages != -1, that bumps used_hpages
> regardless of whether it returns gbl_chg = 0 (rsv slot consumed) or
> gbl_chg > 0 (used_hpages slot only).  If the allocation later fails
> before a folio is returned, the unwind must undo the used_hpages
> bump.  The old cleanup only ran for !gbl_chg, leaking used_hpages on
> the gbl_chg > 0 path.
> 
> For gbl_chg > 0 on max-only subpools (max_hpages != -1, min_hpages
> == -1), hugepage_subpool_get_pages() took only a speculative
> used_hpages slot.  Drop that slot directly under spool->lock.  In
> that configuration hugepage_subpool_put_pages() cannot restore
> rsv_hpages, so the direct decrement is the exact inverse and is
> race-free against concurrent puts.  This matches the used_hpages-only
> part of hugetlb_reserve_pages()'s out_put_pages cleanup, but
> restricts it to the max-only case where no rsv_hpages restoration is
> possible.
> 
> Mounts with min_hpages != -1 are left unchanged for now.  v2's
> approach (hugepage_subpool_put_pages() + h->resv_huge_pages++ to
> back a restored rsv_hpages slot) double-counts global backing under
> concurrent free_huge_folio() and creates phantom reservations under
> concurrent hugetlb_unreserve_pages().  Safe cleanup of that quadrant
> needs a coordinated fix across multiple call sites.
> 
> Reproduced on size=20M hugetlbfs with the faulting task in a hugetlb
> cgroup whose limit is exceeded.  Vanilla leaks 6/8 hugepages of
> subpool quota; this patch leaks 0/8.  Verified under QEMU.

Thanks.

I do like to see a clear statement of the user-visible effects of bugs
when we fix them.  "subpool quota leak" sounds bad, but how does this
visibly manifest?

I asked you-know-what and came up with

  Failed hugetlbfs page allocations can permanently consume the
  mount's size= quota without allocating a huge page.  Repeated
  failures can make the filesystem appear full and cause later
  huge-page faults or allocations to fail with SIGBUS/allocation
  failure despite available huge pages and unused real filesystem
  capacity.

Which I'll paste in there.  Because I do like to tell downstream people
why we want a backport, and to help further downstream people to
understand whether this might fix a problem they're having.  Please lmk
if it's inaccurate/incomplete.

I'll queue this as a backportable hotfix and shall await further
reviewer input.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-09-06  2:31     ` Andrew Morton
@ 2026-09-08  7:12       ` Zhao Li
  0 siblings, 0 replies; 7+ messages in thread
From: Zhao Li @ 2026-09-08  7:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: mawupeng1, Muchun Song, Oscar Salvador, David Hildenbrand,
	linux-mm, linux-kernel, stable

On Sat, 5 Sep 2026 19:31:22 -0700, Andrew Morton wrote:
> I do like to see a clear statement of the user-visible effects of bugs
> when we fix them.  "subpool quota leak" sounds bad, but how does this
> visibly manifest?

Thanks for picking this up. The added description accurately captures
the max-only case.

The leftover used_hpages charge reduces the free space reported by
statfs(), so the mount can appear full despite having fewer allocated
huge pages than its configured size= limit. Later allocations can fail
with ENOSPC, or page faults can result in SIGBUS, even when huge pages
are available.

The patch leaves the min_size= reservation-accounting case unchanged.
I'll make the user-visible impact clearer in future patch descriptions.

Thanks,
Zhao

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
  2026-09-06  2:31     ` Andrew Morton
@ 2026-09-28  5:26     ` Ackerley Tng
  2026-10-07 13:50     ` Muchun Song
  2026-10-13 15:04     ` David Hildenbrand (Arm)
  3 siblings, 0 replies; 7+ messages in thread
From: Ackerley Tng @ 2026-09-28  5:26 UTC (permalink / raw)
  To: Zhao Li
  Cc: akpm, mawupeng1, muchun.song, osalvador, david, linux-mm,
	linux-kernel, stable, Karl Mehltretter, Joshua Hahn

Zhao Li <enderaoelyther@gmail.com> writes:

> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
> is set.  For a subpool with max_hpages != -1, that bumps used_hpages
> regardless of whether it returns gbl_chg = 0 (rsv slot consumed) or
> gbl_chg > 0 (used_hpages slot only).  If the allocation later fails
> before a folio is returned, the unwind must undo the used_hpages
> bump.  The old cleanup only ran for !gbl_chg, leaking used_hpages on
> the gbl_chg > 0 path.
>
> For gbl_chg > 0 on max-only subpools (max_hpages != -1, min_hpages
> == -1), hugepage_subpool_get_pages() took only a speculative
> used_hpages slot.  Drop that slot directly under spool->lock.  In
> that configuration hugepage_subpool_put_pages() cannot restore
> rsv_hpages, so the direct decrement is the exact inverse and is
> race-free against concurrent puts.  This matches the used_hpages-only
> part of hugetlb_reserve_pages()'s out_put_pages cleanup, but
> restricts it to the max-only case where no rsv_hpages restoration is
> possible.
>
> Mounts with min_hpages != -1 are left unchanged for now.  v2's
> approach (hugepage_subpool_put_pages() + h->resv_huge_pages++ to
> back a restored rsv_hpages slot) double-counts global backing under
> concurrent free_huge_folio() and creates phantom reservations under
> concurrent hugetlb_unreserve_pages().  Safe cleanup of that quadrant
> needs a coordinated fix across multiple call sites.
>
> Reproduced on size=20M hugetlbfs with the faulting task in a hugetlb
> cgroup whose limit is exceeded.  Vanilla leaks 6/8 hugepages of
> subpool quota; this patch leaks 0/8.  Verified under QEMU.
>
> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> Cc: stable@vger.kernel.org # v6.15+
> Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
> ---
> Changes in v3:
> - Replace v2's hugepage_subpool_put_pages() + h->resv_huge_pages++ on
>   the gbl_chg > 0 branch with a direct used_hpages-- under spool->lock.
> - Restrict the cleanup to (max_hpages != -1, min_hpages == -1) where
>   the direct decrement is the exact inverse of the speculative bump.
>
> Changes in v2:
> - Skip the gbl_chg > 0 cleanup when max_hpages is unset.
> - Add hugepage_subpool_put_pages() + h->resv_huge_pages++ on the
>   gbl_chg > 0 branch.
>
>  mm/hugetlb.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f24bf49be047e..cfdeaf6394c5b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3025,13 +3025,24 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  		hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h),
>  						    h_cg);
>  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) {
> +		if (!gbl_chg) {
> +			/* Full inverse when subpool_get_pages() consumed rsv_hpages. */
> +			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
> +			hugetlb_acct_memory(h, -gbl_reserve);
> +		} else if (gbl_chg > 0 && spool && spool->min_hpages == -1 &&
> +			   spool->max_hpages != -1) {
> +			unsigned long flags;
> +
> +			/*
> +			 * For max-only subpools, subpool_get_pages() took only a
> +			 * speculative used_hpages slot. Drop that slot directly.
> +			 */
> +			spin_lock_irqsave(&spool->lock, flags);
> +			if (spool->used_hpages > 0)
> +				spool->used_hpages--;
> +			unlock_or_release_subpool(spool, flags);
> +		}
>  	}
>
>
> --
> 2.50.1 (Apple Git-155)

This fixes out_subpool_put in alloc_hugetlb_folio() on a mount that
specifies max_size only, and does not introduce any further bugs AFAICT.

Tested-by: Ackerley Tng <ackerleytng@google.com>

Further details at [1].

[1] https://lore.kernel.org/all/CAEvNRgHQLifmcD1NO6cd7EVcCiQb6gJgNUo4HkFhZAvzuwMq0w@mail.gmail.com/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
  2026-09-06  2:31     ` Andrew Morton
  2026-09-28  5:26     ` Ackerley Tng
@ 2026-10-07 13:50     ` Muchun Song
  2026-10-13 15:04     ` David Hildenbrand (Arm)
  3 siblings, 0 replies; 7+ messages in thread
From: Muchun Song @ 2026-10-07 13:50 UTC (permalink / raw)
  To: Zhao Li
  Cc: Andrew Morton, mawupeng1, Oscar Salvador, David Hildenbrand,
	linux-mm, linux-kernel, stable



> On Apr 28, 2026, at 13:30, Zhao Li <enderaoelyther@gmail.com> wrote:
> 
> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
> is set.  For a subpool with max_hpages != -1, that bumps used_hpages
> regardless of whether it returns gbl_chg = 0 (rsv slot consumed) or
> gbl_chg > 0 (used_hpages slot only).  If the allocation later fails
> before a folio is returned, the unwind must undo the used_hpages
> bump.  The old cleanup only ran for !gbl_chg, leaking used_hpages on
> the gbl_chg > 0 path.
> 
> For gbl_chg > 0 on max-only subpools (max_hpages != -1, min_hpages
> == -1), hugepage_subpool_get_pages() took only a speculative
> used_hpages slot.  Drop that slot directly under spool->lock.  In
> that configuration hugepage_subpool_put_pages() cannot restore
> rsv_hpages, so the direct decrement is the exact inverse and is
> race-free against concurrent puts.  This matches the used_hpages-only
> part of hugetlb_reserve_pages()'s out_put_pages cleanup, but
> restricts it to the max-only case where no rsv_hpages restoration is
> possible.
> 
> Mounts with min_hpages != -1 are left unchanged for now.  v2's
> approach (hugepage_subpool_put_pages() + h->resv_huge_pages++ to
> back a restored rsv_hpages slot) double-counts global backing under
> concurrent free_huge_folio() and creates phantom reservations under
> concurrent hugetlb_unreserve_pages().  Safe cleanup of that quadrant
> needs a coordinated fix across multiple call sites.
> 
> Reproduced on size=20M hugetlbfs with the faulting task in a hugetlb
> cgroup whose limit is exceeded.  Vanilla leaks 6/8 hugepages of
> subpool quota; this patch leaks 0/8.  Verified under QEMU.
> 
> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> Cc: stable@vger.kernel.org # v6.15+
> Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
> ---
> Changes in v3:
> - Replace v2's hugepage_subpool_put_pages() + h->resv_huge_pages++ on
>  the gbl_chg > 0 branch with a direct used_hpages-- under spool->lock.
> - Restrict the cleanup to (max_hpages != -1, min_hpages == -1) where
>  the direct decrement is the exact inverse of the speculative bump.
> 
> Changes in v2:
> - Skip the gbl_chg > 0 cleanup when max_hpages is unset.
> - Add hugepage_subpool_put_pages() + h->resv_huge_pages++ on the
>  gbl_chg > 0 branch.
> 
> mm/hugetlb.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f24bf49be047e..cfdeaf6394c5b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3025,13 +3025,24 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h),
>    h_cg);
> 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) {
> + 		if (!gbl_chg) {
> + 			/* Full inverse when subpool_get_pages() consumed rsv_hpages. */
> + 			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
> +			hugetlb_acct_memory(h, -gbl_reserve);
> + 		} else if (gbl_chg > 0 && spool && spool->min_hpages == -1 &&

The gbl_chg > 0 check can be dropped: negative values jump directly to
out_end_reservation, while zero is handled by the preceding if (!gbl_chg).

> +   			spool->max_hpages != -1) {
> + 			unsigned long flags;
> +
> + 			/*
> +			 * For max-only subpools, subpool_get_pages() took only a
> +			 * speculative used_hpages slot. Drop that slot directly.
> +			 */
> + 			spin_lock_irqsave(&spool->lock, flags);
> + 			if (spool->used_hpages > 0)
> + 				spool->used_hpages--;
> + 			unlock_or_release_subpool(spool, flags);

Could we use hugepage_subpool_put_pages(spool, 1) here?

With the max-only check in place, the helper cannot restore rsv_hpages, so it already
performs the required used_hpages decrement and subpool lifetime handling under the
appropriate lock.

Thanks,
Muchun

> + 		}
> }
> 
> 
> --
> 2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
                       ` (2 preceding siblings ...)
  2026-10-07 13:50     ` Muchun Song
@ 2026-10-13 15:04     ` David Hildenbrand (Arm)
  2026-10-28 15:09       ` David Hildenbrand (Arm)
  3 siblings, 1 reply; 7+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-13 15:04 UTC (permalink / raw)
  To: Zhao Li, Andrew Morton
  Cc: mawupeng1, Muchun Song, Oscar Salvador, linux-mm, linux-kernel,
	stable

On 4/28/26 13:30, Zhao Li wrote:
> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
> is set.  For a subpool with max_hpages != -1, that bumps used_hpages
> regardless of whether it returns gbl_chg = 0 (rsv slot consumed) or
> gbl_chg > 0 (used_hpages slot only).  If the allocation later fails
> before a folio is returned, the unwind must undo the used_hpages
> bump.  The old cleanup only ran for !gbl_chg, leaking used_hpages on
> the gbl_chg > 0 path.
> 
> For gbl_chg > 0 on max-only subpools (max_hpages != -1, min_hpages
> == -1), hugepage_subpool_get_pages() took only a speculative
> used_hpages slot.  Drop that slot directly under spool->lock.  In
> that configuration hugepage_subpool_put_pages() cannot restore
> rsv_hpages, so the direct decrement is the exact inverse and is
> race-free against concurrent puts.  This matches the used_hpages-only
> part of hugetlb_reserve_pages()'s out_put_pages cleanup, but
> restricts it to the max-only case where no rsv_hpages restoration is
> possible.
> 
> Mounts with min_hpages != -1 are left unchanged for now.  v2's
> approach (hugepage_subpool_put_pages() + h->resv_huge_pages++ to
> back a restored rsv_hpages slot) double-counts global backing under
> concurrent free_huge_folio() and creates phantom reservations under
> concurrent hugetlb_unreserve_pages().  Safe cleanup of that quadrant
> needs a coordinated fix across multiple call sites.
> 
> Reproduced on size=20M hugetlbfs with the faulting task in a hugetlb
> cgroup whose limit is exceeded.  Vanilla leaks 6/8 hugepages of
> subpool quota; this patch leaks 0/8.  Verified under QEMU.
> 
> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> Cc: stable@vger.kernel.org # v6.15+
> Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
> ---
> Changes in v3:
> - Replace v2's hugepage_subpool_put_pages() + h->resv_huge_pages++ on
>   the gbl_chg > 0 branch with a direct used_hpages-- under spool->lock.
> - Restrict the cleanup to (max_hpages != -1, min_hpages == -1) where
>   the direct decrement is the exact inverse of the speculative bump.
> 
> Changes in v2:
> - Skip the gbl_chg > 0 cleanup when max_hpages is unset.
> - Add hugepage_subpool_put_pages() + h->resv_huge_pages++ on the
>   gbl_chg > 0 branch.
> 
>  mm/hugetlb.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f24bf49be047e..cfdeaf6394c5b 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3025,13 +3025,24 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  		hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h),
>  						    h_cg);
>  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) {


I agree with Muchuns comment about removing the "gbl_chg > 0" check.

You could consider the following structure, which reduces indentation and avoids
touching the existing handling.

if (map_chg && !gbl_chg) {
	...
} else if (map_chg && spool && spool->min_hpages == -1 &&
	   spool->max_hpages != -1) {
	...
}

But the code would read a lot nicer as

if (map_chg && !gbl_chg) {
	...
} else if (map_chg) {
	hugepage_subpool_XXX_pages()
}

Whereby the spool checks etc are handled in there.

Muchun things that we might just be able to use hugepage_subpool_put_pages(). It
that's possible, great. Otherwise create a new helper that wraps all the logic
below.

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-10-13 15:04     ` David Hildenbrand (Arm)
@ 2026-10-28 15:09       ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 7+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-28 15:09 UTC (permalink / raw)
  To: Zhao Li, Andrew Morton
  Cc: mawupeng1, Muchun Song, Oscar Salvador, linux-mm, linux-kernel,
	stable

On 10/13/26 17:04, David Hildenbrand (Arm) wrote:
> On 4/28/26 13:30, Zhao Li wrote:
>> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
>> is set.  For a subpool with max_hpages != -1, that bumps used_hpages
>> regardless of whether it returns gbl_chg = 0 (rsv slot consumed) or
>> gbl_chg > 0 (used_hpages slot only).  If the allocation later fails
>> before a folio is returned, the unwind must undo the used_hpages
>> bump.  The old cleanup only ran for !gbl_chg, leaking used_hpages on
>> the gbl_chg > 0 path.
>>
>> For gbl_chg > 0 on max-only subpools (max_hpages != -1, min_hpages
>> == -1), hugepage_subpool_get_pages() took only a speculative
>> used_hpages slot.  Drop that slot directly under spool->lock.  In
>> that configuration hugepage_subpool_put_pages() cannot restore
>> rsv_hpages, so the direct decrement is the exact inverse and is
>> race-free against concurrent puts.  This matches the used_hpages-only
>> part of hugetlb_reserve_pages()'s out_put_pages cleanup, but
>> restricts it to the max-only case where no rsv_hpages restoration is
>> possible.
>>
>> Mounts with min_hpages != -1 are left unchanged for now.  v2's
>> approach (hugepage_subpool_put_pages() + h->resv_huge_pages++ to
>> back a restored rsv_hpages slot) double-counts global backing under
>> concurrent free_huge_folio() and creates phantom reservations under
>> concurrent hugetlb_unreserve_pages().  Safe cleanup of that quadrant
>> needs a coordinated fix across multiple call sites.
>>
>> Reproduced on size=20M hugetlbfs with the faulting task in a hugetlb
>> cgroup whose limit is exceeded.  Vanilla leaks 6/8 hugepages of
>> subpool quota; this patch leaks 0/8.  Verified under QEMU.
>>
>> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
>> Cc: stable@vger.kernel.org # v6.15+
>> Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
>> ---
>> Changes in v3:
>> - Replace v2's hugepage_subpool_put_pages() + h->resv_huge_pages++ on
>>   the gbl_chg > 0 branch with a direct used_hpages-- under spool->lock.
>> - Restrict the cleanup to (max_hpages != -1, min_hpages == -1) where
>>   the direct decrement is the exact inverse of the speculative bump.
>>
>> Changes in v2:
>> - Skip the gbl_chg > 0 cleanup when max_hpages is unset.
>> - Add hugepage_subpool_put_pages() + h->resv_huge_pages++ on the
>>   gbl_chg > 0 branch.
>>
>>  mm/hugetlb.c | 25 ++++++++++++++++++-------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index f24bf49be047e..cfdeaf6394c5b 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -3025,13 +3025,24 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>>  		hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h),
>>  						    h_cg);
>>  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) {
> 
> 
> I agree with Muchuns comment about removing the "gbl_chg > 0" check.
> 
> You could consider the following structure, which reduces indentation and avoids
> touching the existing handling.
> 
> if (map_chg && !gbl_chg) {
> 	...
> } else if (map_chg && spool && spool->min_hpages == -1 &&
> 	   spool->max_hpages != -1) {
> 	...
> }
> 
> But the code would read a lot nicer as
> 
> if (map_chg && !gbl_chg) {
> 	...
> } else if (map_chg) {
> 	hugepage_subpool_XXX_pages()
> }
> 
> Whereby the spool checks etc are handled in there.
> 
> Muchun things that we might just be able to use hugepage_subpool_put_pages(). It
> that's possible, great. Otherwise create a new helper that wraps all the logic
> below.
> 

Any update on this?

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-28 15:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260427145247.84157-2-enderaoelyther@gmail.com>
     [not found] ` <20260428030712.66256-2-enderaoelyther@gmail.com>
2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
2026-09-06  2:31     ` Andrew Morton
2026-09-08  7:12       ` Zhao Li
2026-09-28  5:26     ` Ackerley Tng
2026-10-07 13:50     ` Muchun Song
2026-10-13 15:04     ` David Hildenbrand (Arm)
2026-10-28 15:09       ` David Hildenbrand (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox