| Message ID | 20250221081109.734170-5-zhengqixing@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | badblocks: bugfix and cleanup for badblocks | expand |
Hi, +CC Linan 在 2025/02/21 16:11, Zheng Qixing 写道: > From: Li Nan <linan122@huawei.com> > > In the current handling of badblocks settings, a lot of processing has > been done for scenarios where the number of badblocks exceeds 512. > This makes the code look quite complex and also introduces some issues, It's better to add explanations about these issues here. > > Fixing those issues wouldn’t be too complicated, but it wouldn’t > simplify the code. In fact, a disk shouldn’t have too many badblocks, > and for disks with 512 badblocks, attempting to set more bad blocks > doesn’t make much sense. At that point, the more appropriate action > would be to replace the disk. Therefore, to resolve these issues and > simplify the code somewhat, return error directly when setting badblocks > exceeds 512. > > Fixes: aa511ff8218b ("badblocks: switch to the improved badblock handling code") > Signed-off-by: Li Nan <linan122@huawei.com> > --- > block/badblocks.c | 121 ++++++++-------------------------------------- > 1 file changed, 19 insertions(+), 102 deletions(-) > Reviewed-by: Yu Kuai <yukuai3@huawei.com> > diff --git a/block/badblocks.c b/block/badblocks.c > index ad8652fbe1c8..1c8b8f65f6df 100644 > --- a/block/badblocks.c > +++ b/block/badblocks.c > @@ -527,51 +527,6 @@ static int prev_badblocks(struct badblocks *bb, struct badblocks_context *bad, > return ret; > } > > -/* > - * Return 'true' if the range indicated by 'bad' can be backward merged > - * with the bad range (from the bad table) index by 'behind'. > - */ > -static bool can_merge_behind(struct badblocks *bb, > - struct badblocks_context *bad, int behind) > -{ > - sector_t sectors = bad->len; > - sector_t s = bad->start; > - u64 *p = bb->page; > - > - if ((s < BB_OFFSET(p[behind])) && > - ((s + sectors) >= BB_OFFSET(p[behind])) && > - ((BB_END(p[behind]) - s) <= BB_MAX_LEN) && > - BB_ACK(p[behind]) == bad->ack) > - return true; > - return false; > -} > - > -/* > - * Do backward merge for range indicated by 'bad' and the bad range > - * (from the bad table) indexed by 'behind'. The return value is merged > - * sectors from bad->len. > - */ > -static int behind_merge(struct badblocks *bb, struct badblocks_context *bad, > - int behind) > -{ > - sector_t sectors = bad->len; > - sector_t s = bad->start; > - u64 *p = bb->page; > - int merged = 0; > - > - WARN_ON(s >= BB_OFFSET(p[behind])); > - WARN_ON((s + sectors) < BB_OFFSET(p[behind])); > - > - if (s < BB_OFFSET(p[behind])) { > - merged = BB_OFFSET(p[behind]) - s; > - p[behind] = BB_MAKE(s, BB_LEN(p[behind]) + merged, bad->ack); > - > - WARN_ON((BB_LEN(p[behind]) + merged) >= BB_MAX_LEN); > - } > - > - return merged; > -} > - > /* > * Return 'true' if the range indicated by 'bad' can be forward > * merged with the bad range (from the bad table) indexed by 'prev'. > @@ -884,11 +839,9 @@ static bool try_adjacent_combine(struct badblocks *bb, int prev) > static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > int acknowledged) > { > - int retried = 0, space_desired = 0; > - int orig_len, len = 0, added = 0; > + int len = 0, added = 0; > struct badblocks_context bad; > int prev = -1, hint = -1; > - sector_t orig_start; > unsigned long flags; > int rv = 0; > u64 *p; > @@ -912,8 +865,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > > write_seqlock_irqsave(&bb->lock, flags); > > - orig_start = s; > - orig_len = sectors; > bad.ack = acknowledged; > p = bb->page; > > @@ -922,6 +873,11 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > bad.len = sectors; > len = 0; > > + if (badblocks_full(bb)) { > + rv = 1; > + goto out; > + } > + > if (badblocks_empty(bb)) { > len = insert_at(bb, 0, &bad); > bb->count++; > @@ -933,32 +889,14 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > > /* start before all badblocks */ > if (prev < 0) { > - if (!badblocks_full(bb)) { > - /* insert on the first */ > - if (bad.len > (BB_OFFSET(p[0]) - bad.start)) > - bad.len = BB_OFFSET(p[0]) - bad.start; > - len = insert_at(bb, 0, &bad); > - bb->count++; > - added++; > - hint = 0; > - goto update_sectors; > - } > - > - /* No sapce, try to merge */ > - if (overlap_behind(bb, &bad, 0)) { > - if (can_merge_behind(bb, &bad, 0)) { > - len = behind_merge(bb, &bad, 0); > - added++; > - } else { > - len = BB_OFFSET(p[0]) - s; > - space_desired = 1; > - } > - hint = 0; > - goto update_sectors; > - } > - > - /* no table space and give up */ > - goto out; > + /* insert on the first */ > + if (bad.len > (BB_OFFSET(p[0]) - bad.start)) > + bad.len = BB_OFFSET(p[0]) - bad.start; > + len = insert_at(bb, 0, &bad); > + bb->count++; > + added++; > + hint = 0; > + goto update_sectors; > } > > /* in case p[prev-1] can be merged with p[prev] */ > @@ -978,6 +916,11 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > int extra = 0; > > if (!can_front_overwrite(bb, prev, &bad, &extra)) { > + if (extra > 0) { > + rv = 1; > + goto out; > + } > + > len = min_t(sector_t, > BB_END(p[prev]) - s, sectors); > hint = prev; > @@ -1004,24 +947,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > goto update_sectors; > } > > - /* if no space in table, still try to merge in the covered range */ > - if (badblocks_full(bb)) { > - /* skip the cannot-merge range */ > - if (((prev + 1) < bb->count) && > - overlap_behind(bb, &bad, prev + 1) && > - ((s + sectors) >= BB_END(p[prev + 1]))) { > - len = BB_END(p[prev + 1]) - s; > - hint = prev + 1; > - goto update_sectors; > - } > - > - /* no retry any more */ > - len = sectors; > - space_desired = 1; > - hint = -1; > - goto update_sectors; > - } > - > /* cannot merge and there is space in bad table */ > if ((prev + 1) < bb->count && > overlap_behind(bb, &bad, prev + 1)) > @@ -1049,14 +974,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, > */ > try_adjacent_combine(bb, prev); > > - if (space_desired && !badblocks_full(bb)) { > - s = orig_start; > - sectors = orig_len; > - space_desired = 0; > - if (retried++ < 3) > - goto re_insert; > - } > - > out: > if (added) { > set_changed(bb); >
diff --git a/block/badblocks.c b/block/badblocks.c index ad8652fbe1c8..1c8b8f65f6df 100644 --- a/block/badblocks.c +++ b/block/badblocks.c @@ -527,51 +527,6 @@ static int prev_badblocks(struct badblocks *bb, struct badblocks_context *bad, return ret; } -/* - * Return 'true' if the range indicated by 'bad' can be backward merged - * with the bad range (from the bad table) index by 'behind'. - */ -static bool can_merge_behind(struct badblocks *bb, - struct badblocks_context *bad, int behind) -{ - sector_t sectors = bad->len; - sector_t s = bad->start; - u64 *p = bb->page; - - if ((s < BB_OFFSET(p[behind])) && - ((s + sectors) >= BB_OFFSET(p[behind])) && - ((BB_END(p[behind]) - s) <= BB_MAX_LEN) && - BB_ACK(p[behind]) == bad->ack) - return true; - return false; -} - -/* - * Do backward merge for range indicated by 'bad' and the bad range - * (from the bad table) indexed by 'behind'. The return value is merged - * sectors from bad->len. - */ -static int behind_merge(struct badblocks *bb, struct badblocks_context *bad, - int behind) -{ - sector_t sectors = bad->len; - sector_t s = bad->start; - u64 *p = bb->page; - int merged = 0; - - WARN_ON(s >= BB_OFFSET(p[behind])); - WARN_ON((s + sectors) < BB_OFFSET(p[behind])); - - if (s < BB_OFFSET(p[behind])) { - merged = BB_OFFSET(p[behind]) - s; - p[behind] = BB_MAKE(s, BB_LEN(p[behind]) + merged, bad->ack); - - WARN_ON((BB_LEN(p[behind]) + merged) >= BB_MAX_LEN); - } - - return merged; -} - /* * Return 'true' if the range indicated by 'bad' can be forward * merged with the bad range (from the bad table) indexed by 'prev'. @@ -884,11 +839,9 @@ static bool try_adjacent_combine(struct badblocks *bb, int prev) static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, int acknowledged) { - int retried = 0, space_desired = 0; - int orig_len, len = 0, added = 0; + int len = 0, added = 0; struct badblocks_context bad; int prev = -1, hint = -1; - sector_t orig_start; unsigned long flags; int rv = 0; u64 *p; @@ -912,8 +865,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, write_seqlock_irqsave(&bb->lock, flags); - orig_start = s; - orig_len = sectors; bad.ack = acknowledged; p = bb->page; @@ -922,6 +873,11 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, bad.len = sectors; len = 0; + if (badblocks_full(bb)) { + rv = 1; + goto out; + } + if (badblocks_empty(bb)) { len = insert_at(bb, 0, &bad); bb->count++; @@ -933,32 +889,14 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, /* start before all badblocks */ if (prev < 0) { - if (!badblocks_full(bb)) { - /* insert on the first */ - if (bad.len > (BB_OFFSET(p[0]) - bad.start)) - bad.len = BB_OFFSET(p[0]) - bad.start; - len = insert_at(bb, 0, &bad); - bb->count++; - added++; - hint = 0; - goto update_sectors; - } - - /* No sapce, try to merge */ - if (overlap_behind(bb, &bad, 0)) { - if (can_merge_behind(bb, &bad, 0)) { - len = behind_merge(bb, &bad, 0); - added++; - } else { - len = BB_OFFSET(p[0]) - s; - space_desired = 1; - } - hint = 0; - goto update_sectors; - } - - /* no table space and give up */ - goto out; + /* insert on the first */ + if (bad.len > (BB_OFFSET(p[0]) - bad.start)) + bad.len = BB_OFFSET(p[0]) - bad.start; + len = insert_at(bb, 0, &bad); + bb->count++; + added++; + hint = 0; + goto update_sectors; } /* in case p[prev-1] can be merged with p[prev] */ @@ -978,6 +916,11 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, int extra = 0; if (!can_front_overwrite(bb, prev, &bad, &extra)) { + if (extra > 0) { + rv = 1; + goto out; + } + len = min_t(sector_t, BB_END(p[prev]) - s, sectors); hint = prev; @@ -1004,24 +947,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, goto update_sectors; } - /* if no space in table, still try to merge in the covered range */ - if (badblocks_full(bb)) { - /* skip the cannot-merge range */ - if (((prev + 1) < bb->count) && - overlap_behind(bb, &bad, prev + 1) && - ((s + sectors) >= BB_END(p[prev + 1]))) { - len = BB_END(p[prev + 1]) - s; - hint = prev + 1; - goto update_sectors; - } - - /* no retry any more */ - len = sectors; - space_desired = 1; - hint = -1; - goto update_sectors; - } - /* cannot merge and there is space in bad table */ if ((prev + 1) < bb->count && overlap_behind(bb, &bad, prev + 1)) @@ -1049,14 +974,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors, */ try_adjacent_combine(bb, prev); - if (space_desired && !badblocks_full(bb)) { - s = orig_start; - sectors = orig_len; - space_desired = 0; - if (retried++ < 3) - goto re_insert; - } - out: if (added) { set_changed(bb);