| Message ID | 20240314021713.240796-1-joseph.qi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ocfs2: correctly use ocfs2_find_next_zero_bit() | expand |
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 --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! */
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(-)