diff options
author | Artem Bityutskiy <Artem.Bityutskiy@nokia.com> | 2009-05-12 08:43:44 -0400 |
---|---|---|
committer | Artem Bityutskiy <Artem.Bityutskiy@nokia.com> | 2009-05-18 05:28:25 -0400 |
commit | ffb6b7e4fdef715061859651fe46cd27afc6acec (patch) | |
tree | 7b01298488f897b1176fc405fa8b4813f50ebf7b /drivers/mtd/ubi | |
parent | 2cb81e218f336dc5438a960d1ae098188db9ff11 (diff) |
UBI: fix races in I/O debugging checks
When paranoid checs are enabled, the 'io_paral' test from the
'mtd-utils' package fails. The symptoms are:
UBI error: paranoid_check_all_ff: flash region at PEB 3973:512, length 15872 does not contain all 0xFF bytes
UBI error: paranoid_check_all_ff: paranoid check failed for PEB 3973
UBI: hex dump of the 512-16384 region
It turned out to be a bug in the checking function. Suppose there
are 2 tasks - A and B. Task A is the wear-levelling working
('wear_leveling_worker()'). It is reading the VID header to find
which LEB this PEB belongs to. Say, task A is reading header
of PEB X. Suppose PEB X is unmapped, and has no VID header.
Task B is trying to write to PEB X.
Task A: in 'ubi_io_read_vid_hdr()': reads the VID header from PEB X.
The read data contain all 0xFF bytes.
Task B: writes VID header and some data to PEB X
Task A: assumes PEB X is empty, calls 'paranoid_check_all_ff()', which
fails.
The solution for this problem is to make 'paranoid_check_all_ff()'
re-read the VID header, re-check it, and only if it is not there,
check the rest. This now implemented by the 'paranoid_check_empty()'
function.
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Diffstat (limited to 'drivers/mtd/ubi')
-rw-r--r-- | drivers/mtd/ubi/io.c | 80 |
1 files changed, 73 insertions, 7 deletions
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index fe81039f2a7c..ac6604aeb728 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c | |||
@@ -100,6 +100,7 @@ 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_all_ff(struct ubi_device *ubi, int pnum, int offset, | 101 | static int paranoid_check_all_ff(struct ubi_device *ubi, int pnum, int offset, |
102 | int len); | 102 | int len); |
103 | static int paranoid_check_empty(struct ubi_device *ubi, int pnum); | ||
103 | #else | 104 | #else |
104 | #define paranoid_check_not_bad(ubi, pnum) 0 | 105 | #define paranoid_check_not_bad(ubi, pnum) 0 |
105 | #define paranoid_check_peb_ec_hdr(ubi, pnum) 0 | 106 | #define paranoid_check_peb_ec_hdr(ubi, pnum) 0 |
@@ -107,6 +108,7 @@ static int paranoid_check_all_ff(struct ubi_device *ubi, int pnum, int offset, | |||
107 | #define paranoid_check_peb_vid_hdr(ubi, pnum) 0 | 108 | #define paranoid_check_peb_vid_hdr(ubi, pnum) 0 |
108 | #define paranoid_check_vid_hdr(ubi, pnum, vid_hdr) 0 | 109 | #define paranoid_check_vid_hdr(ubi, pnum, vid_hdr) 0 |
109 | #define paranoid_check_all_ff(ubi, pnum, offset, len) 0 | 110 | #define paranoid_check_all_ff(ubi, pnum, offset, len) 0 |
111 | #define paranoid_check_empty(ubi, pnum) 0 | ||
110 | #endif | 112 | #endif |
111 | 113 | ||
112 | /** | 114 | /** |
@@ -670,11 +672,6 @@ int ubi_io_read_ec_hdr(struct ubi_device *ubi, int pnum, | |||
670 | if (read_err != -EBADMSG && | 672 | if (read_err != -EBADMSG && |
671 | check_pattern(ec_hdr, 0xFF, UBI_EC_HDR_SIZE)) { | 673 | check_pattern(ec_hdr, 0xFF, UBI_EC_HDR_SIZE)) { |
672 | /* The physical eraseblock is supposedly empty */ | 674 | /* The physical eraseblock is supposedly empty */ |
673 | |||
674 | /* | ||
675 | * The below is just a paranoid check, it has to be | ||
676 | * compiled out if paranoid checks are disabled. | ||
677 | */ | ||
678 | err = paranoid_check_all_ff(ubi, pnum, 0, | 675 | err = paranoid_check_all_ff(ubi, pnum, 0, |
679 | ubi->peb_size); | 676 | ubi->peb_size); |
680 | if (err) | 677 | if (err) |
@@ -955,8 +952,7 @@ int ubi_io_read_vid_hdr(struct ubi_device *ubi, int pnum, | |||
955 | * The below is just a paranoid check, it has to be | 952 | * The below is just a paranoid check, it has to be |
956 | * compiled out if paranoid checks are disabled. | 953 | * compiled out if paranoid checks are disabled. |
957 | */ | 954 | */ |
958 | err = paranoid_check_all_ff(ubi, pnum, ubi->leb_start, | 955 | err = paranoid_check_empty(ubi, pnum); |
959 | ubi->leb_size); | ||
960 | if (err) | 956 | if (err) |
961 | return err > 0 ? UBI_IO_BAD_VID_HDR : err; | 957 | return err > 0 ? UBI_IO_BAD_VID_HDR : err; |
962 | 958 | ||
@@ -1280,4 +1276,74 @@ error: | |||
1280 | return err; | 1276 | return err; |
1281 | } | 1277 | } |
1282 | 1278 | ||
1279 | /** | ||
1280 | * paranoid_check_empty - whether a PEB is empty. | ||
1281 | * @ubi: UBI device description object | ||
1282 | * @pnum: the physical eraseblock number to check | ||
1283 | * | ||
1284 | * This function makes sure PEB @pnum is empty, which means it contains only | ||
1285 | * %0xFF data bytes. Returns zero if the PEB is empty, %1 if not, and a | ||
1286 | * negative error code in case of failure. | ||
1287 | * | ||
1288 | * Empty PEBs have the EC header, and do not have the VID header. The caller of | ||
1289 | * this function should have already made sure the PEB does not have the VID | ||
1290 | * header. However, this function re-checks that, because it is possible that | ||
1291 | * the header and data has already been written to the PEB. | ||
1292 | * | ||
1293 | * Let's consider a possible scenario. Suppose there are 2 tasks - A and B. | ||
1294 | * Task A is in 'wear_leveling_worker()'. It is reading VID header of PEB X to | ||
1295 | * find which LEB it corresponds to. PEB X is currently unmapped, and has no | ||
1296 | * VID header. Task B is trying to write to PEB X. | ||
1297 | * | ||
1298 | * Task A: in 'ubi_io_read_vid_hdr()': reads the VID header from PEB X. The | ||
1299 | * read data contain all 0xFF bytes; | ||
1300 | * Task B: writes VID header and some data to PEB X; | ||
1301 | * Task A: assumes PEB X is empty, calls 'paranoid_check_empty()'. And if we | ||
1302 | * do not re-read the VID header, and do not cancel the checking if it | ||
1303 | * is there, we fail. | ||
1304 | */ | ||
1305 | static int paranoid_check_empty(struct ubi_device *ubi, int pnum) | ||
1306 | { | ||
1307 | int err, offs = ubi->vid_hdr_aloffset, len = ubi->vid_hdr_alsize; | ||
1308 | size_t read; | ||
1309 | uint32_t magic; | ||
1310 | const struct ubi_vid_hdr *vid_hdr; | ||
1311 | |||
1312 | mutex_lock(&ubi->dbg_buf_mutex); | ||
1313 | err = ubi->mtd->read(ubi->mtd, offs, len, &read, ubi->dbg_peb_buf); | ||
1314 | if (err && err != -EUCLEAN) { | ||
1315 | ubi_err("error %d while reading %d bytes from PEB %d:%d, " | ||
1316 | "read %zd bytes", err, len, pnum, offs, read); | ||
1317 | goto error; | ||
1318 | } | ||
1319 | |||
1320 | vid_hdr = ubi->dbg_peb_buf; | ||
1321 | magic = be32_to_cpu(vid_hdr->magic); | ||
1322 | if (magic == UBI_VID_HDR_MAGIC) | ||
1323 | /* The PEB contains VID header, so it is not empty */ | ||
1324 | goto out; | ||
1325 | |||
1326 | err = check_pattern(ubi->dbg_peb_buf, 0xFF, len); | ||
1327 | if (err == 0) { | ||
1328 | ubi_err("flash region at PEB %d:%d, length %d does not " | ||
1329 | "contain all 0xFF bytes", pnum, offs, len); | ||
1330 | goto fail; | ||
1331 | } | ||
1332 | |||
1333 | out: | ||
1334 | mutex_unlock(&ubi->dbg_buf_mutex); | ||
1335 | return 0; | ||
1336 | |||
1337 | fail: | ||
1338 | ubi_err("paranoid check failed for PEB %d", pnum); | ||
1339 | ubi_msg("hex dump of the %d-%d region", offs, offs + len); | ||
1340 | print_hex_dump(KERN_DEBUG, "", DUMP_PREFIX_OFFSET, 32, 1, | ||
1341 | ubi->dbg_peb_buf, len, 1); | ||
1342 | err = 1; | ||
1343 | error: | ||
1344 | ubi_dbg_dump_stack(); | ||
1345 | mutex_unlock(&ubi->dbg_buf_mutex); | ||
1346 | return err; | ||
1347 | } | ||
1348 | |||
1283 | #endif /* CONFIG_MTD_UBI_DEBUG_PARANOID */ | 1349 | #endif /* CONFIG_MTD_UBI_DEBUG_PARANOID */ |