Hi Jan

> -----Original Message-----
> From: Jan Beulich <[email protected]>
> Sent: Monday, July 25, 2022 11:36 PM
> To: Penny Zheng <[email protected]>
> Cc: Wei Chen <[email protected]>; Andrew Cooper
> <[email protected]>; George Dunlap <[email protected]>;
> Julien Grall <[email protected]>; Stefano Stabellini <[email protected]>;
> Wei Liu <[email protected]>; [email protected]
> Subject: Re: [PATCH v9 6/8] xen/arm: unpopulate memory when domain is
> static
> 
> On 20.07.2022 07:46, Penny Zheng wrote:
> > Today when a domain unpopulates the memory on runtime, they will
> > always hand the memory back to the heap allocator. And it will be a
> > problem if domain is static.
> >
> > Pages as guest RAM for static domain shall be reserved to only this
> > domain and not be used for any other purposes, so they shall never go
> > back to heap allocator.
> >
> > This commit puts reserved pages on the new list resv_page_list only
> > after having taken them off the "normal" list, when the last ref dropped.
> 
> I guess this wording somehow relates to ...
> 
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2674,10 +2674,14 @@ void free_domstatic_page(struct page_info
> > *page)
> >
> >      drop_dom_ref = !domain_adjust_tot_pages(d, -1);
> >
> > -    spin_unlock_recursive(&d->page_alloc_lock);
> > -
> >      free_staticmem_pages(page, 1, scrub_debug);
> >
> > +    /* Add page on the resv_page_list *after* it has been freed. */
> > +    if ( !drop_dom_ref )
> > +        page_list_add_tail(page, &d->resv_page_list);
> 
> ... the conditional used here. I cannot, however, figure why there is this
> conditional (and said part of the description also doesn't help me figure it
> out).
> 

I was thinking that if drop_dom_ref true, then later the whole domain struct
will be released, then there is no need to add the page to d->resv_page_list

> As an aside: A title prefix of xen/arm: suggests you're mostly touching Arm
> code. But you're touching exclusively common code here.
> I for one would almost have skipped this patch (more than once) when
> deciding which ones (may) want me looking at them.
> 

Sorry for that, I’ll fix it
> Jan

Reply via email to