[PATCH 0/2] xen/mm: Fix off-by-one for tail merge in reserve_offlined_page()

[PATCH 0/2] xen/mm: Fix off-by-one for tail merge in reserve_offlined_page()

[Bernhard Kaindl]

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.

Patch 1 adds a native regression test that reproduces the problem.
Patch 2 fixes the off-by-one error and updates the regression test.

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          |
+---------------+               +---------------+---------------+

This series is based on the native test environment v3 for NUMA claims
https://lists.xen.org/archives/html/xen-devel/2026-05/msg01163.html

In turn, it is based on the NUMA claim sets v7 series:
https://lists.xen.org/archives/html/xen-devel/2026-05/msg00363.html

You can pull this series with all dependencies for review and test:
$ git pull git@gitlab.com:bernhardkaindl/xen.git offline-merge-tail-v1
$ make -C tools/tests/native TARGETS=offline-merge-tail test

Fixes: e4865c2315 ('Page offline support in Xen side')
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>

Bernhard Kaindl (2):
  tools/tests/native: Test merging the tail after an offlined page
  xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page

 tools/tests/native/offline-merge-tail.c | 87 +++++++++++++++++++++++++
 xen/common/page_alloc.c                 |  4 +-
 2 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 tools/tests/native/offline-merge-tail.c

-- 
2.39.5

[PATCH 1/2] tools/tests/native: Test merging the tail after an offlined page

[Bernhard Kaindl]

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

[PATCH 2/2] xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page

[Bernhard Kaindl]

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

Re: [PATCH 2/2] xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page

[Jan Beulich]

On 28.05.2026 16:55, Bernhard Kaindl wrote:
> --- 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. */

Much like for the other series: With this coming first (and separate from the
new test), with the stray blank removed from the line that you touch, and with
parentheses also added to reserve_offlined_page() in the subject (like you
have it in the description):
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Maybe also s/stopping/preventing/ in the subject?

Jan

RE: [PATCH 2/2] xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page

[Bernhard Kaindl]

[...]
> Much like for the other series: With this coming first (and separate from the
> new test), with the stray blank removed from the line that you touch, and with
> parentheses also added to reserve_offlined_page() in the subject (like you
> have it in the description):
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Ack, thanks for the review!

I'll submit v2 with all changes and the suggested title improvement momentarily.

   Bernhard