Hi Jan > -----Original Message----- > From: Jan Beulich <jbeul...@suse.com> > Sent: Monday, July 25, 2022 11:36 PM > To: Penny Zheng <penny.zh...@arm.com> > Cc: Wei Chen <wei.c...@arm.com>; Andrew Cooper > <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>; > Julien Grall <jul...@xen.org>; Stefano Stabellini <sstabell...@kernel.org>; > Wei Liu <w...@xen.org>; xen-devel@lists.xenproject.org > 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
