Hi Penny, > On 9 Aug 2022, at 09:58, Penny Zheng <penny.zh...@arm.com> wrote: > > Hi jan > >> -----Original Message----- >> From: Jan Beulich <jbeul...@suse.com> >> Sent: Tuesday, August 9, 2022 4:27 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 8/8] xen: retrieve reserved pages on >> populate_physmap >> >> On 09.08.2022 10:07, Penny Zheng wrote: >>>> -----Original Message----- >>>> From: Jan Beulich <jbeul...@suse.com> >>>> Sent: Tuesday, August 9, 2022 3:59 PM >>>> >>>> On 09.08.2022 09:53, Penny Zheng wrote: >>>>>> -----Original Message----- >>>>>> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf >>>>>> Of Jan Beulich >>>>>> Sent: Monday, July 25, 2022 11:44 PM >>>>>> >>>>>> On 20.07.2022 07:46, 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> >>>>>>> --- >>>>>>> v9 changes: >>>>>>> - Use ASSERT_ALLOC_CONTEXT() in acquire_reserved_page >>>>>>> - Add free_staticmem_pages to undo prepare_staticmem_pages when >>>>>>> assign_domstatic_pages fails >>>>>> >>>>>> May I suggest to re-consider naming of the various functions? >>>>>> Undoing what "prepare" did by "free" is, well, counterintuitive. >>>>>> >>>>> >>>>> How about change the name "prepare_staticmem_pages" to >>>> "allocate_staticmem_pages"? >>>> >>>> Perhaps - if what the function does really resembles allocation in some >> way. >>>> So far I wasn't really certain in that regard, and hence I was >>>> wondering whether "prepare" doesn't better describe what it does, but >>>> then its inverse also doesn't really "free" anything. >>>> >>> >>> Hmmmm, “prepare” with “destroy” in its inverse? Do you have any >> suggestion in mind? >> >> To be honest I was hoping you would make an attempt at finding a suitable >> pair of verbs. To me "destroy" is more the opposite of "create", and I'm >> unable to think of a good opposite of "prepare" (short of resorting to >> "unprepare"); if I really needed to come up with something then it would >> likely be "cleanup", albeit I'd not be overly happy with that either. >> > > Maybe unprepare is better here, I was searching linux code for the help, and > they are using prepare/unprepare as a pair of verbs a lot in drivers codes. > > For the renaming here, I suggest to fix it with a new commit, since > free_staticmem_pages > has already been merged.
I think that unprepare is ok and it make sense to do this in an independent patch. @jan: can you confirm that you agree with this way to go ? Cheers Bertrand
