[PATCH 1/2] md/bitmap: use proper accessor macro

[PATCH 1/2] md/bitmap: use proper accessor macro

[Namhyung Kim]

Use COUNTER()/NEEDED() macro instead of open-coding them.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 drivers/md/bitmap.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 70bd738b8b99..8b40bd71bb4a 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1332,7 +1332,7 @@ int bitmap_startwrite(struct bitmap *bitmap, sector_t offset, unsigned long sect
 			return 0;
 		}
 
-		if (unlikely((*bmc & COUNTER_MAX) == COUNTER_MAX)) {
+		if (unlikely(COUNTER(*bmc) == COUNTER_MAX)) {
 			DEFINE_WAIT(__wait);
 			/* note that it is safe to do the prepare_to_wait
 			 * after the test as long as we do it before dropping
@@ -1404,10 +1404,10 @@ void bitmap_endwrite(struct bitmap *bitmap, sector_t offset, unsigned long secto
 			sysfs_notify_dirent_safe(bitmap->sysfs_can_clear);
 		}
 
-		if (!success && ! (*bmc & NEEDED_MASK))
+		if (!success && !NEEDED(*bmc))
 			*bmc |= NEEDED_MASK;
 
-		if ((*bmc & COUNTER_MAX) == COUNTER_MAX)
+		if (COUNTER(*bmc) == COUNTER_MAX)
 			wake_up(&bitmap->overflow_wait);
 
 		(*bmc)--;
-- 
1.7.5.2

[PATCH 2/2] md/bitmap: remove unused fields from struct bitmap

[Namhyung Kim]

Get rid of ->syncchunk and ->counter_bits since they're never used.

Signed-off-by: Namhyung Kim <namhyung@gmail.com>
---
 drivers/md/bitmap.c |    3 ---
 drivers/md/bitmap.h |    9 ---------
 2 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
index 8b40bd71bb4a..0e3b314917ab 100644
--- a/drivers/md/bitmap.c
+++ b/drivers/md/bitmap.c
@@ -1754,9 +1754,6 @@ int bitmap_create(mddev_t *mddev)
 	bitmap->chunks = chunks;
 	bitmap->pages = pages;
 	bitmap->missing_pages = pages;
-	bitmap->counter_bits = COUNTER_BITS;
-
-	bitmap->syncchunk = ~0UL;
 
 #ifdef INJECT_FATAL_FAULT_1
 	bitmap->bp = NULL;
diff --git a/drivers/md/bitmap.h b/drivers/md/bitmap.h
index d0aeaf46d932..0a239f5d0ca1 100644
--- a/drivers/md/bitmap.h
+++ b/drivers/md/bitmap.h
@@ -196,19 +196,10 @@ struct bitmap {
 
 	mddev_t *mddev; /* the md device that the bitmap is for */
 
-	int counter_bits; /* how many bits per block counter */
-
 	/* bitmap chunksize -- how much data does each bit represent? */
 	unsigned long chunkshift; /* chunksize = 2^chunkshift (for bitops) */
 	unsigned long chunks; /* total number of data chunks for the array */
 
-	/* We hold a count on the chunk currently being synced, and drop
-	 * it when the last block is started.  If the resync is aborted
-	 * midway, we need to be able to drop that count, so we remember
-	 * the counted chunk..
-	 */
-	unsigned long syncchunk;
-
 	__u64	events_cleared;
 	int need_sync;
 
-- 
1.7.5.2

Re: [PATCH 2/2] md/bitmap: remove unused fields from struct bitmap

[NeilBrown]

On Tue,  7 Jun 2011 23:49:32 +0900 Namhyung Kim <namhyung@gmail.com> wrote:

> Get rid of ->syncchunk and ->counter_bits since they're never used.
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  drivers/md/bitmap.c |    3 ---
>  drivers/md/bitmap.h |    9 ---------
>  2 files changed, 0 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 8b40bd71bb4a..0e3b314917ab 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -1754,9 +1754,6 @@ int bitmap_create(mddev_t *mddev)
>  	bitmap->chunks = chunks;
>  	bitmap->pages = pages;
>  	bitmap->missing_pages = pages;
> -	bitmap->counter_bits = COUNTER_BITS;
> -
> -	bitmap->syncchunk = ~0UL;
>  
>  #ifdef INJECT_FATAL_FAULT_1
>  	bitmap->bp = NULL;
> diff --git a/drivers/md/bitmap.h b/drivers/md/bitmap.h
> index d0aeaf46d932..0a239f5d0ca1 100644
> --- a/drivers/md/bitmap.h
> +++ b/drivers/md/bitmap.h
> @@ -196,19 +196,10 @@ struct bitmap {
>  
>  	mddev_t *mddev; /* the md device that the bitmap is for */
>  
> -	int counter_bits; /* how many bits per block counter */
> -
>  	/* bitmap chunksize -- how much data does each bit represent? */
>  	unsigned long chunkshift; /* chunksize = 2^chunkshift (for bitops) */
>  	unsigned long chunks; /* total number of data chunks for the array */
>  
> -	/* We hold a count on the chunk currently being synced, and drop
> -	 * it when the last block is started.  If the resync is aborted
> -	 * midway, we need to be able to drop that count, so we remember
> -	 * the counted chunk..
> -	 */
> -	unsigned long syncchunk;
> -
>  	__u64	events_cleared;
>  	int need_sync;
>  


Always happy to see code disappear!  Thanks.

I took the opportunity to also remove COUNTER_BYTE_RATIO in the same patch.

Thanks,
NeilBrown

Re: [PATCH 1/2] md/bitmap: use proper accessor macro

[NeilBrown]

On Tue,  7 Jun 2011 23:49:31 +0900 Namhyung Kim <namhyung@gmail.com> wrote:

> Use COUNTER()/NEEDED() macro instead of open-coding them.
> 
> Signed-off-by: Namhyung Kim <namhyung@gmail.com>
> ---
>  drivers/md/bitmap.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> index 70bd738b8b99..8b40bd71bb4a 100644
> --- a/drivers/md/bitmap.c
> +++ b/drivers/md/bitmap.c
> @@ -1332,7 +1332,7 @@ int bitmap_startwrite(struct bitmap *bitmap, sector_t offset, unsigned long sect
>  			return 0;
>  		}
>  
> -		if (unlikely((*bmc & COUNTER_MAX) == COUNTER_MAX)) {
> +		if (unlikely(COUNTER(*bmc) == COUNTER_MAX)) {
>  			DEFINE_WAIT(__wait);
>  			/* note that it is safe to do the prepare_to_wait
>  			 * after the test as long as we do it before dropping
> @@ -1404,10 +1404,10 @@ void bitmap_endwrite(struct bitmap *bitmap, sector_t offset, unsigned long secto
>  			sysfs_notify_dirent_safe(bitmap->sysfs_can_clear);
>  		}
>  
> -		if (!success && ! (*bmc & NEEDED_MASK))
> +		if (!success && !NEEDED(*bmc))
>  			*bmc |= NEEDED_MASK;
>  
> -		if ((*bmc & COUNTER_MAX) == COUNTER_MAX)
> +		if (COUNTER(*bmc) == COUNTER_MAX)
>  			wake_up(&bitmap->overflow_wait);
>  
>  		(*bmc)--;


Thanks....

Personally I loathe such macros - I prefer things to be open codes so I can
see what is happening without having to guess.
But as we have the macros and they are already in use we should be consistent
and use them everywhere.
So I'll apply the patch.

Thanks,
NeilBrown