Hi Julien

> -----Original Message-----
> From: Julien Grall <jul...@xen.org>
> Sent: Saturday, June 25, 2022 3:51 AM
> To: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <wei.c...@arm.com>; Andrew Cooper
> <andrew.coop...@citrix.com>; George Dunlap <george.dun...@citrix.com>;
> Jan Beulich <jbeul...@suse.com>; Stefano Stabellini <sstabell...@kernel.org>;
> Wei Liu <w...@xen.org>
> Subject: Re: [PATCH v7 9/9] xen: retrieve reserved pages on
> populate_physmap
> 
> Hi Penny,
> 
> On 20/06/2022 03:44, Penny Zheng wrote:
> > When a static domain populates memory through populate_physmap at
> > runtime, it shall retrieve reserved pages from resv_page_list to make
> > sure that guest RAM is still restricted in statically configured memory
> regions.
> > This commit also introduces a new helper acquire_reserved_page to make
> it work.
> >
> > Signed-off-by: Penny Zheng <penny.zh...@arm.com>
> > ---
> > v7 changes:
> > - remove the lock, since we add the page to rsv_page_list after it has
> > been totally freed.
> 
> Hmmm... Adding the page after it has been totally freed doesn't mean you
> can get away with the lock. AFAICT you can still have concurrent free/allocate
> that could modify the list.
> 
> Therefore if you add/remove pages without the list, you would end up to
> corrupt the list.
> 
> If you disagree, then please point out which lock (or mechanism) will prevent
> concurrent access.
> 

Ok. Combined with the last serie comments, actually, you suggest that we need 
to add
two locks, right?

One is the lock for concurrent free/allocation on page_info, and we will use
heap_lock, one stays in free_staticmem_pages, one stays in its reversed function
prepare_staticmem_pages.

The other is for concurrent free/allocation on resv_page_list, and we will use
d->page_alloc_lock tp guard it. One stays in put_static_page, and another
stays in reversed function acquire_reserved_page.

> Cheers,
> 
> --
> Julien Grall

Reply via email to