[PATCH 1/3] UBI: remember copy_flag while scanning

[PATCH 1/3] UBI: remember copy_flag while scanning

[Artem Bityutskiy]

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

While scanning the flash we read all VID headers and store some important
information in 'struct ubi_scan_leb'. Store also the 'copy_flag' value there
as it is needed when comparing LEBs. We do not increase memory consumption
because this is just one bit and we have plenty of spare bits in
'struct ubi_scan_leb' (sizeof(struct ubi_scan_leb) is 48 both with and
without this patch).

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

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 30b7102..2fbb571 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -330,12 +330,18 @@ static int compare_lebs(struct ubi_device *ubi, const struct ubi_scan_leb *seb,
 			return 1;
 		}
 	} else {
-		pnum = seb->pnum;
+		if (!seb->copy_flag) {
+			/* It is not a copy, so it is newer */
+			dbg_bld("first PEB %d is newer, copy_flag is unset",
+				pnum);
+			return bitflips << 1;
+		}
 
 		vh = ubi_zalloc_vid_hdr(ubi, GFP_KERNEL);
 		if (!vh)
 			return -ENOMEM;
 
+		pnum = seb->pnum;
 		err = ubi_io_read_vid_hdr(ubi, pnum, vh, 0);
 		if (err) {
 			if (err == UBI_IO_BITFLIPS)
@@ -350,14 +356,6 @@ static int compare_lebs(struct ubi_device *ubi, const struct ubi_scan_leb *seb,
 			}
 		}
 
-		if (!vh->copy_flag) {
-			/* It is not a copy, so it is newer */
-			dbg_bld("first PEB %d is newer, copy_flag is unset",
-				pnum);
-			err = bitflips << 1;
-			goto out_free_vidh;
-		}
-
 		vid_hdr = vh;
 	}
 
@@ -516,6 +514,7 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 			seb->ec = ec;
 			seb->pnum = pnum;
 			seb->scrub = ((cmp_res & 2) || bitflips);
+			seb->copy_flag = vid_hdr->copy_flag;
 			seb->sqnum = sqnum;
 
 			if (sv->highest_lnum == lnum)
@@ -549,8 +548,9 @@ int ubi_scan_add_used(struct ubi_device *ubi, struct ubi_scan_info *si,
 	seb->ec = ec;
 	seb->pnum = pnum;
 	seb->lnum = lnum;
-	seb->sqnum = sqnum;
 	seb->scrub = bitflips;
+	seb->copy_flag = vid_hdr->copy_flag;
+	seb->sqnum = sqnum;
 
 	if (sv->highest_lnum <= lnum) {
 		sv->highest_lnum = lnum;
diff --git a/drivers/mtd/ubi/scan.h b/drivers/mtd/ubi/scan.h
index 12ac852..a3264f0 100644
--- a/drivers/mtd/ubi/scan.h
+++ b/drivers/mtd/ubi/scan.h
@@ -30,6 +30,7 @@
  * @pnum: physical eraseblock number
  * @lnum: logical eraseblock number
  * @scrub: if this physical eraseblock needs scrubbing
+ * @copy_flag: this LEB is a copy (@copy_flag is set in VID header of this LEB)
  * @sqnum: sequence number
  * @u: unions RB-tree or @list links
  * @u.rb: link in the per-volume RB-tree of &struct ubi_scan_leb objects
@@ -42,7 +43,8 @@ struct ubi_scan_leb {
 	int ec;
 	int pnum;
 	int lnum;
-	int scrub;
+	unsigned int scrub:1;
+	unsigned int copy_flag:1;
 	unsigned long long sqnum;
 	union {
 		struct rb_node rb;
-- 
1.7.2.3

[PATCH 2/3] UBI: fix check_data_ff return code

[Artem Bityutskiy]

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

When the data does not contain all 0xFF bytes, 'check_data_ff()' should return
1, not -EINVAL; Also, the caller ('process_eb()') should not add the PEB to the
"corrupted" list if there was a read error.

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

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index 2fbb571..e7b800b 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -784,7 +784,7 @@ static int check_data_ff(struct ubi_device *ubi, struct ubi_vid_hdr *vid_hdr,
 	ubi_dbg_print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1,
 			       ubi->peb_buf1, ubi->leb_size, 1);
 	mutex_unlock(&ubi->buf_mutex);
-	return -EINVAL;
+	return 1;
 }
 
 /**
@@ -936,7 +936,10 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 			 * have to check what is in the data area.
 			 */
 			err = check_data_ff(ubi, vidh, pnum);
-		if (!err)
+
+		if (err < 0)
+			return err;
+		else if (!err)
 			/* This corruption is caused by a power cut */
 			err = add_to_list(si, pnum, ec, 1, &si->erase);
 		else
-- 
1.7.2.3

[PATCH 3/3] UBI: tighten the corrupted PEB criteria

[Artem Bityutskiy]

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

If we get a bit-flip of ECC error while reading the data area, do not add it to
corrupted list, because it is possible that this is just unstable PEB with
corruptions caused by unclean reboots.

This patch also improves commentaries.

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

diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c
index e7b800b..c1191b7 100644
--- a/drivers/mtd/ubi/scan.c
+++ b/drivers/mtd/ubi/scan.c
@@ -45,14 +45,26 @@
  *    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.
+ *    layers (e.g., UBIFS) 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.
+ *
+ * However, it is difficult to reliably distinguish between these types of
+ * corruptions and UBI's strategy is as follows. UBI assumes (2.) if the VID
+ * header is corrupted and the data area does not contain all 0xFFs, and there
+ * were not bit-flips or integrity errors while reading the data area. Otherwise
+ * UBI assumes (1.). The assumptions are:
+ *   o if the data area contains only 0xFFs, there is no data, and it is safe
+ *     to just erase this PEB.
+ *   o if the data area has bit-flips and data integrity errors (ECC errors on
+ *     NAND), it is probably a PEB which was being erased when power cut
+ *     happened.
  */
 
 #include <linux/err.h>
@@ -741,23 +753,23 @@ struct ubi_scan_leb *ubi_scan_get_free_peb(struct ubi_device *ubi,
 }
 
 /**
- * check_data_ff - make sure PEB contains only 0xFF data.
+ * check_corruption - check the data area of PEB.
  * @ubi: UBI device description object
  * @vid_hrd: the (corrupted) VID header of this PEB
  * @pnum: the physical eraseblock number to check
  *
  * This is a helper function which is used to distinguish between VID header
  * corruptions caused by power cuts and other reasons. If the PEB contains only
- * 0xFF bytes at the data area, the VID header is most probably corrupted
+ * 0xFF bytes in the data area, the VID header is most probably corrupted
  * because of a power cut (%0 is returned in this case). Otherwise, it was
- * corrupted for some other reasons (%1 is returned in this case). A negative
- * error code is returned if a read error occurred.
+ * probably corrupted for some other reasons (%1 is returned in this case). A
+ * negative error code is returned if a read error occurred.
  *
  * If the corruption reason was a power cut, UBI can safely erase this PEB.
  * Otherwise, it should preserve it to avoid possibly destroying important
  * information.
  */
-static int check_data_ff(struct ubi_device *ubi, struct ubi_vid_hdr *vid_hdr,
+static int check_corruption(struct ubi_device *ubi, struct ubi_vid_hdr *vid_hdr,
 			 int pnum)
 {
 	int err;
@@ -767,7 +779,18 @@ static int check_data_ff(struct ubi_device *ubi, struct ubi_vid_hdr *vid_hdr,
 
 	err = ubi_io_read(ubi, ubi->peb_buf1, pnum, ubi->leb_start,
 			  ubi->leb_size);
-	if (err && err != UBI_IO_BITFLIPS && err != -EBADMSG)
+	if (err == UBI_IO_BITFLIPS || err == -EBADMSG) {
+		/*
+		 * Bit-flips or integrity errors while reading the data area.
+		 * It is difficult to say for sure what type of corruption is
+		 * this, but presumably a power cut happened while this PEB was
+		 * erased, so it became unstable and corrupted, and should be
+		 * erased.
+		 */
+		return 0;
+	}
+
+	if (err)
 		return err;
 
 	if (ubi_check_pattern(ubi->peb_buf1, 0xFF, ubi->leb_size)) {
@@ -926,7 +949,7 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si,
 			 * that this a valid UBI PEB which has corresponding
 			 * LEB, but the headers are corrupted. However, it is
 			 * impossible to distinguish it from a PEB which just
-			 * contains garbage because a power cut during erase
+			 * contains garbage because of a power cut during erase
 			 * operation. So we just schedule this PEB for erasure.
 			 */
 			err = 0;
-- 
1.7.2.3