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 | ||
