* [PATCH 1/2] tools/tests/native: Test merging the tail after an offlined page
2026-05-28 14:55 [PATCH 0/2] xen/mm: Fix off-by-one for tail merge in reserve_offlined_page() Bernhard Kaindl
@ 2026-05-28 14:55 ` Bernhard Kaindl
2026-05-28 14:55 ` [PATCH 2/2] xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page Bernhard Kaindl
1 sibling, 0 replies; 5+ messages in thread
From: Bernhard Kaindl @ 2026-05-28 14:55 UTC (permalink / raw)
To: xen-devel; +Cc: Bernhard Kaindl, Anthony PERARD
After offlining pages, reserve_offlined_page() returns healthy
spans of pages from the buddy it isolated the offlined pages
back to the free lists.
Naturally, it attempts to grow larger buddies, but due to an
off-by-one, this fails at the tail end of the span of pages.
The test seeds the heap with an order-2 buddy and offlines tail page 1:
+---------------+---------------+---------------+---------------+
| head page tail page 1, tail page 2 tail page 3 |
| PFN_ORDER(pg) marked as to |
| == 2 be offlined |
+---------------+---------------+---------------+---------------+
After reserve_offlined_page(), the healthy pages should be:
+---------------+ +---------------+---------------+
| single page | offlined page | head page tail page |
| PFN_ORDER(pg) | not returned | PFN_ORDER(pg) |
| == 0 | to the heap | == 1 |
+---------------+ +---------------+---------------+
A trivial off-by-one error in the growth loop stops the growth loop
early, before the tail end of the original buddy, and we end up with:
+---------------+ +---------------+---------------+
| single page | offlined page | single page | single page |
| PFN_ORDER(pg) | not returned | PFN_ORDER(pg) | PFN_ORDER(pg) |
| == 0 | to the heap | == 0 | == 0 |
+---------------+ +---------------+---------------+
Running the test:
make -C tools/tests/native TARGETS=offline-merge-tail test
Test result:
| - Test assertion failed as expected at offline-merge-tail.c:63:
| The pair of tail pages should be merged into an order-1 buddy
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
---
tools/tests/native/offline-merge-tail.c | 93 +++++++++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 tools/tests/native/offline-merge-tail.c
diff --git a/tools/tests/native/offline-merge-tail.c b/tools/tests/native/offline-merge-tail.c
new file mode 100644
index 000000000000..11c79e3ecc1b
--- /dev/null
+++ b/tools/tests/native/offline-merge-tail.c
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Tests using offline_page() to verify reserve_offlined_page()
+ *
+ * The workflow tested here is offlining a free page:
+ *
+ * 1. offline_page() calls mark_page_offlined() to mark the page.
+ * 2. It calls reserve_heap_page() to find the containing buddy.
+ * 3. It calls reserve_offlined_page() to reserve the marked pages within
+ * that buddy.
+ *
+ * reserve_offlined_page() then:
+ *
+ * 1. Removes the buddy, a 2^order group of pages, from the free list.
+ * 2. Finds size-aligned spans of healthy pages within it.
+ * 3. Rebuilds healthy buddies from those spans and
+ * adds them back to the free list via page_list_add_scrub().
+ * 4. Moves offlined subpages to the offlined page lists.
+ *
+ * Another workflow marks an in-use page for offlining and then
+ * relies on free_heap_pages() to call reserve_offlined_page()
+ * when that page is eventually freed.
+ *
+ * Copyright (C) 2026 Cloud Software Group
+ */
+#define TEST_ENABLE_XC_DOMAIN_C
+#include "harness/native.h"
+
+/* Test merging a surviving tail pair into an order-1 buddy. */
+static void test_merge_tail_pair(int start_mfn)
+{
+ struct page_info *pages = frame_table + start_mfn;
+ uint32_t status = 0;
+
+ /*
+ * Prepare a valid order-2 buddy (4 pages) with this layout:
+ * +---------------+-----------------+-----------------+----------------+
+ * | head page | tail page 1 | tail page 2 | tail page 3 |
+ * +---------------+-----------------+-----------------+----------------+
+ */
+ test_page_list_add_buddy(pages, order2);
+
+ /* Mark the tail page 3 dirty to verify dirty-state preservation. */
+ pages[3].count_info |= PGC_need_scrub;
+ pages[0].u.free.first_dirty = 3;
+
+ /* Act: Offline the second page. */
+ ASSERT(offline_page(page_to_mfn(pages + 1), 0, &status) == 0);
+ ASSERT(status & PG_OFFLINE_OFFLINED);
+ ASSERT(FREE_PAGES == 3);
+
+ /*
+ * Offlining page 1 results in splitting the original order-2 buddy into:
+ * - pages[0] as an order-0 buddy
+ * - pages[1] is the offlined page, removed from the free list
+ * - pages[2] as an order-0 buddy
+ * - pages[3] as an order-0 buddy:
+ * +---------------+ +---------------+---------------+
+ * | single page | offlined page | single page | single page |
+ * +---------------+ +---------------+---------------+
+ *
+ * Tail 2 & 3 are aligned, so they should be merged into an order-1 buddy:
+ * +---------------+ +---------------+---------------+
+ * | single page | offlined page | head page with a tail page |
+ * +---------------+ +---------------+---------------+
+ */
+ CHECK(PFN_ORDER(&pages[0]) == 0, "Former head page, now order-0");
+ CHECK(PFN_ORDER(&pages[1]) == 0, "Offlined page should be order-0");
+ /* pages[0] and pages[1] were prepared as clean pages and still are. */
+ ASSERT(pages[0].u.free.first_dirty == INVALID_DIRTY_IDX);
+ ASSERT(pages[1].u.free.first_dirty == INVALID_DIRTY_IDX);
+
+ /* The tail pair is expected to be merged into one order-1 buddy. */
+ EXPECT_FAIL_BEGIN();
+ CHECK(PFN_ORDER(&pages[2]) == 1,
+ "The pair of tail pages should be merged into an order-1 buddy");
+ CHECK(pages[2].u.free.first_dirty == 1, "In tail buddy, the 2nd is dirty");
+ /* The tail page of the merged buddy does not use first_dirty. */
+ CHECK(pages[3].u.free.first_dirty == INVALID_DIRTY_IDX,
+ "Tail page of the merged buddy should not set first_dirty");
+ EXPECT_FAIL_END(3);
+}
+
+int main(int argc, char *argv[])
+{
+ const char *topic = "Integration test of offline_page()";
+ if ( !parse_args(argc, argv, topic) )
+ return EXIT_FAILURE;
+
+ init_page_alloc_tests();
+ RUN_TESTCASE("TMTP", test_merge_tail_pair, 4);
+ return test_complete();
+}
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page
2026-05-28 14:55 [PATCH 0/2] xen/mm: Fix off-by-one for tail merge in reserve_offlined_page() Bernhard Kaindl
2026-05-28 14:55 ` [PATCH 1/2] tools/tests/native: Test merging the tail after an offlined page Bernhard Kaindl
@ 2026-05-28 14:55 ` Bernhard Kaindl
2026-06-02 14:43 ` Jan Beulich
1 sibling, 1 reply; 5+ messages in thread
From: Bernhard Kaindl @ 2026-05-28 14:55 UTC (permalink / raw)
To: xen-devel
Cc: Bernhard Kaindl, Anthony PERARD, Andrew Cooper, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
reserve_offlined_page() reserves pages marked for offlining and
returns free buddies from the remaining healthy tail pages back
to the free list.
Consider an order-2 buddy (4 pages) with the following layout:
+---------------+---------------+---------------+---------------+
| head page tail page 1, tail page 2 tail page 3 |
| PFN_ORDER(pg) marked as to |
| == 2 be offlined |
+---------------+---------------+---------------+---------------+
The expected result after removing tail page 1 and returning the
remaining healthy pages to the free list would be:
+---------------+ +---------------+---------------+
| single page | offlined page | head page tail page |
| PFN_ORDER(pg) | not returned | PFN_ORDER(pg) |
| == 0 | to the heap | == 1 |
+---------------+ +---------------+---------------+
A trivial off-by-one error in the growth loop stops the growth loop
early before the tail end of the original buddy and we end up with:
+---------------+ +---------------+---------------+
| single page | offlined page | single page | single page |
| PFN_ORDER(pg) | not returned | PFN_ORDER(pg) | PFN_ORDER(pg) |
| == 0 | to the heap | == 0 | == 0 |
+---------------+ +---------------+---------------+
If the offlined page was in a much larger buddy, this would lead
to much more memory not available for higher order allocations
requiring the full tail end of the original buddy for allocation.
Fix the growth loop to correctly grow the buddy to the tail end
to make the full allocation unit available for future allocation
and update the test case to confirm the expected result and to
prevent regressions in the future.
Update the test case to verify the fix and prevent future regressions.
Fixes: e4865c2315 ('Page offline support in Xen side')
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
---
tools/tests/native/offline-merge-tail.c | 8 +-------
xen/common/page_alloc.c | 4 +++-
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/tools/tests/native/offline-merge-tail.c b/tools/tests/native/offline-merge-tail.c
index 11c79e3ecc1b..1893b23c7249 100644
--- a/tools/tests/native/offline-merge-tail.c
+++ b/tools/tests/native/offline-merge-tail.c
@@ -53,11 +53,7 @@ static void test_merge_tail_pair(int start_mfn)
* Offlining page 1 results in splitting the original order-2 buddy into:
* - pages[0] as an order-0 buddy
* - pages[1] is the offlined page, removed from the free list
- * - pages[2] as an order-0 buddy
- * - pages[3] as an order-0 buddy:
- * +---------------+ +---------------+---------------+
- * | single page | offlined page | single page | single page |
- * +---------------+ +---------------+---------------+
+ * - pages[2] and pages[3] as an unaligned order-1 buddy
*
* Tail 2 & 3 are aligned, so they should be merged into an order-1 buddy:
* +---------------+ +---------------+---------------+
@@ -71,14 +67,12 @@ static void test_merge_tail_pair(int start_mfn)
ASSERT(pages[1].u.free.first_dirty == INVALID_DIRTY_IDX);
/* The tail pair is expected to be merged into one order-1 buddy. */
- EXPECT_FAIL_BEGIN();
CHECK(PFN_ORDER(&pages[2]) == 1,
"The pair of tail pages should be merged into an order-1 buddy");
CHECK(pages[2].u.free.first_dirty == 1, "In tail buddy, the 2nd is dirty");
/* The tail page of the merged buddy does not use first_dirty. */
CHECK(pages[3].u.free.first_dirty == INVALID_DIRTY_IDX,
"Tail page of the merged buddy should not set first_dirty");
- EXPECT_FAIL_END(3);
}
int main(int argc, char *argv[])
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index d923ae02ade0..dd0b7c67008d 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1427,11 +1427,13 @@ static int reserve_offlined_page(struct page_info *head)
next_order = cur_order = 0;
+ /* Attempt to grow the order (size) of the buddy as much as possible. */
while ( cur_order < head_order )
{
next_order = cur_order + 1;
- if ( (cur_head + (1 << next_order)) >= (head + ( 1 << head_order)) )
+ /* Do not grow to next_order if it would go beyond the buddy. */
+ if ( (cur_head + (1 << next_order)) > (head + ( 1 << head_order)) )
goto merge;
/* Do not grow to next_order if cur_head is not aligned to it. */
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread