* [PATCH 0/2] xen/mm: Reset PFN_ORDER for offlined buddy heads
@ 2026-05-28 14:58 Bernhard Kaindl
2026-05-28 14:58 ` [PATCH 1/2] tools/tests/native: Add test for offlined buddy head PFN_ORDER Bernhard Kaindl
2026-05-28 14:58 ` [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
0 siblings, 2 replies; 5+ messages in thread
From: Bernhard Kaindl @ 2026-05-28 14:58 UTC (permalink / raw)
To: xen-devel
Cc: Bernhard Kaindl, Anthony PERARD, Andrew Cooper, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
This series fixes an inconsistency in the PFN_ORDER annotation for
pages that are offlined when they are the head of a free buddy.
When reserve_offlined_page() splits a buddy and moves offlined
sub-pages to the offlined lists, the former buddy head would be
left annotated with its original order even though it is now a
single page. While this does not cause functional regressions,
it is misleading.
Patch 1 adds a native regression test that reproduces and documents
the problem. The test seeds an order-1 buddy, offlines the head page,
verifies the tail becomes order-0, and checks the offlined head's order.
Patch 2 resets PFN_ORDER to 0 for offlined buddy heads inside
reserve_offlined_page() when moving the page to the offlined list
and updates the regression test to assert the corrected behaviour.
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
which in turn is based on the NUMA claim sets v7 series:
https://lists.xen.org/archives/html/xen-devel/2026-05/msg00363.html
Please help to review and ack it (It should be eligible for Xen 4.22)
Pull this series with all dependencies for review/test:
$ git pull git@gitlab.com:bernhardkaindl/xen.git offline-head-order
$ make -C tools/tests/native TARGETS=offline-head-order test |
grep -C3 PG_OFFLINE_STATUS_OFFLINED
Log of checking the PFN_ORDER() of the offlined page before the fix is applied:
| offline-head-order.c:32: ASSERT(PFN_ORDER(page) == 1)
| offline-head-order.c:34: ASSERT(offline_page(page_to_mfn(page), 0, &status) == 0)
| offline-head-order.c:35: ASSERT(status == PG_OFFLINE_OFFLINED)
| offline-head-order.c:39: ASSERT(query_page_offline(page_to_mfn(page), &status) == 0)
| offline-head-order.c:40: ASSERT(status == PG_OFFLINE_STATUS_OFFLINED)
|
| - Test assertion failed as expected at offline-head-order.c:44:
| Assertion failed: PFN_ORDER(page) == 0
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
Bernhard Kaindl (2):
tools/tests/native: Add test for offlined buddy head PFN_ORDER
xen/mm: reset PFN_ORDER for offlined buddy heads
tools/tests/native/offline-head-order.c | 79 +++++++++++++++++++++++++
xen/common/page_alloc.c | 7 +++
2 files changed, 86 insertions(+)
create mode 100644 tools/tests/native/offline-head-order.c
--
2.39.5
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] tools/tests/native: Add test for offlined buddy head PFN_ORDER
2026-05-28 14:58 [PATCH 0/2] xen/mm: Reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
@ 2026-05-28 14:58 ` Bernhard Kaindl
2026-05-28 14:58 ` [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
1 sibling, 0 replies; 5+ messages in thread
From: Bernhard Kaindl @ 2026-05-28 14:58 UTC (permalink / raw)
To: xen-devel; +Cc: Bernhard Kaindl, Anthony PERARD
Add a regression test that reproduces and documents an inconsistent
PFN_ORDER observed when the head page of a free buddy is offlined.
When the head of a free buddy is offlined it remains annotated with its
original PFN_ORDER while offline, even though the buddy has been split
into single pages. This mismatch is surprising and can hinder debugging,
although it does not currently cause functional failures.
What the test does:
- Seed a single order-1 buddy on the heap and offline the head page.
- Document the current failing behaviour by asserting (EXPECT_FAIL)
that the offlined head's PFN_ORDER is (not) 0.
- Allocate the successor to prevent a merge, online the page, and
verify that onlining restores the PFN_ORDER to 0.
How to run:
make -C tools/tests/native TARGETS=offline-head-order test
Example failing output (current behaviour):
- Test assertion failed as expected at offline-head-order.c:40:
Offlined former buddy head should be order0
This test is intended as a regression test and should be updated
with the same commit that resets the PFN_ORDER to 0 on offlining.
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
---
tools/tests/native/offline-head-order.c | 87 +++++++++++++++++++++++++
1 file changed, 87 insertions(+)
create mode 100644 tools/tests/native/offline-head-order.c
diff --git a/tools/tests/native/offline-head-order.c b/tools/tests/native/offline-head-order.c
new file mode 100644
index 000000000000..5c666a319523
--- /dev/null
+++ b/tools/tests/native/offline-head-order.c
@@ -0,0 +1,87 @@
+/* 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.
+ *
+ * Copyright (C) 2026 Cloud Software Group
+ */
+#define TEST_ENABLE_XC_DOMAIN_C
+#include "harness/native.h"
+
+static void test_offline_head_order(int start_mfn)
+{
+ struct page_info *page = frame_table + start_mfn;
+ uint32_t status = 0;
+
+ /* Seed a single order-1 buddy onto the heap. */
+ test_page_list_add_buddy(page, order1);
+ ASSERT(PFN_ORDER(page) == 1);
+ /* Offline the head page. */
+ ASSERT(offline_page(page_to_mfn(page), 0, &status) == 0);
+ ASSERT(status == PG_OFFLINE_OFFLINED);
+
+ /* Confirm the status of the page as status offlined. */
+ status = 0;
+ ASSERT(query_page_offline(page_to_mfn(page), &status) == 0);
+ ASSERT(status == PG_OFFLINE_STATUS_OFFLINED);
+
+ /* Check the order of the offlined head page. */
+ EXPECT_FAIL_BEGIN(); /* PFN_ORDER(page) should 0, but is still 1 */
+ ASSERT(PFN_ORDER(page) == 0);
+ EXPECT_FAIL_END(1);
+
+ /*
+ * Allocate the successor page of the offlined page. This prevents
+ * the normal successor page merge when the page is re-onlined below.
+ */
+ struct page_info *pg = alloc_domheap_pages(dom1, order0, 0);
+ ASSERT(pg == page + 1);
+ ASSERT(FREE_PAGES == 0);
+
+ /*
+ * The order of the split head page is still 1. Online the page again to
+ * confirm that onlining it causes the order to be corrected to 0.
+ */
+ ASSERT(PFN_ORDER(page) == 1);
+
+ /* Online the offlined former head page. */
+ ASSERT(online_page(page_to_mfn(page), &status) == 0);
+ ASSERT(status & PG_ONLINE_ONLINED);
+ ASSERT(FREE_PAGES == 1);
+
+ /*
+ * Confirm the order of the onlined former head page is 0, independently
+ * of the order returned by PFN_ORDER() for the offlined page. This should
+ * always be successful because page_offlined_list only contains single
+ * pages and online_page() ignores PFN_ORDER(pg) of the page. It calls
+ * free_heap_pages() passing the order hardcoded to 0. This causes it to
+ * pass the given order 0 to page_list_add_scrub(). This causes it to set
+ * the order of the page to 0 before it adds the page to the free list.
+ */
+ ASSERT(PFN_ORDER(page) == 0);
+}
+
+int main(int argc, char *argv[])
+{
+ const char *topic = "Test offlined head page to be updated to PFN_ORDER 0";
+ if ( !parse_args(argc, argv, topic) )
+ return EXIT_FAILURE;
+
+ init_page_alloc_tests();
+ RUN_TESTCASE("TOHP", test_offline_head_order, 2);
+ return test_complete();
+}
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads
2026-05-28 14:58 [PATCH 0/2] xen/mm: Reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
2026-05-28 14:58 ` [PATCH 1/2] tools/tests/native: Add test for offlined buddy head PFN_ORDER Bernhard Kaindl
@ 2026-05-28 14:58 ` Bernhard Kaindl
2026-06-02 15:57 ` Jan Beulich
1 sibling, 1 reply; 5+ messages in thread
From: Bernhard Kaindl @ 2026-05-28 14:58 UTC (permalink / raw)
To: xen-devel
Cc: Bernhard Kaindl, Anthony PERARD, Andrew Cooper, Michal Orzel,
Jan Beulich, Julien Grall, Roger Pau Monné,
Stefano Stabellini
Ensure offlined buddy head pages are annotated as order-0 pages.
When a buddy containing pages marked for offlining is processed,
reserve_offlined_page() rebuilds any surviving healthy buddies
and moves the offlined subpages onto the offlined lists.
If the buddy head itself is offlined it was previously left
annotated with the original buddy order even though it has
been split into a single page.
This has no functional impact as the order of an offlined
page is not used for any decision making and onlining, but
it is misleading when inspecting the page's metadata.
Reset PFN_ORDER(cur_head) to 0 for an offlined buddy head
when moving it to the offlined list so the page's stored
order reflects its actual size.
Also update the regression test to assert the consistent
behavior.
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>
---
tools/tests/native/offline-head-order.c | 8 --------
xen/common/page_alloc.c | 7 +++++++
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/tools/tests/native/offline-head-order.c b/tools/tests/native/offline-head-order.c
index 5c666a319523..5239fc34518f 100644
--- a/tools/tests/native/offline-head-order.c
+++ b/tools/tests/native/offline-head-order.c
@@ -40,9 +40,7 @@ static void test_offline_head_order(int start_mfn)
ASSERT(status == PG_OFFLINE_STATUS_OFFLINED);
/* Check the order of the offlined head page. */
- EXPECT_FAIL_BEGIN(); /* PFN_ORDER(page) should 0, but is still 1 */
ASSERT(PFN_ORDER(page) == 0);
- EXPECT_FAIL_END(1);
/*
* Allocate the successor page of the offlined page. This prevents
@@ -52,12 +50,6 @@ static void test_offline_head_order(int start_mfn)
ASSERT(pg == page + 1);
ASSERT(FREE_PAGES == 0);
- /*
- * The order of the split head page is still 1. Online the page again to
- * confirm that onlining it causes the order to be corrected to 0.
- */
- ASSERT(PFN_ORDER(page) == 1);
-
/* Online the offlined former head page. */
ASSERT(online_page(page_to_mfn(page), &status) == 0);
ASSERT(status & PG_ONLINE_ONLINED);
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index dd0b7c67008d..1801afc96a0a 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1485,6 +1485,13 @@ static int reserve_offlined_page(struct page_info *head)
ASSERT(node_avail_pages[node] > 0);
node_avail_pages[node]--;
+ /*
+ * All offlined pages are standalone pages: If this offlined page was
+ * the head of a higher-order buddy, we need to reset its order to 0:
+ */
+ if ( cur_head == head && head_order != 0 )
+ PFN_ORDER(cur_head) = 0;
+
page_list_add_tail(cur_head,
test_bit(_PGC_broken, &cur_head->count_info) ?
&page_broken_list : &page_offlined_list);
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads
2026-05-28 14:58 ` [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
@ 2026-06-02 15:57 ` Jan Beulich
2026-06-03 15:10 ` Bernhard Kaindl
0 siblings, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2026-06-02 15:57 UTC (permalink / raw)
To: Bernhard Kaindl
Cc: Anthony PERARD, Andrew Cooper, Michal Orzel, Julien Grall,
Roger Pau Monné, Stefano Stabellini, xen-devel
On 28.05.2026 16:58, Bernhard Kaindl wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1485,6 +1485,13 @@ static int reserve_offlined_page(struct page_info *head)
> ASSERT(node_avail_pages[node] > 0);
> node_avail_pages[node]--;
>
> + /*
> + * All offlined pages are standalone pages: If this offlined page was
> + * the head of a higher-order buddy, we need to reset its order to 0:
> + */
> + if ( cur_head == head && head_order != 0 )
> + PFN_ORDER(cur_head) = 0;
> +
> page_list_add_tail(cur_head,
> test_bit(_PGC_broken, &cur_head->count_info) ?
> &page_broken_list : &page_offlined_list);
Again, with this separated from and moved ahead of the new test:
Reviewed-by: Jan Beulich <jbeulich@suse.com>
I'd suggest to drop the rhs of the && though: There's nothing wrong with storing
0 when the field already is 0. I actually wonder whether the if() is needed at
all. Unconditionally storing 0 is going to make the code more robust against
future changes elsewhere.
Jan
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads
2026-06-02 15:57 ` Jan Beulich
@ 2026-06-03 15:10 ` Bernhard Kaindl
0 siblings, 0 replies; 5+ messages in thread
From: Bernhard Kaindl @ 2026-06-03 15:10 UTC (permalink / raw)
To: Jan Beulich
Cc: Anthony PERARD, Andrew Cooper, Michal Orzel, Julien Grall,
Roger Pau Monne, Stefano Stabellini,
xen-devel@lists.xenproject.org
[...]
> Again, with this separated from and moved ahead of the new test:
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Ack, thanks for the review!
Like before, I'll submit v2 with the same changes momentarily.
Also applied setting 'PFN_ORDER(pg) = 0' unconditionally as suggested.
Bernhard
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-06-03 15:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 14:58 [PATCH 0/2] xen/mm: Reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
2026-05-28 14:58 ` [PATCH 1/2] tools/tests/native: Add test for offlined buddy head PFN_ORDER Bernhard Kaindl
2026-05-28 14:58 ` [PATCH 2/2] xen/mm: reset PFN_ORDER for offlined buddy heads Bernhard Kaindl
2026-06-02 15:57 ` Jan Beulich
2026-06-03 15:10 ` Bernhard Kaindl
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.