Re: [PATCH v1 3/5] xen/arm: unpopulate memory when domain on static allocation

[Julien Grall <julien@xxxxxxx>]

Hi Penny,

On 31/03/2022 11:30, Penny Zheng wrote:
> Another reason I want to keep page allocated is that if putting pages in
> resv_page_list upon dropping the last ref, we need to do a lot things on
> pages to totally let it free, like set its owner to NULL, changing page state
> from in_use to free, etc.
This is not only about optimization here. Bad things can happen if you 
let a page in state free that is not meant to be used by the buddy 
allocator (e.g. it was reserved for static memory).
I discovered it earlier this year when trying to optimize 
init_heap_pages() for Live-Update. It was quite hard to debug because 
the corruption very rarely happened. So let me explain it before you 
face the same issue :).
free_heap_pages() will try to merge the about-to-be-freed chunk with the 
predecessor and/or successor. To know if the page can be merged, the 
algorithm is looking at whether the chunk is suitably aligned (e.g. same 
order) and if the page is in state free.
AFAICT, the pages belonging to the buddy allocator could be right next 
to region reserved memory. So there is a very slim chance that 
free_heap_pages() may decide to merge a chunk from the static region 
with the about-to-be-free chunk. Nothing very good will ensue.
Technically, this is already a bug in the already merged implementation 
of the static memory allocator.
I can see two ways to fix it:
  1) Update free_heap_pages() to check whether the page has 
PGC_reserved set.
  2) Use a different state for pages used by the static allocator.

So far my preference is leaning towards 1. But I would like to hear 
other opinions.
Cheers,

--
Julien Grall