Hi Julien > -----Original Message----- > From: Julien Grall <[email protected]> > Sent: Saturday, June 25, 2022 3:51 AM > To: Penny Zheng <[email protected]>; [email protected] > Cc: Wei Chen <[email protected]>; Andrew Cooper > <[email protected]>; George Dunlap <[email protected]>; > Jan Beulich <[email protected]>; Stefano Stabellini <[email protected]>; > Wei Liu <[email protected]> > 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 <[email protected]> > > --- > > 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
