Skip to content

Commit 7f500f0

Browse files
Li Nanaxboe
authored andcommitted
badblocks: return error if any badblock set fails
_badblocks_set() returns success if at least one badblock is set successfully, even if others fail. This can lead to data inconsistencies in raid, where a failed badblock set should trigger the disk to be kicked out to prevent future reads from failed write areas. _badblocks_set() should return error if any badblock set fails. Instead of relying on 'rv', directly returning 'sectors' for clearer logic. If all badblocks are successfully set, 'sectors' will be 0, otherwise it indicates the number of badblocks that have not been set yet, thus signaling failure. By the way, it can also fix an issue: when a newly set unack badblock is included in an existing ack badblock, the setting will return an error. ··· echo "0 100" /sys/block/md0/md/dev-loop1/bad_blocks echo "0 100" /sys/block/md0/md/dev-loop1/unacknowledged_bad_blocks -bash: echo: write error: No space left on device ``` After fix, it will return success. Fixes: aa511ff ("badblocks: switch to the improved badblock handling code") Signed-off-by: Li Nan <[email protected]> Reviewed-by: Yu Kuai <[email protected]> Acked-by: Coly Li <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent 28243dc commit 7f500f0

File tree

1 file changed

+5
-12
lines changed

1 file changed

+5
-12
lines changed

block/badblocks.c

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -843,7 +843,6 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
843843
struct badblocks_context bad;
844844
int prev = -1, hint = -1;
845845
unsigned long flags;
846-
int rv = 0;
847846
u64 *p;
848847

849848
if (bb->shift < 0)
@@ -873,10 +872,8 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
873872
bad.len = sectors;
874873
len = 0;
875874

876-
if (badblocks_full(bb)) {
877-
rv = 1;
875+
if (badblocks_full(bb))
878876
goto out;
879-
}
880877

881878
if (badblocks_empty(bb)) {
882879
len = insert_at(bb, 0, &bad);
@@ -916,10 +913,8 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
916913
int extra = 0;
917914

918915
if (!can_front_overwrite(bb, prev, &bad, &extra)) {
919-
if (extra > 0) {
920-
rv = 1;
916+
if (extra > 0)
921917
goto out;
922-
}
923918

924919
len = min_t(sector_t,
925920
BB_END(p[prev]) - s, sectors);
@@ -986,10 +981,7 @@ static int _badblocks_set(struct badblocks *bb, sector_t s, int sectors,
986981

987982
write_sequnlock_irqrestore(&bb->lock, flags);
988983

989-
if (!added)
990-
rv = 1;
991-
992-
return rv;
984+
return sectors;
993985
}
994986

995987
/*
@@ -1353,7 +1345,8 @@ EXPORT_SYMBOL_GPL(badblocks_check);
13531345
*
13541346
* Return:
13551347
* 0: success
1356-
* 1: failed to set badblocks (out of space)
1348+
* other: failed to set badblocks (out of space). Parital setting will be
1349+
* treated as failure.
13571350
*/
13581351
int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
13591352
int acknowledged)

0 commit comments

Comments
 (0)