From mboxrd@z Thu Jan  1 00:00:00 1970
From: Keir Fraser <keir.xen@gmail.com>
Subject: Re: [PATCH] Re: [RFC PATCH] xen: comment opaque
 expression in __page_to_virt
Date: Fri, 31 Aug 2012 16:50:35 +0100
Message-ID: <CC669BDB.3D815%keir.xen@gmail.com>
References: <5040EBED0200007800097E2B@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <5040EBED0200007800097E2B@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

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