* [PATCH v4 0/4] block/badblocks: fix badblocks setting error
@ 2023-06-26 8:09 linan666
2023-06-26 8:09 ` [PATCH v4 1/4] block/badblocks: change some members of badblocks to bool linan666
` (5 more replies)
0 siblings, 6 replies; 7+ messages in thread
From: linan666 @ 2023-06-26 8:09 UTC (permalink / raw
To: axboe, linan122, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
This patch series fixes some simple bugs of setting badblocks and
optimizing struct badblocks. Coly Li has been trying to refactor badblocks
in patch series "badblocks improvement for multiple bad block ranges"
(https://lore.kernel.org/all/20220721121152.4180-1-colyli@suse.de). but the
workload is significant. Before that, I will fix some easily triggered
issues and optimize some code that does not conflict with Coly's changes.
Changes in v4:
- patch 1, remove the part of reorder fields
- patch 3/4, improve commit log.
Changes in v3:
- delete patchs with significant changes.
Li Nan (4):
block/badblocks: change some members of badblocks to bool
block/badblocks: only set bb->changed/unacked_exist when badblocks
changes
block/badblocks: fix badblocks loss when badblocks combine
block/badblocks: fix the bug of reverse order
include/linux/badblocks.h | 9 +++++----
block/badblocks.c | 38 ++++++++++++++++++++++----------------
2 files changed, 27 insertions(+), 20 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/4] block/badblocks: change some members of badblocks to bool
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
@ 2023-06-26 8:09 ` linan666
2023-06-26 8:09 ` [PATCH v4 2/4] block/badblocks: only set bb->changed/unacked_exist when badblocks changes linan666
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: linan666 @ 2023-06-26 8:09 UTC (permalink / raw
To: axboe, linan122, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
"changed" and "unacked_exist" are used as boolean type. Change the type
of them to bool. No functional changed intended.
Signed-off-by: Li Nan <linan122@huawei.com>
---
include/linux/badblocks.h | 9 +++++----
block/badblocks.c | 14 +++++++-------
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h
index 2426276b9bd3..2feb143a1856 100644
--- a/include/linux/badblocks.h
+++ b/include/linux/badblocks.h
@@ -27,15 +27,16 @@
struct badblocks {
struct device *dev; /* set by devm_init_badblocks */
int count; /* count of bad blocks */
- int unacked_exist; /* there probably are unacknowledged
- * bad blocks. This is only cleared
- * when a read discovers none
+ bool unacked_exist; /* there probably are unacknowledged
+ * bad blocks. This is only cleared
+ * when a read of unacknowledged bad
+ * blocks discovers none
*/
int shift; /* shift from sectors to block size
* a -ve shift means badblocks are
* disabled.*/
u64 *page; /* badblock list */
- int changed;
+ bool changed;
seqlock_t lock;
sector_t sector;
sector_t size; /* in sectors */
diff --git a/block/badblocks.c b/block/badblocks.c
index 3afb550c0f7b..f28e971a4b94 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -141,7 +141,7 @@ static void badblocks_update_acked(struct badblocks *bb)
}
if (!unacked)
- bb->unacked_exist = 0;
+ bb->unacked_exist = false;
}
/**
@@ -302,9 +302,9 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
}
}
- bb->changed = 1;
+ bb->changed = true;
if (!acknowledged)
- bb->unacked_exist = 1;
+ bb->unacked_exist = true;
else
badblocks_update_acked(bb);
write_sequnlock_irqrestore(&bb->lock, flags);
@@ -414,7 +414,7 @@ int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
}
badblocks_update_acked(bb);
- bb->changed = 1;
+ bb->changed = true;
out:
write_sequnlock_irq(&bb->lock);
return rv;
@@ -435,7 +435,7 @@ void ack_all_badblocks(struct badblocks *bb)
return;
write_seqlock_irq(&bb->lock);
- if (bb->changed == 0 && bb->unacked_exist) {
+ if (!bb->changed && bb->unacked_exist) {
u64 *p = bb->page;
int i;
@@ -447,7 +447,7 @@ void ack_all_badblocks(struct badblocks *bb)
p[i] = BB_MAKE(start, len, 1);
}
}
- bb->unacked_exist = 0;
+ bb->unacked_exist = false;
}
write_sequnlock_irq(&bb->lock);
}
@@ -493,7 +493,7 @@ ssize_t badblocks_show(struct badblocks *bb, char *page, int unack)
length << bb->shift);
}
if (unack && len == 0)
- bb->unacked_exist = 0;
+ bb->unacked_exist = false;
if (read_seqretry(&bb->lock, seq))
goto retry;
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/4] block/badblocks: only set bb->changed/unacked_exist when badblocks changes
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
2023-06-26 8:09 ` [PATCH v4 1/4] block/badblocks: change some members of badblocks to bool linan666
@ 2023-06-26 8:09 ` linan666
2023-06-26 8:09 ` [PATCH v4 3/4] block/badblocks: fix badblocks loss when badblocks combine linan666
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: linan666 @ 2023-06-26 8:09 UTC (permalink / raw
To: axboe, linan122, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
In badblocks_set(), even if no badblocks changes, bb->changed and
unacked_exist will still be set. Only set them when badblocks changes.
Fixes: 9e0e252a048b ("badblocks: Add core badblock management code")
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index f28e971a4b94..7b1ad364e85c 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -166,6 +166,7 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
int lo, hi;
int rv = 0;
unsigned long flags;
+ bool changed = false;
if (bb->shift < 0)
/* badblocks are disabled */
@@ -229,6 +230,7 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
s = a + BB_MAX_LEN;
}
sectors = e - s;
+ changed = true;
}
}
if (sectors && hi < bb->count) {
@@ -259,6 +261,7 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
sectors = e - s;
lo = hi;
hi++;
+ changed = true;
}
}
if (sectors == 0 && hi < bb->count) {
@@ -277,6 +280,7 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
memmove(p + hi, p + hi + 1,
(bb->count - hi - 1) * 8);
bb->count--;
+ changed = true;
}
}
while (sectors) {
@@ -299,14 +303,17 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
p[hi] = BB_MAKE(s, this_sectors, acknowledged);
sectors -= this_sectors;
s += this_sectors;
+ changed = true;
}
}
- bb->changed = true;
- if (!acknowledged)
- bb->unacked_exist = true;
- else
- badblocks_update_acked(bb);
+ if (changed) {
+ bb->changed = changed;
+ if (!acknowledged)
+ bb->unacked_exist = true;
+ else
+ badblocks_update_acked(bb);
+ }
write_sequnlock_irqrestore(&bb->lock, flags);
return rv;
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 3/4] block/badblocks: fix badblocks loss when badblocks combine
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
2023-06-26 8:09 ` [PATCH v4 1/4] block/badblocks: change some members of badblocks to bool linan666
2023-06-26 8:09 ` [PATCH v4 2/4] block/badblocks: only set bb->changed/unacked_exist when badblocks changes linan666
@ 2023-06-26 8:09 ` linan666
2023-06-26 8:09 ` [PATCH v4 4/4] block/badblocks: fix the bug of reverse order linan666
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: linan666 @ 2023-06-26 8:09 UTC (permalink / raw
To: axboe, linan122, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
badblocks will loss if set it as below:
$ echo 1 1 > bad_blocks
$ echo 3 1 > bad_blocks
$ echo 1 4 > bad_blocks
$ cat bad_blocks
1 3
After the fix, in the same scenario, it will be:
$ cat bad_blocks
1 4
In badblocks_set(), if set a new badblocks, first find two existing
badblocks adjacent to it, named lo and hi. Then try to merge new with lo.
If merge success and there is an intersection between lo and hi, try to
combine lo and hi.
set 1 4
binary-search:
lo: 1 1 |____|
hi: 3 1 |____|
merge with lo:
lo: 1 4 |___________________|
hi: 3 1 |____|
combine lo and hi:
result: 1 3 |______________|
| -> hi's end
|____| -> lost
Now, the end of combined badblocks must be hi's end. However, it should be
the larger one between lo and hi. Fix it.
Fixes: 9e0e252a048b ("badblocks: Add core badblock management code")
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/block/badblocks.c b/block/badblocks.c
index 7b1ad364e85c..c1745b76d8f1 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -267,16 +267,14 @@ int badblocks_set(struct badblocks *bb, sector_t s, int 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 a = BB_OFFSET(p[hi]);
- int lolen = BB_LEN(p[lo]);
- int hilen = BB_LEN(p[hi]);
- int newlen = lolen + hilen - (s - a);
+ sector_t a = BB_OFFSET(p[lo]);
+ int newlen = max(s, BB_OFFSET(p[hi]) + BB_LEN(p[hi])) - a;
- if (s >= a && newlen < BB_MAX_LEN) {
+ if (s >= BB_OFFSET(p[hi]) && newlen < BB_MAX_LEN) {
/* yes, we can combine them */
int ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
- p[lo] = BB_MAKE(BB_OFFSET(p[lo]), newlen, ack);
+ p[lo] = BB_MAKE(a, newlen, ack);
memmove(p + hi, p + hi + 1,
(bb->count - hi - 1) * 8);
bb->count--;
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 4/4] block/badblocks: fix the bug of reverse order
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
` (2 preceding siblings ...)
2023-06-26 8:09 ` [PATCH v4 3/4] block/badblocks: fix badblocks loss when badblocks combine linan666
@ 2023-06-26 8:09 ` linan666
2023-08-16 8:54 ` [PATCH v4 0/4] block/badblocks: fix badblocks setting error Li Nan
2023-08-17 1:22 ` Li Nan
5 siblings, 0 replies; 7+ messages in thread
From: linan666 @ 2023-06-26 8:09 UTC (permalink / raw
To: axboe, linan122, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
From: Li Nan <linan122@huawei.com>
Badblocks are arranged from small to large, but order of badblocks will
be reversed if we set a large area at once as below:
$ echo 0 2048 > bad_blocks
$ cat bad_blocks
1536 512
1024 512
512 512
0 512
Actually, it should be:
$ echo 0 2048 > bad_blocks
$ cat bad_blocks
0 512
512 512
1024 512
1536 512
'hi' remains unchanged while setting continuous badblocks is wrong, the
next badblocks is greater than 'p[hi]', and it should be added to
'p[hi+1]'. Let 'hi' +1 each cycle.
(0 512) 0 512
|_________| |_________|
p[hi] p[hi]
(512 512) (0 512) fix 0 512 1024
|_________|_________| ===> |_________|_________|
p[hi] p[hi+1]
(1024 512)(512 512) (0 512) 0 512 1024 1536
|_________|_________|_________| |_________|_________|_________|
p[hi] p[hi+2]
...
Fixes: 9e0e252a048b ("badblocks: Add core badblock management code")
Signed-off-by: Li Nan <linan122@huawei.com>
---
block/badblocks.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/badblocks.c b/block/badblocks.c
index c1745b76d8f1..b79d37a4bf0e 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -301,6 +301,7 @@ int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
p[hi] = BB_MAKE(s, this_sectors, acknowledged);
sectors -= this_sectors;
s += this_sectors;
+ hi++;
changed = true;
}
}
--
2.39.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4 0/4] block/badblocks: fix badblocks setting error
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
` (3 preceding siblings ...)
2023-06-26 8:09 ` [PATCH v4 4/4] block/badblocks: fix the bug of reverse order linan666
@ 2023-08-16 8:54 ` Li Nan
2023-08-17 1:22 ` Li Nan
5 siblings, 0 replies; 7+ messages in thread
From: Li Nan @ 2023-08-16 8:54 UTC (permalink / raw
To: axboe, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
Friendly ping.
在 2023/6/26 16:09, linan666@huaweicloud.com 写道:
> From: Li Nan <linan122@huawei.com>
>
> This patch series fixes some simple bugs of setting badblocks and
> optimizing struct badblocks. Coly Li has been trying to refactor badblocks
> in patch series "badblocks improvement for multiple bad block ranges"
> (https://lore.kernel.org/all/20220721121152.4180-1-colyli@suse.de). but the
> workload is significant. Before that, I will fix some easily triggered
> issues and optimize some code that does not conflict with Coly's changes.
>
> Changes in v4:
> - patch 1, remove the part of reorder fields
> - patch 3/4, improve commit log.
>
> Changes in v3:
> - delete patchs with significant changes.
>
> Li Nan (4):
> block/badblocks: change some members of badblocks to bool
> block/badblocks: only set bb->changed/unacked_exist when badblocks
> changes
> block/badblocks: fix badblocks loss when badblocks combine
> block/badblocks: fix the bug of reverse order
>
> include/linux/badblocks.h | 9 +++++----
> block/badblocks.c | 38 ++++++++++++++++++++++----------------
> 2 files changed, 27 insertions(+), 20 deletions(-)
>
--
Thanks,
Nan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 0/4] block/badblocks: fix badblocks setting error
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
` (4 preceding siblings ...)
2023-08-16 8:54 ` [PATCH v4 0/4] block/badblocks: fix badblocks setting error Li Nan
@ 2023-08-17 1:22 ` Li Nan
5 siblings, 0 replies; 7+ messages in thread
From: Li Nan @ 2023-08-17 1:22 UTC (permalink / raw
To: axboe, vishal.l.verma, dan.j.williams, ashok_raj
Cc: linux-block, linux-kernel, yukuai3, yi.zhang, houtao1, yangerkun
在 2023/6/26 16:09, linan666@huaweicloud.com 写道:
> From: Li Nan <linan122@huawei.com>
>
> This patch series fixes some simple bugs of setting badblocks and
> optimizing struct badblocks. Coly Li has been trying to refactor badblocks
> in patch series "badblocks improvement for multiple bad block ranges"
> (https://lore.kernel.org/all/20220721121152.4180-1-colyli@suse.de). but the
Coly has sent the lastest version of his patch series.
Now this patch series can be discarded.
> workload is significant. Before that, I will fix some easily triggered
> issues and optimize some code that does not conflict with Coly's changes.
>
> Changes in v4:
> - patch 1, remove the part of reorder fields
> - patch 3/4, improve commit log.
>
> Changes in v3:
> - delete patchs with significant changes.
>
> Li Nan (4):
> block/badblocks: change some members of badblocks to bool
> block/badblocks: only set bb->changed/unacked_exist when badblocks
> changes
> block/badblocks: fix badblocks loss when badblocks combine
> block/badblocks: fix the bug of reverse order
>
> include/linux/badblocks.h | 9 +++++----
> block/badblocks.c | 38 ++++++++++++++++++++++----------------
> 2 files changed, 27 insertions(+), 20 deletions(-)
>
--
Thanks,
Nan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-08-17 1:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-26 8:09 [PATCH v4 0/4] block/badblocks: fix badblocks setting error linan666
2023-06-26 8:09 ` [PATCH v4 1/4] block/badblocks: change some members of badblocks to bool linan666
2023-06-26 8:09 ` [PATCH v4 2/4] block/badblocks: only set bb->changed/unacked_exist when badblocks changes linan666
2023-06-26 8:09 ` [PATCH v4 3/4] block/badblocks: fix badblocks loss when badblocks combine linan666
2023-06-26 8:09 ` [PATCH v4 4/4] block/badblocks: fix the bug of reverse order linan666
2023-08-16 8:54 ` [PATCH v4 0/4] block/badblocks: fix badblocks setting error Li Nan
2023-08-17 1:22 ` Li Nan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).