[Qemu-devel] [PATCH] get rid of private bitmap functions in block/sheepdog.c, use generic ones

[Qemu-devel] [PATCH] get rid of private bitmap functions in block/sheepdog.c, use generic ones

[Michael Tokarev]

qemu now has generic bitmap functions,
so don't redefine them in sheepdog.c,
use common header instead.  A small cleanup.

Here's only one function which is actually
used in sheepdog and gets replaced with
a generic one (simplified):

- static inline int test_bit(int nr, const volatile unsigned long *addr)
+ static inline int test_bit(int nr, const unsigned long *addr)
 {
-  return ((1UL << (nr % BITS_PER_LONG))
            & ((unsigned long*)addr)[nr / BITS_PER_LONG])) != 0;
+  return 1UL & (addr[nr / BITS_PER_LONG] >> (nr & (BITS_PER_LONG-1)));
 }

The body is equivalent, but the argument is not: there's
"volatile" in there.  Why it is used for - I'm not sure.

Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a54e0de..98946d7 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -13,6 +13,7 @@
 #include "qemu-error.h"
 #include "qemu_socket.h"
 #include "block_int.h"
+#include "bitops.h"
 
 #define SD_PROTO_VER 0x01
 
@@ -1829,20 +1830,6 @@ static int sd_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
     return 0;
 }
 
-#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
-#define BITS_PER_BYTE        8
-#define BITS_TO_LONGS(nr)    DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
-#define DECLARE_BITMAP(name,bits)               \
-    unsigned long name[BITS_TO_LONGS(bits)]
-
-#define BITS_PER_LONG (BITS_PER_BYTE * sizeof(long))
-
-static inline int test_bit(unsigned int nr, const unsigned long *addr)
-{
-    return ((1UL << (nr % BITS_PER_LONG)) &
-            (((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0;
-}
-
 static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
 {
     BDRVSheepdogState *s = bs->opaque;

Re: [Qemu-devel] [PATCH] get rid of private bitmap functions in block/sheepdog.c, use generic ones

[MORITA Kazutaka]

On Thu, Mar 10, 2011 at 11:03 PM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> qemu now has generic bitmap functions,
> so don't redefine them in sheepdog.c,
> use common header instead.  A small cleanup.
>
> Here's only one function which is actually
> used in sheepdog and gets replaced with
> a generic one (simplified):
>
> - static inline int test_bit(int nr, const volatile unsigned long *addr)
> + static inline int test_bit(int nr, const unsigned long *addr)
>  {
> -  return ((1UL << (nr % BITS_PER_LONG))
>            & ((unsigned long*)addr)[nr / BITS_PER_LONG])) != 0;
> +  return 1UL & (addr[nr / BITS_PER_LONG] >> (nr & (BITS_PER_LONG-1)));
>  }
>
> The body is equivalent, but the argument is not: there's
> "volatile" in there.  Why it is used for - I'm not sure.
>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

Looks good.  Thanks!

Acked-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>

Re: [Qemu-devel] [PATCH] get rid of private bitmap functions in block/sheepdog.c, use generic ones

[Aurelien Jarno]

On Thu, Mar 10, 2011 at 05:03:41PM +0300, Michael Tokarev wrote:
> qemu now has generic bitmap functions,
> so don't redefine them in sheepdog.c,
> use common header instead.  A small cleanup.
> 
> Here's only one function which is actually
> used in sheepdog and gets replaced with
> a generic one (simplified):
> 
> - static inline int test_bit(int nr, const volatile unsigned long *addr)
> + static inline int test_bit(int nr, const unsigned long *addr)
>  {
> -  return ((1UL << (nr % BITS_PER_LONG))
>             & ((unsigned long*)addr)[nr / BITS_PER_LONG])) != 0;
> +  return 1UL & (addr[nr / BITS_PER_LONG] >> (nr & (BITS_PER_LONG-1)));
>  }
> 
> The body is equivalent, but the argument is not: there's
> "volatile" in there.  Why it is used for - I'm not sure.
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>

Thanks, applied.

> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index a54e0de..98946d7 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -13,6 +13,7 @@
>  #include "qemu-error.h"
>  #include "qemu_socket.h"
>  #include "block_int.h"
> +#include "bitops.h"
>  
>  #define SD_PROTO_VER 0x01
>  
> @@ -1829,20 +1830,6 @@ static int sd_snapshot_delete(BlockDriverState *bs, const char *snapshot_id)
>      return 0;
>  }
>  
> -#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
> -#define BITS_PER_BYTE        8
> -#define BITS_TO_LONGS(nr)    DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
> -#define DECLARE_BITMAP(name,bits)               \
> -    unsigned long name[BITS_TO_LONGS(bits)]
> -
> -#define BITS_PER_LONG (BITS_PER_BYTE * sizeof(long))
> -
> -static inline int test_bit(unsigned int nr, const unsigned long *addr)
> -{
> -    return ((1UL << (nr % BITS_PER_LONG)) &
> -            (((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0;
> -}
> -
>  static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
>  {
>      BDRVSheepdogState *s = bs->opaque;
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net