From 29a88c99d29834fb3314e0144900b187ede83106 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 19 Jul 2009 14:03:16 +0300 Subject: [PATCH 1/6] UBI: print a message if ECH is corrupted and VIDH is ok If the EC header is corrupted, but the VID header is OK, UBI accepts the PEB and treats it as "used". However, generally this should not happen. Print a warning if this happens. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/scan.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c index b847745394b4..93361eadab8d 100644 --- a/drivers/mtd/ubi/scan.c +++ b/drivers/mtd/ubi/scan.c @@ -864,7 +864,9 @@ static int process_eb(struct ubi_device *ubi, struct ubi_scan_info *si, } } - /* Both UBI headers seem to be fine */ + if (ec_corr) + ubi_warn("valid VID header but corrupted EC header at PEB %d", + pnum); err = ubi_scan_add_used(ubi, si, pnum, ec, vidh, bitflips); if (err) return err; From 5b289b562f6d236108569a880cb38cc03d17a50d Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 19 Jul 2009 14:09:46 +0300 Subject: [PATCH 2/6] UBI: amend NOR flash pre-erase quirk In case of NOR flash, UBI zeroes EC and VID headers' magic, in order to detect interrupted erasures. It first zeroes out the EC magic, then VID magic. However, if a power cut happens in between, we'll end up with a corrupted EC header and a valid VID header, in which case UBI accepts the PEB, but prints a warning. This patch makes sure we first zero out the VID magic, then the EC magic, not vice versa. This is just a small amendment to prevent warning messages. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/io.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index 4cb69925d8d9..4e7bcb215075 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -480,16 +480,7 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) loff_t addr; uint32_t data = 0; - addr = (loff_t)pnum * ubi->peb_size; - err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&data); - if (err) { - ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " - "%zd bytes", err, pnum, 0, written); - ubi_dbg_dump_stack(); - return err; - } - - addr += ubi->vid_hdr_aloffset; + addr = (loff_t)pnum * ubi->peb_size + ubi->vid_hdr_aloffset; err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&data); if (err) { ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " @@ -498,6 +489,15 @@ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) return err; } + addr -= ubi->vid_hdr_aloffset; + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&data); + if (err) { + ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " + "%zd bytes", err, pnum, 0, written); + ubi_dbg_dump_stack(); + return err; + } + return 0; } From 4a406856ea6830d8b8dba6a27d9f9331c5f4c13a Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Sun, 19 Jul 2009 14:33:14 +0300 Subject: [PATCH 3/6] UBI: print a warning if too many PEBs are corrupted There was a bug report recently where UBI prints: UBI error: ubi_attach_mtd_dev: failed to attach by scanning, error -22 error messages and refuses to attach a PEB. It turned out to be a buggy flash driver which returned garbage to almost every UBI read. This patch makes UBI print a better message in such cases. Namely, if UBI finds 8 or more corrupted PEBs, it prints a warning and lists the corrupted PEBs. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/scan.c | 18 ++++++++++++++++-- drivers/mtd/ubi/scan.h | 2 ++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/mtd/ubi/scan.c b/drivers/mtd/ubi/scan.c index 93361eadab8d..e7161adc419d 100644 --- a/drivers/mtd/ubi/scan.c +++ b/drivers/mtd/ubi/scan.c @@ -75,9 +75,10 @@ static int add_to_list(struct ubi_scan_info *si, int pnum, int ec, dbg_bld("add to free: PEB %d, EC %d", pnum, ec); else if (list == &si->erase) dbg_bld("add to erase: PEB %d, EC %d", pnum, ec); - else if (list == &si->corr) + else if (list == &si->corr) { dbg_bld("add to corrupted: PEB %d, EC %d", pnum, ec); - else if (list == &si->alien) + si->corr_count += 1; + } else if (list == &si->alien) dbg_bld("add to alien: PEB %d, EC %d", pnum, ec); else BUG(); @@ -937,6 +938,19 @@ struct ubi_scan_info *ubi_scan(struct ubi_device *ubi) if (si->is_empty) ubi_msg("empty MTD device detected"); + /* + * 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. Print a warning in this case. + */ + if (si->corr_count >= 8 || si->corr_count >= ubi->peb_count / 4) { + ubi_warn("%d PEBs are corrupted", si->corr_count); + printk(KERN_WARNING "corrupted PEBs are:"); + list_for_each_entry(seb, &si->corr, u.list) + printk(KERN_CONT " %d", seb->pnum); + printk(KERN_CONT "\n"); + } + /* * In case of unknown erase counter we use the mean erase counter * value. diff --git a/drivers/mtd/ubi/scan.h b/drivers/mtd/ubi/scan.h index 1017cf12def5..bab31695dace 100644 --- a/drivers/mtd/ubi/scan.h +++ b/drivers/mtd/ubi/scan.h @@ -102,6 +102,7 @@ struct ubi_scan_volume { * @mean_ec: mean erase counter value * @ec_sum: a temporary variable used when calculating @mean_ec * @ec_count: a temporary variable used when calculating @mean_ec + * @corr_count: count of corrupted PEBs * @image_seq_set: indicates @ubi->image_seq is known * * This data structure contains the result of scanning and may be used by other @@ -125,6 +126,7 @@ struct ubi_scan_info { int mean_ec; uint64_t ec_sum; int ec_count; + int corr_count; int image_seq_set; }; From 758d8e46347aee199e7025b8c571bab75d2de63f Mon Sep 17 00:00:00 2001 From: Phil Carmody Date: Thu, 23 Jul 2009 15:29:10 +0200 Subject: [PATCH 4/6] UBI: eliminate possible undefined behaviour The assignment to pos when rb is finally NULL is undefined behaviour. Upon seeing that assignment, GCC may assume that rb is not NULL, and the loop condition ``rb'' may be optimised away. Signed-off-by: Phil Carmody Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/ubi.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h index 6a5fe9633783..c290f51dd178 100644 --- a/drivers/mtd/ubi/ubi.h +++ b/drivers/mtd/ubi/ubi.h @@ -579,7 +579,8 @@ void ubi_do_get_volume_info(struct ubi_device *ubi, struct ubi_volume *vol, for (rb = rb_first(root), \ pos = (rb ? container_of(rb, typeof(*pos), member) : NULL); \ rb; \ - rb = rb_next(rb), pos = container_of(rb, typeof(*pos), member)) + rb = rb_next(rb), \ + pos = (rb ? container_of(rb, typeof(*pos), member) : NULL)) /** * ubi_zalloc_vid_hdr - allocate a volume identifier header object. From 867996b15c1f0a98d2c405bada907e97499ba8c2 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Fri, 24 Jul 2009 15:31:33 +0300 Subject: [PATCH 5/6] UBI: introduce flash dump helper Useful for debugging problems, compiled in only if UBI debugging is enabled. This patch also makes the UBI writing function dump the flash if it fails to write. Signed-off-by: Artem Bityutskiy --- drivers/mtd/ubi/debug.c | 32 ++++++++++++++++++++++++++++++++ drivers/mtd/ubi/debug.h | 2 ++ drivers/mtd/ubi/io.c | 1 + 3 files changed, 35 insertions(+) diff --git a/drivers/mtd/ubi/debug.c b/drivers/mtd/ubi/debug.c index 54b0186915fb..4876977e52cb 100644 --- a/drivers/mtd/ubi/debug.c +++ b/drivers/mtd/ubi/debug.c @@ -196,4 +196,36 @@ void ubi_dbg_dump_mkvol_req(const struct ubi_mkvol_req *req) printk(KERN_DEBUG "\t1st 16 characters of name: %s\n", nm); } +/** + * ubi_dbg_dump_flash - dump a region of flash. + * @ubi: UBI device description object + * @pnum: the physical eraseblock number to dump + * @offset: the starting offset within the physical eraseblock to dump + * @len: the length of the region to dump + */ +void ubi_dbg_dump_flash(struct ubi_device *ubi, int pnum, int offset, int len) +{ + int err; + size_t read; + void *buf; + loff_t addr = (loff_t)pnum * ubi->peb_size + offset; + + buf = vmalloc(len); + if (!buf) + return; + err = ubi->mtd->read(ubi->mtd, addr, len, &read, buf); + if (err && err != -EUCLEAN) { + ubi_err("error %d while reading %d bytes from PEB %d:%d, " + "read %zd bytes", err, len, pnum, offset, read); + goto out; + } + + dbg_msg("dumping %d bytes of data from PEB %d, offset %d", + len, pnum, offset); + print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, buf, len, 1); +out: + vfree(buf); + return; +} + #endif /* CONFIG_MTD_UBI_DEBUG */ diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index a4da7a09b949..f30bcb372c05 100644 --- a/drivers/mtd/ubi/debug.h +++ b/drivers/mtd/ubi/debug.h @@ -55,6 +55,7 @@ void ubi_dbg_dump_vtbl_record(const struct ubi_vtbl_record *r, int idx); void ubi_dbg_dump_sv(const struct ubi_scan_volume *sv); void ubi_dbg_dump_seb(const struct ubi_scan_leb *seb, int type); void ubi_dbg_dump_mkvol_req(const struct ubi_mkvol_req *req); +void ubi_dbg_dump_flash(struct ubi_device *ubi, int pnum, int offset, int len); #ifdef CONFIG_MTD_UBI_DEBUG_MSG /* General debugging messages */ @@ -167,6 +168,7 @@ static inline int ubi_dbg_is_erase_failure(void) #define ubi_dbg_dump_sv(sv) ({}) #define ubi_dbg_dump_seb(seb, type) ({}) #define ubi_dbg_dump_mkvol_req(req) ({}) +#define ubi_dbg_dump_flash(ubi, pnum, offset, len) ({}) #define UBI_IO_DEBUG 0 #define DBG_DISABLE_BGT 0 diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index 4e7bcb215075..b693138fc519 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -269,6 +269,7 @@ int ubi_io_write(struct ubi_device *ubi, const void *buf, int pnum, int offset, ubi_err("error %d while writing %d bytes to PEB %d:%d, written " "%zd bytes", err, len, pnum, offset, written); ubi_dbg_dump_stack(); + ubi_dbg_dump_flash(ubi, pnum, offset, len); } else ubi_assert(written == len); From de75c771b4cc4da963164a538a8448128301bc35 Mon Sep 17 00:00:00 2001 From: Artem Bityutskiy Date: Fri, 24 Jul 2009 16:18:04 +0300 Subject: [PATCH 6/6] UBI: improve NOR flash erasure quirk More testing of NOR flash against power cuts showed that sometimes eraseblocks may be unwritable, and we cannot really invalidate them before erasure. But in this case the eraseblock probably contains garbage anyway, and we do not have to invalidate the headers. This assumption might be not true, but this is at least what I have observed. So if we cannot invalidate the headers, we make sure that the PEB does not contain valid VID header. If this is true, everything is fine, otherwise we panic. --- drivers/mtd/ubi/io.c | 46 +++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index b693138fc519..8aa51e7a6a7d 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -476,30 +476,46 @@ out: */ static int nor_erase_prepare(struct ubi_device *ubi, int pnum) { - int err; + int err, err1; size_t written; loff_t addr; uint32_t data = 0; + struct ubi_vid_hdr vid_hdr; addr = (loff_t)pnum * ubi->peb_size + ubi->vid_hdr_aloffset; err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&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; + if (!err) { + addr -= ubi->vid_hdr_aloffset; + err = ubi->mtd->write(ubi->mtd, addr, 4, &written, + (void *)&data); + if (!err) + return 0; } - addr -= ubi->vid_hdr_aloffset; - err = ubi->mtd->write(ubi->mtd, addr, 4, &written, (void *)&data); - if (err) { - ubi_err("error %d while writing 4 bytes to PEB %d:%d, written " - "%zd bytes", err, pnum, 0, written); - ubi_dbg_dump_stack(); - return err; - } + /* + * We failed to write to the media. This was observed with Spansion + * S29GL512N NOR flash. Most probably the eraseblock erasure was + * interrupted at a very inappropriate moment, so it became unwritable. + * 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_VID_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 + * case of an unclean reboot. + */ + return 0; - return 0; + /* + * The PEB contains a valid VID header, but we cannot invalidate it. + * Supposedly the flash media or the driver is screwed up, so return an + * error. + */ + ubi_err("cannot invalidate PEB %d, write returned %d read returned %d", + pnum, err, err1); + ubi_dbg_dump_flash(ubi, pnum, 0, ubi->peb_size); + return -EIO; } /**