Re: [PATCH] Re: [RFC PATCH] xen: comment opaque expression in __page_to_virt

[Keir Fraser <keir.xen@gmail.com>]

On 31/08/2012 15:53, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 31.08.12 at 16:36, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
>> Jan Beulich writes ("Re: [RFC PATCH] xen: comment opaque expression in
>> __page_to_virt"):
>>> No, that's not precise. There's really not much of a win to be had
>>> on 32-bit (division by 3 and division by 24 (sizeof(struct page_info))
>>> should be the same in speed.
>>> 
>>> The win is on x86-64, where sizeof(struct page_info) is a power
>>> of 2, and hence the pair of shifts (right, then left) can be reduced
>>> to a single one.
>>> 
>>> Yet (for obvious reasons) the code ought to not break anything
>>> if even on x86-64 the size of the structure would change, hence
>>> it needs to be that complex (and can't be broken into separate,
>>> simpler implementations for 32- and 64-bits).
>> 
>> Thanks.  Do you want to post a revised version of my patch or shall I
>> do so ?  (If so please confirm that I should put your s-o-b on it for
>> your wording above.)
> 
> x86: comment opaque expression in __page_to_virt()
> 
> mm.h's __page_to_virt() has a rather opaque expression. Comment it.
> 
> Reported-By: Ian Campbell <ian.campbell@citrix.com>
> Suggested-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

> --- 2012-08-08.orig/xen/include/asm-x86/mm.h 2012-06-20 17:34:02.000000000
> +0200
> +++ 2012-08-08/xen/include/asm-x86/mm.h 2012-08-31 16:50:50.000000000 +0200
> @@ -323,6 +323,12 @@ static inline struct page_info *__virt_t
>  static inline void *__page_to_virt(const struct page_info *pg)
>  {
>      ASSERT((unsigned long)pg - FRAMETABLE_VIRT_START < FRAMETABLE_VIRT_END);
> +    /*
> +     * (sizeof(*pg) & -sizeof(*pg)) selects the LS bit of sizeof(*pg). The
> +     * division and re-multiplication avoids one shift when sizeof(*pg) is a
> +     * power of two (otherwise there would be a right shift followed by a
> +     * left shift, which the compiler can't know it can fold into one).
> +     */
>      return (void *)(DIRECTMAP_VIRT_START +
>                      ((unsigned long)pg - FRAMETABLE_VIRT_START) /
>                      (sizeof(*pg) / (sizeof(*pg) & -sizeof(*pg))) *
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel