Hi Penny,

Sorry I only spotted a couple of typos in the commit message now.

On 20/07/2022 06:46, 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.

s/accidently/accidentally/ I think.


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,

s/PGC_reserved/PGC_staticmem/

I think both #1 and #3 can go in now. So I would be happy to handle the typoes on commit.

when trying to merge the about-to-be-freed chunk with the predecessor
and/or successor.

Suggested-by: Julien Grall <jgr...@amazon.com>
Signed-off-by: Penny Zheng <penny.zh...@arm.com>
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Reviewed-by: Julien Grall <jgr...@amazon.com>
---
v9 changes:
- no change
---
v8 changes:
- no change
---
v7 changes:
- no change
---
v6 changes:
- adapt to PGC_static
---
v5 changes:
- change PGC_reserved to adapt to PGC_staticmem
---
v4 changes:
- no changes
---
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 a12622e921..45bd88a685 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1475,6 +1475,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_static) ||
                   (PFN_ORDER(predecessor) != order) ||
                   (phys_to_nid(page_to_maddr(predecessor)) != node) )
                  break;
@@ -1498,6 +1499,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_static) ||
                   (PFN_ORDER(successor) != order) ||
                   (phys_to_nid(page_to_maddr(successor)) != node) )
                  break;

Cheers,

--
Julien Grall

Reply via email to