[Drbd-dev] [PATCH] drbd: use bitmap_weight()

[Drbd-dev] [PATCH] drbd: use bitmap_weight()

[Akinobu Mita]

Use bitmap_weight to count the total number of bits set in bitmap.
This change just simplifies the code a bit.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Philipp Reisner <lars.ellenberg@linbit.com>
Cc: Lars Ellenberg <philipp.reisner@linbit.com>
Cc: drbd-dev@lists.linbit.com
---
 drivers/block/drbd/drbd_bitmap.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 434c77d..39d31af 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -24,7 +24,7 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
 #include <linux/vmalloc.h>
 #include <linux/string.h>
 #include <linux/drbd.h>
@@ -559,21 +559,19 @@ static unsigned long bm_count_bits(struct drbd_bitmap *b)
 	unsigned long *p_addr;
 	unsigned long bits = 0;
 	unsigned long mask = (1UL << (b->bm_bits & BITS_PER_LONG_MASK)) -1;
-	int idx, i, last_word;
+	int idx, last_word;
 
 	/* all but last page */
 	for (idx = 0; idx < b->bm_number_of_pages - 1; idx++) {
 		p_addr = __bm_map_pidx(b, idx);
-		for (i = 0; i < LWPP; i++)
-			bits += hweight_long(p_addr[i]);
+		bits += bitmap_weight(p_addr, PAGE_SIZE * BITS_PER_BYTE);
 		__bm_unmap(p_addr);
 		cond_resched();
 	}
 	/* last (or only) page */
 	last_word = ((b->bm_bits - 1) & BITS_PER_PAGE_MASK) >> LN2_BPL;
 	p_addr = __bm_map_pidx(b, idx);
-	for (i = 0; i < last_word; i++)
-		bits += hweight_long(p_addr[i]);
+	bits += bitmap_weight(p_addr, last_word * BITS_PER_LONG);
 	p_addr[last_word] &= cpu_to_lel(mask);
 	bits += hweight_long(p_addr[last_word]);
 	/* 32bit arch, may have an unused padding long */
@@ -1424,15 +1422,12 @@ int drbd_bm_clear_bits(struct drbd_device *device, const unsigned long s, const
 static inline void bm_set_full_words_within_one_page(struct drbd_bitmap *b,
 		int page_nr, int first_word, int last_word)
 {
-	int i;
-	int bits;
-	int changed = 0;
 	unsigned long *paddr = kmap_atomic(b->bm_pages[page_nr]);
-	for (i = first_word; i < last_word; i++) {
-		bits = hweight_long(paddr[i]);
-		paddr[i] = ~0UL;
-		changed += BITS_PER_LONG - bits;
-	}
+	int nbits = (last_word - first_word) * BITS_PER_LONG;
+	int changed;
+
+	changed = nbits - bitmap_weight(paddr + first_word, nbits);
+	bitmap_fill(paddr + first_word, nbits);
 	kunmap_atomic(paddr);
 	if (changed) {
 		/* We only need lazy writeout, the information is still in the
@@ -1637,8 +1632,7 @@ int drbd_bm_e_weight(struct drbd_device *device, unsigned long enr)
 		int n = e-s;
 		p_addr = bm_map_pidx(b, bm_word_to_page_idx(b, s));
 		bm = p_addr + MLPP(s);
-		while (n--)
-			count += hweight_long(*bm++);
+		count = bitmap_weight(bm, n * BITS_PER_LONG);
 		bm_unmap(p_addr);
 	} else {
 		drbd_err(device, "start offset (%d) too large in drbd_bm_e_weight\n", s);
-- 
1.9.1

Re: [Drbd-dev] [PATCH] drbd: use bitmap_weight()

[Lars Ellenberg]

On Sat, Mar 14, 2015 at 10:12:56AM +0900, Akinobu Mita wrote:
> Use bitmap_weight to count the total number of bits set in bitmap.
> This change just simplifies the code a bit.

"Simplifies", not sure about that, but ok, maybe.

For the "bm_set_full_words_within_one_page", I disagree.
I think it is more cpu cache friendly to hweight_long then set to ~0UL
each word in turn, than to first bitmap_weight() all words, then
bitmap_fill() all words.

Thanks,

	Lars Ellenberg

BTW, you swapped names and email addresses of phil and me.  Are those
listed incorrectly somewhere, or was that just a mishap on your part?

Re: [Drbd-dev] [PATCH] drbd: use bitmap_weight()

[Akinobu Mita]

2015-03-21 0:14 GMT+09:00 Lars Ellenberg <lars.ellenberg@linbit.com>:
> On Sat, Mar 14, 2015 at 10:12:56AM +0900, Akinobu Mita wrote:
>> Use bitmap_weight to count the total number of bits set in bitmap.
>> This change just simplifies the code a bit.
>
> "Simplifies", not sure about that, but ok, maybe.
>
> For the "bm_set_full_words_within_one_page", I disagree.
> I think it is more cpu cache friendly to hweight_long then set to ~0UL
> each word in turn, than to first bitmap_weight() all words, then
> bitmap_fill() all words.

I see. I'll remove the change in bm_set_full_words_within_one_page().

> Thanks,
>
>         Lars Ellenberg
>
> BTW, you swapped names and email addresses of phil and me.  Are those
> listed incorrectly somewhere, or was that just a mishap on your part?

It's just my mistake.  I copied your names from MAINTAINERS, but it
doesn't list your email addresses then I incorrectly swapped while
retrieving email addrsses from git log.

[Drbd-dev] [PATCH] drbd: use bitmap_weight()

[Akinobu Mita]

Use bitmap_weight to count the total number of bits set in bitmap.
This change just simplifies the code a bit.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Philipp Reisner <philipp.reisner@linbit.com>
Cc: Lars Ellenberg <lars.ellenberg@linbit.com>
Cc: drbd-dev@lists.linbit.com
---
 drivers/block/drbd/drbd_bitmap.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c
index 434c77d..1e62c49 100644
--- a/drivers/block/drbd/drbd_bitmap.c
+++ b/drivers/block/drbd/drbd_bitmap.c
@@ -24,7 +24,7 @@
 
 #define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
 
-#include <linux/bitops.h>
+#include <linux/bitmap.h>
 #include <linux/vmalloc.h>
 #include <linux/string.h>
 #include <linux/drbd.h>
@@ -559,21 +559,19 @@ static unsigned long bm_count_bits(struct drbd_bitmap *b)
 	unsigned long *p_addr;
 	unsigned long bits = 0;
 	unsigned long mask = (1UL << (b->bm_bits & BITS_PER_LONG_MASK)) -1;
-	int idx, i, last_word;
+	int idx, last_word;
 
 	/* all but last page */
 	for (idx = 0; idx < b->bm_number_of_pages - 1; idx++) {
 		p_addr = __bm_map_pidx(b, idx);
-		for (i = 0; i < LWPP; i++)
-			bits += hweight_long(p_addr[i]);
+		bits += bitmap_weight(p_addr, PAGE_SIZE * BITS_PER_BYTE);
 		__bm_unmap(p_addr);
 		cond_resched();
 	}
 	/* last (or only) page */
 	last_word = ((b->bm_bits - 1) & BITS_PER_PAGE_MASK) >> LN2_BPL;
 	p_addr = __bm_map_pidx(b, idx);
-	for (i = 0; i < last_word; i++)
-		bits += hweight_long(p_addr[i]);
+	bits += bitmap_weight(p_addr, last_word * BITS_PER_LONG);
 	p_addr[last_word] &= cpu_to_lel(mask);
 	bits += hweight_long(p_addr[last_word]);
 	/* 32bit arch, may have an unused padding long */
@@ -1637,8 +1635,7 @@ int drbd_bm_e_weight(struct drbd_device *device, unsigned long enr)
 		int n = e-s;
 		p_addr = bm_map_pidx(b, bm_word_to_page_idx(b, s));
 		bm = p_addr + MLPP(s);
-		while (n--)
-			count += hweight_long(*bm++);
+		count = bitmap_weight(bm, n * BITS_PER_LONG);
 		bm_unmap(p_addr);
 	} else {
 		drbd_err(device, "start offset (%d) too large in drbd_bm_e_weight\n", s);
-- 
1.9.1