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

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


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: George Dunlap <George.Dunlap@xxxxxxxxxx>
  • Date: Mon, 7 Feb 2022 15:49:17 +0000
  • Accept-language: en-US
  • 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=MY7Z5FwUiaPGBlJzHz/bNmCDZrW81h1hOhMJKyQZMPM=; b=b16+GZ30saxSAKczhlUa5ZbPNAcht2c55cx8eJXbRgTBYbStnysb1LGhWfaJrnnreVb++4PVqCTYOrn9c+FBGT4BPZf1SM1MY3gKSj9tgQ7o5hwjKkc5uIVu4BSW/kwj4lo/VGFnmDJQZcolmTJvoSnSVUu0Jr5k734cJmcleJ1SkNjkFyn5PRSBT2/mN3k7lPE7sJHilrBXzIonCWPVZ0z5b+7pCorBlHbN5FY3YTsXZeZq9OodD+b2yTtnWrgfcFZ1jBWZd7RKsBQjh6hmyWu1/JZiVf8UEoF396w4DCurEhAE/8MPuXpM6rekEW6Bb9uqf8OZ07Vbhy5ciTgNxg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=DhK1kFYLvZJahA6Fc41McMyoB24nrqJKRS9Z3JdykS4jVdSO1sl94LfnROWRN+DokjLLxFrTrnXztgSmwTfbzIyHVll0nsMBkJ/eE2jRtAAkoed+sEfExAnt04Wb8Z2QuVAtRgbStxhEpevEwdANgSbJZB0Dz08r/4e26kxgV0kDBmKosy3JBAjLwU8FZx2Xc69bL6L+A3+2bbp42OoaX7BMzcJdqE4k8bmhij7IrvkZHMWll+WnJp4P2DCcFb3Us2Ambt1EAEudT0NxEEbbHiRowTyhVf2GCYs5gAEUxgLcAC3iSvOVopRr3+FkxXrHbQXM1yXtSwEN56lK7xuzTg==
  • Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: George Dunlap <dunlapg@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>
  • Delivery-date: Mon, 07 Feb 2022 15:49:32 +0000
  • Ironport-data: A9a23:ccr59K97bi8VEnwJOIF4DrUDeXmTJUtcMsCJ2f8bNWPcYEJGY0x3x mdMXGqGafmJNjH8KYp1aozk9B5V65bTyoRgQFZuqSE8E34SpcT7XtnIdU2Y0wF+jyHgoOCLy +1EN7Es+ehtFie0Si9AttENlFEkvU2ybuOU5NXsZ2YhFWeIdA970Ug5w7Rh3dYy6TSEK1jlV e3a8pW31GCNg1aYAkpMg05UgEoy1BhakGpwUm0WPZinjneH/5UmJMt3yZWKB2n5WuFp8tuSH I4v+l0bElTxpH/BAvv9+lryn9ZjrrT6ZWBigVIOM0Sub4QrSoXfHc/XOdJFAXq7hQllkPhXw YwQm6aBFDsgI63DpdpEaxJ4I3xXaPguFL/veRBTsOSWxkzCNXDt3+9vHAc9OohwFuRfWD8Us 6ZCcXZUM07F17neLLGTE4GAguwMIc7xM45ZlmxmyTjBJf0nXYrCU+PB4towMDIY2JsTQ6mHP JRxhTxHdgT7bj4SBHMrMJ8HocyXnkvieWFTkQfAzUYwyzeKl1EguFT3C/LKfvSaSMMTmVyXz kre9nn9KgEXMpqY0zXt2mKhgKrDkD32XKoWFaak7bh6jVuL3GsRBRYKE1yhrpGRiESzRtZeI Ew84Tc1oO4580nDZtXgWRmxuHGsoxsdUNoWHuEngDxh0YKNvVzfXDJdCGccNpp276faWADGy Hevloq0KwZe7YGcVHKSy4nNlTiSCXU8eDpqiTA/cSMJ5NzqoYcWhx3JT8p+HKPdsuAZCQ0c0 BjR8nFg2ux7YdojkvzioAuZ227ESo3hE1Ztjjg7SF5J+e+QiGSNQ4WzoWbW4v9bRGpyZgnQ5 SNU8yRyAQ1nMH1sqMBvaLhXdF1Kz6zcWNE5vbKIN8NwnwlBA1b5IehtDMhWfS+FyPosdz7ze 1P0sghM/pJVN3bCRfYpP97oWp1zk/m6TI+NuhXogj1mOMkZSeN61Hs2OR74M57FzCDAbp3Ty b/EKJ3xXB72+IxszSasRvd17FPY7ntW+I8nfriil07P+ePHPBa9EO5ZWHPTPrFRxP7V+239r ocAX+PUkE83eLOlPUHqHXs7cAliwY4TXsut9aS6t4erf2JbJY3WI6WNmON6Jd05wPg9eyWh1 ijVZ3K0AWHX3BXvAQ6LdmpiePXoW5N+pmg8JisiIRCj3H1LXGplxP13m0IfceZ1+ep94+RzS vVZKcyMDu4WEmbM+igHbIm7p4tnLUz5iQWLNiujQT4+Y58/GFCZpo66JlPipHsUEy66lcoiu Ln8hAnVdoUOGlZ5B8HMZfPxk17o5SoBmPh/VlfjK8VIfBm+65BjLiH816dlI8wFJRjZ6CGd0 gKaXUURqeXX+tdn+9jVn6GU6YyuFrImTEZdGmDa65ewNDXboTX/kdMRDr7QcGmEBm3u+aika eFE9N3GMaUKzARQro5xM7d31qZitdHhkKBXk1Z/F3LRYlX1Vr45eiua3dNCv7Fmz6NCvVfkQ VqG/9RXNOnbOM7hF1JNdgMpYv7aiKMRkzjWq/80PF/79Gl8+7/eCRdeOByFiSp8KrppMdx6n bd96ZBOswHv2AA3NtumjzxP8zXeJ3MNZKwrq5UGDdK5kQEs0FxDPcTRByKeDEtjsDmQ3p3G+ gOpuZc=
  • Ironport-hdrordr: A9a23:5M7dxqF+ei1SautspLqF5JLXdLJyesId70hD6qkvc3Bom52j+v xGws5w6fatskdrZJhSo6H7BEDgewKXyXcR2+Ys1NiZLW7bUQeTTL2KqLGSuwEIeBeOu9K1t5 0QFZSWYeeYZTMV7PoSojPIaurIq+P3kpxA8N2uq0uFOjsaDp2IgT0JaTpzQHcGHDWvpPcCZc ahD5981nydUEVSReiAPD0iQ/XJocHNmdbdeBYDQyUq8Q+UkFqTmeTHOind9TslaXdo3aoo93 TDnkjC/62m98uwzATRvlWjtKh+qZ/L5uF4QOqRl8MSNjLgzjywbIAkYaCLoSwuydvftWrCxO O8+isIDoBW0Tf8b2u1qRzi103LyzA18ULvzleenD/KvdH5bChSMbsPuatpNj/ir2YwttB116 xGm0iDsYBMMB/GlCPho/DVShBRkFauq3ZKq59cs5Ufa/peVFZil/1dwKpnKuZDIMs80vFjLA BaNrCd2B+RSyLfU5mWhBgp/DXmZAVPIv7PeDl/hiXS6UkboJkx9Tpm+OUP2ngH754zUJ9C+q DNNblpjqhHSosMYbt6H/ppe7r/NoXhe2O6DIupGyWQKEjHAQO/l7fnpLEuoO26cp0By5U/3J zHTVNDrGY3P0bjE9eH0pFH+g3EBDzVZ0Wn9uhOo5xi/rHsTrviNiOODFgojsu7uv0aRsnWQe y6Np5aC+LqaWHuBYFK1QvjXIQ6EwhXbOQF/tIgH16eqMPCLYPn8uTdbfbIPbLoVS0pX2vua0 FzKAQb5P8wm3xDdkWI8SQ5akmdCHAXp6gAbpQy19JjuLQwCg==
  • Ironport-sdr: d523P19b6Rg0LPIMo/cyDpWhQJ1Ir6Y5uhxQSl8blDEit1iJsLYp1g6jkD/jpfM+SZT/QTLTBW n+of6KM87F5uXZx/c59ClsO3FQiMwK8M2MIMAoZEF6J7Qx6NeZblvwpOoRPO+kqcDm4TE5XNKL G6DzypXYdDiQtY4rFHk8sL4ufxJFR6LUsGBKMwSjEEVfIcZMhPaheQd+569vsJmwJW+TxpCSxT jnhwb+aoXMgE6rhKK6Cj/hnuxNAHGLDJRhbthpCV53Uyw2R8LoJJi0ytaMxmAPJi+J+x/TGki8 qUGtRCNv6jOJ8Z/IMNXr0yDp
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXcbe4nq39qHT4NkaFiXaxZQKxtayFQ+EAgAPllgCAAGedgA==
  • Thread-topic: [PATCH 02/16] x86/P2M: introduce p2m_{add,remove}_page()


> On Feb 7, 2022, at 9:38 AM, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> 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.

The way you wrote it is ambiguous grammatically; it could either mean, “Right 
now p2m_add_page() is simply a rename, and so…” or it could mean, “In this 
patch, p2m_add_page() is simply a rename.”  If a reader starts interpreting it 
the first way, then they’ll read along until it doesn’t make sense any more, 
then have to re-evaluate the whole paragraph.

It seems to me that my proposal is unambiguous.

> I'm further slightly puzzled by you keeping the use of present tense in
> "That way callers can use ...".

I wouldn’t call that the present tense; I’m sure a real linguist would have a 
name for it. Consider the sentence, “Put the box near the door; that way we can 
find it easily when we need it.”  The second half of the sentence is set in the 
hypothetical universe in which the imperative has been carried out.

 -George



 


Rackspace

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