From: Li Nan <[email protected]>
The patch series fix the bug of setting badblocks, which may not match
expectations in complex scenarios. Optimize the code to make it more
readable at the same time.
Li Nan (10):
block/badblocks: only set bb->changed when badblocks changes
block/badblocks: fix badblocks loss when badblocks combine
block/badblocks: fix badblocks overlap
block/badblocks: fix the bug of reverse order
block/badblocks: fix ack set fail in badblocks_set
block/badblocks: check bb->count instead of 'hi > lo'
block/badblocks: factor out a helper to merge badblocks
block/badblocks: factor out a helper to combine badblocks
block/badblocks: factor out a helper to create badblocks
block/badblocks: try to merge badblocks as much as possible
block/badblocks.c | 300 +++++++++++++++++++++++++++++-----------------
1 file changed, 187 insertions(+), 113 deletions(-)
--
2.31.1
From: Li Nan <[email protected]>
If we try to set ack for a BB_MAX_LEN badblocks, it will return
0(success) but did not set ack at all in badblocks_set(). Check ack
before setting to fix it, and do not set badblocks already exist.
Fixes: 9e0e252a048b ("badblocks: Add core badblock management code")
Signed-off-by: Li Nan <[email protected]>
---
block/badblocks.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index 11e3a3ae2c72..c11eb869f2f3 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -219,18 +219,18 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
if (e < s + sectors)
e = s + sectors;
if (e - a <= BB_MAX_LEN) {
- p[lo] = BB_MAKE(a, e-a, ack);
s = e;
} else {
/* does not all fit in one range,
* make p[lo] maximal
*/
- if (BB_LEN(p[lo]) != BB_MAX_LEN)
- p[lo] = BB_MAKE(a, BB_MAX_LEN, ack);
s = a + BB_MAX_LEN;
}
+ if (s - a != BB_LEN(p[lo]) || ack != BB_ACK(p[lo])) {
+ p[lo] = BB_MAKE(a, s - a, ack);
+ changed = true;
+ }
sectors = e - s;
- changed = true;
}
}
if (sectors && hi < bb->count) {
--
2.31.1
From: Li Nan <[email protected]>
If we set a new badblocks, we first merge it with existing region, then
try to combine lo and hi. If there are still badblocks need to be set,
create it. It is a bad way when setting a laget number of badblocks. for
example, it will become chaotic if we set as below:
# echo 1 1 > bad_blocks
# echo 512 1 > bad_blocks
# echo 0 513 > bad_blocks
# cat bad_blocks
0 512
512 1
512 1
Fix it by trying to merge as much as possible. If we have merged any
badblocks, retry to merge next sectors. Do not check sectors while
combining, we should combine lo and hi each sycle.
Fixes: 9e0e252a048b ("badblocks: Add core badblock management code")
Signed-off-by: Li Nan <[email protected]>
---
block/badblocks.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index bb0324b66f57..7e6fce10c82d 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -347,8 +347,6 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
lo = 0;
hi = bb->count;
if (bb->count) {
- int merged_sectors;
-
/* Find the last range that starts at-or-before 's' */
while (hi - lo > 1) {
int mid = (lo + hi) / 2;
@@ -360,12 +358,19 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
hi = mid;
}
- merged_sectors = badblocks_merge(bb, s, sectors, acknowledged,
- &lo, &hi, &changed);
- s += merged_sectors;
- sectors -= merged_sectors;
- if (sectors == 0)
+ while (sectors) {
+ int merged_sectors;
+
+ merged_sectors = badblocks_merge(bb, s, sectors, acknowledged,
+ &lo, &hi, &changed);
+ /* can't merge, break to create */
+ if (!merged_sectors)
+ break;
+
+ s += merged_sectors;
+ sectors -= merged_sectors;
badblocks_combine(bb, lo);
+ }
}
rv = badblocks_create(bb, s, sectors, hi, acknowledged, &changed);
--
2.31.1
From: Li Nan <[email protected]>
Add a helper badblocks_combine() to combine badblocks, it makes code more
readable. No functional change.
Signed-off-by: Li Nan <[email protected]>
---
block/badblocks.c | 87 ++++++++++++++++++++++++++---------------------
1 file changed, 48 insertions(+), 39 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index f498fae201a1..c87c68d4bcac 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -218,6 +218,51 @@ static int badblocks_merge(struct badblocks *bb, sector_t s, int sectors,
return merged_sectors;
}
+/*
+ * try to combine lo and hi(lo + 1) if lo intersects with hi
+ */
+static void badblocks_combine(struct badblocks *bb, int lo)
+{
+ u64 *p = bb->page;
+ sector_t loe = BB_OFFSET(p[lo]) + BB_LEN(p[lo]);
+ int hi = lo + 1;
+
+ if (hi >= bb->count)
+ return;
+ /* we might be able to combine lo and hi */
+
+ if (loe >= BB_OFFSET(p[hi])) {
+ sector_t loa = BB_OFFSET(p[lo]), hia = BB_OFFSET(p[hi]);
+ sector_t hie = hia + BB_LEN(p[hi]);
+ int newlen = max(loe, hie) - loa;
+ int ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
+
+ while (loe >= hie) {
+ /* lo contains hi, just remove hi */
+ memmove(p + hi, p + hi + 1,
+ (bb->count - hi - 1) * 8);
+ bb->count--;
+ if (hi >= bb->count)
+ break;
+ hia = BB_OFFSET(p[hi]);
+ hie = hia + BB_LEN(p[hi]);
+ }
+ if (loe >= hia && hi < bb->count) {
+ if (newlen > BB_MAX_LEN) {
+ p[lo] = BB_MAKE(loa, BB_MAX_LEN, ack);
+ p[hi] = BB_MAKE(loa + BB_MAX_LEN,
+ newlen - BB_MAX_LEN,
+ BB_ACK(p[hi]));
+ } else {
+ p[lo] = BB_MAKE(loa, newlen, ack);
+ memmove(p + hi, p + hi + 1,
+ (bb->count - hi - 1) * 8);
+ bb->count--;
+ }
+ }
+ }
+}
+
/**
* badblocks_set() - Add a range of bad blocks to the table.
* @bb: the badblocks structure that holds all badblock information
@@ -262,16 +307,13 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
lo = 0;
hi = bb->count;
if (bb->count) {
- sector_t a;
- sector_t e;
- int ack;
int merged_sectors;
/* Find the last range that starts at-or-before 's' */
while (hi - lo > 1) {
int mid = (lo + hi) / 2;
+ int a = BB_OFFSET(p[mid]);
- a = BB_OFFSET(p[mid]);
if (a <= s)
lo = mid;
else
@@ -282,41 +324,8 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
&lo, &hi, &changed);
s += merged_sectors;
sectors -= merged_sectors;
- if (sectors == 0 && hi < bb->count) {
- /* we might be able to combine lo and hi */
- /* Note: 's' is at the end of 'lo' */
- sector_t loa = BB_OFFSET(p[lo]), hia = BB_OFFSET(p[hi]);
- sector_t hie = hia + BB_LEN(p[hi]);
- int newlen = max(s, hie) - loa;
-
- ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
- if (s >= hia) {
- while (s >= hie) {
- /* lo contains hi, just remove hi */
- memmove(p + hi, p + hi + 1,
- (bb->count - hi - 1) * 8);
- bb->count--;
- if (hi >= bb->count)
- break;
- hia = BB_OFFSET(p[hi]);
- hie = hia + BB_LEN(p[hi]);
- }
- if (s >= hia && hi < bb->count) {
- if (newlen > BB_MAX_LEN) {
- p[lo] = BB_MAKE(loa, BB_MAX_LEN, ack);
- p[hi] = BB_MAKE(loa + BB_MAX_LEN,
- newlen - BB_MAX_LEN,
- BB_ACK(p[hi]));
- } else {
- p[lo] = BB_MAKE(loa, newlen, ack);
- memmove(p + hi, p + hi + 1,
- (bb->count - hi - 1) * 8);
- bb->count--;
- }
- }
- changed = true;
- }
- }
+ if (sectors == 0)
+ badblocks_combine(bb, lo);
}
while (sectors) {
/* didn't merge (it all).
--
2.31.1
From: Li Nan <[email protected]>
Add a helper badblocks_create() to create badblocks, it makes code more
readable. No functional change.
Signed-off-by: Li Nan <[email protected]>
---
block/badblocks.c | 65 ++++++++++++++++++++++++++++++-----------------
1 file changed, 41 insertions(+), 24 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index c87c68d4bcac..bb0324b66f57 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -263,6 +263,46 @@ static void badblocks_combine(struct badblocks *bb, int lo)
}
}
+/*
+ * creat new badblocks if it can't merge with existing region
+ *
+ * Return:
+ * 0: success
+ * 1: failed to set badblocks (out of space)
+ */
+static int badblocks_create(struct badblocks *bb, sector_t s, sector_t sectors,
+ int hi, int acknowledged, bool *changed)
+{
+ u64 *p = bb->page;
+ int rv = 0;
+
+ while (sectors) {
+ int this_sectors = sectors;
+
+ /* didn't merge (it all).
+ * Need to add a range just before 'hi'
+ */
+ if (bb->count >= MAX_BADBLOCKS) {
+ /* No room for more */
+ rv = 1;
+ break;
+ }
+
+ memmove(p + hi + 1, p + hi,
+ (bb->count - hi) * 8);
+ bb->count++;
+
+ if (this_sectors > BB_MAX_LEN)
+ this_sectors = BB_MAX_LEN;
+ p[hi] = BB_MAKE(s, this_sectors, acknowledged);
+ sectors -= this_sectors;
+ s += this_sectors;
+ hi++;
+ *changed = true;
+ }
+ return rv;
+}
+
/**
* badblocks_set() - Add a range of bad blocks to the table.
* @bb: the badblocks structure that holds all badblock information
@@ -327,30 +367,7 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
if (sectors == 0)
badblocks_combine(bb, lo);
}
- while (sectors) {
- /* didn't merge (it all).
- * Need to add a range just before 'hi'
- */
- if (bb->count >= MAX_BADBLOCKS) {
- /* No room for more */
- rv = 1;
- break;
- } else {
- int this_sectors = sectors;
-
- memmove(p + hi + 1, p + hi,
- (bb->count - hi) * 8);
- bb->count++;
-
- if (this_sectors > BB_MAX_LEN)
- this_sectors = BB_MAX_LEN;
- p[hi] = BB_MAKE(s, this_sectors, acknowledged);
- sectors -= this_sectors;
- s += this_sectors;
- hi++;
- changed = true;
- }
- }
+ rv = badblocks_create(bb, s, sectors, hi, acknowledged, &changed);
if (changed) {
bb->changed = changed;
--
2.31.1