[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ms7dntlb.fsf@bootlin.com>
Date: Tue, 02 Sep 2025 10:31:44 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Wang Zhaolong <wangzhaolong@...weicloud.com>
Cc: richard@....at, vigneshr@...com, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org, chengzhihao1@...wei.com,
yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH] mtd: core: only increment ecc_stats.badblocks on
confirmed good->bad transition
Hello Wang,
> @@ -2349,17 +2351,35 @@ 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);
> +
> + /* Pre-check: remember current state if available. */
> + if (master->_block_isbad)
> + oldbad = master->_block_isbad(master, moffs);
> +
> + ret = master->_block_markbad(master, moffs);
> if (ret)
> return ret;
>
> - while (mtd->parent) {
> - mtd->ecc_stats.badblocks++;
> - mtd = mtd->parent;
> + /*
> + * Post-check and bump stats only on a confirmed good->bad transition.
> + * If _block_isbad is not implemented, be conservative and do not bump.
> + */
> + if (master->_block_isbad) {
> + /* If it was already bad, nothing to do. */
> + if (oldbad > 0)
> + return 0;
> +
> + if (master->_block_isbad(master, moffs) > 0) {
> + while (mtd->parent) {
> + mtd->ecc_stats.badblocks++;
> + mtd = mtd->parent;
> + }
> + }
I don't think you can assume the block was already bad and must not be
accounted as a new bad block if you cannot verify that. In this case we
must remain conservative and tell userspace a new block was marked bad,
I believe.
Said otherwise, the { while () badblocks++ } block shall remain outside
of the if (_block_isbad) condition and remain untouched. Just bail out
early if you are sure this is not needed.
Thanks,
Miquèl
Powered by blists - more mailing lists