|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH 2/2] xen/mm: Fix off-by-one stopping tail merge in reserve_offlined_page
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@xxxxxxxxxx>
---
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
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |