* [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
@ 2025-09-02 9:27 ` Wang Zhaolong
0 siblings, 0 replies; 10+ messages in thread
From: Wang Zhaolong @ 2025-09-02 9:27 UTC (permalink / raw)
To: miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, chengzhihao1, yi.zhang, yangerkun
Repeatedly marking the same eraseblock bad inflates
mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
increments the counter on success, while some implementations (e.g.
NAND) return 0 both when the block was already bad and when it has just
been marked[1].
Fix by checking if the block is already bad before calling
->_block_markbad() when _block_isbad is available. Only skip the counter
increment when we can confirm the block was already bad. In all other
cases continue incrementing the counter.
This keeps the logic centralized in mtdcore without requiring driver
changes.
Link: https://lore.kernel.org/all/ef573188-9815-4a6b-bad1-3d8ff7c9b16f@huaweicloud.com/ [1]
Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
---
V2:
- Checks old state when _block_isbad exists and bails out early if already bad
drivers/mtd/mtdcore.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 5ba9a741f5ac..096a3c94670f 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -2337,10 +2337,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
EXPORT_SYMBOL_GPL(mtd_block_isbad);
int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
{
struct mtd_info *master = mtd_get_master(mtd);
+ loff_t moffs;
int ret;
if (!master->_block_markbad)
return -EOPNOTSUPP;
if (ofs < 0 || ofs >= mtd->size)
@@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
return -EROFS;
if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
- ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
+ moffs = mtd_get_master_ofs(mtd, ofs);
+
+ if (master->_block_isbad) {
+ ret = master->_block_isbad(master, moffs);
+ if (ret > 0)
+ return 0;
+ }
+
+ ret = master->_block_markbad(master, moffs);
if (ret)
return ret;
while (mtd->parent) {
mtd->ecc_stats.badblocks++;
--
2.39.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
2025-09-02 9:27 ` Wang Zhaolong
@ 2025-09-02 11:41 ` Zhihao Cheng
-1 siblings, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2025-09-02 11:41 UTC (permalink / raw)
To: Wang Zhaolong, miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, yi.zhang, yangerkun
在 2025/9/2 17:27, Wang Zhaolong 写道:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
>
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
>
> This keeps the logic centralized in mtdcore without requiring driver
> changes.
>
> Link: https://lore.kernel.org/all/ef573188-9815-4a6b-bad1-3d8ff7c9b16f@huaweicloud.com/ [1]
> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> ---
>
> V2:
> - Checks old state when _block_isbad exists and bails out early if already bad
>
> drivers/mtd/mtdcore.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5ba9a741f5ac..096a3c94670f 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -2337,10 +2337,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
> EXPORT_SYMBOL_GPL(mtd_block_isbad);
>
> int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> {
> struct mtd_info *master = mtd_get_master(mtd);
> + loff_t moffs;
> int ret;
>
> if (!master->_block_markbad)
> return -EOPNOTSUPP;
> if (ofs < 0 || ofs >= mtd->size)
> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> return -EROFS;
>
> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
> ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>
> - ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> + moffs = mtd_get_master_ofs(mtd, ofs);
> +
> + if (master->_block_isbad) {
> + ret = master->_block_isbad(master, moffs);
> + if (ret > 0)
> + return 0;
Hi, Miquèl.
Here, should we keep the same logic with the lower level(eg.
nand_block_markbad, onenand_block_markbad) when 'ret < 0' is returned by
master->_block_isbad. Many specific nand drivers(markbad) return the
negative code when 'isbad' fails.
> + }
> +
> + ret = master->_block_markbad(master, moffs);
> if (ret)
> return ret;
>
> while (mtd->parent) {
> mtd->ecc_stats.badblocks++;
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
@ 2025-09-02 11:41 ` Zhihao Cheng
0 siblings, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2025-09-02 11:41 UTC (permalink / raw)
To: Wang Zhaolong, miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, yi.zhang, yangerkun
在 2025/9/2 17:27, Wang Zhaolong 写道:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
>
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
>
> This keeps the logic centralized in mtdcore without requiring driver
> changes.
>
> Link: https://lore.kernel.org/all/ef573188-9815-4a6b-bad1-3d8ff7c9b16f@huaweicloud.com/ [1]
> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> ---
>
> V2:
> - Checks old state when _block_isbad exists and bails out early if already bad
>
> drivers/mtd/mtdcore.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5ba9a741f5ac..096a3c94670f 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -2337,10 +2337,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
> EXPORT_SYMBOL_GPL(mtd_block_isbad);
>
> int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> {
> struct mtd_info *master = mtd_get_master(mtd);
> + loff_t moffs;
> int ret;
>
> if (!master->_block_markbad)
> return -EOPNOTSUPP;
> if (ofs < 0 || ofs >= mtd->size)
> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> return -EROFS;
>
> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
> ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>
> - ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> + moffs = mtd_get_master_ofs(mtd, ofs);
> +
> + if (master->_block_isbad) {
> + ret = master->_block_isbad(master, moffs);
> + if (ret > 0)
> + return 0;
Hi, Miquèl.
Here, should we keep the same logic with the lower level(eg.
nand_block_markbad, onenand_block_markbad) when 'ret < 0' is returned by
master->_block_isbad. Many specific nand drivers(markbad) return the
negative code when 'isbad' fails.
> + }
> +
> + ret = master->_block_markbad(master, moffs);
> if (ret)
> return ret;
>
> while (mtd->parent) {
> mtd->ecc_stats.badblocks++;
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
2025-09-02 11:41 ` Zhihao Cheng
@ 2025-09-02 13:16 ` Miquel Raynal
-1 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2025-09-02 13:16 UTC (permalink / raw)
To: Zhihao Cheng
Cc: Wang Zhaolong, richard, vigneshr, linux-mtd, linux-kernel,
yi.zhang, yangerkun
>> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
>> return -EROFS;
>> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>> ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>> - ret = master->_block_markbad(master, mtd_get_master_ofs(mtd,
>> ofs));
>> + moffs = mtd_get_master_ofs(mtd, ofs);
>> +
>> + if (master->_block_isbad) {
>> + ret = master->_block_isbad(master, moffs);
>> + if (ret > 0)
>> + return 0;
>
> Hi, Miquèl.
> Here, should we keep the same logic with the lower
> level(eg. nand_block_markbad, onenand_block_markbad) when 'ret < 0' is
> returned by master->_block_isbad. Many specific nand drivers(markbad)
> return the negative code when 'isbad' fails.
Good question, I guess in case of error in isbad() we shall probably
still try to mark the block bad because actually marking a block bad is
probably more important than returning correct statistics.
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
@ 2025-09-02 13:16 ` Miquel Raynal
0 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2025-09-02 13:16 UTC (permalink / raw)
To: Zhihao Cheng
Cc: Wang Zhaolong, richard, vigneshr, linux-mtd, linux-kernel,
yi.zhang, yangerkun
>> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
>> return -EROFS;
>> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
>> ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>> - ret = master->_block_markbad(master, mtd_get_master_ofs(mtd,
>> ofs));
>> + moffs = mtd_get_master_ofs(mtd, ofs);
>> +
>> + if (master->_block_isbad) {
>> + ret = master->_block_isbad(master, moffs);
>> + if (ret > 0)
>> + return 0;
>
> Hi, Miquèl.
> Here, should we keep the same logic with the lower
> level(eg. nand_block_markbad, onenand_block_markbad) when 'ret < 0' is
> returned by master->_block_isbad. Many specific nand drivers(markbad)
> return the negative code when 'isbad' fails.
Good question, I guess in case of error in isbad() we shall probably
still try to mark the block bad because actually marking a block bad is
probably more important than returning correct statistics.
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
2025-09-02 9:27 ` Wang Zhaolong
@ 2025-09-03 2:11 ` Zhihao Cheng
-1 siblings, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2025-09-03 2:11 UTC (permalink / raw)
To: Wang Zhaolong, miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, yi.zhang, yangerkun
在 2025/9/2 17:27, Wang Zhaolong 写道:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
>
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
>
> This keeps the logic centralized in mtdcore without requiring driver
> changes.
>
> Link: https://lore.kernel.org/all/ef573188-9815-4a6b-bad1-3d8ff7c9b16f@huaweicloud.com/ [1]
> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> ---
>
> V2:
> - Checks old state when _block_isbad exists and bails out early if already bad
>
> drivers/mtd/mtdcore.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5ba9a741f5ac..096a3c94670f 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -2337,10 +2337,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
> EXPORT_SYMBOL_GPL(mtd_block_isbad);
>
> int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> {
> struct mtd_info *master = mtd_get_master(mtd);
> + loff_t moffs;
> int ret;
>
> if (!master->_block_markbad)
> return -EOPNOTSUPP;
> if (ofs < 0 || ofs >= mtd->size)
> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> return -EROFS;
>
> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
> ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>
> - ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> + moffs = mtd_get_master_ofs(mtd, ofs);
> +
> + if (master->_block_isbad) {
> + ret = master->_block_isbad(master, moffs);
> + if (ret > 0)
> + return 0;
> + }
> +
> + ret = master->_block_markbad(master, moffs);
> if (ret)
> return ret;
>
> while (mtd->parent) {
> mtd->ecc_stats.badblocks++;
>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
@ 2025-09-03 2:11 ` Zhihao Cheng
0 siblings, 0 replies; 10+ messages in thread
From: Zhihao Cheng @ 2025-09-03 2:11 UTC (permalink / raw)
To: Wang Zhaolong, miquel.raynal, richard, vigneshr
Cc: linux-mtd, linux-kernel, yi.zhang, yangerkun
在 2025/9/2 17:27, Wang Zhaolong 写道:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
>
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
>
> This keeps the logic centralized in mtdcore without requiring driver
> changes.
>
> Link: https://lore.kernel.org/all/ef573188-9815-4a6b-bad1-3d8ff7c9b16f@huaweicloud.com/ [1]
> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com>
> ---
>
> V2:
> - Checks old state when _block_isbad exists and bails out early if already bad
>
> drivers/mtd/mtdcore.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 5ba9a741f5ac..096a3c94670f 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -2337,10 +2337,11 @@ int mtd_block_isbad(struct mtd_info *mtd, loff_t ofs)
> EXPORT_SYMBOL_GPL(mtd_block_isbad);
>
> int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> {
> struct mtd_info *master = mtd_get_master(mtd);
> + loff_t moffs;
> int ret;
>
> if (!master->_block_markbad)
> return -EOPNOTSUPP;
> if (ofs < 0 || ofs >= mtd->size)
> @@ -2349,11 +2350,19 @@ int mtd_block_markbad(struct mtd_info *mtd, loff_t ofs)
> return -EROFS;
>
> if (mtd->flags & MTD_SLC_ON_MLC_EMULATION)
> ofs = (loff_t)mtd_div_by_eb(ofs, mtd) * master->erasesize;
>
> - ret = master->_block_markbad(master, mtd_get_master_ofs(mtd, ofs));
> + moffs = mtd_get_master_ofs(mtd, ofs);
> +
> + if (master->_block_isbad) {
> + ret = master->_block_isbad(master, moffs);
> + if (ret > 0)
> + return 0;
> + }
> +
> + ret = master->_block_markbad(master, moffs);
> if (ret)
> return ret;
>
> while (mtd->parent) {
> mtd->ecc_stats.badblocks++;
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
2025-09-02 9:27 ` Wang Zhaolong
@ 2025-09-05 14:56 ` Miquel Raynal
-1 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2025-09-05 14:56 UTC (permalink / raw)
To: richard, vigneshr, Wang Zhaolong
Cc: linux-mtd, linux-kernel, chengzhihao1, yi.zhang, yangerkun
On Tue, 02 Sep 2025 17:27:32 +0800, Wang Zhaolong wrote:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
>
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
>
> [...]
Applied to mtd/next, thanks!
[1/1] mtd: core: skip badblocks increment for blocks already known bad
commit: 1b2dd17dd514b699818afeac1b513651b003ec10
Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).
Kind regards,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH V2] mtd: core: skip badblocks increment for blocks already known bad
@ 2025-09-05 14:56 ` Miquel Raynal
0 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2025-09-05 14:56 UTC (permalink / raw)
To: richard, vigneshr, Wang Zhaolong
Cc: linux-mtd, linux-kernel, chengzhihao1, yi.zhang, yangerkun
On Tue, 02 Sep 2025 17:27:32 +0800, Wang Zhaolong wrote:
> Repeatedly marking the same eraseblock bad inflates
> mtd->ecc_stats.badblocks because mtd_block_markbad() unconditionally
> increments the counter on success, while some implementations (e.g.
> NAND) return 0 both when the block was already bad and when it has just
> been marked[1].
>
> Fix by checking if the block is already bad before calling
> ->_block_markbad() when _block_isbad is available. Only skip the counter
> increment when we can confirm the block was already bad. In all other
> cases continue incrementing the counter.
>
> [...]
Applied to mtd/next, thanks!
[1/1] mtd: core: skip badblocks increment for blocks already known bad
commit: 1b2dd17dd514b699818afeac1b513651b003ec10
Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).
Kind regards,
Miquèl
^ permalink raw reply [flat|nested] 10+ messages in thread