mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] Respect mempolicy when calculating surplus huge pages.
@ 2026-05-27 22:48 Charles Haithcock
  2026-05-30  3:35 ` Andrew Morton
  2026-06-02 15:20 ` Joshua Hahn
  0 siblings, 2 replies; 4+ messages in thread
From: Charles Haithcock @ 2026-05-27 22:48 UTC (permalink / raw)
  To: muchun.song, osalvador, akpm; +Cc: Charles Haithcock, linux-mm, linux-kernel

Presently, when calculating how many huge pages are needed when
reserving surplus huge pages, the global count of free huge pages
are used. When reserving with a mempolicy, the global count of free huge
pages is used even if some/all of those free huge pages are on numa
nodes outside of the mempolicy. Fix it so free huge pages only on nodes
within the mempolicy are considered.

Signed-off-by: Charles Haithcock <chaithco@redhat.com>
---
 mm/hugetlb.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f24bf49be0..02752ce735 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2255,6 +2255,23 @@ static nodemask_t *policy_mbind_nodemask(gfp_t gfp)
 	return NULL;
 }
 
+static unsigned int allowed_mems_nr(struct hstate *h)
+{
+	int node;
+	unsigned int nr = 0;
+	nodemask_t *mbind_nodemask;
+	unsigned int *array = h->free_huge_pages_node;
+	gfp_t gfp_mask = htlb_alloc_mask(h);
+
+	mbind_nodemask = policy_mbind_nodemask(gfp_mask);
+	for_each_node_mask(node, cpuset_current_mems_allowed) {
+		if (!mbind_nodemask || node_isset(node, *mbind_nodemask))
+			nr += array[node];
+	}
+
+	return nr;
+}
+
 /*
  * Increase the hugetlb pool such that it can accommodate a reservation
  * of size 'delta'.
@@ -2277,7 +2294,7 @@ static int gather_surplus_pages(struct hstate *h, long delta)
 		alloc_nodemask = cpuset_current_mems_allowed;
 
 	lockdep_assert_held(&hugetlb_lock);
-	needed = (h->resv_huge_pages + delta) - h->free_huge_pages;
+	needed = (h->resv_huge_pages + delta) - allowed_mems_nr(h);
 	if (needed <= 0) {
 		h->resv_huge_pages += delta;
 		return 0;
@@ -2312,7 +2329,7 @@ static int gather_surplus_pages(struct hstate *h, long delta)
 	 */
 	spin_lock_irq(&hugetlb_lock);
 	needed = (h->resv_huge_pages + delta) -
-			(h->free_huge_pages + allocated);
+			(allowed_mems_nr(h) + allocated);
 	if (needed > 0) {
 		if (alloc_ok)
 			goto retry;
@@ -4513,23 +4530,6 @@ static int __init hugepage_alloc_threads_setup(char *s)
 }
 __setup("hugepage_alloc_threads=", hugepage_alloc_threads_setup);
 
-static unsigned int allowed_mems_nr(struct hstate *h)
-{
-	int node;
-	unsigned int nr = 0;
-	nodemask_t *mbind_nodemask;
-	unsigned int *array = h->free_huge_pages_node;
-	gfp_t gfp_mask = htlb_alloc_mask(h);
-
-	mbind_nodemask = policy_mbind_nodemask(gfp_mask);
-	for_each_node_mask(node, cpuset_current_mems_allowed) {
-		if (!mbind_nodemask || node_isset(node, *mbind_nodemask))
-			nr += array[node];
-	}
-
-	return nr;
-}
-
 void hugetlb_report_meminfo(struct seq_file *m)
 {
 	struct hstate *h;
-- 
2.54.0


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

* Re: [PATCH] Respect mempolicy when calculating surplus huge pages.
  2026-05-27 22:48 [PATCH] Respect mempolicy when calculating surplus huge pages Charles Haithcock
@ 2026-05-30  3:35 ` Andrew Morton
  2026-06-02 15:20 ` Joshua Hahn
  1 sibling, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2026-05-30  3:35 UTC (permalink / raw)
  To: Charles Haithcock; +Cc: muchun.song, osalvador, linux-mm, linux-kernel

On Wed, 27 May 2026 16:48:46 -0600 Charles Haithcock <chaithco@redhat.com> wrote:

> Presently, when calculating how many huge pages are needed when
> reserving surplus huge pages, the global count of free huge pages
> are used. When reserving with a mempolicy, the global count of free huge
> pages is used even if some/all of those free huge pages are on numa
> nodes outside of the mempolicy. Fix it so free huge pages only on nodes
> within the mempolicy are considered.
> 

Thanks.   AI review asked a thing:
	https://sashiko.dev/#/patchset/20260527224848.1753560-1-chaithco@redhat.com



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

* Re: [PATCH] Respect mempolicy when calculating surplus huge pages.
  2026-05-27 22:48 [PATCH] Respect mempolicy when calculating surplus huge pages Charles Haithcock
  2026-05-30  3:35 ` Andrew Morton
@ 2026-06-02 15:20 ` Joshua Hahn
  1 sibling, 0 replies; 4+ messages in thread
From: Joshua Hahn @ 2026-06-02 15:20 UTC (permalink / raw)
  To: Charles Haithcock; +Cc: muchun.song, osalvador, akpm, linux-mm, linux-kernel

On Wed, 27 May 2026 16:48:46 -0600 Charles Haithcock <chaithco@redhat.com> wrote:

> Presently, when calculating how many huge pages are needed when
> reserving surplus huge pages, the global count of free huge pages
> are used. When reserving with a mempolicy, the global count of free huge
> pages is used even if some/all of those free huge pages are on numa
> nodes outside of the mempolicy. Fix it so free huge pages only on nodes
> within the mempolicy are considered.

Hello Charles, thank you for the patch!

I just wanted to add that it seems like this is a known issue. From the
comment in hugetlb_acct_memory (the only caller of gather_surplus_pages)
we have the following comment block:

        /*
         * When cpuset is configured, it breaks the strict hugetlb page
         * reservation as the accounting is done on a global variable. Such
         * reservation is completely rubbish in the presence of cpuset because
         * the reservation is not checked against page availability for the
         * current cpuset. Application can still potentially OOM'ed by kernel
         * with lack of free htlb page in cpuset that the task is in.
         * Attempt to enforce strict accounting with cpuset is almost
         * impossible (or too ugly) because cpuset is too fluid that
         * task or memory node can be dynamically moved between cpusets.
         *
         * The change of semantics for shared hugetlb mapping with cpuset is
         * undesirable. However, in order to preserve some of the semantics,
         * we fall back to check against current free page availability as
         * a best attempt and hopefully to minimize the impact of changing
         * semantics that cpuset has.
         *
         * Apart from cpuset, we also have memory policy mechanism that
         * also determines from which node the kernel will allocate memory
         * in a NUMA system. So similar to cpuset, we also should consider
         * the memory policy of the current task. Similar to the description
         * above.

So it would appear that getting an exact number of pages to allocate,
and ensure that there are no changes with the reservation or which nodes
those reservations actually go to is a lot more difficult. But I think
we can do a bit better.

FWIW, I think over-allocating is actually not fatal (although overallocating
by a lot is obviously not desirable) since we free all the unused hugetlb
pages at the end of gather_surplus_pages. I wonder if an approach like this
could work:

@@ -2260,7 +2277,8 @@ static int gather_surplus_pages(struct hstate *h, long delta)
                alloc_nodemask = cpuset_current_mems_allowed;

        lockdep_assert_held(&hugetlb_lock);
-       needed = (h->resv_huge_pages + delta) - h->free_huge_pages;
+       needed = max(delta - allowed_mems_nr(h),
+                    (h->resv_huge_pages + delta) - h->free_huge_pages);
        if (needed <= 0) {
                h->resv_huge_pages += delta;
                return 0;
@@ -2294,8 +2312,8 @@ static int gather_surplus_pages(struct hstate *h, long delta)
         * because either resv_huge_pages or free_huge_pages may have changed.
         */
        spin_lock_irq(&hugetlb_lock);
-       needed = (h->resv_huge_pages + delta) -
-                       (h->free_huge_pages + allocated);
+       needed = max((h->resv_huge_pages + delta) - h->free_huge_pages,
+                    delta - allowed_mems_nr(h)) - allocated;
        if (needed > 0) {
                if (alloc_ok)
                        goto retry;

So we compare the mempolicy-perspective "needed" and compare it to the
global "needed" and take whatever. Since we are taking a max it should
only ever make it more likely to actually succeed with the mempolicy-bound
hugetlb page usage, even though we still can't make guarantees since
a free page on our node may be taken by a different reservation later.

Let me know what you think. Thanks again!
Joshua

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

* Re: [PATCH] Respect mempolicy when calculating surplus huge pages.
       [not found] <CAMcbwu5=TK_EjZ=wN5fqLC-DVUendE+CBWX0pN7KBF+1ZwD7Vw@mail.gmail.com>
@ 2026-06-09  4:23 ` Joshua Hahn
  0 siblings, 0 replies; 4+ messages in thread
From: Joshua Hahn @ 2026-06-09  4:23 UTC (permalink / raw)
  To: Charles Haithcock; +Cc: muchun.song, osalvador, akpm, linux-mm, linux-kernel

On Mon, 8 Jun 2026 10:21:22 -0600 Charles Haithcock <chaithco@redhat.com> wrote:

> On Tue, Jun 2, 2026, 9:20 AM Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> > Hello Charles, thank you for the patch!
> 
> You are so welcome, and thank you! I'm newer to contributing to upstream so
> I welcome any and all feedback!

Hello Charles, welcome!! : -)

> > I just wanted to add that it seems like this is a known issue. From the
> > comment in hugetlb_acct_memory (the only caller of gather_surplus_pages)
> > we have the following comment block:
> > [...]
> > So it would appear that getting an exact number of pages to allocate,
> > and ensure that there are no changes with the reservation or which nodes
> > those reservations actually go to is a lot more difficult. But I think
> > we can do a bit better.
> 
> Agreed; actually, after submitting the initial version, I went back and
> have been working on adding per-node reserve hugepage counts to the hstates
> and adding some selftests for them. It's going well so far, but I'm
> hitting an issue in accounting where, after reserving a static hugetlb page
> on node0, reserving and then freeing a surplus hugetlb page on node1, and
> then freeing the static hugetlb page on node0, followup reservations on
> node0 additionally count towards surplus pages even when there are free
> static hugetlb pages on that node. Trace-cmd revealed allowed_mems_nr_free
> is returning more than what was requested resulting in the over-allocated
> surplus pages to stick around rather than be freed back.

I see. So from the perspective of the system, even though you free the
static hugetlb page, the system views that as unavailable to other
reservations and so future allocations go towards surplus pages?
How exactly are you freeing the page in this case? And how long do you
wait between freeing the static page and allocating the next page?

I wonder if you are seeing this issue without your per-node
hugetlb page situation as well. I have seen quite a few hugetlb folio
reservation restoration accounting bugs recently, including some that
I think are still in mm-new. If you're developing against anything
other than mm-new, I'd suggest trying to build on top of that to see
if your bug still persists. 

> I plan to dig into
> what's causing that, however, if doing so leads down a nasty path of adding
> per-node accounting, I may need to fall back to the first version of this
> patch (but with selftests) and be ok with simply doing "a bit better" like
> you mention.

Sounds good! I wonder if it would be a good exercise to measure how
much we typically over-allocate by when using the simplified calculation
I noted in my previously reply.

> > FWIW, I think over-allocating is actually not fatal (although
> > overallocating
> > by a lot is obviously not desirable) since we free all the unused hugetlb
> > pages at the end of gather_surplus_pages.
> 
> Agreed on this point as well; afaik, surplus hugetlb pages are viewed quite
> differently from static hugetlb pages where surplus hugetlb pages are "best
> effort." Obviously we should try to reserve the requested amount, true
> users of large quantities of hugetlb pages should tend towards setting them
> on boot rather than dynamically.

Yup, totally agree here.

> > [...] I wonder if an approach like this
> > could work:
> > [...]
> > So we compare the mempolicy-perspective "needed" and compare it to the
> > global "needed" and take whatever. Since we are taking a max it should
> > only ever make it more likely to actually succeed with the mempolicy-bound
> > hugetlb page usage, even though we still can't make guarantees since
> > a free page on our node may be taken by a different reservation later.
> 
> This is considerably simpler than a per-node reserved hugetlb page count!
> If I end up falling back to my initial patch, I will absolutely be adding
> this and attributing you. Thank you!

Sounds good with me : -) Good luck and have a great day!
Joshua

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

end of thread, other threads:[~2026-06-09  4:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-27 22:48 [PATCH] Respect mempolicy when calculating surplus huge pages Charles Haithcock
2026-05-30  3:35 ` Andrew Morton
2026-06-02 15:20 ` Joshua Hahn
     [not found] <CAMcbwu5=TK_EjZ=wN5fqLC-DVUendE+CBWX0pN7KBF+1ZwD7Vw@mail.gmail.com>
2026-06-09  4:23 ` Joshua Hahn

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

all inboxes | Powered by JetHome®