All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure
@ 2026-04-27 14:52 Catherine
  2026-04-27 15:12 ` Andrew Morton
  2026-04-28  3:07 ` [PATCH v2] " Zhao Li
  0 siblings, 2 replies; 16+ messages in thread
From: Catherine @ 2026-04-27 14:52 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
	linux-kernel, Catherine

alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
is set.  For subpools with max_hpages, that increments used_hpages even
when the returned gbl_chg is positive.

If a later hugetlb cgroup charge fails, the cleanup currently calls
hugepage_subpool_put_pages() only for !gbl_chg.  The gbl_chg > 0 path
therefore leaks one used_hpages charge per failure.

Always undo the subpool charge after a successful subpool get.  Keep the
global reservation accounting under !gbl_chg, because only that path
consumed a reservation from the subpool.

Signed-off-by: Catherine <enderaoelyther@gmail.com>
---
 mm/hugetlb.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f24bf49be..b3ad024a0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3026,12 +3026,14 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 						    h_cg);
 out_subpool_put:
 	/*
-	 * put page to subpool iff the quota of subpool's rsv_hpages is used
-	 * during hugepage_subpool_get_pages.
+	 * map_chg means hugepage_subpool_get_pages() succeeded above.
+	 * Always undo the subpool quota charge; only drop global reservation
+	 * accounting if the subpool consumed a reservation.
 	 */
-	if (map_chg && !gbl_chg) {
+	if (map_chg) {
 		gbl_reserve = hugepage_subpool_put_pages(spool, 1);
-		hugetlb_acct_memory(h, -gbl_reserve);
+		if (!gbl_chg)
+			hugetlb_acct_memory(h, -gbl_reserve);
 	}
 
 
-- 
2.50.1 (Apple Git-155)



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

* Re: [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-27 14:52 [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure Catherine
@ 2026-04-27 15:12 ` Andrew Morton
  2026-04-27 15:19   ` Catherine
  2026-04-28  3:07 ` [PATCH v2] " Zhao Li
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2026-04-27 15:12 UTC (permalink / raw)
  To: Catherine
  Cc: Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
	linux-kernel

On Mon, 27 Apr 2026 22:52:48 +0800 Catherine <enderaoelyther@gmail.com> wrote:

> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
> is set.  For subpools with max_hpages, that increments used_hpages even
> when the returned gbl_chg is positive.
> 
> If a later hugetlb cgroup charge fails, the cleanup currently calls
> hugepage_subpool_put_pages() only for !gbl_chg.  The gbl_chg > 0 path
> therefore leaks one used_hpages charge per failure.
> 
> Always undo the subpool charge after a successful subpool get.  Keep the
> global reservation accounting under !gbl_chg, because only that path
> consumed a reservation from the subpool.

Thanks.

We do prefer full, real names for kernel alterations.  Can you please
provide that?



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

* Re: [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-27 15:12 ` Andrew Morton
@ 2026-04-27 15:19   ` Catherine
  2026-04-27 21:12     ` Andrew Morton
  0 siblings, 1 reply; 16+ messages in thread
From: Catherine @ 2026-04-27 15:19 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Catherine, Muchun Song, Oscar Salvador, David Hildenbrand,
	linux-mm, linux-kernel

Hi Andrew,

My real name is Zhao Li. Please use Zhao Li <enderaoelyther@gmail.com>
for authorship and Signed-off-by.

Thanks,
Catherine


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

* Re: [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-27 15:19   ` Catherine
@ 2026-04-27 21:12     ` Andrew Morton
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2026-04-27 21:12 UTC (permalink / raw)
  To: Catherine
  Cc: Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
	linux-kernel

On Mon, 27 Apr 2026 23:19:35 +0800 Catherine <enderaoelyther@gmail.com> wrote:

> Hi Andrew,
> 
> My real name is Zhao Li. Please use Zhao Li <enderaoelyther@gmail.com>
> for authorship and Signed-off-by.

Thanks.

AI review might have found an accounting imbalance:
	https://sashiko.dev/#/patchset/20260427145247.84157-2-enderaoelyther@gmail.com


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

* [PATCH v2] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-27 14:52 [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure Catherine
  2026-04-27 15:12 ` Andrew Morton
@ 2026-04-28  3:07 ` Zhao Li
  2026-04-28  9:08   ` Oscar Salvador
  2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
  1 sibling, 2 replies; 16+ messages in thread
From: Zhao Li @ 2026-04-28  3:07 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Muchun Song, Oscar Salvador, David Hildenbrand, linux-mm,
	linux-kernel

alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
is set.  For subpools with max_hpages, that increments used_hpages.
If the later hugetlb cgroup charge fails, the unwind must undo that
charge even when gbl_chg > 0.

hugepage_subpool_put_pages() can also restore rsv_hpages if concurrent
frees move used_hpages below min_hpages between the get and put.  When
that happens on the gbl_chg > 0 path, restore the matching global
reservation as well.

Skip the gbl_chg > 0 put when max_hpages is unset.  For a min_size-only
subpool, get_pages() did not change subpool state and put_pages() would
create a false reservation.

Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
---
Changes in v2:
- Handle rsv_hpages restoration when racing frees cross min_hpages.
- Skip gbl_chg > 0 put_pages() when max_hpages is unset.

 mm/hugetlb.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f24bf49be047e..4065d66fdcb5c 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3026,12 +3026,23 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 						    h_cg);
 out_subpool_put:
 	/*
-	 * put page to subpool iff the quota of subpool's rsv_hpages is used
-	 * during hugepage_subpool_get_pages.
+	 * map_chg means hugepage_subpool_get_pages() succeeded above.
+	 * If max_hpages accounting was touched, undo it.  If racing frees
+	 * moved the subpool below min_hpages, the put path may restore a
+	 * subpool reservation.  Restore the matching global reservation too.
 	 */
-	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) {
+			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
+			hugetlb_acct_memory(h, -gbl_reserve);
+		} else if (spool && spool->max_hpages != -1) {
+			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
+			if (!gbl_reserve) {
+				spin_lock_irq(&hugetlb_lock);
+				h->resv_huge_pages++;
+				spin_unlock_irq(&hugetlb_lock);
+			}
+		}
 	}


--
2.50.1 (Apple Git-155)


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

* Re: [PATCH v2] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-28  3:07 ` [PATCH v2] " Zhao Li
@ 2026-04-28  9:08   ` Oscar Salvador
  2026-04-28 11:30     ` Lance Yang
  2026-04-28 11:41     ` Zhao Li
  2026-04-28 11:30   ` [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure Zhao Li
  1 sibling, 2 replies; 16+ messages in thread
From: Oscar Salvador @ 2026-04-28  9:08 UTC (permalink / raw)
  To: Zhao Li
  Cc: Andrew Morton, Muchun Song, David Hildenbrand, linux-mm,
	linux-kernel

On Tue, Apr 28, 2026 at 11:07:13AM +0800, Zhao Li wrote:
> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
> is set.  For subpools with max_hpages, that increments used_hpages.
> If the later hugetlb cgroup charge fails, the unwind must undo that
> charge even when gbl_chg > 0.

I found that last sentence misleading, because we do not really care
about hugetlb cgroup charge/uncharge (besides that being of the reasons
we end up on error path) but rather the fact that we fiddle with
subpool->used_hpages and we need to undo that when we rollback.

> hugepage_subpool_put_pages() can also restore rsv_hpages if concurrent
> frees move used_hpages below min_hpages between the get and put.  When
> that happens on the gbl_chg > 0 path, restore the matching global
> reservation as well.

Well, that does not quite explain the problem I think, at least not clear enough?
So the problem at hand (IIUC) is that

1) if we took a global reservation
2) and concurrent hugepage_subpool_put_pages operations made
   used_hpages be below min_hpages, and so subpool's reservation
   was incremented, but we need to increment the global one as well
   otherwise the next time we pull a page from the spool,
   global resv_huge_pages will be inbalanced


> Skip the gbl_chg > 0 put when max_hpages is unset.  For a min_size-only
> subpool, get_pages() did not change subpool state and put_pages() would
> create a false reservation.
> 
> Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
> ---
> Changes in v2:
> - Handle rsv_hpages restoration when racing frees cross min_hpages.
> - Skip gbl_chg > 0 put_pages() when max_hpages is unset.
> 
>  mm/hugetlb.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f24bf49be047e..4065d66fdcb5c 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3026,12 +3026,23 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  						    h_cg);
>  out_subpool_put:
>  	/*
> -	 * put page to subpool iff the quota of subpool's rsv_hpages is used
> -	 * during hugepage_subpool_get_pages.
> +	 * map_chg means hugepage_subpool_get_pages() succeeded above.
> +	 * If max_hpages accounting was touched, undo it.  If racing frees
> +	 * moved the subpool below min_hpages, the put path may restore a
> +	 * subpool reservation.  Restore the matching global reservation too.

I would split the comment in two parts and place them within the block
they belong, otherwise it sounds confusing.
And maybe elaborate a little bit more.

Subpools, reservations and hugetlb make a very head-spinning situation, so let
us make our life easier.


-- 
Oscar Salvador
SUSE Labs


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

* [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
  2026-04-28  3:07 ` [PATCH v2] " Zhao Li
  2026-04-28  9:08   ` Oscar Salvador
@ 2026-04-28 11:30   ` Zhao Li
  2026-09-06  2:31     ` Andrew Morton
                       ` (3 more replies)
  1 sibling, 4 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH v2] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-28  9:08   ` Oscar Salvador
@ 2026-04-28 11:30     ` Lance Yang
  2026-04-28 11:41       ` Zhao Li
  2026-04-28 11:41     ` Zhao Li
  1 sibling, 1 reply; 16+ messages in thread
From: Lance Yang @ 2026-04-28 11:30 UTC (permalink / raw)
  To: osalvador, enderaoelyther
  Cc: akpm, muchun.song, david, linux-mm, linux-kernel, Lance Yang


On Tue, Apr 28, 2026 at 11:08:04AM +0200, Oscar Salvador wrote:
>On Tue, Apr 28, 2026 at 11:07:13AM +0800, Zhao Li wrote:
>> alloc_hugetlb_folio() calls hugepage_subpool_get_pages() when map_chg
>> is set.  For subpools with max_hpages, that increments used_hpages.
>> If the later hugetlb cgroup charge fails, the unwind must undo that
>> charge even when gbl_chg > 0.
>
>I found that last sentence misleading, because we do not really care
>about hugetlb cgroup charge/uncharge (besides that being of the reasons
>we end up on error path) but rather the fact that we fiddle with
>subpool->used_hpages and we need to undo that when we rollback.
>
>> hugepage_subpool_put_pages() can also restore rsv_hpages if concurrent
>> frees move used_hpages below min_hpages between the get and put.  When
>> that happens on the gbl_chg > 0 path, restore the matching global
>> reservation as well.
>
>Well, that does not quite explain the problem I think, at least not clear enough?
>So the problem at hand (IIUC) is that
>
>1) if we took a global reservation
>2) and concurrent hugepage_subpool_put_pages operations made
>   used_hpages be below min_hpages, and so subpool's reservation
>   was incremented, but we need to increment the global one as well
>   otherwise the next time we pull a page from the spool,
>   global resv_huge_pages will be inbalanced
>
>
>> Skip the gbl_chg > 0 put when max_hpages is unset.  For a min_size-only
>> subpool, get_pages() did not change subpool state and put_pages() would
>> create a false reservation.
>> 
>> Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
>> ---
>> Changes in v2:
>> - Handle rsv_hpages restoration when racing frees cross min_hpages.
>> - Skip gbl_chg > 0 put_pages() when max_hpages is unset.
>> 
>>  mm/hugetlb.c | 21 ++++++++++++++++-----
>>  1 file changed, 16 insertions(+), 5 deletions(-)
>> 
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index f24bf49be047e..4065d66fdcb5c 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -3026,12 +3026,23 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>>  						    h_cg);
>>  out_subpool_put:
>>  	/*
>> -	 * put page to subpool iff the quota of subpool's rsv_hpages is used
>> -	 * during hugepage_subpool_get_pages.
>> +	 * map_chg means hugepage_subpool_get_pages() succeeded above.
>> +	 * If max_hpages accounting was touched, undo it.  If racing frees
>> +	 * moved the subpool below min_hpages, the put path may restore a
>> +	 * subpool reservation.  Restore the matching global reservation too.
>
>I would split the comment in two parts and place them within the block
>they belong, otherwise it sounds confusing.
>And maybe elaborate a little bit more.
>
>Subpools, reservations and hugetlb make a very head-spinning situation, so let
>us make our life easier.

Yep, the comment is confusing to me as well ...

+	if (map_chg) {
+		if (!gbl_chg) {
+			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
+			hugetlb_acct_memory(h, -gbl_reserve);
+		} else if (spool && spool->max_hpages != -1) {
+			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
+			if (!gbl_reserve) {
+				spin_lock_irq(&hugetlb_lock);
+				h->resv_huge_pages++;
+				spin_unlock_irq(&hugetlb_lock);
+			}
+		}
 	}

IIUC, there are three cases:

1) !gbl_chg

hugepage_subpool_get_pages() consumed one reservation from
spool->rsv_hpages.  So the error path needs to call
hugepage_subpool_put_pages() and then drop the matching global
reservation with hugetlb_acct_memory().

2) gbl_chg > 0 && spool->max_hpages != -1

hugepage_subpool_get_pages() did not consume spool->rsv_hpages, but it
did increment spool->used_hpages after the max_hpages check passed.  So
the error path still needs to call hugepage_subpool_put_pages() to undo
that.

If hugepage_subpool_put_pages() returns 0 here, it restored one
reservation in spool->rsv_hpages, so we also need to increment
h->resv_huge_pages.

3) gbl_chg > 0 && spool->max_hpages == -1

hugepage_subpool_get_pages() did not change spool->rsv_hpages or
spool->used_hpages, so the error path should not call
hugepage_subpool_put_pages(), otherwise it could create a false
reservation in spool->rsv_hpages.

Hopefully I didn't miss anything :)
Lance


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

* Re: [PATCH v2] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-28  9:08   ` Oscar Salvador
  2026-04-28 11:30     ` Lance Yang
@ 2026-04-28 11:41     ` Zhao Li
  1 sibling, 0 replies; 16+ messages in thread
From: Zhao Li @ 2026-04-28 11:41 UTC (permalink / raw)
  To: Oscar Salvador
  Cc: Zhao Li, Andrew Morton, Muchun Song, David Hildenbrand,
	Lance Yang, linux-mm, linux-kernel

On Tue, Apr 28, 2026 at 11:08:04AM +0200, Oscar Salvador wrote:
> I found that last sentence misleading, because we do not really
> care about hugetlb cgroup charge/uncharge (besides that being of
> the reasons we end up on error path) but rather the fact that we
> fiddle with subpool->used_hpages and we need to undo that when we
> rollback.

Agreed - reframed in v3.  The commit body now states the bug as
the unwind missing the used_hpages rollback, without pinning it to
the cgroup-charge case, and the subject is narrowed to "fix
max-only subpool accounting on alloc_hugetlb_folio failure".

> Well, that does not quite explain the problem I think, at least
> not clear enough?  [...]

Fair - that explanation got tangled because v2's design itself was
trying to compensate for racing min crossings.  v3 sidesteps it
entirely: the gbl_chg > 0 cleanup is now restricted to
(max_hpages != -1, min_hpages == -1).  In that configuration
hugepage_subpool_put_pages()'s min-restoration branch is dead, so a
direct used_hpages-- under spool->lock is the exact inverse of the
speculative bump - no h->resv_huge_pages++ needed, no rsv_hpages
publication, no racing-put reasoning.

Mounts with min_hpages != -1 are left at v1 behaviour for now.
That quadrant has an inherited race that also exists at
hugetlb_reserve_pages()'s out_put_pages cleanup, so a coordinated
fix belongs in a separate RFC rather than this stable backport.

> I would split the comment in two parts and place them within the
> block they belong, otherwise it sounds confusing.
>
> Subpools, reservations and hugetlb make a very head-spinning
> situation, so let us make our life easier.

Done - one short comment per branch placed inside the relevant
code block in v3.  Hopefully easier to follow now.

v3:
  https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@gmail.com/

Thanks for the review.

--
Zhao Li


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

* Re: [PATCH v2] mm/hugetlb: fix subpool accounting after cgroup charge failure
  2026-04-28 11:30     ` Lance Yang
@ 2026-04-28 11:41       ` Zhao Li
  0 siblings, 0 replies; 16+ messages in thread
From: Zhao Li @ 2026-04-28 11:41 UTC (permalink / raw)
  To: Lance Yang
  Cc: Zhao Li, Oscar Salvador, Andrew Morton, Muchun Song,
	David Hildenbrand, linux-mm, linux-kernel

On Tue, Apr 28, 2026 at 07:30:59PM +0800, Lance Yang wrote:
> IIUC, there are three cases:
> [...]
> 2) gbl_chg > 0 && spool->max_hpages != -1
> [...]
> If hugepage_subpool_put_pages() returns 0 here, it restored one
> reservation in spool->rsv_hpages, so we also need to increment
> h->resv_huge_pages.

Thanks for working through the three cases - that's a clean
breakdown and matches what v2 was trying to do.  We ran into trouble
on case 2 specifically: the h->resv_huge_pages++ on the
put-returned-0 path looked right in isolation but turns out to be
unsafe once you put it next to concurrent hugetlb_unreserve_pages()
or free_huge_folio().  Two reachable orderings break it:

 * free_huge_folio() with HPageRestoreReserve set already does
   h->resv_huge_pages++ on its own.  v2's bump on the gbl_chg > 0
   cleanup double-counts against that.

 * hugetlb_unreserve_pages() does hugetlb_acct_memory(h, -X) which
   subtracts from h->resv_huge_pages and may also return surplus
   backing.  v2's bump then leaves rsv_hpages backed by no
   h->resv_huge_pages - a phantom reservation that the next
   subpool_get_pages() consumes without real backing.

v3 ended up going a different way: the gbl_chg > 0 cleanup is now
restricted to (max_hpages != -1, min_hpages == -1).  In that
configuration hugepage_subpool_put_pages()'s min-restoration branch
is dead, so a direct used_hpages-- under spool->lock is the exact
inverse of the speculative bump - no put_pages(), no
h->resv_huge_pages++, no concurrent-races to reason about.

Your case 3 (max_hpages == -1) is unchanged: cleanup is a no-op,
because get_pages() didn't touch any subpool field.

Mounts with min_hpages != -1 are left at v1 behaviour for now.  That
quadrant has an inherited race that also exists at
hugetlb_reserve_pages()'s out_put_pages cleanup; a coordinated fix
belongs in a separate RFC rather than this stable backport.

v3:
  https://lore.kernel.org/linux-mm/20260428113037.88766-2-enderaoelyther@gmail.com/

Thanks for the review.

--
Zhao Li


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

* Re: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
@ 2026-05-02  8:58 kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2026-05-02  8:58 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20260428113037.88766-2-enderaoelyther@gmail.com>
References: <20260428113037.88766-2-enderaoelyther@gmail.com>
TO: Zhao Li <enderaoelyther@gmail.com>
TO: Andrew Morton <akpm@linux-foundation.org>
CC: Linux Memory Management List <linux-mm@kvack.org>
CC: mawupeng1@huawei.com
CC: Zhao Li <enderaoelyther@gmail.com>
CC: Muchun Song <muchun.song@linux.dev>
CC: Oscar Salvador <osalvador@suse.de>
CC: David Hildenbrand <david@kernel.org>
CC: linux-kernel@vger.kernel.org
CC: stable@vger.kernel.org

Hi Zhao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on linus/master]
[also build test WARNING on v7.1-rc1 next-20260430]
[cannot apply to akpm-mm/mm-everything]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zhao-Li/mm-hugetlb-fix-max-only-subpool-accounting-on-alloc_hugetlb_folio-failure/20260429-135834
base:   linus/master
patch link:    https://lore.kernel.org/r/20260428113037.88766-2-enderaoelyther%40gmail.com
patch subject: [PATCH v3] mm/hugetlb: fix max-only subpool accounting on alloc_hugetlb_folio failure
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: powerpc64-randconfig-r071-20260501 (https://download.01.org/0day-ci/archive/20260502/202605021600.IYZD9zHl-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 5bac06718f502014fade905512f1d26d578a18f3)
smatch: v0.5.0-9065-ge9cc34fd

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202605021600.IYZD9zHl-lkp@intel.com/

New smatch warnings:
mm/hugetlb.c:3041 alloc_hugetlb_folio() warn: mixing irq and irqsave

Old smatch warnings:
arch/powerpc/include/asm/mmu.h:223 early_mmu_has_feature() warn: bitwise AND condition is false here
arch/powerpc/include/asm/book3s/64/pgtable.h:678 pte_swp_soft_dirty() warn: bitwise AND condition is false here

vim +3041 mm/hugetlb.c

923682a0dd57065 Peter Xu         2025-01-07  2864  
30cef82bc6e8975 Peter Xu         2025-01-07  2865  /*
30cef82bc6e8975 Peter Xu         2025-01-07  2866   * NOTE! "cow_from_owner" represents a very hacky usage only used in CoW
30cef82bc6e8975 Peter Xu         2025-01-07  2867   * faults of hugetlb private mappings on top of a non-page-cache folio (in
30cef82bc6e8975 Peter Xu         2025-01-07  2868   * which case even if there's a private vma resv map it won't cover such
30cef82bc6e8975 Peter Xu         2025-01-07  2869   * allocation).  New call sites should (probably) never set it to true!!
30cef82bc6e8975 Peter Xu         2025-01-07  2870   * When it's set, the allocation will bypass all vma level reservations.
30cef82bc6e8975 Peter Xu         2025-01-07  2871   */
d0ce0e47b323a8d Sidhartha Kumar  2023-01-25  2872  struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
30cef82bc6e8975 Peter Xu         2025-01-07  2873  				    unsigned long addr, bool cow_from_owner)
^1da177e4c3f415 Linus Torvalds   2005-04-16  2874  {
90481622d75715b David Gibson     2012-03-21  2875  	struct hugepage_subpool *spool = subpool_vma(vma);
a5516438959d90b Andi Kleen       2008-07-23  2876  	struct hstate *h = hstate_vma(vma);
d4ab0316cc33aee Sidhartha Kumar  2022-11-01  2877  	struct folio *folio;
a833a693a490ecf Wupeng Ma        2025-04-10  2878  	long retval, gbl_chg, gbl_reserve;
923682a0dd57065 Peter Xu         2025-01-07  2879  	map_chg_state map_chg;
991135774c0e05a Joshua Hahn      2024-12-11  2880  	int ret, idx;
d0ce0e47b323a8d Sidhartha Kumar  2023-01-25  2881  	struct hugetlb_cgroup *h_cg = NULL;
8cba9576df601c3 Nhat Pham        2023-10-06  2882  	gfp_t gfp = htlb_alloc_mask(h) | __GFP_RETRY_MAYFAIL;
8cba9576df601c3 Nhat Pham        2023-10-06  2883  
6d76dcf40405144 Aneesh Kumar K.V 2012-07-31  2884  	idx = hstate_index(h);
923682a0dd57065 Peter Xu         2025-01-07  2885  
923682a0dd57065 Peter Xu         2025-01-07  2886  	/* Whether we need a separate per-vma reservation? */
923682a0dd57065 Peter Xu         2025-01-07  2887  	if (cow_from_owner) {
923682a0dd57065 Peter Xu         2025-01-07  2888  		/*
923682a0dd57065 Peter Xu         2025-01-07  2889  		 * Special case!  Since it's a CoW on top of a reserved
923682a0dd57065 Peter Xu         2025-01-07  2890  		 * page, the private resv map doesn't count.  So it cannot
923682a0dd57065 Peter Xu         2025-01-07  2891  		 * consume the per-vma resv map even if it's reserved.
923682a0dd57065 Peter Xu         2025-01-07  2892  		 */
923682a0dd57065 Peter Xu         2025-01-07  2893  		map_chg = MAP_CHG_ENFORCED;
923682a0dd57065 Peter Xu         2025-01-07  2894  	} else {
a1e78772d72b261 Mel Gorman       2008-07-23  2895  		/*
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2896  		 * Examine the region/reserve map to determine if the process
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2897  		 * has a reservation for the page to be allocated.  A return
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2898  		 * code of zero indicates a reservation exists (no change).
a1e78772d72b261 Mel Gorman       2008-07-23  2899  		 */
923682a0dd57065 Peter Xu         2025-01-07  2900  		retval = vma_needs_reservation(h, vma, addr);
923682a0dd57065 Peter Xu         2025-01-07  2901  		if (retval < 0)
76dcee75c1aff61 Aneesh Kumar K.V 2012-07-31  2902  			return ERR_PTR(-ENOMEM);
923682a0dd57065 Peter Xu         2025-01-07  2903  		map_chg = retval ? MAP_CHG_NEEDED : MAP_CHG_REUSE;
923682a0dd57065 Peter Xu         2025-01-07  2904  	}
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2905  
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2906  	/*
923682a0dd57065 Peter Xu         2025-01-07  2907  	 * Whether we need a separate global reservation?
923682a0dd57065 Peter Xu         2025-01-07  2908  	 *
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2909  	 * Processes that did not create the mapping will have no
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2910  	 * reserves as indicated by the region/reserve map. Check
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2911  	 * that the allocation will not exceed the subpool limit.
923682a0dd57065 Peter Xu         2025-01-07  2912  	 * Or if it can get one from the pool reservation directly.
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2913  	 */
923682a0dd57065 Peter Xu         2025-01-07  2914  	if (map_chg) {
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2915  		gbl_chg = hugepage_subpool_get_pages(spool, 1);
8cba9576df601c3 Nhat Pham        2023-10-06  2916  		if (gbl_chg < 0)
8cba9576df601c3 Nhat Pham        2023-10-06  2917  			goto out_end_reservation;
923682a0dd57065 Peter Xu         2025-01-07  2918  	} else {
923682a0dd57065 Peter Xu         2025-01-07  2919  		/*
923682a0dd57065 Peter Xu         2025-01-07  2920  		 * If we have the vma reservation ready, no need for extra
923682a0dd57065 Peter Xu         2025-01-07  2921  		 * global reservation.
923682a0dd57065 Peter Xu         2025-01-07  2922  		 */
923682a0dd57065 Peter Xu         2025-01-07  2923  		gbl_chg = 0;
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2924  	}
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2925  
923682a0dd57065 Peter Xu         2025-01-07  2926  	/*
923682a0dd57065 Peter Xu         2025-01-07  2927  	 * If this allocation is not consuming a per-vma reservation,
923682a0dd57065 Peter Xu         2025-01-07  2928  	 * charge the hugetlb cgroup now.
08cf9faf7558020 Mina Almasry     2020-04-01  2929  	 */
923682a0dd57065 Peter Xu         2025-01-07  2930  	if (map_chg) {
08cf9faf7558020 Mina Almasry     2020-04-01  2931  		ret = hugetlb_cgroup_charge_cgroup_rsvd(
08cf9faf7558020 Mina Almasry     2020-04-01  2932  			idx, pages_per_huge_page(h), &h_cg);
8f34af6f93aee88 Jianyu Zhan      2014-06-04  2933  		if (ret)
8f34af6f93aee88 Jianyu Zhan      2014-06-04  2934  			goto out_subpool_put;
08cf9faf7558020 Mina Almasry     2020-04-01  2935  	}
08cf9faf7558020 Mina Almasry     2020-04-01  2936  
08cf9faf7558020 Mina Almasry     2020-04-01  2937  	ret = hugetlb_cgroup_charge_cgroup(idx, pages_per_huge_page(h), &h_cg);
08cf9faf7558020 Mina Almasry     2020-04-01  2938  	if (ret)
08cf9faf7558020 Mina Almasry     2020-04-01  2939  		goto out_uncharge_cgroup_reservation;
8f34af6f93aee88 Jianyu Zhan      2014-06-04  2940  
db71ef79b59bb2e Mike Kravetz     2021-05-04  2941  	spin_lock_irq(&hugetlb_lock);
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2942  	/*
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2943  	 * glb_chg is passed to indicate whether or not a page must be taken
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2944  	 * from the global free pool (global change).  gbl_chg == 0 indicates
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2945  	 * a reservation exists for the allocation.
d85f69b0b533ec6 Mike Kravetz     2015-09-08  2946  	 */
58db7c5fbe7daa4 Peter Xu         2025-01-07  2947  	folio = dequeue_hugetlb_folio_vma(h, vma, addr, gbl_chg);
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2948  	if (!folio) {
db71ef79b59bb2e Mike Kravetz     2021-05-04  2949  		spin_unlock_irq(&hugetlb_lock);
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2950  		folio = alloc_buddy_hugetlb_folio_with_mpol(h, vma, addr);
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2951  		if (!folio)
8f34af6f93aee88 Jianyu Zhan      2014-06-04  2952  			goto out_uncharge_cgroup;
12df140f0bdfae5 Rik van Riel     2022-10-17  2953  		spin_lock_irq(&hugetlb_lock);
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2954  		list_add(&folio->lru, &h->hugepage_activelist);
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2955  		folio_ref_unfreeze(folio, 1);
81a6fcae3ff3f6a Joonsoo Kim      2013-09-11  2956  		/* Fall through */
68842c9b94560e6 Ken Chen         2008-01-14  2957  	}
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2958  
f931af2e41ab406 Peter Xu         2025-01-07  2959  	/*
f931af2e41ab406 Peter Xu         2025-01-07  2960  	 * Either dequeued or buddy-allocated folio needs to add special
f931af2e41ab406 Peter Xu         2025-01-07  2961  	 * mark to the folio when it consumes a global reservation.
f931af2e41ab406 Peter Xu         2025-01-07  2962  	 */
f931af2e41ab406 Peter Xu         2025-01-07  2963  	if (!gbl_chg) {
f931af2e41ab406 Peter Xu         2025-01-07  2964  		folio_set_hugetlb_restore_reserve(folio);
f931af2e41ab406 Peter Xu         2025-01-07  2965  		h->resv_huge_pages--;
f931af2e41ab406 Peter Xu         2025-01-07  2966  	}
f931af2e41ab406 Peter Xu         2025-01-07  2967  
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2968  	hugetlb_cgroup_commit_charge(idx, pages_per_huge_page(h), h_cg, folio);
08cf9faf7558020 Mina Almasry     2020-04-01  2969  	/* If allocation is not consuming a reservation, also store the
08cf9faf7558020 Mina Almasry     2020-04-01  2970  	 * hugetlb_cgroup pointer on the page.
08cf9faf7558020 Mina Almasry     2020-04-01  2971  	 */
923682a0dd57065 Peter Xu         2025-01-07  2972  	if (map_chg) {
08cf9faf7558020 Mina Almasry     2020-04-01  2973  		hugetlb_cgroup_commit_charge_rsvd(idx, pages_per_huge_page(h),
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2974  						  h_cg, folio);
08cf9faf7558020 Mina Almasry     2020-04-01  2975  	}
08cf9faf7558020 Mina Almasry     2020-04-01  2976  
db71ef79b59bb2e Mike Kravetz     2021-05-04  2977  	spin_unlock_irq(&hugetlb_lock);
7893d1d505d59db Adam Litke       2007-10-16  2978  
ff7d853b0313023 Sidhartha Kumar  2023-01-13  2979  	hugetlb_set_folio_subpool(folio, spool);
a1e78772d72b261 Mel Gorman       2008-07-23  2980  
923682a0dd57065 Peter Xu         2025-01-07  2981  	if (map_chg != MAP_CHG_ENFORCED) {
923682a0dd57065 Peter Xu         2025-01-07  2982  		/* commit() is only needed if the map_chg is not enforced */
923682a0dd57065 Peter Xu         2025-01-07  2983  		retval = vma_commit_reservation(h, vma, addr);
33039678c8da813 Mike Kravetz     2015-06-24  2984  		/*
923682a0dd57065 Peter Xu         2025-01-07  2985  		 * Check for possible race conditions. When it happens..
33039678c8da813 Mike Kravetz     2015-06-24  2986  		 * The page was added to the reservation map between
33039678c8da813 Mike Kravetz     2015-06-24  2987  		 * vma_needs_reservation and vma_commit_reservation.
33039678c8da813 Mike Kravetz     2015-06-24  2988  		 * This indicates a race with hugetlb_reserve_pages.
33039678c8da813 Mike Kravetz     2015-06-24  2989  		 * Adjust for the subpool count incremented above AND
33039678c8da813 Mike Kravetz     2015-06-24  2990  		 * in hugetlb_reserve_pages for the same page.	Also,
33039678c8da813 Mike Kravetz     2015-06-24  2991  		 * the reservation count added in hugetlb_reserve_pages
33039678c8da813 Mike Kravetz     2015-06-24  2992  		 * no longer applies.
33039678c8da813 Mike Kravetz     2015-06-24  2993  		 */
923682a0dd57065 Peter Xu         2025-01-07  2994  		if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) {
33039678c8da813 Mike Kravetz     2015-06-24  2995  			long rsv_adjust;
33039678c8da813 Mike Kravetz     2015-06-24  2996  
33039678c8da813 Mike Kravetz     2015-06-24  2997  			rsv_adjust = hugepage_subpool_put_pages(spool, 1);
33039678c8da813 Mike Kravetz     2015-06-24  2998  			hugetlb_acct_memory(h, -rsv_adjust);
b76b46902c2d039 Peter Xu         2024-04-17  2999  			spin_lock_irq(&hugetlb_lock);
923682a0dd57065 Peter Xu         2025-01-07  3000  			hugetlb_cgroup_uncharge_folio_rsvd(
a1c655f55444156 Joshua Hahn      2026-01-16  3001  			    hstate_index(h), pages_per_huge_page(h), folio);
b76b46902c2d039 Peter Xu         2024-04-17  3002  			spin_unlock_irq(&hugetlb_lock);
b76b46902c2d039 Peter Xu         2024-04-17  3003  		}
33039678c8da813 Mike Kravetz     2015-06-24  3004  	}
8cba9576df601c3 Nhat Pham        2023-10-06  3005  
991135774c0e05a Joshua Hahn      2024-12-11  3006  	ret = mem_cgroup_charge_hugetlb(folio, gfp);
991135774c0e05a Joshua Hahn      2024-12-11  3007  	/*
991135774c0e05a Joshua Hahn      2024-12-11  3008  	 * Unconditionally increment NR_HUGETLB here. If it turns out that
991135774c0e05a Joshua Hahn      2024-12-11  3009  	 * mem_cgroup_charge_hugetlb failed, then immediately free the page and
991135774c0e05a Joshua Hahn      2024-12-11  3010  	 * decrement NR_HUGETLB.
991135774c0e05a Joshua Hahn      2024-12-11  3011  	 */
05d4532b60e3e6e Joshua Hahn      2024-11-01  3012  	lruvec_stat_mod_folio(folio, NR_HUGETLB, pages_per_huge_page(h));
991135774c0e05a Joshua Hahn      2024-12-11  3013  
991135774c0e05a Joshua Hahn      2024-12-11  3014  	if (ret == -ENOMEM) {
991135774c0e05a Joshua Hahn      2024-12-11  3015  		free_huge_folio(folio);
991135774c0e05a Joshua Hahn      2024-12-11  3016  		return ERR_PTR(-ENOMEM);
991135774c0e05a Joshua Hahn      2024-12-11  3017  	}
8cba9576df601c3 Nhat Pham        2023-10-06  3018  
d0ce0e47b323a8d Sidhartha Kumar  2023-01-25  3019  	return folio;
8f34af6f93aee88 Jianyu Zhan      2014-06-04  3020  
8f34af6f93aee88 Jianyu Zhan      2014-06-04  3021  out_uncharge_cgroup:
8f34af6f93aee88 Jianyu Zhan      2014-06-04  3022  	hugetlb_cgroup_uncharge_cgroup(idx, pages_per_huge_page(h), h_cg);
08cf9faf7558020 Mina Almasry     2020-04-01  3023  out_uncharge_cgroup_reservation:
923682a0dd57065 Peter Xu         2025-01-07  3024  	if (map_chg)
08cf9faf7558020 Mina Almasry     2020-04-01  3025  		hugetlb_cgroup_uncharge_cgroup_rsvd(idx, pages_per_huge_page(h),
08cf9faf7558020 Mina Almasry     2020-04-01  3026  						    h_cg);
8f34af6f93aee88 Jianyu Zhan      2014-06-04  3027  out_subpool_put:
f7c7f89890cf4f3 Zhao Li          2026-04-28  3028  	if (map_chg) {
f7c7f89890cf4f3 Zhao Li          2026-04-28  3029  		if (!gbl_chg) {
f7c7f89890cf4f3 Zhao Li          2026-04-28  3030  			/* Full inverse when subpool_get_pages() consumed rsv_hpages. */
a833a693a490ecf Wupeng Ma        2025-04-10  3031  			gbl_reserve = hugepage_subpool_put_pages(spool, 1);
a833a693a490ecf Wupeng Ma        2025-04-10  3032  			hugetlb_acct_memory(h, -gbl_reserve);
f7c7f89890cf4f3 Zhao Li          2026-04-28  3033  		} else if (gbl_chg > 0 && spool && spool->min_hpages == -1 &&
f7c7f89890cf4f3 Zhao Li          2026-04-28  3034  			   spool->max_hpages != -1) {
f7c7f89890cf4f3 Zhao Li          2026-04-28  3035  			unsigned long flags;
f7c7f89890cf4f3 Zhao Li          2026-04-28  3036  
f7c7f89890cf4f3 Zhao Li          2026-04-28  3037  			/*
f7c7f89890cf4f3 Zhao Li          2026-04-28  3038  			 * For max-only subpools, subpool_get_pages() took only a
f7c7f89890cf4f3 Zhao Li          2026-04-28  3039  			 * speculative used_hpages slot. Drop that slot directly.
f7c7f89890cf4f3 Zhao Li          2026-04-28  3040  			 */
f7c7f89890cf4f3 Zhao Li          2026-04-28 @3041  			spin_lock_irqsave(&spool->lock, flags);
f7c7f89890cf4f3 Zhao Li          2026-04-28  3042  			if (spool->used_hpages > 0)
f7c7f89890cf4f3 Zhao Li          2026-04-28  3043  				spool->used_hpages--;
f7c7f89890cf4f3 Zhao Li          2026-04-28  3044  			unlock_or_release_subpool(spool, flags);
f7c7f89890cf4f3 Zhao Li          2026-04-28  3045  		}
a833a693a490ecf Wupeng Ma        2025-04-10  3046  	}
a833a693a490ecf Wupeng Ma        2025-04-10  3047  
a833a693a490ecf Wupeng Ma        2025-04-10  3048  
8cba9576df601c3 Nhat Pham        2023-10-06  3049  out_end_reservation:
923682a0dd57065 Peter Xu         2025-01-07  3050  	if (map_chg != MAP_CHG_ENFORCED)
feba16e25a57808 Mike Kravetz     2015-09-08  3051  		vma_end_reservation(h, vma, addr);
8f34af6f93aee88 Jianyu Zhan      2014-06-04  3052  	return ERR_PTR(-ENOSPC);
^1da177e4c3f415 Linus Torvalds   2005-04-16  3053  }
b45b5bd65f668a6 David Gibson     2006-03-22  3054  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ 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-23  6:57     ` Karl Mehltretter
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ 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-23  6:57     ` Karl Mehltretter
  2026-09-28  5:26     ` Ackerley Tng
  2026-10-07 13:50     ` Muchun Song
  3 siblings, 0 replies; 16+ messages in thread
From: Karl Mehltretter @ 2026-09-23  6:57 UTC (permalink / raw)
  To: Zhao Li
  Cc: Karl Mehltretter, Andrew Morton, Muchun Song, Oscar Salvador,
	David Hildenbrand, Ma Wupeng, Jinmeng Zhou, linux-mm,
	linux-kernel

On Tue, Apr 28, 2026, Zhao Li wrote:
> Mounts with min_hpages != -1 are left unchanged for now.

In case it helps with that follow-up, here is a small reproducer for the
min_size case. It needs no hugetlb cgroup.

The pool has two huge pages and no overcommit, and the mount is
size=8M,min_size=4M. A test maps a 4-page file MAP_SHARED | MAP_NORESERVE,
touches the first N pages, then unmaps and removes the file. Five rounds
on one mount:

  N   faults ok / SIGBUS   statfs free / blocks   HugePages_Rsvd
  -                                               2 (after mount)
  2   2 / 0                4 / 4                  2
  2   2 / 0                4 / 4                  2
  4   2 / 2                2 / 4                  0
  2   2 / 0                2 / 4                  0
  2   2 / 0                2 / 4                  0

The two SIGBUS faults keep their used_hpages charge. Half of the quota
stays gone with no file on the mount. used_hpages then no longer drops
below min_hpages, so hugepage_subpool_put_pages() never restores
rsv_hpages, and the mount has lost its minimum reservation for good.

The out_subpool_put: path skips this case on purpose, as the commit
message says.

Same result on v7.3-rc4 (f0100363d8c3), on v7.3-rc4 with this patch, and
on next-20260922. That tree also has 6f8b22e85eac ("mm/hugetlb: fix subpool
minimum reservation rollback"), which fixes the hugetlb_reserve_pages()
error path. This is the fault path. Tested under QEMU x86_64, one CPU.

Thanks,
Karl

#include <fcntl.h>
#include <setjmp.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <sys/statfs.h>
#include <unistd.h>

#define HPAGE (2UL << 20)

static sigjmp_buf env;

static void on_sigbus(int sig)
{
	siglongjmp(env, 1);
}

/* usage: t <hugetlbfs dir> <pages to touch, at most 4> */
int main(int argc, char **argv)
{
	int n = atoi(argv[2]), fd, i;
	volatile int ok = 0, bus = 0;
	struct statfs st;
	char path[256];
	char *p;

	signal(SIGBUS, on_sigbus);
	snprintf(path, sizeof(path), "%s/f", argv[1]);
	fd = open(path, O_CREAT | O_RDWR, 0600);
	if (fd < 0 || ftruncate(fd, 4 * HPAGE))
		return 1;
	p = mmap(NULL, 4 * HPAGE, PROT_READ | PROT_WRITE,
		 MAP_SHARED | MAP_NORESERVE, fd, 0);
	if (p == MAP_FAILED)
		return 1;
	for (i = 0; i < n; i++) {
		if (!sigsetjmp(env, 1)) {
			p[i * HPAGE] = 1;
			ok++;
		} else {
			bus++;
		}
	}
	munmap(p, 4 * HPAGE);
	close(fd);
	unlink(path);
	statfs(argv[1], &st);
	printf("ok=%d sigbus=%d free=%ld/%ld\n", ok, bus,
	       (long)st.f_bfree, (long)st.f_blocks);
	return 0;
}

  echo 0 > /proc/sys/vm/nr_overcommit_hugepages
  echo 2 > /proc/sys/vm/nr_hugepages
  mount -t hugetlbfs -o size=8M,min_size=4M none /mnt/h
  for n in 2 2 4 2 2; do ./t /mnt/h $n; grep HugePages_Rsvd /proc/meminfo; done


^ permalink raw reply	[flat|nested] 16+ 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-23  6:57     ` Karl Mehltretter
@ 2026-09-28  5:26     ` Ackerley Tng
  2026-10-07 13:50     ` Muchun Song
  3 siblings, 0 replies; 16+ 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] 16+ 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-09-28  5:26     ` Ackerley Tng
@ 2026-10-07 13:50     ` Muchun Song
  3 siblings, 0 replies; 16+ 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] 16+ messages in thread

end of thread, other threads:[~2026-10-07 13:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27 14:52 [PATCH] mm/hugetlb: fix subpool accounting after cgroup charge failure Catherine
2026-04-27 15:12 ` Andrew Morton
2026-04-27 15:19   ` Catherine
2026-04-27 21:12     ` Andrew Morton
2026-04-28  3:07 ` [PATCH v2] " Zhao Li
2026-04-28  9:08   ` Oscar Salvador
2026-04-28 11:30     ` Lance Yang
2026-04-28 11:41       ` Zhao Li
2026-04-28 11:41     ` Zhao Li
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-23  6:57     ` Karl Mehltretter
2026-09-28  5:26     ` Ackerley Tng
2026-10-07 13:50     ` Muchun Song
  -- strict thread matches above, loose matches on Subject: below --
2026-05-02  8:58 kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.