[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 02/16] x86/P2M: introduce p2m_{add,remove}_page()


  • To: George Dunlap <dunlapg@xxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Mon, 7 Feb 2022 10:38:24 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=SnC2UznGjDVROvbGU1mF5qH5Fp/581JGnSKukwxZp0U=; b=e995fgMt7vlRerm7P1muB9oEwayJSjq+vFrRDLajJulNLL2ekdOebl968EpK8YaIhKJIu32pD7RqBa21/rdUxxrGJCm/+S1s38t0G3EJD3KjyS0QviZSlN6/JyH+sMcmZ0ZRZSNrF+YUyub6jl3ap2fzB3UDCOvSTsVzmnVGNf9tRjVM6fNFElgshrZ9XOL1+yf4bicovnp5hCDXsmm84/xE56WREALZ3k4tDay0sCi4GYVhdFhawTiO3pr2stOIQ3Y1Sxx7HzomQu4HP9LMaNbZAXzU/Re/hHwDmtEgpNvJId0CCJOqwPi33q3mqEiVkj0mjQLdFzUoPZOEGmiWNw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LMey19zXuLcnWftzZCtL1aofE9Y6oz4aPyYGvlQMjDeCAKxTZ9DGnyaIojEWEJjAyl58fMZqDwuD7mPOF+rVnG8RwodbS3dPhJc/T3IA1RIOn9gBOOmzJeM6nlv9dZMtYX8FqOOwKe0hNUmo9b72TpPZPaGO2cpovnhhpqpCqyXXZOxb9P+gIeUqQHud25AuNHMGF5onUrHB4iC9sTu8AqSdaZR4I1VwOpu5mfZp+vZ86JUFO54QFaex7lRRHdp/DjtOvdI9fE1dCRcMTI9XnnRK/b8ir9TYsi8O/I+nKDolhOenEEs0HF3mL4XbEHUT91/XwLoIja9s+jRtwf0U8A==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Mon, 07 Feb 2022 09:38:37 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 04.02.2022 23:07, George Dunlap wrote:
> On Mon, Jul 5, 2021 at 5:06 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
>> p2m_add_page() is simply a rename from guest_physmap_add_entry().
>> p2m_remove_page() then is its counterpart, despite rendering
>> guest_physmap_remove_page().

First of all: It has been long ago that I noticed that this sentence
misses words. It now ends "...  a trivial wrapper."

>> This way callers can use suitable pairs of
>> functions (previously violated by hvm/grant_table.c).
>>
> 
> Obviously this needs some clarification.  While we're here, I find this a
> bit confusing; I tend to use the present tense for the way the code is
> before the patch, and the imperative for what the patch does; so Id' say:
> 
> Rename guest_physmap_add_entry() to p2m_add_page; make
> guest_physmap_remove_page a wrapper with p2m_remove_page.  That way callers
> can use suitable pairs...

Well, yes, I understand you might word it this way. I'm not convinced
of the fixed scheme you mention for present vs imperative use to be a
universal fit though, requiring to always be followed. When reading
the description with the title in mind (and with the previously missing
words added), I find the use of present tense quite reasonable here.
I'm further slightly puzzled by you keeping the use of present tense in
"That way callers can use ...".

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.