From patchwork Mon Sep 6 08:54:49 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Artem Bityutskiy X-Patchwork-Id: 63902 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6DFE4B70EE for ; Mon, 6 Sep 2010 18:56:36 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1OsXTz-0000lz-0e; Mon, 06 Sep 2010 08:55:19 +0000 Received: from smtp.nokia.com ([192.100.122.233] helo=mgw-mx06.nokia.com) by bombadil.infradead.org with esmtps (Exim 4.72 #1 (Red Hat Linux)) id 1OsXTq-0000Va-8q for linux-mtd@lists.infradead.org; Mon, 06 Sep 2010 08:55:13 +0000 Received: from vaebh105.NOE.Nokia.com (vaebh105.europe.nokia.com [10.160.244.31]) by mgw-mx06.nokia.com (Switch-3.3.3/Switch-3.3.3) with ESMTP id o868t0xe029883; Mon, 6 Sep 2010 11:55:07 +0300 Received: from esebh102.NOE.Nokia.com ([172.21.138.183]) by vaebh105.NOE.Nokia.com with Microsoft SMTPSVC(6.0.3790.4675); Mon, 6 Sep 2010 11:55:05 +0300 Received: from mgw-da01.ext.nokia.com ([147.243.128.24]) by esebh102.NOE.Nokia.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Mon, 6 Sep 2010 11:55:04 +0300 Received: from eru.research.nokia.com (helruo-dhcp022138.ntc.nokia.com [172.21.22.138]) by mgw-da01.ext.nokia.com (Switch-3.3.3/Switch-3.3.3) with ESMTP id o868srqv018627; Mon, 6 Sep 2010 11:55:02 +0300 From: Artem Bityutskiy To: linux-mtd@lists.infradead.org Subject: [PATCH 05/13] UBI: handle bit-flips when no header found Date: Mon, 6 Sep 2010 11:54:49 +0300 Message-Id: <1283763293-1882-6-git-send-email-dedekind1@gmail.com> X-Mailer: git-send-email 1.7.1.1 In-Reply-To: <1283763293-1882-1-git-send-email-dedekind1@gmail.com> References: <1283763293-1882-1-git-send-email-dedekind1@gmail.com> X-OriginalArrivalTime: 06 Sep 2010 08:55:04.0927 (UTC) FILETIME=[32E716F0:01CB4DA1] X-Nokia-AV: Clean X-CRM114-Version: 20090807-BlameThorstenAndJenny ( TRE 0.7.6 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20100906_045512_682985_6F41D170 X-CRM114-Status: GOOD ( 25.17 ) X-Spam-Score: 1.1 (+) X-Spam-Report: SpamAssassin version 3.3.1 on bombadil.infradead.org summary: Content analysis details: (1.1 points) pts rule name description ---- ---------------------- -------------------------------------------------- -2.3 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [192.100.122.233 listed in list.dnswl.org] 0.0 FREEMAIL_FROM Sender email is freemail (dedekind1[at]gmail.com) 0.0 DKIM_ADSP_CUSTOM_MED No valid author signature, adsp_override is CUSTOM_MED 2.2 FREEMAIL_ENVFROM_END_DIGIT Envelope-from freemail username ends in digit (dedekind1[at]gmail.com) 1.2 NML_ADSP_CUSTOM_MED ADSP custom_med hit, and not from a mailing list 0.0 T_TO_NO_BRKTS_FREEMAIL T_TO_NO_BRKTS_FREEMAIL Cc: Matthieu CASTET , "Matthew L. Creech" X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org From: Artem Bityutskiy 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 --- 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",