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

Re: [PATCH 3/5] x86_64/mm: map and unmap page tables in share_hotadd_m2p_table



On 23.03.2020 10:41, Hongyan Xia wrote:
> --- a/xen/include/asm-x86/page.h
> +++ b/xen/include/asm-x86/page.h
> @@ -196,6 +196,24 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa, 
> unsigned int flags)
>  #define map_l2t_from_l3e(x)        (l2_pgentry_t 
> *)map_domain_page(l3e_get_mfn(x))
>  #define map_l3t_from_l4e(x)        (l3_pgentry_t 
> *)map_domain_page(l4e_get_mfn(x))
>  
> +#define l1e_from_l2e(l2e, off) ({                   \
> +        l1_pgentry_t *l1t = map_l1t_from_l2e(l2e);  \
> +        l1_pgentry_t l1e = l1t[off];                \
> +        UNMAP_DOMAIN_PAGE(l1t);                     \
> +        l1e; })
> +
> +#define l2e_from_l3e(l3e, off) ({                   \
> +        l2_pgentry_t *l2t = map_l2t_from_l3e(l3e);  \
> +        l2_pgentry_t l2e = l2t[off];                \
> +        UNMAP_DOMAIN_PAGE(l2t);                     \
> +        l2e; })
> +
> +#define l3e_from_l4e(l4e, off) ({                   \
> +        l3_pgentry_t *l3t = map_l3t_from_l4e(l4e);  \
> +        l3_pgentry_t l3e = l3t[off];                \
> +        UNMAP_DOMAIN_PAGE(l3t);                     \
> +        l3e; })

There's a reason these are macros rather than inline functions,
I assume? (This reason would be nice to be stated in the
description.)

I don't see why you use UNMAP_DOMAIN_PAGE() rather than
unmap_domain_page() here - the variables in question go out of
scope right afterwards, so the storing of NULL into them is
pointless (and very likely to be eliminated by the compiler
anyway).

Finally the pointers should each be "to const", as you're
only reading through them.

Jan



 


Rackspace

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