* [PATCH 1/3] mtd: ubi: mark PEBs as bad on erase failure
@ 2019-07-15 13:54 Sascha Hauer
2019-07-15 13:54 ` [PATCH 2/3] mtd: peb: Do not mark as bad in mtd_peb_torture() Sascha Hauer
2019-07-15 13:54 ` [PATCH 3/3] ubiformat: handle write errors correctly Sascha Hauer
0 siblings, 2 replies; 4+ messages in thread
From: Sascha Hauer @ 2019-07-15 13:54 UTC (permalink / raw)
To: Barebox List
70542a9c65 converted UBI to use mtd_peb_torture(). It was assumed that a
block was marked as bad when it didn't pass the torture test. However,
not all possibly bad blocks went through the torture test, so it could
happen that a block that could not be erased was still kept as good
block. This patch fixes this and explicitly calls ubi_io_mark_bad() when
a block cannot be erased.
Fixes: 70542a9c65 ("mtd: ubi: Use mtd_peb_torture")
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mtd/ubi/io.c | 29 +++++++++++++++++++++++++++++
drivers/mtd/ubi/ubi.h | 1 +
drivers/mtd/ubi/wl.c | 5 +++++
3 files changed, 35 insertions(+)
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index d0ea23b124..78458b58e1 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -356,6 +356,35 @@ int ubi_io_is_bad(const struct ubi_device *ubi, int pnum)
return 0;
}
+/**
+ * ubi_io_mark_bad - mark a physical eraseblock as bad.
+ * @ubi: UBI device description object
+ * @pnum: the physical eraseblock number to mark
+ *
+ * This function returns zero in case of success and a negative error code in
+ * case of failure.
+ */
+int ubi_io_mark_bad(const struct ubi_device *ubi, int pnum)
+{
+ int err;
+ struct mtd_info *mtd = ubi->mtd;
+
+ ubi_assert(pnum >= 0 && pnum < ubi->peb_count);
+
+ if (ubi->ro_mode) {
+ ubi_err(ubi, "read-only mode");
+ return -EROFS;
+ }
+
+ if (!ubi->bad_allowed)
+ return 0;
+
+ err = mtd_block_markbad(mtd, (loff_t)pnum * ubi->peb_size);
+ if (err)
+ ubi_err(ubi, "cannot mark PEB %d bad, error %d", pnum, err);
+ return err;
+}
+
/**
* validate_ec_hdr - validate an erase counter header.
* @ubi: UBI device description object
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index b85d602055..922c1a3c8b 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -865,6 +865,7 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset,
int len);
int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture);
int ubi_io_is_bad(const struct ubi_device *ubi, int pnum);
+int ubi_io_mark_bad(const struct ubi_device *ubi, int pnum);
int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
struct ubi_ec_hdr *ec_hdr, int verbose);
int ubi_io_write_ec_hdr(struct ubi_device *ubi, int pnum,
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index fa1b813702..cf90ecfb23 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -1085,6 +1085,11 @@ static int __erase_worker(struct ubi_device *ubi, struct ubi_work *wl_wrk)
available_consumed = 1;
}
+ ubi_msg(ubi, "mark PEB %d as bad", pnum);
+ err = ubi_io_mark_bad(ubi, pnum);
+ if (err)
+ goto out_ro;
+
if (ubi->beb_rsvd_pebs > 0) {
if (available_consumed) {
/*
--
2.20.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/3] mtd: peb: Do not mark as bad in mtd_peb_torture()
2019-07-15 13:54 [PATCH 1/3] mtd: ubi: mark PEBs as bad on erase failure Sascha Hauer
@ 2019-07-15 13:54 ` Sascha Hauer
2019-07-15 14:48 ` Ahmad Fatoum
2019-07-15 13:54 ` [PATCH 3/3] ubiformat: handle write errors correctly Sascha Hauer
1 sibling, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2019-07-15 13:54 UTC (permalink / raw)
To: Barebox List
Both the Kernel and mtd-utils have peb torture functions and both
do not mark the block as bad automatically. Instead, the caller
must mark the block as bad when -EIO is returned from the torture
function. Do the same in barebox. This is necessary as the UBI code
otherwise may mark a block as bad twice: Once indirectly in
mtd_peb_torture() and then directly afterwards.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/imx-bbu-nand-fcb.c | 5 +++++
common/state/backend_bucket_circular.c | 2 ++
common/ubiformat.c | 18 ++++++++++++------
drivers/mtd/peb.c | 9 +++------
4 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/common/imx-bbu-nand-fcb.c b/common/imx-bbu-nand-fcb.c
index 6d773b59df..a62e2a2975 100644
--- a/common/imx-bbu-nand-fcb.c
+++ b/common/imx-bbu-nand-fcb.c
@@ -531,6 +531,9 @@ again:
if (ret == -EBADMSG) {
ret = mtd_peb_torture(mtd, block);
+ if (ret == -EIO)
+ mtd_peb_mark_bad(mtd, block);
+
if (!ret && retries++ < 3)
goto again;
}
@@ -771,6 +774,8 @@ out:
if (ret == -EBADMSG) {
ret = mtd_peb_torture(mtd, block);
+ if (ret == -EIO)
+ mtd_peb_mark_bad(mtd, block);
if (!ret && retries++ < 3)
goto again;
diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
index 4676730d05..9c106b7b63 100644
--- a/common/state/backend_bucket_circular.c
+++ b/common/state/backend_bucket_circular.c
@@ -95,6 +95,7 @@ static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ
if (ret == -EBADMSG) {
ret = mtd_peb_torture(circ->mtd, circ->eraseblock);
if (ret == -EIO) {
+ mtd_peb_mark_bad(mtd, circ->eraseblock);
dev_err(circ->dev, "Tortured eraseblock failed and is marked bad now, PEB %u\n",
circ->eraseblock);
return -EIO;
@@ -132,6 +133,7 @@ static int state_mtd_peb_write(struct state_backend_storage_bucket_circular *cir
if (ret == -EBADMSG) {
ret = mtd_peb_torture(circ->mtd, circ->eraseblock);
if (ret == -EIO) {
+ mtd_peb_mark_bad(mtd, circ->eraseblock);
dev_err(circ->dev, "Tortured eraseblock failed and is marked bad now, PEB %u\n",
circ->eraseblock);
return -EIO;
diff --git a/common/ubiformat.c b/common/ubiformat.c
index fe02270b78..4f0df6fd5c 100644
--- a/common/ubiformat.c
+++ b/common/ubiformat.c
@@ -310,10 +310,13 @@ static int flash_image(struct ubiformat_args *args, struct mtd_info *mtd,
goto out_close;
err = mtd_peb_torture(mtd, eb);
- if (err < 0 && err != -EIO)
- goto out_close;
- if (err == -EIO && consecutive_bad_check(args, eb))
+ if (err == -EIO) {
+ err = mark_bad(args, mtd, si, eb);
+ if (err)
+ goto out_close;
+ } else if (err) {
goto out_close;
+ }
continue;
}
@@ -426,10 +429,13 @@ static int format(struct ubiformat_args *args, struct mtd_info *mtd,
}
err = mtd_peb_torture(mtd, eb);
- if (err < 0 && err != -EIO)
- goto out_free;
- if (err == -EIO && consecutive_bad_check(args, eb))
+ if (err == -EIO) {
+ err = mark_bad(args, mtd, si, eb);
+ if (err)
+ goto out_free;
+ } else if (err) {
goto out_free;
+ }
continue;
diff --git a/drivers/mtd/peb.c b/drivers/mtd/peb.c
index 388db7f587..c82b3e8bba 100644
--- a/drivers/mtd/peb.c
+++ b/drivers/mtd/peb.c
@@ -493,9 +493,8 @@ static uint8_t patterns[] = {0xa5, 0x5a, 0x0};
* is passed then this function will with the block freshly erased and
* the positive number returned indicaties how often the block has been
* erased during this test.
- * If the block does not pass the test the block is marked as bad and
- * -EIO is returned.
- * Other negative errors are returned in case of other errors.
+ * If the block does not pass the test -EIO is returned.
+ * Other negative errors are returned in case of other errors.
*/
int mtd_peb_torture(struct mtd_info *mtd, int pnum)
{
@@ -549,11 +548,9 @@ out:
* has not passed because it happened on a freshly erased
* physical eraseblock which means something is wrong with it.
*/
- dev_err(&mtd->class_dev, "read problems on freshly erased PEB %d, marking it bad\n",
+ dev_err(&mtd->class_dev, "read problems on freshly erased PEB %d, must be bad\n",
pnum);
- mtd_peb_mark_bad(mtd, pnum);
-
err = -EIO;
}
--
2.20.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 3/3] ubiformat: handle write errors correctly
2019-07-15 13:54 [PATCH 1/3] mtd: ubi: mark PEBs as bad on erase failure Sascha Hauer
2019-07-15 13:54 ` [PATCH 2/3] mtd: peb: Do not mark as bad in mtd_peb_torture() Sascha Hauer
@ 2019-07-15 13:54 ` Sascha Hauer
1 sibling, 0 replies; 4+ messages in thread
From: Sascha Hauer @ 2019-07-15 13:54 UTC (permalink / raw)
To: Barebox List
This is a barebox adoption of mtd-utils commit
d9cbf6a ("ubiformat: handle write errors correctly"):
| ubiformat: handle write errors correctly
|
| This issue was reported and analyzed by
| Anton Olofsson <anol.martinsson@gmail.com>:
|
| when ubiformat encounters a write error while flashing the UBI image (which may
| come from a file of from stdout), it correctly marks the faulty eraseblock as
| bad and skips it. However, it also incorrectly drops the data buffer which was
| supposed to be written, and reads next block of data.
|
| This patch fixes this issue - in case of a write error, we preserve the current
| data and write it to the next eraseblock, instead of dropping it.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/ubiformat.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/common/ubiformat.c b/common/ubiformat.c
index 4f0df6fd5c..655c5323ba 100644
--- a/common/ubiformat.c
+++ b/common/ubiformat.c
@@ -188,6 +188,7 @@ static int flash_image(struct ubiformat_args *args, struct mtd_info *mtd,
const struct ubigen_info *ui, struct ubi_scan_info *si)
{
int fd = 0, img_ebs, eb, written_ebs = 0, ret = -1, eb_cnt;
+ int skip_data_read = 0;
off_t st_size;
char *buf = NULL;
uint64_t lastprint = 0;
@@ -266,17 +267,20 @@ static int flash_image(struct ubiformat_args *args, struct mtd_info *mtd,
continue;
}
- if (args->image) {
- err = read_full(fd, buf, mtd->erasesize);
- if (err < 0) {
- sys_errmsg("failed to read eraseblock %d from image",
- written_ebs);
- goto out_close;
+ if (!skip_data_read) {
+ if (args->image) {
+ err = read_full(fd, buf, mtd->erasesize);
+ if (err < 0) {
+ sys_errmsg("failed to read eraseblock %d from image",
+ written_ebs);
+ goto out_close;
+ }
+ } else {
+ memcpy(buf, inbuf, mtd->erasesize);
+ inbuf += mtd->erasesize;
}
- } else {
- memcpy(buf, inbuf, mtd->erasesize);
- inbuf += mtd->erasesize;
}
+ skip_data_read = 0;
if (args->override_ec)
ec = args->ec;
@@ -318,6 +322,13 @@ static int flash_image(struct ubiformat_args *args, struct mtd_info *mtd,
goto out_close;
}
+ /*
+ * We have to make sure that we do not read next block
+ * of data from the input image or stdin - we have to
+ * write buf first instead.
+ */
+ skip_data_read = 1;
+
continue;
}
if (++written_ebs >= img_ebs)
--
2.20.1
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/3] mtd: peb: Do not mark as bad in mtd_peb_torture()
2019-07-15 13:54 ` [PATCH 2/3] mtd: peb: Do not mark as bad in mtd_peb_torture() Sascha Hauer
@ 2019-07-15 14:48 ` Ahmad Fatoum
0 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2019-07-15 14:48 UTC (permalink / raw)
To: barebox, Sascha Hauer
Hello Sascha,
On 15/7/19 15:54, Sascha Hauer wrote:
> diff --git a/common/state/backend_bucket_circular.c b/common/state/backend_bucket_circular.c
> index 4676730d05..9c106b7b63 100644
> --- a/common/state/backend_bucket_circular.c
> +++ b/common/state/backend_bucket_circular.c
> @@ -95,6 +95,7 @@ static int state_mtd_peb_read(struct state_backend_storage_bucket_circular *circ
> if (ret == -EBADMSG) {
> ret = mtd_peb_torture(circ->mtd, circ->eraseblock);
> if (ret == -EIO) {
> + mtd_peb_mark_bad(mtd, circ->eraseblock);
There's no `mtd' in this scope. Do you mean `circ->mtd'?
> dev_err(circ->dev, "Tortured eraseblock failed and is marked bad now, PEB %u\n",
> circ->eraseblock);
> return -EIO;
> @@ -132,6 +133,7 @@ static int state_mtd_peb_write(struct state_backend_storage_bucket_circular *cir
> if (ret == -EBADMSG) {
> ret = mtd_peb_torture(circ->mtd, circ->eraseblock);
> if (ret == -EIO) {
> + mtd_peb_mark_bad(mtd, circ->eraseblock);
Likewise.
> dev_err(circ->dev, "Tortured eraseblock failed and is marked bad now, PEB %u\n",
> circ->eraseblock);
> return -EIO;
Thanks,
Ahmad
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-15 14:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 13:54 [PATCH 1/3] mtd: ubi: mark PEBs as bad on erase failure Sascha Hauer
2019-07-15 13:54 ` [PATCH 2/3] mtd: peb: Do not mark as bad in mtd_peb_torture() Sascha Hauer
2019-07-15 14:48 ` Ahmad Fatoum
2019-07-15 13:54 ` [PATCH 3/3] ubiformat: handle write errors correctly Sascha Hauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox