Re: [PATCH v4 2/6] xen: do not merge reserved pages in free_heap_pages()

[Julien Grall]

Hi Penny,

On 10/05/2022 03:27, Penny Zheng wrote:

The code in free_heap_pages() will try to merge pages with the
successor/predecessor if pages are suitably aligned. So if the pages
reserved are right next to the pages given to the heap allocator,
free_heap_pages() will merge them, and give the reserved pages to heap
allocator accidently as a result.

So in order to avoid the above scenario, this commit updates free_heap_pages()
to check whether the predecessor and/or successor has PGC_reserved set,
when trying to merge the about-to-be-freed chunk with the predecessor
and/or successor.

Signed-off-by: Penny Zheng 
Suggested-by: Julien Grall 


NIT: In general, the tags are historically ordered. I.e I first sugested 
and then you wrote the patch. So the two tags should be inverted.


This can be done on commit:

Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

[PATCH v4 2/6] xen: do not merge reserved pages in free_heap_pages()

[Penny Zheng]

The code in free_heap_pages() will try to merge pages with the
successor/predecessor if pages are suitably aligned. So if the pages
reserved are right next to the pages given to the heap allocator,
free_heap_pages() will merge them, and give the reserved pages to heap
allocator accidently as a result.

So in order to avoid the above scenario, this commit updates free_heap_pages()
to check whether the predecessor and/or successor has PGC_reserved set,
when trying to merge the about-to-be-freed chunk with the predecessor
and/or successor.

Signed-off-by: Penny Zheng 
Suggested-by: Julien Grall 
Reviewed-by: Jan Beulich 
---
v4 changes:
- commit message refinement
---
v3 changes:
- no changes
---
v2 changes:
- new commit
---
 xen/common/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 5e569a48a2..290526adaf 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1483,6 +1483,7 @@ static void free_heap_pages(
 /* Merge with predecessor block? */
 if ( !mfn_valid(page_to_mfn(predecessor)) ||
  !page_state_is(predecessor, free) ||
+ (predecessor->count_info & PGC_reserved) ||
  (PFN_ORDER(predecessor) != order) ||
  (phys_to_nid(page_to_maddr(predecessor)) != node) )
 break;
@@ -1506,6 +1507,7 @@ static void free_heap_pages(
 /* Merge with successor block? */
 if ( !mfn_valid(page_to_mfn(successor)) ||
  !page_state_is(successor, free) ||
+ (successor->count_info & PGC_reserved) ||
  (PFN_ORDER(successor) != order) ||
  (phys_to_nid(page_to_maddr(successor)) != node) )
 break;
-- 
2.25.1