diff options
author | Artem Bityutskiy <Artem.Bityutskiy@nokia.com> | 2009-06-29 08:58:36 -0400 |
---|---|---|
committer | Artem Bityutskiy <Artem.Bityutskiy@nokia.com> | 2009-07-05 11:47:05 -0400 |
commit | 1398788fe7b730db10e97dcb9f838603e41922d5 (patch) | |
tree | 67edd2fb58aca4628c870679a39530838c8de76a | |
parent | 40a71a87fa8e0cb3ec0fca4d152263734b203eb2 (diff) |
UBI: remove bogus debugging checks
The 'paranoid_check_empty()' is bogus because, which is easilly
seen on NOR flash, which has long erase cycles, and which may
easilly end-up with half-erased eraseblocks. In this case the
paranoid check fails. I is just wrong to assume that PEBs which
do not have EC headers always contain all 0xFF. Such assumption
should not be made on the I/O level, which is quite low.
Thus, just kill the check.
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
-rw-r--r-- | drivers/mtd/ubi/debug.h | 1 | ||||
-rw-r--r-- | drivers/mtd/ubi/io.c | 87 | ||||
-rw-r--r-- | drivers/mtd/ubi/wl.c | 2 |
3 files changed, 2 insertions, 88 deletions
diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index 6fc7fda2ab91..a4da7a09b949 100644 --- a/drivers/mtd/ubi/debug.h +++ b/drivers/mtd/ubi/debug.h | |||
@@ -173,6 +173,7 @@ static inline int ubi_dbg_is_erase_failure(void) | |||
173 | #define ubi_dbg_is_bitflip() 0 | 173 | #define ubi_dbg_is_bitflip() 0 |
174 | #define ubi_dbg_is_write_failure() 0 | 174 | #define ubi_dbg_is_write_failure() 0 |
175 | #define ubi_dbg_is_erase_failure() 0 | 175 | #define ubi_dbg_is_erase_failure() 0 |
176 | #define ubi_dbg_check_all_ff(ubi, pnum, offset, len) 0 | ||
176 | 177 | ||
177 | #endif /* !CONFIG_MTD_UBI_DEBUG */ | 178 | #endif /* !CONFIG_MTD_UBI_DEBUG */ |
178 | #endif /* !__UBI_DEBUG_H__ */ | 179 | #endif /* !__UBI_DEBUG_H__ */ |
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index 6c5441e8791d..c8edbfd09b64 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c | |||
@@ -98,14 +98,12 @@ static int paranoid_check_ec_hdr(const struct ubi_device *ubi, int pnum, | |||
98 | static int paranoid_check_peb_vid_hdr(const struct ubi_device *ubi, int pnum); | 98 | static int paranoid_check_peb_vid_hdr(const struct ubi_device *ubi, int pnum); |
99 | static int paranoid_check_vid_hdr(const struct ubi_device *ubi, int pnum, | 99 | static int paranoid_check_vid_hdr(const struct ubi_device *ubi, int pnum, |
100 | const struct ubi_vid_hdr *vid_hdr); | 100 | const struct ubi_vid_hdr *vid_hdr); |
101 | static int paranoid_check_empty(struct ubi_device *ubi, int pnum); | ||
102 | #else | 101 | #else |
103 | #define paranoid_check_not_bad(ubi, pnum) 0 | 102 | #define paranoid_check_not_bad(ubi, pnum) 0 |
104 | #define paranoid_check_peb_ec_hdr(ubi, pnum) 0 | 103 | #define paranoid_check_peb_ec_hdr(ubi, pnum) 0 |
105 | #define paranoid_check_ec_hdr(ubi, pnum, ec_hdr) 0 | 104 | #define paranoid_check_ec_hdr(ubi, pnum, ec_hdr) 0 |
106 | #define paranoid_check_peb_vid_hdr(ubi, pnum) 0 | 105 | #define paranoid_check_peb_vid_hdr(ubi, pnum) 0 |
107 | #define paranoid_check_vid_hdr(ubi, pnum, vid_hdr) 0 | 106 | #define paranoid_check_vid_hdr(ubi, pnum, vid_hdr) 0 |
108 | #define paranoid_check_empty(ubi, pnum) 0 | ||
109 | #endif | 107 | #endif |
110 | 108 | ||
111 | /** | 109 | /** |
@@ -669,10 +667,6 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum, | |||
669 | if (read_err != -EBADMSG && | 667 | if (read_err != -EBADMSG && |
670 | check_pattern(ec_hdr, 0xFF, UBI_EC_HDR_SIZE)) { | 668 | check_pattern(ec_hdr, 0xFF, UBI_EC_HDR_SIZE)) { |
671 | /* The physical eraseblock is supposedly empty */ | 669 | /* The physical eraseblock is supposedly empty */ |
672 | err = ubi_dbg_check_all_ff(ubi, pnum, 0, ubi->peb_size); | ||
673 | if (err) | ||
674 | return err > 0 ? UBI_IO_BAD_EC_HDR : err; | ||
675 | |||
676 | if (verbose) | 670 | if (verbose) |
677 | ubi_warn("no EC header found at PEB %d, " | 671 | ubi_warn("no EC header found at PEB %d, " |
678 | "only 0xFF bytes", pnum); | 672 | "only 0xFF bytes", pnum); |
@@ -943,15 +937,6 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum, | |||
943 | if (read_err != -EBADMSG && | 937 | if (read_err != -EBADMSG && |
944 | check_pattern(vid_hdr, 0xFF, UBI_VID_HDR_SIZE)) { | 938 | check_pattern(vid_hdr, 0xFF, UBI_VID_HDR_SIZE)) { |
945 | /* The physical eraseblock is supposedly free */ | 939 | /* The physical eraseblock is supposedly free */ |
946 | |||
947 | /* | ||
948 | * The below is just a paranoid check, it has to be | ||
949 | * compiled out if paranoid checks are disabled. | ||
950 | */ | ||
951 | err = paranoid_check_empty(ubi, pnum); | ||
952 | if (err) | ||
953 | return err > 0 ? UBI_IO_BAD_VID_HDR : err; | ||
954 | |||
955 | if (verbose) | 940 | if (verbose) |
956 | ubi_warn("no VID header found at PEB %d, " | 941 | ubi_warn("no VID header found at PEB %d, " |
957 | "only 0xFF bytes", pnum); | 942 | "only 0xFF bytes", pnum); |
@@ -1241,8 +1226,6 @@ int ubi_dbg_check_all_ff(struct ubi_device *ubi, int pnum, int offset, int len) | |||
1241 | int err; | 1226 | int err; |
1242 | loff_t addr = (loff_t)pnum * ubi->peb_size + offset; | 1227 | loff_t addr = (loff_t)pnum * ubi->peb_size + offset; |
1243 | 1228 | ||
1244 | ubi_assert(!mutex_is_locked(&ubi->dbg_buf_mutex)); | ||
1245 | |||
1246 | mutex_lock(&ubi->dbg_buf_mutex); | 1229 | mutex_lock(&ubi->dbg_buf_mutex); |
1247 | err = ubi->mtd->read(ubi->mtd, addr, len, &read, ubi->dbg_peb_buf); | 1230 | err = ubi->mtd->read(ubi->mtd, addr, len, &read, ubi->dbg_peb_buf); |
1248 | if (err && err != -EUCLEAN) { | 1231 | if (err && err != -EUCLEAN) { |
@@ -1273,74 +1256,4 @@ error: | |||
1273 | return err; | 1256 | return err; |
1274 | } | 1257 | } |
1275 | 1258 | ||
1276 | /** | ||
1277 | * paranoid_check_empty - whether a PEB is empty. | ||
1278 | * @ubi: UBI device description object | ||
1279 | * @pnum: the physical eraseblock number to check | ||
1280 | * | ||
1281 | * This function makes sure PEB @pnum is empty, which means it contains only | ||
1282 | * %0xFF data bytes. Returns zero if the PEB is empty, %1 if not, and a | ||
1283 | * negative error code in case of failure. | ||
1284 | * | ||
1285 | * Empty PEBs have the EC header, and do not have the VID header. The caller of | ||
1286 | * this function should have already made sure the PEB does not have the VID | ||
1287 | * header. However, this function re-checks that, because it is possible that | ||
1288 | * the header and data has already been written to the PEB. | ||
1289 | * | ||
1290 | * Let's consider a possible scenario. Suppose there are 2 tasks - A and B. | ||
1291 | * Task A is in 'wear_leveling_worker()'. It is reading VID header of PEB X to | ||
1292 | * find which LEB it corresponds to. PEB X is currently unmapped, and has no | ||
1293 | * VID header. Task B is trying to write to PEB X. | ||
1294 | * | ||
1295 | * Task A: in 'ubi_io_read_vid_hdr()': reads the VID header from PEB X. The | ||
1296 | * read data contain all 0xFF bytes; | ||
1297 | * Task B: writes VID header and some data to PEB X; | ||
1298 | * Task A: assumes PEB X is empty, calls 'paranoid_check_empty()'. And if we | ||
1299 | * do not re-read the VID header, and do not cancel the checking if it | ||
1300 | * is there, we fail. | ||
1301 | */ | ||
1302 | static int paranoid_check_empty(struct ubi_device *ubi, int pnum) | ||
1303 | { | ||
1304 | int err, offs = ubi->vid_hdr_aloffset, len = ubi->vid_hdr_alsize; | ||
1305 | size_t read; | ||
1306 | uint32_t magic; | ||
1307 | const struct ubi_vid_hdr *vid_hdr; | ||
1308 | |||
1309 | mutex_lock(&ubi->dbg_buf_mutex); | ||
1310 | err = ubi->mtd->read(ubi->mtd, offs, len, &read, ubi->dbg_peb_buf); | ||
1311 | if (err && err != -EUCLEAN) { | ||
1312 | ubi_err("error %d while reading %d bytes from PEB %d:%d, " | ||
1313 | "read %zd bytes", err, len, pnum, offs, read); | ||
1314 | goto error; | ||
1315 | } | ||
1316 | |||
1317 | vid_hdr = ubi->dbg_peb_buf; | ||
1318 | magic = be32_to_cpu(vid_hdr->magic); | ||
1319 | if (magic == UBI_VID_HDR_MAGIC) | ||
1320 | /* The PEB contains VID header, so it is not empty */ | ||
1321 | goto out; | ||
1322 | |||
1323 | err = check_pattern(ubi->dbg_peb_buf, 0xFF, len); | ||
1324 | if (err == 0) { | ||
1325 | ubi_err("flash region at PEB %d:%d, length %d does not " | ||
1326 | "contain all 0xFF bytes", pnum, offs, len); | ||
1327 | goto fail; | ||
1328 | } | ||
1329 | |||
1330 | out: | ||
1331 | mutex_unlock(&ubi->dbg_buf_mutex); | ||
1332 | return 0; | ||
1333 | |||
1334 | fail: | ||
1335 | ubi_err("paranoid check failed for PEB %d", pnum); | ||
1336 | ubi_msg("hex dump of the %d-%d region", offs, offs + len); | ||
1337 | print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, | ||
1338 | ubi->dbg_peb_buf, len, 1); | ||
1339 | err = 1; | ||
1340 | error: | ||
1341 | ubi_dbg_dump_stack(); | ||
1342 | mutex_unlock(&ubi->dbg_buf_mutex); | ||
1343 | return err; | ||
1344 | } | ||
1345 | |||
1346 | #endif /* CONFIG_MTD_UBI_DEBUG_PARANOID */ | 1259 | #endif /* CONFIG_MTD_UBI_DEBUG_PARANOID */ |
diff --git a/drivers/mtd/ubi/wl.c b/drivers/mtd/ubi/wl.c index e4be446e05ed..600c7229d5cf 100644 --- a/drivers/mtd/ubi/wl.c +++ b/drivers/mtd/ubi/wl.c | |||
@@ -463,7 +463,7 @@ retry: | |||
463 | err = ubi_dbg_check_all_ff(ubi, e->pnum, ubi->vid_hdr_aloffset, | 463 | err = ubi_dbg_check_all_ff(ubi, e->pnum, ubi->vid_hdr_aloffset, |
464 | ubi->peb_size - ubi->vid_hdr_aloffset); | 464 | ubi->peb_size - ubi->vid_hdr_aloffset); |
465 | if (err) { | 465 | if (err) { |
466 | dbg_err("new PEB does not contain all 0xFF bytes"); | 466 | ubi_err("new PEB %d does not contain all 0xFF bytes", e->pnum); |
467 | return err > 0 ? -EINVAL : err; | 467 | return err > 0 ? -EINVAL : err; |
468 | } | 468 | } |
469 | 469 | ||