diff mbox series

ocfs2: correctly use ocfs2_find_next_zero_bit()

Message ID 20240314021713.240796-1-joseph.qi@linux.alibaba.com (mailing list archive)
State New, archived
Headers show
Series ocfs2: correctly use ocfs2_find_next_zero_bit() | expand

Commit Message

Joseph Qi March 14, 2024, 2:17 a.m. UTC
If no bits are zero, ocfs2_find_next_zero_bit() will return max size, so
check the return value with -1 is meaningless.
Correct this usage and cleanup the code.

Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/localalloc.c   | 19 ++++++-------------
 fs/ocfs2/reservations.c |  2 +-
 fs/ocfs2/suballoc.c     |  6 ++----
 3 files changed, 9 insertions(+), 18 deletions(-)

Comments

heming.zhao@suse.com March 14, 2024, 3:48 a.m. UTC | #1
On 3/14/24 10:17, Joseph Qi wrote:
> If no bits are zero, ocfs2_find_next_zero_bit() will return max size, so
> check the return value with -1 is meaningless.
> Correct this usage and cleanup the code.
> 
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>   fs/ocfs2/localalloc.c   | 19 ++++++-------------
>   fs/ocfs2/reservations.c |  2 +-
>   fs/ocfs2/suballoc.c     |  6 ++----
>   3 files changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
> index c803c10dd97e..33aeaaa056d7 100644
> --- a/fs/ocfs2/localalloc.c
> +++ b/fs/ocfs2/localalloc.c
> @@ -863,14 +863,8 @@ static int ocfs2_local_alloc_find_clear_bits(struct ocfs2_super *osb,
>   
>   	numfound = bitoff = startoff = 0;
>   	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
> -	while ((bitoff = ocfs2_find_next_zero_bit(bitmap, left, startoff)) != -1) {
> -		if (bitoff == left) {
> -			/* mlog(0, "bitoff (%d) == left", bitoff); */
> -			break;
> -		}
> -		/* mlog(0, "Found a zero: bitoff = %d, startoff = %d, "
> -		   "numfound = %d\n", bitoff, startoff, numfound);*/
> -
> +	while ((bitoff = ocfs2_find_next_zero_bit(bitmap, left, startoff)) <
> +	       left) {
>   		/* Ok, we found a zero bit... is it contig. or do we
>   		 * start over?*/
>   		if (bitoff == startoff) {
> @@ -976,9 +970,9 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>   	start = count = 0;
>   	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
>   
> -	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start))
> -	       != -1) {
> -		if ((bit_off < left) && (bit_off == start)) {
> +	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start)) <
> +	       left) {
> +		if (bit_off == start) {
>   			count++;
>   			start++;
>   			continue;
> @@ -1002,8 +996,7 @@ static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
>   				goto bail;
>   			}
>   		}
> -		if (bit_off >= left)
> -			break;
> +
>   		count = 1;
>   		start = bit_off + 1;
>   	}
> diff --git a/fs/ocfs2/reservations.c b/fs/ocfs2/reservations.c
> index a9d1296d736d..1fe61974d9f0 100644
> --- a/fs/ocfs2/reservations.c
> +++ b/fs/ocfs2/reservations.c
> @@ -414,7 +414,7 @@ static int ocfs2_resmap_find_free_bits(struct ocfs2_reservation_map *resmap,
>   
>   	start = search_start;
>   	while ((offset = ocfs2_find_next_zero_bit(bitmap, resmap->m_bitmap_len,
> -						 start)) != -1) {
> +					start)) < resmap->m_bitmap_len) {
>   		/* Search reached end of the region */
>   		if (offset >= (search_start + search_len))
>   			break;
> diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
> index 166c8918c825..961998415308 100644
> --- a/fs/ocfs2/suballoc.c
> +++ b/fs/ocfs2/suballoc.c
> @@ -1290,10 +1290,8 @@ static int ocfs2_block_group_find_clear_bits(struct ocfs2_super *osb,
>   	found = start = best_offset = best_size = 0;
>   	bitmap = bg->bg_bitmap;
>   
> -	while((offset = ocfs2_find_next_zero_bit(bitmap, total_bits, start)) != -1) {
> -		if (offset == total_bits)
> -			break;
> -
> +	while ((offset = ocfs2_find_next_zero_bit(bitmap, total_bits, start)) <
> +	       total_bits) {
>   		if (!ocfs2_test_bg_bit_allocatable(bg_bh, offset)) {
>   			/* We found a zero, but we can't use it as it
>   			 * hasn't been put to disk yet! */

I found the same issue when I was writing the IO performance patch.
(So my patch doesn't follow the existing handling style.)
Thank you for taking time to fix it.

Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Thanks,
Heming
diff mbox series

Patch

diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index c803c10dd97e..33aeaaa056d7 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -863,14 +863,8 @@  static int ocfs2_local_alloc_find_clear_bits(struct ocfs2_super *osb,
 
 	numfound = bitoff = startoff = 0;
 	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
-	while ((bitoff = ocfs2_find_next_zero_bit(bitmap, left, startoff)) != -1) {
-		if (bitoff == left) {
-			/* mlog(0, "bitoff (%d) == left", bitoff); */
-			break;
-		}
-		/* mlog(0, "Found a zero: bitoff = %d, startoff = %d, "
-		   "numfound = %d\n", bitoff, startoff, numfound);*/
-
+	while ((bitoff = ocfs2_find_next_zero_bit(bitmap, left, startoff)) <
+	       left) {
 		/* Ok, we found a zero bit... is it contig. or do we
 		 * start over?*/
 		if (bitoff == startoff) {
@@ -976,9 +970,9 @@  static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
 	start = count = 0;
 	left = le32_to_cpu(alloc->id1.bitmap1.i_total);
 
-	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start))
-	       != -1) {
-		if ((bit_off < left) && (bit_off == start)) {
+	while ((bit_off = ocfs2_find_next_zero_bit(bitmap, left, start)) <
+	       left) {
+		if (bit_off == start) {
 			count++;
 			start++;
 			continue;
@@ -1002,8 +996,7 @@  static int ocfs2_sync_local_to_main(struct ocfs2_super *osb,
 				goto bail;
 			}
 		}
-		if (bit_off >= left)
-			break;
+
 		count = 1;
 		start = bit_off + 1;
 	}
diff --git a/fs/ocfs2/reservations.c b/fs/ocfs2/reservations.c
index a9d1296d736d..1fe61974d9f0 100644
--- a/fs/ocfs2/reservations.c
+++ b/fs/ocfs2/reservations.c
@@ -414,7 +414,7 @@  static int ocfs2_resmap_find_free_bits(struct ocfs2_reservation_map *resmap,
 
 	start = search_start;
 	while ((offset = ocfs2_find_next_zero_bit(bitmap, resmap->m_bitmap_len,
-						 start)) != -1) {
+					start)) < resmap->m_bitmap_len) {
 		/* Search reached end of the region */
 		if (offset >= (search_start + search_len))
 			break;
diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c
index 166c8918c825..961998415308 100644
--- a/fs/ocfs2/suballoc.c
+++ b/fs/ocfs2/suballoc.c
@@ -1290,10 +1290,8 @@  static int ocfs2_block_group_find_clear_bits(struct ocfs2_super *osb,
 	found = start = best_offset = best_size = 0;
 	bitmap = bg->bg_bitmap;
 
-	while((offset = ocfs2_find_next_zero_bit(bitmap, total_bits, start)) != -1) {
-		if (offset == total_bits)
-			break;
-
+	while ((offset = ocfs2_find_next_zero_bit(bitmap, total_bits, start)) <
+	       total_bits) {
 		if (!ocfs2_test_bg_bit_allocatable(bg_bh, offset)) {
 			/* We found a zero, but we can't use it as it
 			 * hasn't been put to disk yet! */