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

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

[Bernhard Kaindl]

After offlining pages, reserve_offlined_page() attempts to grow larger
buddies between the offlined pages, but due to an off-by-one, this
fails at the tail end of the span of pages of the containing buddy.

As requested for backporting the fix, the first patch fixes the
issue while the second patch adds the regression test for it.

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

You can pull this series with the regression test environment to run it:
$ git pull git@gitlab.com:bernhardkaindl/xen.git offline-merge-tail-v2
$ 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):
  xen/mm: Fix off-by-one preventing tail merge in
    reserve_offlined_page()
  tools/tests: Add regression test for tail buddy growth

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

-- 
2.39.5

[PATCH v2 1/2] xen/mm: Fix off-by-one preventing 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.

Fixes: e4865c2315 ('Page offline support in Xen side')
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v2:
- Bugfix isolated from the test case for backporting.
- Removed stray blank from the line that I touch.
- Title: Replaced stopping with preventing.
- Title: Added parentheses after reserve_offlined_page().
---
 xen/common/page_alloc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 2767376a710b..e01ac3e99c72 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1195,11 +1195,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

[PATCH v2 2/2] tools/tests: Add regression test for tail buddy growth

[Bernhard Kaindl]

Add a regression test covering reserve_offlined_page() to verify
that, when composing healthy buddies between offlined pages,
healthy spans at the tail of the containing buddy are promoted to
larger-order buddies as expected.

Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
---
 tools/tests/native/offline-merge-tail.c | 81 +++++++++++++++++++++++++
 1 file changed, 81 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..217595e70ece
--- /dev/null
+++ b/tools/tests/native/offline-merge-tail.c
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Test merging a surviving tail pair into an order-1 buddy.
+ *
+ * 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.
+ *
+ * Copyright (C) 2026 Cloud Software Group
+ */
+#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
+     * 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. */
+    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");
+}
+
+int main(int argc, char *argv[])
+{
+    const char *topic = "Test offlining to merge tails into an order-1 buddy";
+
+    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

Re: [PATCH v2 1/2] xen/mm: Fix off-by-one preventing tail merge in reserve_offlined_page()

[Jan Beulich]

On 03.06.2026 16:27, Bernhard Kaindl wrote:
> 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.
> 
> Fixes: e4865c2315 ('Page offline support in Xen side')
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Oleksii, same question again here.

Jan

> ---
> v2:
> - Bugfix isolated from the test case for backporting.
> - Removed stray blank from the line that I touch.
> - Title: Replaced stopping with preventing.
> - Title: Added parentheses after reserve_offlined_page().
> ---
>  xen/common/page_alloc.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 2767376a710b..e01ac3e99c72 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1195,11 +1195,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. */

Re: [PATCH v2 1/2] xen/mm: Fix off-by-one preventing tail merge in reserve_offlined_page()

[Oleksii Kurochko]

On 6/3/26 4:33 PM, Jan Beulich wrote:
> On 03.06.2026 16:27, Bernhard Kaindl wrote:
>> 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.
>>
>> Fixes: e4865c2315 ('Page offline support in Xen side')
>> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Oleksii, same question again here.
> 


Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

Re: [PATCH v2 1/2] xen/mm: Fix off-by-one preventing tail merge in reserve_offlined_page()

[Oleksii Kurochko]

On 6/3/26 4:33 PM, Jan Beulich wrote:
> On 03.06.2026 16:27, Bernhard Kaindl wrote:
>> 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.
>>
>> Fixes: e4865c2315 ('Page offline support in Xen side')
>> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> Oleksii, same question again here.
> 
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>

~ Oleksii