From mboxrd@z Thu Jan  1 00:00:00 1970
From: Keir Fraser <keir.xen@gmail.com>
Subject: Re: [PATCH] tools/libxc: make volatile keyword for
 bitmap operations optional
Date: Mon, 30 Jan 2012 07:32:31 +0000
Message-ID: <CB4BF80F.29F55%keir.xen@gmail.com>
References: <5da4c273e819d5a1d42f.1327883776@probook.site>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <5da4c273e819d5a1d42f.1327883776@probook.site>
List-Unsubscribe: <http://lists.xensource.com/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Olaf Hering <olaf@aepfle.de>, xen-devel@lists.xensource.com
List-Id: xen-devel@lists.xenproject.org

On 30/01/2012 00:36, "Olaf Hering" <olaf@aepfle.de> wrote:

> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1327883694 -3600
> # Node ID 5da4c273e819d5a1d42f3186f0829f8e00d83132
> # Parent  b06617b0398c73c63ce153f39464fd1edac788e5
> tools/libxc: make volatile keyword for bitmap operations optional
> 
> Except for xc_save, all bitmaps maintained by xc_bitops.h are used in single
> threaded applications. So nothing will change the bitmaps content, adding
> volatile adds just unneeded memory reloads.

The bitops aren't threadsafe anyway, as none of them use atomic rmw
instructions. I suspect the volatile declarations are completely pointless
and can just be removed.

 -- Keir

> Adjust the bitmap helpers to make the volatile keyword optional. Users of
> xc_bitops.h can define XC_BITMAPS_VOLATILE before inclusion to declare all
> bitmaps as volatile.
> 
> xc_save passes the bitmap to the hypervisor which in turn modifies it. To be
> safe, declare the used bitmaps as volatile.
> 
> xenpaging uses bitmaps alot and using non-volatile versions will slightly
> improve performance.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> 
> diff -r b06617b0398c -r 5da4c273e819 tools/libxc/xc_bitops.h
> --- a/tools/libxc/xc_bitops.h
> +++ b/tools/libxc/xc_bitops.h
> @@ -3,6 +3,13 @@
>  
>  /* bitmap operations for single threaded access */
>  
> +/* Declare bitmaps passed to the hypervisor as volatile */
> +#ifdef XC_BITMAPS_VOLATILE
> +#define __XC_BITMAP_VOLATILE volatile
> +#else
> +#define __XC_BITMAP_VOLATILE
> +#endif
> +
>  #include <stdlib.h>
>  #include <string.h>
>  
> @@ -31,29 +38,29 @@ static inline void bitmap_clear(unsigned
>      memset(addr, 0, bitmap_size(nr_bits));
>  }
>  
> -static inline int test_bit(int nr, volatile unsigned long *addr)
> +static inline int test_bit(int nr, __XC_BITMAP_VOLATILE unsigned long *addr)
>  {
>      return (BITMAP_ENTRY(nr, addr) >> BITMAP_SHIFT(nr)) & 1;
>  }
>  
> -static inline void clear_bit(int nr, volatile unsigned long *addr)
> +static inline void clear_bit(int nr, __XC_BITMAP_VOLATILE unsigned long
> *addr)
>  {
>      BITMAP_ENTRY(nr, addr) &= ~(1UL << BITMAP_SHIFT(nr));
>  }
>  
> -static inline void set_bit(int nr, volatile unsigned long *addr)
> +static inline void set_bit(int nr, __XC_BITMAP_VOLATILE unsigned long *addr)
>  {
>      BITMAP_ENTRY(nr, addr) |= (1UL << BITMAP_SHIFT(nr));
>  }
>  
> -static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
> +static inline int test_and_clear_bit(int nr, __XC_BITMAP_VOLATILE unsigned
> long *addr)
>  {
>      int oldbit = test_bit(nr, addr);
>      clear_bit(nr, addr);
>      return oldbit;
>  }
>  
> -static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> +static inline int test_and_set_bit(int nr, __XC_BITMAP_VOLATILE unsigned long
> *addr)
>  {
>      int oldbit = test_bit(nr, addr);
>      set_bit(nr, addr);
> diff -r b06617b0398c -r 5da4c273e819 tools/libxc/xc_domain_save.c
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -27,6 +27,7 @@
>  #include <sys/time.h>
>  
>  #include "xc_private.h"
> +#define XC_BITMAPS_VOLATILE
>  #include "xc_bitops.h"
>  #include "xc_dom.h"
>  #include "xg_private.h"
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel