From patchwork Mon Jul 6 06:43:55 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Artem Bityutskiy X-Patchwork-Id: 29486 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 bilbo.ozlabs.org (Postfix) with ESMTPS id 412DCB6F1F for ; Mon, 6 Jul 2009 16:49:06 +1000 (EST) Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1MNhwh-0000Op-NK; Mon, 06 Jul 2009 06:44:59 +0000 Received: from smtp.nokia.com ([192.100.122.230] helo=mgw-mx03.nokia.com) by bombadil.infradead.org with esmtps (Exim 4.69 #1 (Red Hat Linux)) id 1MNhwX-0000Of-Qd for linux-mtd@lists.infradead.org; Mon, 06 Jul 2009 06:44:57 +0000 Received: from esebh106.NOE.Nokia.com (esebh106.ntc.nokia.com [172.21.138.213]) by mgw-mx03.nokia.com (Switch-3.3.3/Switch-3.3.3) with ESMTP id n666iGRq014272; Mon, 6 Jul 2009 09:44:20 +0300 Received: from vaebh104.NOE.Nokia.com ([10.160.244.30]) by esebh106.NOE.Nokia.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 6 Jul 2009 09:43:59 +0300 Received: from mgw-sa01.ext.nokia.com ([147.243.1.47]) by vaebh104.NOE.Nokia.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.3959); Mon, 6 Jul 2009 09:43:58 +0300 Received: from [172.21.42.232] (esdhcp042232.research.nokia.com [172.21.42.232]) by mgw-sa01.ext.nokia.com (Switch-3.2.6/Switch-3.2.6) with ESMTP id n666hsiD012744; Mon, 6 Jul 2009 09:43:55 +0300 Subject: RE: UBIFS Corrupt during power failure From: Artem Bityutskiy To: Urs Muff In-Reply-To: <1246855913.20721.287.camel@localhost.localdomain> References: <1239979018.3390.298.camel@localhost.localdomain> <200905150916.54091.sr@denx.de> <1242721105.3623.0.camel@localhost.localdomain> <1246627562.20721.190.camel@localhost.localdomain> <1246627771.20721.191.camel@localhost.localdomain> <7207AAC68CE347458026863515A07DA102901F3C@usw-am-xch-02.am.trimblecorp.net> <1246629940.20721.219.camel@localhost.localdomain> <7207AAC68CE347458026863515A07DA102901F9C@usw-am-xch-02.am.trimblecorp.net> <1246633131.20721.224.camel@localhost.localdomain> <1246854654.20721.271.camel@localhost.localdomain> <1246855913.20721.287.camel@localhost.localdomain> Date: Mon, 06 Jul 2009 09:43:55 +0300 Message-Id: <1246862635.20721.291.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.24.5 (2.24.5-1.fc10) X-OriginalArrivalTime: 06 Jul 2009 06:43:58.0682 (UTC) FILETIME=[23DE1BA0:01C9FE05] X-Nokia-AV: Clean X-Spam-Score: -4.0 (----) X-Spam-Report: SpamAssassin version 3.2.5 on bombadil.infradead.org summary: Content analysis details: (-4.0 points) pts rule name description ---- ---------------------- -------------------------------------------------- -4.0 RCVD_IN_DNSWL_MED RBL: Sender listed at http://www.dnswl.org/, medium trust [192.100.122.230 listed in list.dnswl.org] Cc: Nicolas Pitre , Stefan Roese , linux-mtd@lists.infradead.org, Eric Holmberg , Adrian Hunter X-BeenThere: linux-mtd@lists.infradead.org X-Mailman-Version: 2.1.11 Precedence: list Reply-To: dedekind@infradead.org List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-mtd-bounces@lists.infradead.org Errors-To: linux-mtd-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org On Mon, 2009-07-06 at 07:51 +0300, Artem Bityutskiy wrote: > > [CCed Nicolas Pitre] > > > > OK, I've written a small user-space program which first fills the NOR > > flash with an '0x89ABCDEF' pattern, then starts erasing it, and then > > I cut the power at random point. > > > > And unfortunately the power cut results in eraseblocks which have > > 0x89ABCDEF at the beginning, and all zeroes at the end. I've attached > > one example. > > > > So it indeed looks like NOR erasure includes writing zeroes from the > > end. Unfortunately UBI/UBIFS cannot handle this correctly ATM. > > Although I can easily fix this by writing few zeroes at the beginning of > the eraseblock _before) erasing it, so that UBI will be happy. But it is > still interesting whether I may just ask NOR to amend it's embedded > erase algorithm. This patch seems to fix the UBI issues Eric and me observed (issue N2). I'll test the patch for few days, but I couldn't see any problem so far. From: Artem Bityutskiy Subject: [PATCH] UBI: fix NOR flash recovery This commit fixes NOR flash recovery issues observed with Spansion S29GL512N NOR. When NOR erases, it first fills PEBs with zeroes, then sets all bytes to 0xFF. Filling with zeroes starts from the end of the PEB. And when power is cut, this results in PEBs containing correct EC and VID headers but corrupted with zeros at the end. This confuses UBI and it mistakinly accepts these PEBs and associate them with LEBs. Fis this issue by zeroing EC and VID magics before erasing PEBs, to make UBI later refuse zem. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/build.c | 5 ++++ drivers/mtd/ubi/io.c | 58 +++++++++++++++++++++++++++++++++++++++++++++- drivers/mtd/ubi/ubi.h | 4 ++- 3 files changed, 64 insertions(+), 3 deletions(-) diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c index 286ed59..4dd4f73 100644 --- a/drivers/mtd/ubi/build.c +++ b/drivers/mtd/ubi/build.c @@ -657,6 +657,11 @@ static int io_init(struct ubi_device *ubi) if (ubi->mtd->block_isbad && ubi->mtd->block_markbad) ubi->bad_allowed = 1; + if (ubi->mtd->type == MTD_NORFLASH) { + ubi_assert(ubi->mtd->writesize == 1); + ubi->nor_flash = 1; + } + ubi->min_io_size = ubi->mtd->writesize; ubi->hdrs_min_io_size = ubi->mtd->writesize >> ubi->mtd->subpage_sft; diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index c8edbfd..275e533 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -266,8 +266,8 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset, addr = (loff_t)pnum * ubi->peb_size + offset; err = ubi->mtd->write(ubi->mtd, addr, len, &written, buf); if (err) { - ubi_err("error %d while writing %d bytes to PEB %d:%d, written" - " %zd bytes", err, len, pnum, offset, written); + ubi_err("error %d while writing %d bytes to PEB %d:%d, written " + "%zd bytes", err, len, pnum, offset, written); ubi_dbg_dump_stack(); } else ubi_assert(written == len); @@ -454,6 +454,54 @@ out: } /** + * nor_erase_prepare - prepare a NOR flash PEB for erasure. + * @ubi: UBI device description object + * @pnum: physical eraseblock number to prepare + * + * NOR flash, or at least some of them, have peculiar embedded PEB erasure + * algorithm: the PEB is first filled with zeroes, then it is erased. And + * filling with zeroes starts from the end of the PEB. This was observed with + * Spansion S29GL512N NOR flash. + * + * This means that in case of a power cut we may end up with intact data at the + * beginning of the PEB, and all zeroes at the end of PEB. In other words, the + * EC and VID headers are OK, but a large chunk of data at the end of PEB is + * zeroed. This makes UBI mistakenly treat this PEB as used and associate it + * with an LEB, which leads to subsequent failures (e.g., UBIFS fails). + * + * This function is called before erasing NOR PEBs and it zeroes out EC and VID + * magic numbers in order to invalidate them and prevent the failures. Returns + * zero in case of success and a negative error code in case of failure. + */ +static int nor_erase_prepare(struct ubi_device *ubi, int pnum) +{ + int err; + size_t written; + loff_t addr; + uint32_t data = 0; + + addr = (loff_t)pnum * ubi->peb_size; + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, &data); + if (err) { + ubi_err("error %d while writing 4 bytes to PEB %d:0, written " + "%zd bytes", err, pnum, 0, written); + ubi_dbg_dump_stack(); + return err; + } + + addr += ubi->vid_hdr_aloffset; + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, &data); + if (err) { + ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " + "%zd bytes", err, pnum, ubi->vid_hdr_aloffset, written); + ubi_dbg_dump_stack(); + return err; + } + + return 0; +} + +/** * ubi_io_sync_erase - synchronously erase a physical eraseblock. * @ubi: UBI device description object * @pnum: physical eraseblock number to erase @@ -484,6 +532,12 @@ int ubi_io_sync_erase(struct ubi_device *ubi, int pnum, int torture) return -EROFS; } + if (ubi->nor_flash) { + err = nor_erase_prepare(ubi, pnum); + if (err) + return err; + } + if (torture) { ret = torture_peb(ubi, pnum); if (ret < 0) diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 28acd13..f68cb54 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -372,6 +372,7 @@ struct ubi_wl_entry; * @vid_hdr_shift: contains @vid_hdr_offset - @vid_hdr_aloffset * @bad_allowed: whether the MTD device admits of bad physical eraseblocks or * not + * @nor_flash: non-zero if working on top of NOR flash * @mtd: MTD device descriptor * * @peb_buf1: a buffer of PEB size used for different purposes @@ -452,7 +453,8 @@ struct ubi_device { int vid_hdr_offset; int vid_hdr_aloffset; int vid_hdr_shift; - int bad_allowed; + unsigned int bad_allowed:1; + unsigned int nor_flash:1; struct mtd_info *mtd; void *peb_buf1;