| Message ID | 1289654101.2218.51.camel@localhost |
|---|---|
| State | Accepted |
| Commit | 276832d878d8a892ac7b40fd0ee07fe757e080c7 |
| Headers | show |
On Sat, Nov 13, 2010 at 3:15 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > While trying to figure out what is happening in your system, I realized > one possible scenario which may confuse UBI. I've added a patch below. > This probably won't fix your issue (but you could try), I need more time > to think about what was happening. But a log with all messages (not only > I/O) would help. Thanks. Well I think I already know what's wrong with my driver - it has subpage reads broken. So UBI tries to read a subpage, driver fails there, then it runs a torture test on full PEB that passes (because page reads work right), marks that PEB as good and retries the subpage read that fails again, and the story repeats. Does that sound like reasonable scenario, or do you still want more debugging logs?
On Sat, 2010-11-13 at 16:23 +0200, Grazvydas Ignotas wrote: > On Sat, Nov 13, 2010 at 3:15 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote: > > While trying to figure out what is happening in your system, I realized > > one possible scenario which may confuse UBI. I've added a patch below. > > This probably won't fix your issue (but you could try), I need more time > > to think about what was happening. But a log with all messages (not only > > I/O) would help. Thanks. > > Well I think I already know what's wrong with my driver - it has > subpage reads broken. So UBI tries to read a subpage, driver fails > there, then it runs a torture test on full PEB that passes (because > page reads work right), marks that PEB as good and retries the subpage > read that fails again, and the story repeats. Does that sound like > reasonable scenario, or do you still want more debugging logs? Yaeah, obviously you have driver problems, I'm just interested to improve UBI's resilience.
diff --git a/drivers/mtd/ubi/io.c b/drivers/mtd/ubi/io.c index c2960ac..9ab1a33 100644 --- a/drivers/mtd/ubi/io.c +++ b/drivers/mtd/ubi/io.c @@ -146,6 +146,28 @@ int ubi_io_read(const struct ubi_device *ubi, void *buf, int pnum, int offset, if (err) return err; + /* + * Deliberately corrupt the buffer to improve robustness. Indeed, if we + * do not do this, the following may happen: + * 1. The buffer contains data from previous operation, e.g., read from + * another PEB previously. The data looks like expected, e.g., if we + * just do not read anything and return - the caller would not + * notice this. E.g., if we are reading a VID header, the buffer may + * contain a valid VID header from another PEB. + * 2. The driver is buggy and returns us success or -EBADMSG or + * -EUCLEAN, but it does not actually put any data to the buffer. + * + * This may confuse UBI or upper layers - they may think the buffer + * contains valid data while in fact it is just old data. This is + * especially possible because UBI (and UBIFS) relies on CRC, and + * treats data as correct even in case of ECC errors if the CRC is + * correct. + * + * Try to prevent this situation by changing the first byte of the + * buffer. + */ + *((uint8_t *)buf) ^= 0xFF; + addr = (loff_t)pnum * ubi->peb_size + offset; retry: err = ubi->mtd->read(ubi->mtd, addr, len, &read, buf);