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

[Julien Grall <julien@xxxxxxx>]

Hi,

On 27/04/2022 10:27, Penny Zheng wrote:
> There is a slim chance that free_heap_pages() may decide to merge a chunk
> from the static region(PGC_reserved) with the about-to-be-free chunk.
This sentence tells me that the merge can happen but it doesn't tell me 
the cases. I think the second part is more important to know.
The code in free_heap_Pages() will try to merge with the 
successor/predecessor if they are suitably aligned. So the issue can 
only happen if the pages reserved are right next to the pages given to 
the heap allocator.
> 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 <penny.zheng@xxxxxxx>
> ---
> 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 be501582a3..1f3ad4bd28 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;
Cheers,

--
Julien Grall