Skip to contentSkip to conversation

mm: replace free hugepage folios after migration

PATCH In review Mark Bundschuh 3 participants 8 messages Nov 5 – Nov 7

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mm: replace free hugepage folios after migration

From: yangge <[email protected]>

[ Upstream commit 04f13d241b8b146b23038bffd907cb8278391d07 ]

My machine has 4 NUMA nodes, each equipped with 32GB of memory. I have configured each NUMA node with 16GB of CMA and 16GB of in-use hugetlb pages. The allocation of contiguous memory via cma_alloc() can fail probabilistically.

When there are free hugetlb folios in the hugetlb pool, during the migration of in-use hugetlb folios, new folios are allocated from the free hugetlb pool. After the migration is completed, the old folios are released back to the free hugetlb pool instead of being returned to the buddy system. This can cause test_pages_isolated() check to fail, ultimately leading to the failure of cma_alloc().

Call trace:

cma_alloc()
__alloc_contig_migrate_range() // migrate in-use hugepage test_pages_isolated()
__test_page_isolated_in_pageblock()
PageBuddy(page) // check if the page is in buddy

To address this issue, we introduce a function named replace_free_hugepage_folios(). This function will replace the hugepage in the free hugepage pool with a new one and release the old one to the buddy system. After the migration of in-use hugetlb pages is completed, we will invoke replace_free_hugepage_folios() to ensure that these hugepages are properly released to the buddy system. Following this step, when test_pages_isolated() is executed for inspection, it will successfully pass.

Additionally, when alloc_contig_range() is used to migrate multiple in-use hugetlb pages, it can result in some in-use hugetlb pages being released back to the free hugetlb pool and subsequently being reallocated and used again. For example:

[huge 0] [huge 1]

To migrate huge 0, we obtain huge x from the pool. After the migration is completed, we return the now-freed huge 0 back to the pool. When it's time to migrate huge 1, we can simply reuse the now-freed huge 0 from the pool. As a result, when replace_free_hugepage_folios() is executed, it cannot release huge 0 back to the buddy system. To address this issue, we should prevent the reuse of isolated free hugepages during the migration process.

[ 5.15: Take only the dequeue_huge_page_node_exact() hunk, so memory
offline stops migrating huge pages back into the isolated range. ]

---
Without this, offlining a memory block holding mapped huge pages keeps migrating them back into the block until the offline is interrupted. Reproduced on arm64 on 5.10 and 6.12 (stock AL2023 6.12.113): the offline was still migrating after 60 s. With this patch on 5.10 it completes in 0.15 s with the data intact. Also sent for 5.10, 5.15, 6.1, 6.6 and 6.12.

 mm/hugetlb.c | 4 ++++
 1 file changed, 4 insertions(+)
mm/hugetlb.c +4 -0
index 947610e37a33..6df31569ce36 100644
@@ -43,6 +43,7 @@
 #include <linux/page_owner.h>
 #include "internal.h"
 #include "hugetlb_vmemmap.h"
+#include <linux/page-isolation.h>
 
 int hugetlb_max_hstate __read_mostly;
 unsigned int default_hstate_idx;
@@ -1095,6 +1096,9 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid)
 		if (PageHWPoison(page))
 			continue;
 
+		if (is_migrate_isolate_page(page))
+			continue;
+
 		list_move(&page->lru, &h->hugepage_activelist);
 		set_page_refcounted(page);
 		ClearHPageFreed(page);
Signature · 1 line
2.50.1
#1 lore raw
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] mm/hugetlb: isolated free pages in free_huge_pages break reservations and offline - [Medium] mm…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] mm/hugetlb: isolated free pages in free_huge_pages break reservations and offline
- [Medium] mm: missing free hugepage replacement after migration in alloc_contig_range()
--

commit c4fd411e1512176c3c86e8729268c9e64aee0792
Author: yangge <[email protected]>

mm: replace free hugepage folios after migration

This backport takes the dequeue_huge_page_node_exact() hunk from upstream commit 04f13d241b8b146b23038bffd907cb8278391d07 so that free hugetlb pages in MIGRATE_ISOLATE pageblocks are skipped when allocating from the hugetlb pool.

[Severity: Medium]
In mm/hugetlb.c, dequeue_huge_page_node_exact() is updated to skip pages where is_migrate_isolate_page(page) is true, while the commit subject and changelog state that replace_free_hugepage_folios() is introduced and invoked after migrating in-use hugetlb pages so cma_alloc() and test_pages_isolated() succeed.

In 5.15, hugetlb pages are allocated with GFP_HIGHUSER_MOVABLE via htlb_alloc_mask() and can reside in MIGRATE_CMA and MIGRATE_MOVABLE pageblocks. During cma_alloc() -> alloc_contig_range() -> __alloc_contig_migrate_range() in mm/page_alloc.c, isolate_or_dissolve_huge_page() in mm/hugetlb.c dissolves free hugepages in the target range via alloc_and_dissolve_huge_page(), but for in-use hugepages (page_count(head) > 0) it isolates them onto cc->migratepages via isolate_hugetlb() and advances cc->migrate_pfn past them.

When migrate_pages() migrates those in-use hugepages via alloc_migration_target() -> alloc_huge_page_nodemask(), if unreserved free hugepages exist in the pool (h->free_huge_pages - h->resv_huge_pages > 0), dequeue_huge_page_nodemask() returns a free pool hugepage without HPageTemporary set. Because HPageTemporary(newpage) is false, move_hugetlb_state() does not set HPageTemporary on the old source page, so free_huge_page() enqueues the old page back onto h->hugepage_freelists[nid] inside the MIGRATE_ISOLATE range instead of returning it to the buddy allocator.

Because this backport omits the post-migration replacement step after __alloc_contig_migrate_range() (even though alloc_and_dissolve_huge_page() already exists in mm/hugetlb.c), won't migrating in-use persistent hugetlb pages still leave the freed source hugepages on h->hugepage_freelists inside [start, end) and cause test_pages_isolated() in alloc_contig_range() to fail with -EBUSY? Also, should the commit subject and message be updated if only the dequeue_huge_page_node_exact() change is intended?

[Severity: High]
In mm/hugetlb.c, dequeue_huge_page_node_exact() skips pages where is_migrate_isolate_page(page) is true, but those isolated pages remain on h->hugepage_freelists[nid] and remain counted in h->free_huge_pages. Can this cause both memory offlining (offline_pages()) and reserved hugetlb page faults (alloc_huge_page()) to fail when free hugepages sit in a MIGRATE_ISOLATE range?

Consider a memory block [start_pfn, end_pfn) being offlined via offline_pages() that contains two in-use hugepages H0 and H1 (or one free hugepage H0 and one in-use hugepage H1), while outside the block there is one unreserved free hugepage F0 and one reserved free hugepage F1 backing an existing reservation (h->free_huge_pages == 2, h->resv_huge_pages == 1):

1. offline_pages() calls start_isolate_page_range(), marking [start_pfn, end_pfn) as MIGRATE_ISOLATE, and then calls do_migrate_range().

2. Migrating H0 dequeues F0. When H0 is released via putback_active_hugepage() -> free_huge_page() -> enqueue_huge_page(), H0 is placed onto h->hugepage_freelists[nid] inside the MIGRATE_ISOLATE range and increments h->free_huge_pages back to 2.

3. When migrating H1, alloc_huge_page_nodemask() in mm/hugetlb.c checks:

       if (h->free_huge_pages - h->resv_huge_pages > 0)

Because the isolated page H0 is still counted in h->free_huge_pages, 2 - 1 > 0 succeeds even though the only non-isolated free page (F1) is needed to back h->resv_huge_pages.

4. dequeue_huge_page_node_exact() skips H0 because is_migrate_isolate_page(H0) is true and dequeues the reserved page F1 for H1. H1 is then freed onto h->hugepage_freelists[nid] inside the MIGRATE_ISOLATE range, leaving both free hugepages (H0 and H1) inside the MIGRATE_ISOLATE range while h->resv_huge_pages == 1.

5. In offline_pages(), dissolve_free_huge_pages(start_pfn, end_pfn) dissolves H0 (reducing h->free_huge_pages to 1) and then calls dissolve_free_huge_page() on H1, which hits:

       if (h->free_huge_pages - h->resv_huge_pages == 0)
           goto out;

and fails with -EBUSY, aborting offline_pages().

In addition, gather_surplus_pages() in mm/hugetlb.c computes:

    needed = (h->resv_huge_pages + delta) - h->free_huge_pages;
    if (needed <= 0) {
        h->resv_huge_pages += delta;
        return 0;
    }

which counts MIGRATE_ISOLATE free pages in h->free_huge_pages as usable to satisfy reservations. Whenever the free pages backing h->resv_huge_pages are in MIGRATE_ISOLATE pageblocks (either from migration swapping F1 with H1 above, or from gather_surplus_pages()), a page fault on a VMA with a valid reservation calls alloc_huge_page() -> dequeue_huge_page_vma(), which skips the isolated pages in dequeue_huge_page_node_exact() and returns NULL.

alloc_huge_page() then falls back to alloc_buddy_huge_page_with_mpol() -> alloc_surplus_huge_page(), which fails when h->surplus_huge_pages >= h->nr_overcommit_huge_pages (0 by default) or hstate_is_gigantic(h), returning ERR_PTR(-ENOSPC) and delivering SIGBUS to a process faulting on a reserved mapping.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#2 lore raw
In reply to Mark Bundschuh
Quoted 58 lines · Mark Bundschuh

From: yangge <[email protected]>

[ Upstream commit 04f13d241b8b146b23038bffd907cb8278391d07 ]

My machine has 4 NUMA nodes, each equipped with 32GB of memory. I have configured each NUMA node with 16GB of CMA and 16GB of in-use hugetlb pages. The allocation of contiguous memory via cma_alloc() can fail probabilistically.

When there are free hugetlb folios in the hugetlb pool, during the migration of in-use hugetlb folios, new folios are allocated from the free hugetlb pool. After the migration is completed, the old folios are released back to the free hugetlb pool instead of being returned to the buddy system. This can cause test_pages_isolated() check to fail, ultimately leading to the failure of cma_alloc().

Call trace:

cma_alloc()
__alloc_contig_migrate_range() // migrate in-use hugepage test_pages_isolated()
__test_page_isolated_in_pageblock()
PageBuddy(page) // check if the page is in buddy

To address this issue, we introduce a function named replace_free_hugepage_folios(). This function will replace the hugepage in the free hugepage pool with a new one and release the old one to the buddy system. After the migration of in-use hugetlb pages is completed, we will invoke replace_free_hugepage_folios() to ensure that these hugepages are properly released to the buddy system. Following this step, when test_pages_isolated() is executed for inspection, it will successfully pass.

Additionally, when alloc_contig_range() is used to migrate multiple in-use hugetlb pages, it can result in some in-use hugetlb pages being released back to the free hugetlb pool and subsequently being reallocated and used again. For example:

[huge 0] [huge 1]

To migrate huge 0, we obtain huge x from the pool. After the migration is completed, we return the now-freed huge 0 back to the pool. When it's time to migrate huge 1, we can simply reuse the now-freed huge 0 from the pool. As a result, when replace_free_hugepage_folios() is executed, it cannot release huge 0 back to the buddy system. To address this issue, we should prevent the reuse of isolated free hugepages during the migration process.

[ 5.15: Take only the dequeue_huge_page_node_exact() hunk, so memory
offline stops migrating huge pages back into the isolated range. ]

Quoted 7 lines

---
Without this, offlining a memory block holding mapped huge pages keeps migrating them back into the block until the offline is interrupted. Reproduced on arm64 on 5.10 and 6.12 (stock AL2023 6.12.113): the offline was still migrating after 60 s. With this patch on 5.10 it completes in 0.15 s with the data intact. Also sent for 5.10, 5.15, 6.1, 6.6 and 6.12.

The simplified backport was really only the right thing to do for 5.10, per my other mail.

Signature · 3 lines
Cheers,

David
#3 lore raw

v2 cover mm: replace free hugepage folios after migration

In reply to Mark Bundschuh

Full backport of 04f13d241b8b ("mm: replace free hugepage folios after migration") with its fix 113ed54ad276.

Tested in a VM on 5.15.224: memory offline of a block with mapped huge pages no longer livelocks, and cma_alloc() over mapped huge pages now succeeds (it failed on 5.15.224 and with v1).

v1: https://lore.kernel.org/all/[email protected]/

Ge Yang (1):
mm/hugetlb: fix kernel NULL pointer dereference when replacing free
hugetlb folios

yangge (1):
mm: replace free hugepage folios after migration

 include/linux/hugetlb.h |  7 ++++++
 mm/hugetlb.c            | 49 +++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c         | 12 +++++++++-
 3 files changed, 67 insertions(+), 1 deletion(-)
Signature · 1 line
2.50.1
#4 lore raw

PATCH v2 1/2 mm: replace free hugepage folios after migration

From: yangge <[email protected]>

[ Upstream commit 04f13d241b8b146b23038bffd907cb8278391d07 ]

My machine has 4 NUMA nodes, each equipped with 32GB of memory. I have configured each NUMA node with 16GB of CMA and 16GB of in-use hugetlb pages. The allocation of contiguous memory via cma_alloc() can fail probabilistically.

When there are free hugetlb folios in the hugetlb pool, during the migration of in-use hugetlb folios, new folios are allocated from the free hugetlb pool. After the migration is completed, the old folios are released back to the free hugetlb pool instead of being returned to the buddy system. This can cause test_pages_isolated() check to fail, ultimately leading to the failure of cma_alloc().

Call trace:

cma_alloc()
__alloc_contig_migrate_range() // migrate in-use hugepage test_pages_isolated()
__test_page_isolated_in_pageblock()
PageBuddy(page) // check if the page is in buddy

To address this issue, we introduce a function named replace_free_hugepage_folios(). This function will replace the hugepage in the free hugepage pool with a new one and release the old one to the buddy system. After the migration of in-use hugetlb pages is completed, we will invoke replace_free_hugepage_folios() to ensure that these hugepages are properly released to the buddy system. Following this step, when test_pages_isolated() is executed for inspection, it will successfully pass.

Additionally, when alloc_contig_range() is used to migrate multiple in-use hugetlb pages, it can result in some in-use hugetlb pages being released back to the free hugetlb pool and subsequently being reallocated and used again. For example:

[huge 0] [huge 1]

To migrate huge 0, we obtain huge x from the pool. After the migration is completed, we return the now-freed huge 0 back to the pool. When it's time to migrate huge 1, we can simply reuse the now-freed huge 0 from the pool. As a result, when replace_free_hugepage_folios() is executed, it cannot release huge 0 back to the buddy system. To address this issue, we should prevent the reuse of isolated free hugepages during the migration process.

---
 include/linux/hugetlb.h |  7 +++++++
 mm/hugetlb.c            | 41 +++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c         | 12 +++++++++++-
 3 files changed, 59 insertions(+), 1 deletion(-)
include/linux/hugetlb.h +7 -0
index 254fcd6f9604..e4b6aa016d30 100644
@@ -641,6 +641,7 @@ struct huge_bootmem_page {
 };
 
 int isolate_or_dissolve_huge_page(struct page *page, struct list_head *list);
+int replace_free_hugepage_folios(unsigned long start_pfn, unsigned long end_pfn);
 struct page *alloc_huge_page(struct vm_area_struct *vma,
 				unsigned long addr, int avoid_reserve);
 struct page *alloc_huge_page_nodemask(struct hstate *h, int preferred_nid,
@@ -933,6 +934,12 @@ static inline int isolate_or_dissolve_huge_page(struct page *page,
 	return -ENOMEM;
 }
 
+static inline int replace_free_hugepage_folios(unsigned long start_pfn,
+		unsigned long end_pfn)
+{
+	return 0;
+}
+
 static inline struct page *alloc_huge_page(struct vm_area_struct *vma,
 					   unsigned long addr,
 					   int avoid_reserve)
mm/hugetlb.c +41 -0

59 lines not on this page · raw message

mm/page_alloc.c +11 -1
index cfbfd6309c25..b813570d2b39 100644
@@ -9250,7 +9250,17 @@ int alloc_contig_range(unsigned long start, unsigned long end,
 	ret = __alloc_contig_migrate_range(&cc, start, end);
 	if (ret && ret != -EBUSY)
 		goto done;
-	ret = 0;
+
+	/*
+	 * When in-use hugetlb pages are migrated, they may simply be released
+	 * back into the free hugepage pool instead of being returned to the
+	 * buddy system.  After the migration of in-use huge pages is completed,
+	 * we will invoke replace_free_hugepage_folios() to ensure that these
+	 * hugepages are properly released to the buddy system.
+	 */
+	ret = replace_free_hugepage_folios(start, end);
+	if (ret)
+		goto done;
 
 	/*
 	 * Pages from [start, end) are within a MAX_ORDER_NR_PAGES
Signature · 1 line
2.50.1
#5 lore raw
Thank you for your contribution! Sashiko AI review found 2 potential issues to consider: - [High] mm/hugetlb: isolated free pages in free_huge_pages break reservations and offline - [Low] mm…

Thank you for your contribution! Sashiko AI review found 2 potential issues to consider:
- [High] mm/hugetlb: isolated free pages in free_huge_pages break reservations and offline
- [Low] mm: missing Fixes trailer on commit fixing hugetlb migration in alloc_contig_range()
--

commit d4646288ad2f50427e782b8be06fdbc2d46b6d5b
Author: yangge <[email protected]>

mm: replace free hugepage folios after migration

This commit adds replace_free_hugepage_folios() after migrating in-use hugetlb pages in alloc_contig_range() and skips MIGRATE_ISOLATE pages in dequeue_huge_page_node_exact() so freed hugepages in the isolated range are returned to the buddy allocator instead of being reused from the hugetlb pool.

[Severity: Low]
In the commit message, this is a bug fix for migrating in-use hugetlb pages in alloc_contig_range(). Should it include:

Fixes ae37c7ff79f1 ("mm: make alloc_contig_range handle in-use hugetlb pages")

[Severity: High]
In mm/hugetlb.c at line 1099, does skipping MIGRATE_ISOLATE pages in dequeue_huge_page_node_exact() while still counting them in h->free_huge_pages allow migration to consume non-isolated free pages backing h->resv_huge_pages?

When an in-use hugepage inside an isolated range (from offline_pages() or alloc_contig_range()) is migrated and freed back to h->hugepage_freelists, it remains counted in h->free_huge_pages. On the next hugepage migration, alloc_huge_page_nodemask() (mm/hugetlb.c, line 2184) sees h->free_huge_pages - h->resv_huge_pages > 0, and dequeue_huge_page_node_exact() skips the isolated free page and dequeues a non-isolated free page that was needed to back h->resv_huge_pages.

Once the free pages backing h->resv_huge_pages have been swapped into the MIGRATE_ISOLATE range (or accounted as available by gather_surplus_pages() at mm/hugetlb.c, line 2231), dissolve_free_huge_page() (mm/hugetlb.c, line 1971) hits h->free_huge_pages - h->resv_huge_pages == 0 and fails offline_pages() with -EBUSY after already shrinking h->max_huge_pages. In addition, page faults on reserved hugetlb mappings fail in alloc_huge_page() (mm/hugetlb.c, line 2836) with -ENOSPC (delivering SIGBUS) when dequeue_huge_page_vma() skips the isolated pages and surplus allocation is disallowed (such as when nr_overcommit_huge_pages is 0).

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#6 lore raw

PATCH v2 2/2 mm/hugetlb: fix kernel NULL pointer dereference when replacing free hugetlb folios

In reply to Mark Bundschuh

From: Ge Yang <[email protected]>

[ Upstream commit 113ed54ad276c352ee5ce109bdcf0df118a43bda ]

A kernel crash was observed when replacing free hugetlb folios:

BUG: kernel NULL pointer dereference, address: 0000000000000028
PGD 0 P4D 0
Oops: Oops: 0000 [#1] SMP NOPTI
CPU: 28 UID: 0 PID: 29639 Comm: test_cma.sh Tainted 6.15.0-rc6-zp #41 PREEMPT(voluntary)
RIP: 0010:alloc_and_dissolve_hugetlb_folio+0x1d/0x1f0
RSP: 0018:ffffc9000b30fa90 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000342cca RCX: ffffea0043000000
RDX: ffffc9000b30fb08 RSI: ffffea0043000000 RDI: 0000000000000000
RBP: ffffc9000b30fb20 R08: 0000000000001000 R09: 0000000000000000
R10: ffff88886f92eb00 R11: 0000000000000000 R12: ffffea0043000000
R13: 0000000000000000 R14: 00000000010c0200 R15: 0000000000000004
FS:  00007fcda5f14740(0000) GS:ffff8888ec1d8000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000028 CR3: 0000000391402000 CR4: 0000000000350ef0
Call Trace:
<TASK>
 replace_free_hugepage_folios+0xb6/0x100
 alloc_contig_range_noprof+0x18a/0x590
 ? srso_return_thunk+0x5/0x5f
 ? down_read+0x12/0xa0
 ? srso_return_thunk+0x5/0x5f
 cma_range_alloc.constprop.0+0x131/0x290
 __cma_alloc+0xcf/0x2c0
 cma_alloc_write+0x43/0xb0
 simple_attr_write_xsigned.constprop.0.isra.0+0xb2/0x110
 debugfs_attr_write+0x46/0x70
 full_proxy_write+0x62/0xa0
 vfs_write+0xf8/0x420
 ? srso_return_thunk+0x5/0x5f
 ? filp_flush+0x86/0xa0
 ? srso_return_thunk+0x5/0x5f
 ? filp_close+0x1f/0x30
 ? srso_return_thunk+0x5/0x5f
 ? do_dup2+0xaf/0x160
 ? srso_return_thunk+0x5/0x5f
 ksys_write+0x65/0xe0
 do_syscall_64+0x64/0x170
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

There is a potential race between __update_and_free_hugetlb_folio() and replace_free_hugepage_folios():

CPU1 CPU2
__update_and_free_hugetlb_folio replace_free_hugepage_folios
folio_test_hugetlb(folio) -- It's still hugetlb folio.

__folio_clear_hugetlb(folio) hugetlb_free_folio(folio)
h = folio_hstate(folio) -- Here, h is NULL pointer

When the above race condition occurs, folio_hstate(folio) returns NULL, and subsequent access to this NULL pointer will cause the system to crash. To resolve this issue, execute folio_hstate(folio) under the protection of the hugetlb_lock lock, ensuring that folio_hstate(folio) does not return NULL.

Fixes 04f13d241b8b ("mm: replace free hugepage folios after migration")
Cc Baolin Wang <[email protected]>
Cc Barry Song <[email protected]>
Cc David Hildenbrand <[email protected]>
[ 5.15: Use pages instead of folios. ]
---
 mm/hugetlb.c | 8 ++++++++
 1 file changed, 8 insertions(+)
mm/hugetlb.c +8 -0
index 4fb98320f9df..3d7e8826e452 100644
@@ -2756,12 +2756,20 @@ int replace_free_hugepage_folios(unsigned long start_pfn, unsigned long end_pfn)
 
 	while (start_pfn < end_pfn) {
 		page = compound_head(pfn_to_page(start_pfn));
+
+		/*
+		 * The page might have been dissolved from under our feet, so make sure
+		 * to carefully check the state under the lock.
+		 */
+		spin_lock_irq(&hugetlb_lock);
 		if (PageHuge(page)) {
 			h = page_hstate(page);
 		} else {
+			spin_unlock_irq(&hugetlb_lock);
 			start_pfn++;
 			continue;
 		}
+		spin_unlock_irq(&hugetlb_lock);
 
 		if (!page_count(page)) {
 			ret = alloc_and_dissolve_huge_page(h, page, &isolate_list);
Signature · 1 line
2.50.1
#7 lore raw

Re: [PATCH v2 5.15 2/2] mm/hugetlb: fix kernel NULL pointer dereference when replacing free hugetlb folios

Thank you for your contribution! Sashiko AI review found 1 potential issue to consider: - [Medium] mm/hugetlb: compound_head() evaluated outside hugetlb_lock in replace_free_hugepage_folios() --

Thank you for your contribution! Sashiko AI review found 1 potential issue to consider:
- [Medium] mm/hugetlb: compound_head() evaluated outside hugetlb_lock in replace_free_hugepage_folios()
--

commit de1b340d0acd3398ffa967e9c36bcd2099c3b383
Author: Ge Yang <[email protected]>

mm/hugetlb: fix kernel NULL pointer dereference when replacing free hugetlb folios

This commit acquires hugetlb_lock around PageHuge() and page_hstate() in replace_free_hugepage_folios() to prevent racing with concurrent hugepage freeing.

Quoted 12 lines
mm/hugetlb.c +4 -0
index 4fb98320f9df3..3d7e8826e4525 100644
@@ -2756,12 +2756,20 @@ int replace_free_hugepage_folios(unsigned long start_pfn, unsigned long end_pfn)
 
 	while (start_pfn < end_pfn) {
 		page = compound_head(pfn_to_page(start_pfn));
+
+		/*
+		 * The page might have been dissolved from under our feet, so make sure
+		 * to carefully check the state under the lock.

mm/hugetlb.c int replace_free_hugepage_folios(unsigned long start_pfn, unsigned long end_pfn) · line 2772

Quoted 10 lines
mm/hugetlb.c +4 -0
+		 */
+		spin_lock_irq(&hugetlb_lock);
 		if (PageHuge(page)) {
 			h = page_hstate(page);
 		} else {
+			spin_unlock_irq(&hugetlb_lock);
 			start_pfn++;
 			continue;
 		}
+		spin_unlock_irq(&hugetlb_lock);

[Severity: Medium]
Can page_hstate(page) still return NULL if pfn_to_page(start_pfn) becomes a tail page of a newly prepared hugepage before hugetlb_lock is acquired?

CPU0 CPU1
replace_free_hugepage_folios()
page = compound_head(pfn_to_page(start_pfn))

                                        alloc_gigantic_page() / prep_new_huge_page()
                                          prep_compound_gigantic_page()
                                            /* page becomes a tail page */
                                          spin_lock_irq(&hugetlb_lock)
                                          __prep_new_huge_page()
                                            set_compound_page_dtor(HUGETLB_PAGE_DTOR)
                                          spin_unlock_irq(&hugetlb_lock)

spin_lock_irq(&hugetlb_lock)
if (PageHuge(page))
h = page_hstate(page) <-- returns NULL
spin_unlock_irq(&hugetlb_lock) alloc_and_dissolve_huge_page(h, page, ...)
htlb_alloc_mask(h) <-- NULL pointer dereference

In upstream 113ed54ad276 ("mm/hugetlb: fix kernel NULL pointer dereference when replacing free hugetlb folios"), pfn_folio(start_pfn) is checked with folio_test_hugetlb(), which returns false for tail pages. In 5.15, PageHuge(page) resolves compound_head(page) internally on its local parameter and returns true for tail pages, while page_hstate(page) calls page_size(page) -> compound_order(page), which returns 0 when !PageHead(page) and causes size_to_hstate(PAGE_SIZE) to return NULL.

Re-evaluating page = compound_head(page) under hugetlb_lock after PageHuge(page) succeeds, as isolate_or_dissolve_huge_page() and dissolve_free_huge_page() do, avoids passing a tail page to page_hstate() and alloc_and_dissolve_huge_page().

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2