[PATCH 00/13] UBI reliability improvements

[PATCH 00/13] UBI reliability improvements

[Artem Bityutskiy]

Hi,

here is a set of patches which should improve UBI reliability. I wrote them
during my trip to LinuxCon Brazil.

Patches 1-5 should be interesting for Matthieu CASTET - they improve -EUCLEAN
errors handling a little.

Patches 6-13 should be interesting for Matthew L. Creech - they presimably have
to help in catching why in his setup UBIFS refers unmapped LEB. I'm not sure
they will help, but there is a chance - my theory is that UBI decides that the
LEB is corrupted and wipes it. With this patch-set UBI should instead preserve
corrupted LEBs, so we could take a look what happened.

Please, reveiw, test. I tested them a little as well, but not extensively.

Artem.

[PATCH 01/13] mtd: nand_nase: do not cache pages with uncorrectable ECC errors

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Currently MTD caches the last read NAND page, even if there was an uncorrectable ECC
error. This patch prevents caching in case of uncorrectable ECC errors. The reason
is that we want to allow the user to re-read the NAND page several times. In case of
unstable bits re-trying may help.

Moreover, current behavior is wrong because the first read returns -EBADMSG (correctly)
but the second read succeeds and incorrectly returns 0 (because we read from the cache).

I've pushed these patches to ubi-2.6.git / master.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/nand/nand_base.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index d551ddd..ddffe76 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -1493,7 +1493,8 @@ static int nand_do_read_ops(struct mtd_info *mtd, loff_t from,
 
 			/* Transfer not aligned data */
 			if (!aligned) {
-				if (!NAND_SUBPAGE_READ(chip) && !oob)
+				if (!NAND_SUBPAGE_READ(chip) && !oob &&
+				    !(mtd->ecc_stats.failed - stats.failed))
 					chip->pagebuf = realpage;
 				memcpy(buf, chip->buffers->databuf + col, bytes);
 			}
-- 
1.7.1.1

[PATCH 02/13] UBI: fix small 80 characters limit style issue

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

One line was longer than 80 lines, make it shorter.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/build.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 78ae894..f247c4e 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -95,8 +95,8 @@ DEFINE_MUTEX(ubi_devices_mutex);
 static DEFINE_SPINLOCK(ubi_devices_lock);
 
 /* "Show" method for files in '/<sysfs>/class/ubi/' */
-static ssize_t ubi_version_show(struct class *class, struct class_attribute *attr,
-				char *buf)
+static ssize_t ubi_version_show(struct class *class,
+				struct class_attribute *attr, char *buf)
 {
 	return sprintf(buf, "%d\n", UBI_VERSION);
 }
-- 
1.7.1.1

[PATCH 03/13] UBI: rename IO error code

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Rename UBI_IO_BAD_HDR_READ into UBI_IO_BAD_HDR_EBADMSG which is presumably more
self-documenting and readable. Indeed, the '_READ' suffix does not tell much and
even confuses, while '_EBADMSG' tells about uncorrectable ECC error, because we
use -EBADMSG all over the place to represent ECC errors.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/eba.c  |    4 ++--
 drivers/mtd/ubi/io.c   |   33 +++++++++++++++++++--------------
 drivers/mtd/ubi/scan.c |    8 ++++----
 drivers/mtd/ubi/scan.h |    2 +-
 drivers/mtd/ubi/ubi.h  |    7 ++++---
 5 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index fe74749..334865e 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -418,7 +418,7 @@ retry:
 				 * may try to recover data. FIXME: but this is
 				 * not implemented.
 				 */
-				if (err == UBI_IO_BAD_HDR_READ ||
+				if (err == UBI_IO_BAD_HDR_EBADMSG ||
 				    err == UBI_IO_BAD_HDR) {
 					ubi_warn("corrupted VID header at PEB "
 						 "%d, LEB %d:%d", pnum, vol_id,
@@ -963,7 +963,7 @@ write_error:
 static int is_error_sane(int err)
 {
 	if (err == -EIO || err == -ENOMEM || err == UBI_IO_BAD_HDR ||
-	    err == UBI_IO_BAD_HDR_READ || err == -ETIMEDOUT)
+	    err == UBI_IO_BAD_HDR_EBADMSG || err == -ETIMEDOUT)
 		return 0;
 	return 1;
 }
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 332f992..05774da 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -517,7 +517,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum)
 	 * In this case we probably anyway have garbage in this PEB.
 	 */
 	err1 = ubi_io_read_vid_hdr(ubi, pnum, &vid_hdr, 0);
-	if (err1 == UBI_IO_BAD_HDR_READ || err1 == UBI_IO_BAD_HDR)
+	if (err1 == UBI_IO_BAD_HDR_EBADMSG || err1 == UBI_IO_BAD_HDR)
 		/*
 		 * The VID header is corrupted, so we can safely erase this
 		 * PEB and not afraid that it will be treated as a valid PEB in
@@ -712,6 +712,8 @@ bad:
  *   and corrected by the flash driver; this is harmless but may indicate that
  *   this eraseblock may become bad soon (but may be not);
  * o %UBI_IO_BAD_HDR if the erase counter header is corrupted (a CRC error);
+ * o %UBI_IO_BAD_HDR_EBADMSG is the same as %UBI_IO_BAD_HDR, but there also was
+ *   a data integrity error (uncorrectable ECC error in case of NAND);
  * o %UBI_IO_PEB_EMPTY if the physical eraseblock is empty;
  * o a negative error code in case of failure.
  */
@@ -731,15 +733,15 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 
 		/*
 		 * We read all the data, but either a correctable bit-flip
-		 * occurred, or MTD reported about some data integrity error,
-		 * like an ECC error in case of NAND. The former is harmless,
-		 * the later may mean that the read data is corrupted. But we
-		 * have a CRC check-sum and we will detect this. If the EC
-		 * header is still OK, we just report this as there was a
-		 * bit-flip.
+		 * occurred, or MTD reported a data integrity error
+		 * (uncorrectable ECC error in case of NAND). The former is
+		 * harmless, the later may mean that the read data is
+		 * corrupted. But we have a CRC check-sum and we will detect
+		 * this. If the EC header is still OK, we just report this as
+		 * there was a bit-flip, to force scrubbing.
 		 */
 		if (err == -EBADMSG)
-			read_err = UBI_IO_BAD_HDR_READ;
+			read_err = UBI_IO_BAD_HDR_EBADMSG;
 	}
 
 	magic = be32_to_cpu(ec_hdr->magic);
@@ -983,6 +985,8 @@ bad:
  *   this eraseblock may become bad soon;
  * o %UBI_IO_BAD_HDR if the volume identifier header is corrupted (a CRC
  *   error detected);
+ * o %UBI_IO_BAD_HDR_EBADMSG is the same as %UBI_IO_BAD_HDR, but there also was
+ *   a data integrity error (uncorrectable ECC error in case of NAND);
  * o %UBI_IO_PEB_FREE if the physical eraseblock is free (i.e., there is no VID
  *   header there);
  * o a negative error code in case of failure.
@@ -1006,14 +1010,15 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 
 		/*
 		 * We read all the data, but either a correctable bit-flip
-		 * occurred, or MTD reported about some data integrity error,
-		 * like an ECC error in case of NAND. The former is harmless,
-		 * the later may mean the read data is corrupted. But we have a
-		 * CRC check-sum and we will identify this. If the VID header is
-		 * still OK, we just report this as there was a bit-flip.
+		 * occurred, or MTD reported a data integrity error
+		 * (uncorrectable ECC error in case of NAND). The former is
+		 * harmless, the later may mean that the read data is
+		 * corrupted. But we have a CRC check-sum and we will detect
+		 * this. If the VID header is still OK, we just report this as
+		 * there was a bit-flip, to force scrubbing.
 		 */
 		if (err == -EBADMSG)
-			read_err = UBI_IO_BAD_HDR_READ;
+			read_err = UBI_IO_BAD_HDR_EBADMSG;
 	}
 
 	magic = be32_to_cpu(vid_hdr->magic);
diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 69b52e9..7e7c56d 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -750,7 +750,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		bitflips = 1;
 	else if (err == UBI_IO_PEB_EMPTY)
 		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, &si->erase);
-	else if (err == UBI_IO_BAD_HDR_READ || err == UBI_IO_BAD_HDR) {
+	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR) {
 		/*
 		 * We have to also look at the VID header, possibly it is not
 		 * corrupted. Set %bitflips flag in order to make this PEB be
@@ -816,11 +816,11 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		return err;
 	else if (err == UBI_IO_BITFLIPS)
 		bitflips = 1;
-	else if (err == UBI_IO_BAD_HDR_READ || err == UBI_IO_BAD_HDR ||
+	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR ||
 		 (err == UBI_IO_PEB_FREE && ec_corr)) {
 		/* VID header is corrupted */
-		if (err == UBI_IO_BAD_HDR_READ ||
-		    ec_corr == UBI_IO_BAD_HDR_READ)
+		if (err == UBI_IO_BAD_HDR_EBADMSG ||
+		    ec_corr == UBI_IO_BAD_HDR_EBADMSG)
 			si->read_err_count += 1;
 		err = add_to_list(si, pnum, ec, &si->corr);
 		if (err)
diff --git a/drivers/mtd/ubi/scan.h b/drivers/mtd/ubi/scan.h
index 2576a8d..0876649 100644
--- a/drivers/mtd/ubi/scan.h
+++ b/drivers/mtd/ubi/scan.h
@@ -93,7 +93,7 @@ struct ubi_scan_volume {
  *         those belonging to "preserve"-compatible internal volumes)
  * @used_peb_count: count of used PEBs
  * @corr_peb_count: count of PEBs in the @corr list
- * @read_err_count: count of PEBs read with error (%UBI_IO_BAD_HDR_READ was
+ * @read_err_count: count of PEBs read with error (%UBI_IO_BAD_HDR_EBADMSG was
  *                  returned)
  * @free_peb_count: count of PEBs in the @free list
  * @erase_peb_count: count of PEBs in the @erase list
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 0359e0c..24a7c76 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -90,15 +90,16 @@
  * UBI_IO_PEB_FREE: the physical eraseblock is free, i.e. it contains only a
  *                  valid erase counter header, and the rest are %0xFF bytes
  * UBI_IO_BAD_HDR: the EC or VID header is corrupted (bad magic or CRC)
- * UBI_IO_BAD_HDR_READ: the same as %UBI_IO_BAD_HDR, but also there was a read
- * 			error reported by the flash driver
+ * UBI_IO_BAD_HDR_EBADMSG: the same as %UBI_IO_BAD_HDR, but also there was a
+ *                         data integrity error reported by the MTD driver
+ *                         (uncorrectable ECC error in case of NAND)
  * UBI_IO_BITFLIPS: bit-flips were detected and corrected
  */
 enum {
 	UBI_IO_PEB_EMPTY = 1,
 	UBI_IO_PEB_FREE,
 	UBI_IO_BAD_HDR,
-	UBI_IO_BAD_HDR_READ,
+	UBI_IO_BAD_HDR_EBADMSG,
 	UBI_IO_BITFLIPS
 };
 
-- 
1.7.1.1

[PATCH 04/13] UBI: remove duplicate IO error codes

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

The 'UBI_IO_PEB_EMPTY' and 'UBI_IO_PEB_FREE' are essentially the same
and mean that there are only 0xFF bytes instead of headers. Simplify
UBI a little by turning them into a single 'UBI_IO_FF' error code.

Also, stop maintaining commentaries in 'ubi_io_read_vid_hdr()' which are
almost identical to commentaries in 'ubi_io_read_ec_hdr()'.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/io.c   |   45 +++++++--------------------------------------
 drivers/mtd/ubi/scan.c |    6 +++---
 drivers/mtd/ubi/ubi.h  |    8 ++------
 drivers/mtd/ubi/wl.c   |    2 +-
 4 files changed, 13 insertions(+), 48 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 05774da..1677a21 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -714,7 +714,7 @@ bad:
  * o %UBI_IO_BAD_HDR if the erase counter header is corrupted (a CRC error);
  * o %UBI_IO_BAD_HDR_EBADMSG is the same as %UBI_IO_BAD_HDR, but there also was
  *   a data integrity error (uncorrectable ECC error in case of NAND);
- * o %UBI_IO_PEB_EMPTY if the physical eraseblock is empty;
+ * o %UBI_IO_FF if only 0xFF bytes were read (the PEB is supposedly empty)
  * o a negative error code in case of failure.
  */
 int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
@@ -762,7 +762,7 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 			else if (UBI_IO_DEBUG)
 				dbg_msg("no EC header found at PEB %d, "
 					"only 0xFF bytes", pnum);
-			return UBI_IO_PEB_EMPTY;
+			return UBI_IO_FF;
 		}
 
 		/*
@@ -977,19 +977,11 @@ bad:
  *
  * This function reads the volume identifier header from physical eraseblock
  * @pnum and stores it in @vid_hdr. It also checks CRC checksum of the read
- * volume identifier header. The following codes may be returned:
+ * volume identifier header. The error codes are the same as in
+ * 'ubi_io_read_ec_hdr()'.
  *
- * o %0 if the CRC checksum is correct and the header was successfully read;
- * o %UBI_IO_BITFLIPS if the CRC is correct, but bit-flips were detected
- *   and corrected by the flash driver; this is harmless but may indicate that
- *   this eraseblock may become bad soon;
- * o %UBI_IO_BAD_HDR if the volume identifier header is corrupted (a CRC
- *   error detected);
- * o %UBI_IO_BAD_HDR_EBADMSG is the same as %UBI_IO_BAD_HDR, but there also was
- *   a data integrity error (uncorrectable ECC error in case of NAND);
- * o %UBI_IO_PEB_FREE if the physical eraseblock is free (i.e., there is no VID
- *   header there);
- * o a negative error code in case of failure.
+ * Note, the implementation of this function is also very similar to
+ * 'ubi_io_read_ec_hdr()', so refer commentaries in 'ubi_io_read_ec_hdr()'.
  */
 int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 			struct ubi_vid_hdr *vid_hdr, int verbose)
@@ -1008,15 +1000,6 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 		if (err != UBI_IO_BITFLIPS && err != -EBADMSG)
 			return err;
 
-		/*
-		 * We read all the data, but either a correctable bit-flip
-		 * occurred, or MTD reported a data integrity error
-		 * (uncorrectable ECC error in case of NAND). The former is
-		 * harmless, the later may mean that the read data is
-		 * corrupted. But we have a CRC check-sum and we will detect
-		 * this. If the VID header is still OK, we just report this as
-		 * there was a bit-flip, to force scrubbing.
-		 */
 		if (err == -EBADMSG)
 			read_err = UBI_IO_BAD_HDR_EBADMSG;
 	}
@@ -1026,25 +1009,16 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 		if (read_err)
 			return read_err;
 
-		/*
-		 * If we have read all 0xFF bytes, the VID header probably does
-		 * not exist and the physical eraseblock is assumed to be free.
-		 */
 		if (check_pattern(vid_hdr, 0xFF, UBI_VID_HDR_SIZE)) {
-			/* The physical eraseblock is supposedly free */
 			if (verbose)
 				ubi_warn("no VID header found at PEB %d, "
 					 "only 0xFF bytes", pnum);
 			else if (UBI_IO_DEBUG)
 				dbg_msg("no VID header found at PEB %d, "
 					"only 0xFF bytes", pnum);
-			return UBI_IO_PEB_FREE;
+			return UBI_IO_FF;
 		}
 
-		/*
-		 * This is not a valid VID header, and these are not 0xFF
-		 * bytes. Report that the header is corrupted.
-		 */
 		if (verbose) {
 			ubi_warn("bad magic number at PEB %d: %08x instead of "
 				 "%08x", pnum, magic, UBI_VID_HDR_MAGIC);
@@ -1069,17 +1043,12 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 		return read_err ?: UBI_IO_BAD_HDR;
 	}
 
-	/* Validate the VID header that we have just read */
 	err = validate_vid_hdr(ubi, vid_hdr);
 	if (err) {
 		ubi_err("validation failed for PEB %d", pnum);
 		return -EINVAL;
 	}
 
-	/*
-	 * If there was a read error (%-EBADMSG), but the header CRC is still
-	 * OK, report about a bit-flip to force scrubbing on this PEB.
-	 */
 	return read_err ? UBI_IO_BITFLIPS : 0;
 }
 
diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 7e7c56d..37cb18f 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -748,7 +748,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		return err;
 	else if (err == UBI_IO_BITFLIPS)
 		bitflips = 1;
-	else if (err == UBI_IO_PEB_EMPTY)
+	else if (err == UBI_IO_FF)
 		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, &si->erase);
 	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR) {
 		/*
@@ -817,7 +817,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 	else if (err == UBI_IO_BITFLIPS)
 		bitflips = 1;
 	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR ||
-		 (err == UBI_IO_PEB_FREE && ec_corr)) {
+		 (err == UBI_IO_FF && ec_corr)) {
 		/* VID header is corrupted */
 		if (err == UBI_IO_BAD_HDR_EBADMSG ||
 		    ec_corr == UBI_IO_BAD_HDR_EBADMSG)
@@ -826,7 +826,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		if (err)
 			return err;
 		goto adjust_mean_ec;
-	} else if (err == UBI_IO_PEB_FREE) {
+	} else if (err == UBI_IO_FF) {
 		/* No VID header - the physical eraseblock is free */
 		err = add_to_list(si, pnum, ec, &si->free);
 		if (err)
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 24a7c76..774bdca 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -85,10 +85,7 @@
 /*
  * Error codes returned by the I/O sub-system.
  *
- * UBI_IO_PEB_EMPTY: the physical eraseblock is empty, i.e. it contains only
- *                   %0xFF bytes
- * UBI_IO_PEB_FREE: the physical eraseblock is free, i.e. it contains only a
- *                  valid erase counter header, and the rest are %0xFF bytes
+ * UBI_IO_FF: the read region of flash contains only 0xFFs
  * UBI_IO_BAD_HDR: the EC or VID header is corrupted (bad magic or CRC)
  * UBI_IO_BAD_HDR_EBADMSG: the same as %UBI_IO_BAD_HDR, but also there was a
  *                         data integrity error reported by the MTD driver
@@ -96,8 +93,7 @@
  * UBI_IO_BITFLIPS: bit-flips were detected and corrected
  */
 enum {
-	UBI_IO_PEB_EMPTY = 1,
-	UBI_IO_PEB_FREE,
+	UBI_IO_FF = 1,
 	UBI_IO_BAD_HDR,
 	UBI_IO_BAD_HDR_EBADMSG,
 	UBI_IO_BITFLIPS
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index 97a4356..a9e7c9e 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -745,7 +745,7 @@ static int wear_leveling_worker(struct ubi_device *ubi, struct ubi_work *wrk,
 
 	err = ubi_io_read_vid_hdr(ubi, e1->pnum, vid_hdr, 0);
 	if (err && err != UBI_IO_BITFLIPS) {
-		if (err == UBI_IO_PEB_FREE) {
+		if (err == UBI_IO_FF) {
 			/*
 			 * We are trying to move PEB without a VID header. UBI
 			 * always write VID headers shortly after the PEB was
-- 
1.7.1.1

[PATCH 05/13] UBI: handle bit-flips when no header found

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Currently UBI has one small flaw - when we read EC or VID header, but find only
0xFF bytes, we return UBI_IO_FF and do not report whether we had bit-flips or
not. In case of the VID header, the scanning code adds this PEB to the free list,
even though there were bit-flips.

Imagine the following situation: we start writing VID header to a PEB and have a
power cut, so the PEB becomes unstable. When we scan and read the PEB, we get
a bit-flip. Currently, UBI would just ignore this and treat the PEB as free. This
patch changes UBI behavior and now UBI will schedule this PEB for erasure.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/io.c   |   54 ++++++++++++++++++++++++++---------------------
 drivers/mtd/ubi/scan.c |    4 +-
 drivers/mtd/ubi/ubi.h  |   10 ++++++++-
 drivers/mtd/ubi/wl.c   |   10 ++++++++
 4 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c
index 1677a21..b762524 100644
--- a/drivers/mtd/ubi/io.c
+++ b/drivers/mtd/ubi/io.c
@@ -720,16 +720,16 @@ bad:
 int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 		       struct ubi_ec_hdr *ec_hdr, int verbose)
 {
-	int err, read_err = 0;
+	int err, read_err;
 	uint32_t crc, magic, hdr_crc;
 
 	dbg_io("read EC header from PEB %d", pnum);
 	ubi_assert(pnum >= 0 && pnum < ubi->peb_count);
 
-	err = ubi_io_read(ubi, ec_hdr, pnum, 0, UBI_EC_HDR_SIZE);
-	if (err) {
-		if (err != UBI_IO_BITFLIPS && err != -EBADMSG)
-			return err;
+	read_err = ubi_io_read(ubi, ec_hdr, pnum, 0, UBI_EC_HDR_SIZE);
+	if (read_err) {
+		if (read_err != UBI_IO_BITFLIPS && read_err != -EBADMSG)
+			return read_err;
 
 		/*
 		 * We read all the data, but either a correctable bit-flip
@@ -740,14 +740,12 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 		 * this. If the EC header is still OK, we just report this as
 		 * there was a bit-flip, to force scrubbing.
 		 */
-		if (err == -EBADMSG)
-			read_err = UBI_IO_BAD_HDR_EBADMSG;
 	}
 
 	magic = be32_to_cpu(ec_hdr->magic);
 	if (magic != UBI_EC_HDR_MAGIC) {
-		if (read_err)
-			return read_err;
+		if (read_err == -EBADMSG)
+			return UBI_IO_BAD_HDR_EBADMSG;
 
 		/*
 		 * The magic field is wrong. Let's check if we have read all
@@ -762,7 +760,10 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 			else if (UBI_IO_DEBUG)
 				dbg_msg("no EC header found at PEB %d, "
 					"only 0xFF bytes", pnum);
-			return UBI_IO_FF;
+			if (!read_err)
+				return UBI_IO_FF;
+			else
+				return UBI_IO_FF_BITFLIPS;
 		}
 
 		/*
@@ -790,7 +791,11 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum,
 		} else if (UBI_IO_DEBUG)
 			dbg_msg("bad EC header CRC at PEB %d, calculated "
 				"%#08x, read %#08x", pnum, crc, hdr_crc);
-		return read_err ?: UBI_IO_BAD_HDR;
+
+		if (!read_err)
+			return UBI_IO_BAD_HDR;
+		else
+			return UBI_IO_BAD_HDR_EBADMSG;
 	}
 
 	/* And of course validate what has just been read from the media */
@@ -986,7 +991,7 @@ bad:
 int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 			struct ubi_vid_hdr *vid_hdr, int verbose)
 {
-	int err, read_err = 0;
+	int err, read_err;
 	uint32_t crc, magic, hdr_crc;
 	void *p;
 
@@ -994,20 +999,15 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 	ubi_assert(pnum >= 0 &&  pnum < ubi->peb_count);
 
 	p = (char *)vid_hdr - ubi->vid_hdr_shift;
-	err = ubi_io_read(ubi, p, pnum, ubi->vid_hdr_aloffset,
+	read_err = ubi_io_read(ubi, p, pnum, ubi->vid_hdr_aloffset,
 			  ubi->vid_hdr_alsize);
-	if (err) {
-		if (err != UBI_IO_BITFLIPS && err != -EBADMSG)
-			return err;
-
-		if (err == -EBADMSG)
-			read_err = UBI_IO_BAD_HDR_EBADMSG;
-	}
+	if (read_err && read_err != UBI_IO_BITFLIPS && read_err != -EBADMSG)
+		return read_err;
 
 	magic = be32_to_cpu(vid_hdr->magic);
 	if (magic != UBI_VID_HDR_MAGIC) {
-		if (read_err)
-			return read_err;
+		if (read_err == -EBADMSG)
+			return UBI_IO_BAD_HDR_EBADMSG;
 
 		if (check_pattern(vid_hdr, 0xFF, UBI_VID_HDR_SIZE)) {
 			if (verbose)
@@ -1016,7 +1016,10 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 			else if (UBI_IO_DEBUG)
 				dbg_msg("no VID header found at PEB %d, "
 					"only 0xFF bytes", pnum);
-			return UBI_IO_FF;
+			if (!read_err)
+				return UBI_IO_FF;
+			else
+				return UBI_IO_FF_BITFLIPS;
 		}
 
 		if (verbose) {
@@ -1040,7 +1043,10 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum,
 		} else if (UBI_IO_DEBUG)
 			dbg_msg("bad CRC at PEB %d, calculated %#08x, "
 				"read %#08x", pnum, crc, hdr_crc);
-		return read_err ?: UBI_IO_BAD_HDR;
+		if (!read_err)
+			return UBI_IO_BAD_HDR;
+		else
+			return UBI_IO_BAD_HDR_EBADMSG;
 	}
 
 	err = validate_vid_hdr(ubi, vid_hdr);
diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 37cb18f..6f90807 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -748,7 +748,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		return err;
 	else if (err == UBI_IO_BITFLIPS)
 		bitflips = 1;
-	else if (err == UBI_IO_FF)
+	else if (err == UBI_IO_FF || err == UBI_IO_FF_BITFLIPS)
 		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, &si->erase);
 	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR) {
 		/*
@@ -817,7 +817,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 	else if (err == UBI_IO_BITFLIPS)
 		bitflips = 1;
 	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR ||
-		 (err == UBI_IO_FF && ec_corr)) {
+		 (err == UBI_IO_FF && ec_corr) || err == UBI_IO_FF_BITFLIPS) {
 		/* VID header is corrupted */
 		if (err == UBI_IO_BAD_HDR_EBADMSG ||
 		    ec_corr == UBI_IO_BAD_HDR_EBADMSG)
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 774bdca..1099077 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -86,17 +86,25 @@
  * Error codes returned by the I/O sub-system.
  *
  * UBI_IO_FF: the read region of flash contains only 0xFFs
+ * UBI_IO_FF_BITFLIPS: the same as %UBI_IO_FF, but also also there was a data
+ *                     integrity error reported by the MTD driver
+ *                     (uncorrectable ECC error in case of NAND)
  * UBI_IO_BAD_HDR: the EC or VID header is corrupted (bad magic or CRC)
  * UBI_IO_BAD_HDR_EBADMSG: the same as %UBI_IO_BAD_HDR, but also there was a
  *                         data integrity error reported by the MTD driver
  *                         (uncorrectable ECC error in case of NAND)
  * UBI_IO_BITFLIPS: bit-flips were detected and corrected
+ *
+ * Note, it is probably better to have bit-flip and ebadmsg as flags which can
+ * be or'ed with other error code. But this is a big change because there are
+ * may callers, so it does not worth the risk of introducing a bug
  */
 enum {
 	UBI_IO_FF = 1,
+	UBI_IO_FF_BITFLIPS,
 	UBI_IO_BAD_HDR,
 	UBI_IO_BAD_HDR_EBADMSG,
-	UBI_IO_BITFLIPS
+	UBI_IO_BITFLIPS,
 };
 
 /*
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c
index a9e7c9e..605ecb1 100644
--- a/drivers/mtd/ubi/wl.c
+++ b/drivers/mtd/ubi/wl.c
@@ -759,6 +759,16 @@ static int wear_leveling_worker(struct ubi_device *ubi, struct ubi_work *wrk,
 			dbg_wl("PEB %d has no VID header", e1->pnum);
 			protect = 1;
 			goto out_not_moved;
+		} else if (err == UBI_IO_FF_BITFLIPS) {
+			/*
+			 * The same situation as %UBI_IO_FF, but bit-flips were
+			 * detected. It is better to schedule this PEB for
+			 * scrubbing.
+			 */
+			dbg_wl("PEB %d has no VID header but has bit-flips",
+			       e1->pnum);
+			scrubbing = 1;
+			goto out_not_moved;
 		}
 
 		ubi_err("error %d while reading VID header from PEB %d",
-- 
1.7.1.1

[PATCH 06/13] UBI: rename a local variable

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Rename local variable 'ec_corr' into 'ec_err' to make the code a little bit
more readable. 'ec_err' is more appropriate because it sounds more like 'error
when EC was read' and it looks more logical because we use it together with
'err'. Just a minor nicification which should improve the rather complex
scanning code.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/scan.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 6f90807..a15e9bc 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -725,7 +725,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		      int pnum)
 {
 	long long uninitialized_var(ec);
-	int err, bitflips = 0, vol_id, ec_corr = 0;
+	int err, bitflips = 0, vol_id, ec_err = 0;
 
 	dbg_bld("scan PEB %d", pnum);
 
@@ -756,12 +756,12 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		 * corrupted. Set %bitflips flag in order to make this PEB be
 		 * moved and EC be re-created.
 		 */
-		ec_corr = err;
+		ec_err = err;
 		ec = UBI_SCAN_UNKNOWN_EC;
 		bitflips = 1;
 	}
 
-	if (!ec_corr) {
+	if (!ec_err) {
 		int image_seq;
 
 		/* Make sure UBI version is OK */
@@ -817,10 +817,10 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 	else if (err == UBI_IO_BITFLIPS)
 		bitflips = 1;
 	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR ||
-		 (err == UBI_IO_FF && ec_corr) || err == UBI_IO_FF_BITFLIPS) {
+		 (err == UBI_IO_FF && ec_err) || err == UBI_IO_FF_BITFLIPS) {
 		/* VID header is corrupted */
 		if (err == UBI_IO_BAD_HDR_EBADMSG ||
-		    ec_corr == UBI_IO_BAD_HDR_EBADMSG)
+		    ec_err == UBI_IO_BAD_HDR_EBADMSG)
 			si->read_err_count += 1;
 		err = add_to_list(si, pnum, ec, &si->corr);
 		if (err)
@@ -870,7 +870,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		}
 	}
 
-	if (ec_corr)
+	if (ec_err)
 		ubi_warn("valid VID header but corrupted EC header at PEB %d",
 			 pnum);
 	err = ubi_scan_add_used(ubi, si, pnum, ec, vidh, bitflips);
@@ -878,7 +878,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		return err;
 
 adjust_mean_ec:
-	if (!ec_corr) {
+	if (!ec_err) {
 		si->ec_sum += ec;
 		si->ec_count += 1;
 		if (ec > si->max_ec)
-- 
1.7.1.1

[PATCH 07/13] UBI: change cascade of ifs to switch statements

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

This patch improves readability and simplifies scanning code by changing a
long cascade of 'if' statements to a switch statement. This should presumably
be a little faster as well.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/scan.c |   45 ++++++++++++++++++++++++++++++++-------------
 1 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index a15e9bc..9405e24 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -746,11 +746,18 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 	err = ubi_io_read_ec_hdr(ubi, pnum, ech, 0);
 	if (err < 0)
 		return err;
-	else if (err == UBI_IO_BITFLIPS)
+	switch (err) {
+	case 0:
+		break;
+	case UBI_IO_BITFLIPS:
 		bitflips = 1;
-	else if (err == UBI_IO_FF || err == UBI_IO_FF_BITFLIPS)
+		break;
+	case UBI_IO_FF:
+	case UBI_IO_FF_BITFLIPS:
 		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, &si->erase);
-	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR) {
+	case UBI_IO_BAD_HDR_EBADMSG:
+		si->read_err_count += 1;
+	case UBI_IO_BAD_HDR:
 		/*
 		 * We have to also look at the VID header, possibly it is not
 		 * corrupted. Set %bitflips flag in order to make this PEB be
@@ -759,6 +766,10 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		ec_err = err;
 		ec = UBI_SCAN_UNKNOWN_EC;
 		bitflips = 1;
+		break;
+	default:
+		ubi_err("'ubi_io_read_ec_hdr()' returned unknown code %d", err);
+		return -EINVAL;
 	}
 
 	if (!ec_err) {
@@ -814,24 +825,32 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 	err = ubi_io_read_vid_hdr(ubi, pnum, vidh, 0);
 	if (err < 0)
 		return err;
-	else if (err == UBI_IO_BITFLIPS)
+	switch (err) {
+	case 0:
+		break;
+	case UBI_IO_BITFLIPS:
 		bitflips = 1;
-	else if (err == UBI_IO_BAD_HDR_EBADMSG || err == UBI_IO_BAD_HDR ||
-		 (err == UBI_IO_FF && ec_err) || err == UBI_IO_FF_BITFLIPS) {
-		/* VID header is corrupted */
-		if (err == UBI_IO_BAD_HDR_EBADMSG ||
-		    ec_err == UBI_IO_BAD_HDR_EBADMSG)
-			si->read_err_count += 1;
+		break;
+	case UBI_IO_BAD_HDR_EBADMSG:
+		si->read_err_count += 1;
+	case UBI_IO_BAD_HDR:
+	case UBI_IO_FF_BITFLIPS:
 		err = add_to_list(si, pnum, ec, &si->corr);
 		if (err)
 			return err;
 		goto adjust_mean_ec;
-	} else if (err == UBI_IO_FF) {
-		/* No VID header - the physical eraseblock is free */
-		err = add_to_list(si, pnum, ec, &si->free);
+	case UBI_IO_FF:
+		if (ec_err)
+			err = add_to_list(si, pnum, ec, &si->corr);
+		else
+			err = add_to_list(si, pnum, ec, &si->free);
 		if (err)
 			return err;
 		goto adjust_mean_ec;
+	default:
+		ubi_err("'ubi_io_read_vid_hdr()' returned unknown code %d",
+			err);
+		return -EINVAL;
 	}
 
 	vol_id = be32_to_cpu(vidh->vol_id);
-- 
1.7.1.1

[PATCH 08/13] UBI: separate out corrupted list

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

This patch introduces 'add_corrupted()' function and separates out 'corr' list
manipulation from the common 'add_to_list()' function. This is just a
preparation for further changes - this patch does not change functionality.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/scan.c |   45 ++++++++++++++++++++++++++++++++++-----------
 1 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 9405e24..fba3dc6 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -64,9 +64,9 @@ static struct ubi_vid_hdr *vidh;
  * @ec: erase counter of the physical eraseblock
  * @list: the list to add to
  *
- * This function adds physical eraseblock @pnum to free, erase, corrupted or
- * alien lists. Returns zero in case of success and a negative error code in
- * case of failure.
+ * This function adds physical eraseblock @pnum to free, erase, or alien lists.
+ * Returns zero in case of success and a negative error code in case of
+ * failure.
  */
 static int add_to_list(struct ubi_scan_info *si, int pnum, int ec,
 		       struct list_head *list)
@@ -79,9 +79,6 @@ static int add_to_list(struct ubi_scan_info *si, int pnum, int ec,
 	} else if (list == &si->erase) {
 		dbg_bld("add to erase: PEB %d, EC %d", pnum, ec);
 		si->erase_peb_count += 1;
-	} else if (list == &si->corr) {
-		dbg_bld("add to corrupted: PEB %d, EC %d", pnum, ec);
-		si->corr_peb_count += 1;
 	} else if (list == &si->alien) {
 		dbg_bld("add to alien: PEB %d, EC %d", pnum, ec);
 		si->alien_peb_count += 1;
@@ -99,6 +96,33 @@ static int add_to_list(struct ubi_scan_info *si, int pnum, int ec,
 }
 
 /**
+ * add_corrupted - add a corrupted physical eraseblock.
+ * @si: scanning information
+ * @pnum: physical eraseblock number to add
+ * @ec: erase counter of the physical eraseblock
+ *
+ * This function adds corrupted physical eraseblock @pnum to the 'corr' list.
+ * Returns zero in case of success and a negative error code in case of
+ * failure.
+ */
+static int add_corrupted(struct ubi_scan_info *si, int pnum, int ec)
+{
+	struct ubi_scan_leb *seb;
+
+	dbg_bld("add to corrupted: PEB %d, EC %d", pnum, ec);
+
+	seb = kmalloc(sizeof(struct ubi_scan_leb), GFP_KERNEL);
+	if (!seb)
+		return -ENOMEM;
+
+	si->corr_peb_count += 1;
+	seb->pnum = pnum;
+	seb->ec = ec;
+	list_add(&seb->u.list, &si->corr);
+	return 0;
+}
+
+/**
  * validate_vid_hdr - check volume identifier header.
  * @vid_hdr: the volume identifier header to check
  * @sv: information about the volume this logical eraseblock belongs to
@@ -464,8 +488,7 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 				return err;
 
 			if (cmp_res & 4)
-				err = add_to_list(si, seb->pnum, seb->ec,
-						  &si->corr);
+				err = add_corrupted(si, seb->pnum, seb->ec);
 			else
 				err = add_to_list(si, seb->pnum, seb->ec,
 						  &si->erase);
@@ -488,7 +511,7 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 			 * previously.
 			 */
 			if (cmp_res & 4)
-				return add_to_list(si, pnum, ec, &si->corr);
+				return add_corrupted(si, pnum, ec);
 			else
 				return add_to_list(si, pnum, ec, &si->erase);
 		}
@@ -835,13 +858,13 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		si->read_err_count += 1;
 	case UBI_IO_BAD_HDR:
 	case UBI_IO_FF_BITFLIPS:
-		err = add_to_list(si, pnum, ec, &si->corr);
+		err = add_corrupted(si, pnum, ec);
 		if (err)
 			return err;
 		goto adjust_mean_ec;
 	case UBI_IO_FF:
 		if (ec_err)
-			err = add_to_list(si, pnum, ec, &si->corr);
+			err = add_corrupted(si, pnum, ec);
 		else
 			err = add_to_list(si, pnum, ec, &si->free);
 		if (err)
-- 
1.7.1.1

[PATCH 09/13] UBI: do not put eraseblocks to the corrupted list unnecessarily

[Artem Bityutskiy]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Currently UBI maintains 2 lists of PEBs during scanning:
1. 'erase' list - PEBs which have no corruptions but should be erased
2. 'corr' list - PEBs which have some corruptions and should be erased

But we do not really need 2 lists for PEBs which should be erased after
scanning is done - this is redundant. So this patch makes sure all PEBs
which are corrupted are moved to the head of the 'erase' list. We add
them to the head to make sure they are erased first and we get rid of
corruption ASAP.

However, we do not remove the 'corr' list and realted functions, because
the plan is to use this list for other purposes. Namely, we plan to
put eraseblocks with corruption which does not look like it was caused
by unclean power cut. Then we'll preserve thes PEBs in order to avoid
killing potentially valuable user data.

This patch also amends PEBs accounting, because it was closely tight to
the 'erase'/'corr' lists separation.

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
---
 drivers/mtd/ubi/scan.c |  141 +++++++++++++++++++++++++++---------------------
 drivers/mtd/ubi/scan.h |   15 ++---
 drivers/mtd/ubi/vtbl.c |    2 +-
 3 files changed, 87 insertions(+), 71 deletions(-)

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index fba3dc6..d5666a0 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -29,7 +29,7 @@
  * objects which are kept in volume RB-tree with root at the @volumes field.
  * The RB-tree is indexed by the volume ID.
  *
- * Found logical eraseblocks are represented by &struct ubi_scan_leb objects.
+ * Scanned logical eraseblocks are represented by &struct ubi_scan_leb objects.
  * These objects are kept in per-volume RB-trees with the root at the
  * corresponding &struct ubi_scan_volume object. To put it differently, we keep
  * an RB-tree of per-volume objects and each of these objects is the root of
@@ -38,6 +38,21 @@
  * Corrupted physical eraseblocks are put to the @corr list, free physical
  * eraseblocks are put to the @free list and the physical eraseblock to be
  * erased are put to the @erase list.
+ *
+ * UBI tries to distinguish between 2 types of corruptions.
+ * 1. Corruptions caused by power cuts. These are harmless and expected
+ *    corruptions and UBI tries to handle them gracefully, without printing too
+ *    many warnings and error messages. The idea is that we do not lose
+ *    important data in these case - we may lose only the data which was being
+ *    written to the media just before the power cut happened, and the upper
+ *    layers are supposed to handle these situations. UBI puts these PEBs to
+ *    the head of the @erase list and they are scheduled for erasure.
+ *
+ * 2. Unexpected corruptions which are not caused by power cuts. During
+ *    scanning, such PEBs are put to the @corr list and UBI preserves them.
+ *    Obviously, this lessens the amount of available PEBs, and if at some
+ *    point UBI runs out of free PEBs, it switches to R/O mode. UBI also loudly
+ *    informs about such PEBs every time the MTD device is attached.
  */
 
 #include <linux/err.h>
@@ -62,23 +77,26 @@ static struct ubi_vid_hdr *vidh;
  * @si: scanning information
  * @pnum: physical eraseblock number to add
  * @ec: erase counter of the physical eraseblock
+ * @to_head: if not zero, add to the head of the list
  * @list: the list to add to
  *
  * This function adds physical eraseblock @pnum to free, erase, or alien lists.
- * Returns zero in case of success and a negative error code in case of
+ * If @to_head is not zero, PEB will be added to the head of the list, which
+ * basically means it will be processed first later. E.g., we add corrupted
+ * PEBs (corrupted due to power cuts) to the head of the erase list to make
+ * sure we erase them first and get rid of corruptions ASAP. This function
+ * returns zero in case of success and a negative error code in case of
  * failure.
  */
-static int add_to_list(struct ubi_scan_info *si, int pnum, int ec,
+static int add_to_list(struct ubi_scan_info *si, int pnum, int ec, int to_head,
 		       struct list_head *list)
 {
 	struct ubi_scan_leb *seb;
 
 	if (list == &si->free) {
 		dbg_bld("add to free: PEB %d, EC %d", pnum, ec);
-		si->free_peb_count += 1;
 	} else if (list == &si->erase) {
 		dbg_bld("add to erase: PEB %d, EC %d", pnum, ec);
-		si->erase_peb_count += 1;
 	} else if (list == &si->alien) {
 		dbg_bld("add to alien: PEB %d, EC %d", pnum, ec);
 		si->alien_peb_count += 1;
@@ -91,7 +109,10 @@ static int add_to_list(struct ubi_scan_info *si, int pnum, int ec,
 
 	seb->pnum = pnum;
 	seb->ec = ec;
-	list_add_tail(&seb->u.list, list);
+	if (to_head)
+		list_add(&seb->u.list, list);
+	else
+		list_add_tail(&seb->u.list, list);
 	return 0;
 }
 
@@ -282,8 +303,8 @@ static int compare_lebs(struct ubi_device *ubi, const struct ubi_scan_leb *seb,
 		 * created before sequence numbers support has been added. At
 		 * that times we used 32-bit LEB versions stored in logical
 		 * eraseblocks. That was before UBI got into mainline. We do not
-		 * support these images anymore. Well, those images will work
-		 * still work, but only if no unclean reboots happened.
+		 * support these images anymore. Well, those images still work,
+		 * but only if no unclean reboots happened.
 		 */
 		ubi_err("unsupported on-flash UBI format\n");
 		return -EINVAL;
@@ -321,7 +342,7 @@ static int compare_lebs(struct ubi_device *ubi, const struct ubi_scan_leb *seb,
 				bitflips = 1;
 			else {
 				dbg_err("VID of PEB %d header is bad, but it "
-					"was OK earlier", pnum);
+					"was OK earlier, err %d", pnum, err);
 				if (err > 0)
 					err = -EIO;
 
@@ -487,11 +508,8 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 			if (err)
 				return err;
 
-			if (cmp_res & 4)
-				err = add_corrupted(si, seb->pnum, seb->ec);
-			else
-				err = add_to_list(si, seb->pnum, seb->ec,
-						  &si->erase);
+			err = add_to_list(si, seb->pnum, seb->ec, cmp_res & 4,
+					  &si->erase);
 			if (err)
 				return err;
 
@@ -510,10 +528,8 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 			 * This logical eraseblock is older than the one found
 			 * previously.
 			 */
-			if (cmp_res & 4)
-				return add_corrupted(si, pnum, ec);
-			else
-				return add_to_list(si, pnum, ec, &si->erase);
+			return add_to_list(si, pnum, ec, cmp_res & 4,
+					   &si->erase);
 		}
 	}
 
@@ -544,7 +560,6 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 	sv->leb_count += 1;
 	rb_link_node(&seb->u.rb, parent, p);
 	rb_insert_color(&seb->u.rb, &sv->root);
-	si->used_peb_count += 1;
 	return 0;
 }
 
@@ -776,10 +791,14 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		bitflips = 1;
 		break;
 	case UBI_IO_FF:
+		si->empty_peb_count += 1;
+		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, 0,
+				   &si->erase);
 	case UBI_IO_FF_BITFLIPS:
-		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, &si->erase);
+		si->empty_peb_count += 1;
+		return add_to_list(si, pnum, UBI_SCAN_UNKNOWN_EC, 1,
+				   &si->erase);
 	case UBI_IO_BAD_HDR_EBADMSG:
-		si->read_err_count += 1;
 	case UBI_IO_BAD_HDR:
 		/*
 		 * We have to also look at the VID header, possibly it is not
@@ -855,18 +874,23 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		bitflips = 1;
 		break;
 	case UBI_IO_BAD_HDR_EBADMSG:
-		si->read_err_count += 1;
+		if (ec_err == UBI_IO_BAD_HDR_EBADMSG)
+			/* Both EC and VID headers were read with data
+			 * integrity error, probably this is a bad PEB, bit it
+			 * is not marked a bad yet.
+			 */
+			si->maybe_bad_peb_count += 1;
 	case UBI_IO_BAD_HDR:
 	case UBI_IO_FF_BITFLIPS:
-		err = add_corrupted(si, pnum, ec);
+		err = add_to_list(si, pnum, ec, 1, &si->erase);
 		if (err)
 			return err;
 		goto adjust_mean_ec;
 	case UBI_IO_FF:
 		if (ec_err)
-			err = add_corrupted(si, pnum, ec);
+			err = add_to_list(si, pnum, ec, 1, &si->erase);
 		else
-			err = add_to_list(si, pnum, ec, &si->free);
+			err = add_to_list(si, pnum, ec, 0, &si->free);
 		if (err)
 			return err;
 		goto adjust_mean_ec;
@@ -885,7 +909,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		case UBI_COMPAT_DELETE:
 			ubi_msg("\"delete\" compatible internal volume %d:%d"
 				" found, will remove it", vol_id, lnum);
-			err = add_to_list(si, pnum, ec, &si->erase);
+			err = add_to_list(si, pnum, ec, 1, &si->erase);
 			if (err)
 				return err;
 			return 0;
@@ -900,7 +924,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 		case UBI_COMPAT_PRESERVE:
 			ubi_msg("\"preserve\" compatible internal volume %d:%d"
 				" found", vol_id, lnum);
-			err = add_to_list(si, pnum, ec, &si->alien);
+			err = add_to_list(si, pnum, ec, 0, &si->alien);
 			if (err)
 				return err;
 			return 0;
@@ -946,19 +970,20 @@ adjust_mean_ec:
 static int check_what_we_have(struct ubi_device *ubi, struct ubi_scan_info *si)
 {
 	struct ubi_scan_leb *seb;
-	int max_corr;
+	int max_corr, peb_count;
 
-	max_corr = ubi->peb_count - si->bad_peb_count - si->alien_peb_count;
-	max_corr = max_corr / 20 ?: 8;
+	peb_count = ubi->peb_count - si->bad_peb_count - si->alien_peb_count;
+	max_corr = peb_count / 20 ?: 8;
 
 	/*
 	 * Few corrupted PEBs are not a problem and may be just a result of
 	 * unclean reboots. However, many of them may indicate some problems
 	 * with the flash HW or driver.
 	 */
-	if (si->corr_peb_count >= 8) {
-		ubi_warn("%d PEBs are corrupted", si->corr_peb_count);
-		printk(KERN_WARNING "corrupted PEBs are:");
+	if (si->corr_peb_count) {
+		ubi_err("%d PEBs are corrupted and preserved",
+			si->corr_peb_count);
+		printk(KERN_ERR "Corrupted PEBs are:");
 		list_for_each_entry(seb, &si->corr, u.list)
 			printk(KERN_CONT " %d", seb->pnum);
 		printk(KERN_CONT "\n");
@@ -973,41 +998,35 @@ static int check_what_we_have(struct ubi_device *ubi, struct ubi_scan_info *si)
 		}
 	}
 
-	if (si->free_peb_count + si->used_peb_count +
-	    si->alien_peb_count == 0) {
-		/* No UBI-formatted eraseblocks were found */
-		if (si->corr_peb_count == si->read_err_count &&
-		    si->corr_peb_count < 8) {
-			/* No or just few corrupted PEBs, and all of them had a
-			 * read error. We assume that those are bad PEBs, which
-			 * were just not marked as bad so far.
-			 *
-			 * This piece of code basically tries to distinguish
-			 * between the following 2 situations:
-			 *
-			 * 1. Flash is empty, but there are few bad PEBs, which
-			 *    are not marked as bad so far, and which were read
-			 *    with error. We want to go ahead and format this
-			 *    flash. While formating, the faulty PEBs will
-			 *    probably be marked as bad.
-			 *
-			 * 2. Flash probably contains non-UBI data and we do
-			 * not want to format it and destroy possibly needed
-			 * data (e.g., consider the case when the bootloader
-			 * MTD partition was accidentally fed to UBI).
-			 */
+	if (si->empty_peb_count + si->maybe_bad_peb_count == peb_count) {
+		/*
+		 * All PEBs are empty, or almost all - a couple PEBs look like
+		 * they may be bad PEB which were not marked as bad yet.
+		 *
+		 * This piece of code basically tries to distinguish  between
+		 * the following 2 situations:
+		 *
+		 * 1. Flash is empty, but there are few bad PEBs, which are not
+		 *    marked as bad so far, and which were read with error. We
+		 *    want to go ahead and format this flash. While formating,
+		 *    the faulty PEBs will probably be marked as bad.
+		 *
+		 * 2. Flash contains non-UBI data and we do not want to format
+		 *    it and destroy possibly important information.
+		 */
+		if (si->maybe_bad_peb_count <= 2) {
 			si->is_empty = 1;
 			ubi_msg("empty MTD device detected");
-			get_random_bytes(&ubi->image_seq, sizeof(ubi->image_seq));
+			get_random_bytes(&ubi->image_seq,
+					 sizeof(ubi->image_seq));
 		} else {
-			ubi_err("MTD device possibly contains non-UBI data, "
-				"refusing it");
+			ubi_err("MTD device is not UBI-formatted and possibly "
+				"contains non-UBI data - refusing it");
 			return -EINVAL;
 		}
+
 	}
 
-	if (si->corr_peb_count > 0)
-		ubi_msg("corrupted PEBs will be formatted");
 	return 0;
 }
 
diff --git a/drivers/mtd/ubi/scan.h b/drivers/mtd/ubi/scan.h
index 0876649..12ac852 100644
--- a/drivers/mtd/ubi/scan.h
+++ b/drivers/mtd/ubi/scan.h
@@ -91,14 +91,13 @@ struct ubi_scan_volume {
  * @erase: list of physical eraseblocks which have to be erased
  * @alien: list of physical eraseblocks which should not be used by UBI (e.g.,
  *         those belonging to "preserve"-compatible internal volumes)
- * @used_peb_count: count of used PEBs
  * @corr_peb_count: count of PEBs in the @corr list
- * @read_err_count: count of PEBs read with error (%UBI_IO_BAD_HDR_EBADMSG was
- *                  returned)
- * @free_peb_count: count of PEBs in the @free list
- * @erase_peb_count: count of PEBs in the @erase list
+ * @empty_peb_count: count of PEBs which are presumably empty (contain only
+ *                   0xFF bytes)
  * @alien_peb_count: count of PEBs in the @alien list
  * @bad_peb_count: count of bad physical eraseblocks
+ * @maybe_bad_peb_count: count of bad physical eraseblocks which are not marked
+ *                       as bad yet, but which look like bad
  * @vols_found: number of volumes found during scanning
  * @highest_vol_id: highest volume ID
  * @is_empty: flag indicating whether the MTD device is empty or not
@@ -119,13 +118,11 @@ struct ubi_scan_info {
 	struct list_head free;
 	struct list_head erase;
 	struct list_head alien;
-	int used_peb_count;
 	int corr_peb_count;
-	int read_err_count;
-	int free_peb_count;
-	int erase_peb_count;
+	int empty_peb_count;
 	int alien_peb_count;
 	int bad_peb_count;
+	int maybe_bad_peb_count;
 	int vols_found;
 	int highest_vol_id;
 	int is_empty;
diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
index 14c10be..3bfe00a 100644
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -366,7 +366,7 @@ write_error:
 		 * Probably this physical eraseblock went bad, try to pick
 		 * another one.
 		 */
-		list_add_tail(&new_seb->u.list, &si->corr);
+		list_add(&new_seb->u.list, &si->erase);
 		goto retry;
 	}
 	kfree(new_seb);
-- 
1.7.1.1

Re: [PATCH 00/13] UBI reliability improvements

[Artem Bityutskiy]

On Mon, 2010-09-06 at 11:54 +0300, Artem Bityutskiy wrote:
> Hi,
> 
> here is a set of patches which should improve UBI reliability. I wrote them
> during my trip to LinuxCon Brazil.
> 
> Patches 1-5 should be interesting for Matthieu CASTET - they improve -EUCLEAN
> errors handling a little.
> 
> Patches 6-13 should be interesting for Matthew L. Creech - they presimably have
> to help in catching why in his setup UBIFS refers unmapped LEB. I'm not sure
> they will help, but there is a chance - my theory is that UBI decides that the
> LEB is corrupted and wipes it. With this patch-set UBI should instead preserve
> corrupted LEBs, so we could take a look what happened.
> 
> Please, reveiw, test. I tested them a little as well, but not extensively.

OK, although no one reviewed / tested this so far, I'm pushing this to
the ubi-2.6.git / master tree.

-- 
Best Regards,
Artem Bityutskiy (Битюцкий Артём)