Re: [PATCH v9 8/8] xen: retrieve reserved pages on populate_physmap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.08.2022 11:53, Bertrand Marquis wrote:
>> On 9 Aug 2022, at 09:58, Penny Zheng <Penny.Zheng@xxxxxxx> wrote:
>>> -----Original Message-----
>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>> Sent: Tuesday, August 9, 2022 4:27 PM
>>>
>>> On 09.08.2022 10:07, Penny Zheng wrote:
>>>>> -----Original Message-----
>>>>> From: Jan Beulich <jbeulich@xxxxxxxx>
>>>>> Sent: Tuesday, August 9, 2022 3:59 PM
>>>>>
>>>>> On 09.08.2022 09:53, Penny Zheng wrote:
>>>>>>> -----Original Message-----
>>>>>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> 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.zheng@xxxxxxx>
>>>>>>>> ---
>>>>>>>> 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 ?

I don't mind this, yes.

Jan