Re: [Xen-devel] [PATCH 3/3] x86/mm: Introduce and use l?e_{get, from}_mfn()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 25/08/17 16:03, George Dunlap wrote:
> On 08/25/2017 04:00 PM, George Dunlap wrote:
>> On 08/24/2017 02:14 PM, Andrew Cooper wrote:
>>> This avoids the explicit boxing/unboxing of mfn_t in relevant codepaths.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> [snip]
>>
>>> diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
>>> index 242903f..8463d71 100644
>>> --- a/xen/include/asm-x86/page.h
>>> +++ b/xen/include/asm-x86/page.h
>>> @@ -71,6 +71,12 @@
>>>  #define l4e_get_pfn(x)             \
>>>      ((unsigned long)(((x).l4 & (PADDR_MASK&PAGE_MASK)) >> PAGE_SHIFT))
>>>  
>>> +/* Get mfn mapped by pte (mfn_t). */
>>> +#define l1e_get_mfn(x) _mfn(l1e_get_pfn(x))
>>> +#define l2e_get_mfn(x) _mfn(l2e_get_pfn(x))
>>> +#define l3e_get_mfn(x) _mfn(l3e_get_pfn(x))
>>> +#define l4e_get_mfn(x) _mfn(l4e_get_pfn(x))
>> Hmm, "get" and "put" have specific meanings elsewhere in the code that
>> don't apply here, but the context of which is confusing enough that
>> people might think they apply.
>>
>> What if we did "mfn_from_l1e" instead, to be symmetric with l1e_from_mfn()?
> /me notices all the other #defines of the "lNe_get_FOO" variety
>
> Nevermind - I'm not a fan but it looks like the ship has already sailed;
> not worth the effort of getting it back into port.

Personally, I'd prefer mfn_from_l1e() over l1e_get_mfn(), because it
doesn't collide with our other nomenclature where get means "take a
reference".

However, such a change (if its generally agreed upon) so be done
consistently to all helpers at once, or this code will become even
harder to read.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel