public inbox for linux-mm@kvack.org
 help / color / mirror / Atom feed
From: Zhao Li <enderaoelyther@gmail.com>
To: Lance Yang <lance.yang@linux.dev>
Cc: Zhao Li <enderaoelyther@gmail.com>,
	Oscar Salvador <osalvador@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Muchun Song <muchun.song@linux.dev>,
	David Hildenbrand <david@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mm/hugetlb: fix subpool accounting after cgroup charge failure
Date: Tue, 28 Apr 2026 19:41:39 +0800	[thread overview]
Message-ID: <20260428114138.92159-2-enderaoelyther@gmail.com> (raw)
In-Reply-To: <20260428113059.79001-1-lance.yang@linux.dev>

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


  reply	other threads:[~2026-04-28 11:42 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260428114138.92159-2-enderaoelyther@gmail.com \
    --to=enderaoelyther@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=muchun.song@linux.dev \
    --cc=osalvador@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox