From 52dd83096be5826c9e03f33fe45b279bab3820ea Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 5 Dec 2020 14:29:52 -0600 Subject: [PATCH 01/12] Initial implementation of forward-looking erase-state CRCs This change is necessary to handle out-of-order writes found by pjsg's fuzzing work. The problem is that it is possible for (non-NOR) block devices to write pages in any order, or to even write random data in the case of a power-loss. This breaks littlefs's use of the first bit in a page to indicate the erase-state. pjsg notes this behavior is documented in the W25Q here: https://community.cypress.com/docs/DOC-10507 --- The basic idea here is to CRC the next page, and use this "erase-state CRC" to check if the next page is erased and ready to accept programs. .------------------. \ commit | metadata | | | | +---. | | | | |------------------| | | | erase-state CRC -----. | |------------------| | | | | commit CRC ---|-|-' |------------------| / | | padding | | padding (doesn't need CRC) | | | |------------------| \ | next prog | erased? | +-' | | | | | v | / | | | | '------------------' This is made a bit annoying since littlefs doesn't actually store the page (prog_size) in the superblock, since it doesn't need to know the size for any other operation. We can work around this by storing both the CRC and size of the next page when necessary. Another interesting note is that we don't need to any bit tweaking information, since we read the next page every time we would need to know how to clobber the erase-state CRC. And since we only read prog_size, this works really well with our caching, since the caches must be a multiple of prog_size. This also brings back the internal lfs_bd_crc function, in which we can use some optimizations added to lfs_bd_cmp. Needs some cleanup but the idea is passing most relevant tests. --- lfs.c | 253 ++++++++++++++++++++++++++++++-------- scripts/readmdir.py | 51 ++++++-- tests/test_powerloss.toml | 181 +++++++++++++++++++++++++++ 3 files changed, 419 insertions(+), 66 deletions(-) create mode 100644 tests/test_powerloss.toml diff --git a/lfs.c b/lfs.c index 48a10e8e..abd5e649 100644 --- a/lfs.c +++ b/lfs.c @@ -133,16 +133,15 @@ static int lfs_bd_cmp(lfs_t *lfs, for (lfs_off_t i = 0; i < size; i += diff) { uint8_t dat[8]; - diff = lfs_min(size-i, sizeof(dat)); - int res = lfs_bd_read(lfs, + int err = lfs_bd_read(lfs, pcache, rcache, hint-i, block, off+i, &dat, diff); - if (res) { - return res; + if (err) { + return err; } - res = memcmp(dat, data + i, diff); + int res = memcmp(dat, data + i, diff); if (res) { return res < 0 ? LFS_CMP_LT : LFS_CMP_GT; } @@ -151,6 +150,27 @@ static int lfs_bd_cmp(lfs_t *lfs, return LFS_CMP_EQ; } +static int lfs_bd_crc(lfs_t *lfs, + const lfs_cache_t *pcache, lfs_cache_t *rcache, lfs_size_t hint, + lfs_block_t block, lfs_off_t off, lfs_size_t size, uint32_t *crc) { + lfs_size_t diff = 0; + + for (lfs_off_t i = 0; i < size; i += diff) { + uint8_t dat[8]; + diff = lfs_min(size-i, sizeof(dat)); + int err = lfs_bd_read(lfs, + pcache, rcache, hint-i, + block, off+i, &dat, diff); + if (err) { + return err; + } + + *crc = lfs_crc(*crc, &dat, diff); + } + + return 0; +} + #ifndef LFS_READONLY static int lfs_bd_flush(lfs_t *lfs, lfs_cache_t *pcache, lfs_cache_t *rcache, bool validate) { @@ -413,6 +433,22 @@ static inline void lfs_gstate_tole32(lfs_gstate_t *a) { } #endif +// operations on estate in CRC tags +struct lfs_estate { + lfs_size_t size; + uint32_t crc; +}; + +static void lfs_estate_fromle32(struct lfs_estate *estate) { + estate->size = lfs_fromle32(estate->size); + estate->crc = lfs_fromle32(estate->crc); +} + +static void lfs_estate_tole32(struct lfs_estate *estate) { + estate->size = lfs_tole32(estate->size); + estate->crc = lfs_tole32(estate->crc); +} + // other endianness operations static void lfs_ctz_fromle32(struct lfs_ctz *ctz) { ctz->head = lfs_fromle32(ctz->head); @@ -1056,14 +1092,10 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, } crc = lfs_crc(crc, &tag, sizeof(tag)); - tag = lfs_frombe32(tag) ^ ptag; + tag = (lfs_frombe32(tag) ^ ptag) & 0x7fffffff; - // next commit not yet programmed or we're not in valid range - if (!lfs_tag_isvalid(tag)) { - dir->erased = (lfs_tag_type1(ptag) == LFS_TYPE_CRC && - dir->off % lfs->cfg->prog_size == 0); - break; - } else if (off + lfs_tag_dsize(tag) > lfs->cfg->block_size) { + // out of range? + if (off + lfs_tag_dsize(tag) > lfs->cfg->block_size) { dir->erased = false; break; } @@ -1071,11 +1103,31 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, ptag = tag; if (lfs_tag_type1(tag) == LFS_TYPE_CRC) { + lfs_off_t noff = off + sizeof(tag); + struct lfs_estate estate; + + if (lfs_tag_chunk(tag) == 3) { + err = lfs_bd_read(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], noff, &estate, sizeof(estate)); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + return err; + } + + crc = lfs_crc(crc, &estate, sizeof(estate)); + lfs_estate_fromle32(&estate); + noff += sizeof(estate); + } + // check the crc attr uint32_t dcrc; err = lfs_bd_read(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), &dcrc, sizeof(dcrc)); + dir->pair[0], noff, &dcrc, sizeof(dcrc)); if (err) { if (err == LFS_ERR_CORRUPT) { dir->erased = false; @@ -1090,9 +1142,6 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, break; } - // reset the next bit if we need to - ptag ^= (lfs_tag_t)(lfs_tag_chunk(tag) & 1U) << 31; - // toss our crc into the filesystem seed for // pseudorandom numbers, note we use another crc here // as a collection function because it is sufficiently @@ -1110,24 +1159,72 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, // reset crc crc = 0xffffffff; + + // check if the next page is erased + // + // this may look inefficient, but it's surprisingly efficient + // since cache_size is probably > prog_size, so the data will + // always remain in cache for the next iteration + if (lfs_tag_chunk(tag) == 3) { + // first we get a tag-worth of bits, this is so we can + // tweak our current tag to force future writes to be + // different than the erased state + lfs_tag_t etag; + err = lfs_bd_read(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], dir->off, &etag, sizeof(etag)); + if (err) { + // TODO can we stop duplicating this error condition? + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + return err; + } + + // perturb valid bit? + dir->etag |= 0x80000000 & ~lfs_frombe32(etag); + + // crc the rest the full prog_size, including etag in case + // the actual esize is < tag size (though this shouldn't + // happen normally) + uint32_t tcrc = 0xffffffff; + err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], dir->off, estate.size, &tcrc); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + return err; + } + + if (tcrc == estate.crc) { + dir->erased = true; + break; + } + } else { + // end of block commit + // TODO handle backwards compat? + dir->erased = false; + break; + } + continue; } // crc the entry first, hopefully leaving it in the cache - for (lfs_off_t j = sizeof(tag); j < lfs_tag_dsize(tag); j++) { - uint8_t dat; - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+j, &dat, 1); - if (err) { - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } - return err; + err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], off+sizeof(tag), + lfs_tag_dsize(tag)-sizeof(tag), &crc); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; } - - crc = lfs_crc(crc, &dat, 1); + return err; } // directory modification tags? @@ -1494,9 +1591,18 @@ static int lfs_dir_commitattr(lfs_t *lfs, struct lfs_commit *commit, #ifndef LFS_READONLY static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // align to program units - const lfs_off_t end = lfs_alignup(commit->off + 2*sizeof(uint32_t), + // + // this gets a bit complex as we have two types of crcs: + // - 4-word crc with estate to check following prog (middle of block) + // - 2-word crc with no following prog (end of block) + const lfs_off_t end = lfs_alignup( + lfs_min(commit->off + 4*sizeof(uint32_t), lfs->cfg->block_size), lfs->cfg->prog_size); + // clamp erase size to tag size, this gives us the full tag as potential + // to intentionally invalidate erase CRCs + const lfs_size_t esize = lfs_max(lfs->cfg->prog_size, sizeof(lfs_tag_t)); + lfs_off_t off1 = 0; uint32_t crc1 = 0; @@ -1510,40 +1616,77 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { noff = lfs_min(noff, end - 2*sizeof(uint32_t)); } - // read erased state from next program unit - lfs_tag_t tag = 0xffffffff; - int err = lfs_bd_read(lfs, - NULL, &lfs->rcache, sizeof(tag), - commit->block, noff, &tag, sizeof(tag)); - if (err && err != LFS_ERR_CORRUPT) { - return err; + // build crc tag + lfs_tag_t etag = 0; + lfs_tag_t tag = LFS_MKTAG(LFS_TYPE_CRC + 2, 0x3ff, noff - off); + lfs_size_t size = 2*sizeof(uint32_t); + struct { + lfs_tag_t tag; + union { + struct { + uint32_t crc; + } crc2; + struct { + struct lfs_estate estate; + uint32_t crc; + } crc3; + } u; + } data; + + if (noff <= lfs->cfg->block_size - esize) { + // first we get a tag-worth of bits, this is so we can + // tweak our current tag to force future writes to be + // different than the erased state + int err = lfs_bd_read(lfs, + NULL, &lfs->rcache, esize, + commit->block, noff, &etag, sizeof(etag)); + // TODO handle erased-as-corrupt correctly? + if (err && err != LFS_ERR_CORRUPT) { + return err; + } + + // find expected erased state + uint32_t ecrc = 0xffffffff; + err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, esize, + commit->block, noff, esize, &ecrc); + // TODO handle erased-as-corrupt correctly? + if (err && err != LFS_ERR_CORRUPT) { + return err; + } + + data.u.crc3.estate.size = esize; + data.u.crc3.estate.crc = ecrc; + lfs_estate_tole32(&data.u.crc3.estate); + + // indicate we include estate + tag |= LFS_MKTAG(1, 0, 0); + size += sizeof(struct lfs_estate); } - // build crc tag - bool reset = ~lfs_frombe32(tag) >> 31; - tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff, noff - off); + data.tag = lfs_tobe32(tag ^ commit->ptag); + commit->crc = lfs_crc(commit->crc, &data, size-sizeof(uint32_t)); + ((uint32_t*)&data)[size/sizeof(uint32_t) - 1] = lfs_tole32(commit->crc); - // write out crc - uint32_t footer[2]; - footer[0] = lfs_tobe32(tag ^ commit->ptag); - commit->crc = lfs_crc(commit->crc, &footer[0], sizeof(footer[0])); - footer[1] = lfs_tole32(commit->crc); - err = lfs_bd_prog(lfs, + int err = lfs_bd_prog(lfs, &lfs->pcache, &lfs->rcache, false, - commit->block, commit->off, &footer, sizeof(footer)); + commit->block, commit->off, &data, size); if (err) { return err; } // keep track of non-padding checksum to verify if (off1 == 0) { - off1 = commit->off + sizeof(uint32_t); + //off1 = commit->off + sizeof(uint32_t); + off1 = commit->off + size-sizeof(uint32_t); crc1 = commit->crc; } commit->off += sizeof(tag)+lfs_tag_size(tag); - commit->ptag = tag ^ ((lfs_tag_t)reset << 31); - commit->crc = 0xffffffff; // reset crc for next "commit" + // perturb valid bit? + commit->ptag = tag | (0x80000000 & ~lfs_frombe32(etag)); + // reset crc for next commit + commit->crc = 0xffffffff; } // flush buffers @@ -1556,6 +1699,7 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { lfs_off_t off = commit->begin; lfs_off_t noff = off1; while (off < end) { + // TODO restructure to use lfs_bd_crc? uint32_t crc = 0xffffffff; for (lfs_off_t i = off; i < noff+sizeof(uint32_t); i++) { // check against written crc, may catch blocks that @@ -1564,7 +1708,6 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { return LFS_ERR_CORRUPT; } - // leave it up to caching to make this efficient uint8_t dat; err = lfs_bd_read(lfs, NULL, &lfs->rcache, noff+sizeof(uint32_t)-i, @@ -1581,12 +1724,16 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { return LFS_ERR_CORRUPT; } - // skip padding - off = lfs_min(end - noff, 0x3fe) + noff; + // skip padding, note that these always contain estate + off = noff - 3*sizeof(uint32_t); + off = lfs_min(end - off, 0x3fe) + off; if (off < end) { off = lfs_min(off, end - 2*sizeof(uint32_t)); } noff = off + sizeof(uint32_t); + if (noff <= lfs->cfg->block_size - esize) { + noff += 2*sizeof(uint32_t); + } } return 0; diff --git a/scripts/readmdir.py b/scripts/readmdir.py index b6c3dcca..cf6d9d88 100755 --- a/scripts/readmdir.py +++ b/scripts/readmdir.py @@ -99,7 +99,16 @@ class Tag: return struct.unpack('b', struct.pack('B', self.chunk))[0] def is_(self, type): - return (self.type & TAG_TYPES[type][0]) == TAG_TYPES[type][1] + try: + if ' ' in type: + type1, type3 = type.split() + return (self.is_(type1) and + (self.type & ~TAG_TYPES[type1][0]) == int(type3, 0)) + + return self.type == int(type, 0) + + except (ValueError, KeyError): + return (self.type & TAG_TYPES[type][0]) == TAG_TYPES[type][1] def mkmask(self): return Tag( @@ -109,14 +118,19 @@ class Tag: def chid(self, nid): ntag = Tag(self.type, nid, self.size) - if hasattr(self, 'off'): ntag.off = self.off - if hasattr(self, 'data'): ntag.data = self.data - if hasattr(self, 'crc'): ntag.crc = self.crc + if hasattr(self, 'off'): ntag.off = self.off + if hasattr(self, 'data'): ntag.data = self.data + if hasattr(self, 'crc'): ntag.crc = self.crc + if hasattr(self, 'erased'): ntag.erased = self.erased return ntag def typerepr(self): if self.is_('crc') and getattr(self, 'crc', 0xffffffff) != 0xffffffff: - return 'crc (bad)' + crc_status = ' (bad)' + elif self.is_('crc') and getattr(self, 'erased', False): + crc_status = ' (era)' + else: + crc_status = '' reverse_types = {v: k for k, v in TAG_TYPES.items()} for prefix in range(12): @@ -124,12 +138,12 @@ class Tag: if (mask, self.type & mask) in reverse_types: type = reverse_types[mask, self.type & mask] if prefix > 0: - return '%s %#0*x' % ( - type, prefix//4, self.type & ((1 << prefix)-1)) + return '%s %#x%s' % ( + type, self.type & ((1 << prefix)-1), crc_status) else: - return type + return '%s%s' % (type, crc_status) else: - return '%02x' % self.type + return '%02x%s' % (self.type, crc_status) def idrepr(self): return repr(self.id) if self.id != 0x3ff else '.' @@ -182,11 +196,13 @@ class MetadataPair: while len(block) - off >= 4: ntag, = struct.unpack('>I', block[off:off+4]) - tag = Tag(int(tag) ^ ntag) + tag = Tag((int(tag) ^ ntag) & 0x7fffffff) tag.off = off + 4 tag.data = block[off+4:off+tag.dsize] - if tag.is_('crc'): - crc = binascii.crc32(block[off:off+4+4], crc) + if tag.is_('crc 0x3'): + crc = binascii.crc32(block[off:off+4*4], crc) + elif tag.is_('crc'): + crc = binascii.crc32(block[off:off+2*4], crc) else: crc = binascii.crc32(block[off:off+tag.dsize], crc) tag.crc = crc @@ -201,9 +217,18 @@ class MetadataPair: if not corrupt: self.log = self.all_.copy() + # end of commit? + if tag.is_('crc 0x3'): + esize, ecrc = struct.unpack(' 0; + + lfs_mount(&lfs, cfg) => 0; + lfs_mkdir(&lfs, "notebook") => 0; + lfs_file_t file; + lfs_file_open(&lfs, &file, "notebook/paper", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0; + char buffer[256]; + strcpy(buffer, "hello"); + lfs_size_t size = strlen("hello"); + for (int i = 0; i < 5; i++) { + lfs_file_write(&lfs, &file, buffer, size) => size; + lfs_file_sync(&lfs, &file) => 0; + } + lfs_file_close(&lfs, &file) => 0; + + char rbuffer[256]; + lfs_file_open(&lfs, &file, "notebook/paper", LFS_O_RDONLY) => 0; + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; + + // get pair/rev count + lfs_mount(&lfs, cfg) => 0; + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "notebook") => 0; + lfs_block_t pair[2] = {dir.m.pair[0], dir.m.pair[1]}; + uint32_t rev = dir.m.rev; + lfs_dir_close(&lfs, &dir) => 0; + lfs_unmount(&lfs) => 0; + + // write just the revision count + uint8_t bbuffer[BLOCK_SIZE]; + cfg->read(cfg, pair[1], 0, bbuffer, BLOCK_SIZE) => 0; + + memcpy(bbuffer, &(uint32_t){lfs_tole32(rev+1)}, sizeof(uint32_t)); + + cfg->erase(cfg, pair[1]) => 0; + cfg->prog(cfg, pair[1], 0, bbuffer, BLOCK_SIZE) => 0; + + lfs_mount(&lfs, cfg) => 0; + + // can read? + lfs_file_open(&lfs, &file, "notebook/paper", LFS_O_RDONLY) => 0; + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + + // can write? + lfs_file_open(&lfs, &file, "notebook/paper", + LFS_O_WRONLY | LFS_O_APPEND) => 0; + strcpy(buffer, "goodbye"); + size = strlen("goodbye"); + for (int i = 0; i < 5; i++) { + lfs_file_write(&lfs, &file, buffer, size) => size; + lfs_file_sync(&lfs, &file) => 0; + } + lfs_file_close(&lfs, &file) => 0; + + lfs_file_open(&lfs, &file, "notebook/paper", LFS_O_RDONLY) => 0; + strcpy(buffer, "hello"); + size = strlen("hello"); + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + strcpy(buffer, "goodbye"); + size = strlen("goodbye"); + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + + lfs_unmount(&lfs) => 0; +''' + +# partial prog, may not be byte in order! +[cases.test_powerloss_partial_prog] +defines.BYTE_OFF = ["0", "PROG_SIZE-1", "PROG_SIZE/2"] +defines.BYTE_VALUE = [0x33, 0xcc] +in = "lfs.c" +code = ''' + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + lfs_mount(&lfs, cfg) => 0; + lfs_mkdir(&lfs, "notebook") => 0; + lfs_file_t file; + lfs_file_open(&lfs, &file, "notebook/paper", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_APPEND) => 0; + char buffer[256]; + strcpy(buffer, "hello"); + lfs_size_t size = strlen("hello"); + for (int i = 0; i < 5; i++) { + lfs_file_write(&lfs, &file, buffer, size) => size; + lfs_file_sync(&lfs, &file) => 0; + } + lfs_file_close(&lfs, &file) => 0; + + char rbuffer[256]; + lfs_file_open(&lfs, &file, "notebook/paper", LFS_O_RDONLY) => 0; + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; + + // imitate a partial prog, value should not matter, if littlefs + // doesn't notice the partial prog testbd will assert + + // get offset to next prog + lfs_mount(&lfs, cfg) => 0; + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "notebook") => 0; + lfs_block_t block = dir.m.pair[0]; + lfs_off_t off = dir.m.off; + lfs_dir_close(&lfs, &dir) => 0; + lfs_unmount(&lfs) => 0; + + // tweak byte + uint8_t bbuffer[BLOCK_SIZE]; + cfg->read(cfg, block, 0, bbuffer, BLOCK_SIZE) => 0; + + bbuffer[off + BYTE_OFF] = BYTE_VALUE; + + cfg->erase(cfg, block) => 0; + cfg->prog(cfg, block, 0, bbuffer, BLOCK_SIZE) => 0; + + lfs_mount(&lfs, cfg) => 0; + + // can read? + lfs_file_open(&lfs, &file, "notebook/paper", LFS_O_RDONLY) => 0; + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + + // can write? + lfs_file_open(&lfs, &file, "notebook/paper", + LFS_O_WRONLY | LFS_O_APPEND) => 0; + strcpy(buffer, "goodbye"); + size = strlen("goodbye"); + for (int i = 0; i < 5; i++) { + lfs_file_write(&lfs, &file, buffer, size) => size; + lfs_file_sync(&lfs, &file) => 0; + } + lfs_file_close(&lfs, &file) => 0; + + lfs_file_open(&lfs, &file, "notebook/paper", LFS_O_RDONLY) => 0; + strcpy(buffer, "hello"); + size = strlen("hello"); + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + strcpy(buffer, "goodbye"); + size = strlen("goodbye"); + for (int i = 0; i < 5; i++) { + lfs_file_read(&lfs, &file, rbuffer, size) => size; + assert(memcmp(rbuffer, buffer, size) == 0); + } + lfs_file_close(&lfs, &file) => 0; + + lfs_unmount(&lfs) => 0; +''' From 91ad673c4520c08006272ba3bad133fe21048887 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 6 Dec 2020 00:20:09 -0600 Subject: [PATCH 02/12] Cleaned up a few additional commit corner cases - General cleanup from integration, including cleaning up some older commit code - Partial-prog tests do not make sense when prog_size == block_size (there can't be partial-progs!) - Fixed signed-comparison issue in modified filebd --- lfs.c | 211 +++++++++++++++++--------------------- tests/test_powerloss.toml | 1 + 2 files changed, 97 insertions(+), 115 deletions(-) diff --git a/lfs.c b/lfs.c index abd5e649..c8f87b02 100644 --- a/lfs.c +++ b/lfs.c @@ -1160,12 +1160,13 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, // reset crc crc = 0xffffffff; - // check if the next page is erased - // - // this may look inefficient, but it's surprisingly efficient - // since cache_size is probably > prog_size, so the data will - // always remain in cache for the next iteration if (lfs_tag_chunk(tag) == 3) { + // check if the next page is erased + // + // this may look inefficient, but since cache_size is + // probably > prog_size, the data will always remain in + // cache for the next iteration + // first we get a tag-worth of bits, this is so we can // tweak our current tag to force future writes to be // different than the erased state @@ -1173,12 +1174,7 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, err = lfs_bd_read(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, dir->pair[0], dir->off, &etag, sizeof(etag)); - if (err) { - // TODO can we stop duplicating this error condition? - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } + if (err && err != LFS_ERR_CORRUPT) { return err; } @@ -1192,11 +1188,7 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, err = lfs_bd_crc(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, dir->pair[0], dir->off, estate.size, &tcrc); - if (err) { - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } + if (err && err != LFS_ERR_CORRUPT) { return err; } @@ -1204,51 +1196,21 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->erased = true; break; } - } else { + } else if (lfs_tag_chunk(tag) == 2) { // end of block commit - // TODO handle backwards compat? dir->erased = false; break; + } else { + // for backwards compatibility we fall through here on + // unrecognized tags, leaving it up to the CRC to reject + // bad commits } - - continue; - } - - // crc the entry first, hopefully leaving it in the cache - err = lfs_bd_crc(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), - lfs_tag_dsize(tag)-sizeof(tag), &crc); - if (err) { - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } - return err; - } - - // directory modification tags? - if (lfs_tag_type1(tag) == LFS_TYPE_NAME) { - // increase count of files if necessary - if (lfs_tag_id(tag) >= tempcount) { - tempcount = lfs_tag_id(tag) + 1; - } - } else if (lfs_tag_type1(tag) == LFS_TYPE_SPLICE) { - tempcount += lfs_tag_splice(tag); - - if (tag == (LFS_MKTAG(LFS_TYPE_DELETE, 0, 0) | - (LFS_MKTAG(0, 0x3ff, 0) & tempbesttag))) { - tempbesttag |= 0x80000000; - } else if (tempbesttag != -1 && - lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { - tempbesttag += LFS_MKTAG(0, lfs_tag_splice(tag), 0); - } - } else if (lfs_tag_type1(tag) == LFS_TYPE_TAIL) { - tempsplit = (lfs_tag_chunk(tag) & 1); - - err = lfs_bd_read(lfs, + } else { + // crc the entry first, hopefully leaving it in the cache + err = lfs_bd_crc(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), &temptail, 8); + dir->pair[0], off+sizeof(tag), + lfs_tag_dsize(tag)-sizeof(tag), &crc); if (err) { if (err == LFS_ERR_CORRUPT) { dir->erased = false; @@ -1256,33 +1218,64 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, } return err; } - lfs_pair_fromle32(temptail); - } - // found a match for our fetcher? - if ((fmask & tag) == (fmask & ftag)) { - int res = cb(data, tag, &(struct lfs_diskoff){ - dir->pair[0], off+sizeof(tag)}); - if (res < 0) { - if (res == LFS_ERR_CORRUPT) { - dir->erased = false; - break; + // directory modification tags? + if (lfs_tag_type1(tag) == LFS_TYPE_NAME) { + // increase count of files if necessary + if (lfs_tag_id(tag) >= tempcount) { + tempcount = lfs_tag_id(tag) + 1; } - return res; + } else if (lfs_tag_type1(tag) == LFS_TYPE_SPLICE) { + tempcount += lfs_tag_splice(tag); + + if (tag == (LFS_MKTAG(LFS_TYPE_DELETE, 0, 0) | + (LFS_MKTAG(0, 0x3ff, 0) & tempbesttag))) { + tempbesttag |= 0x80000000; + } else if (tempbesttag != -1 && + lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { + tempbesttag += LFS_MKTAG(0, lfs_tag_splice(tag), 0); + } + } else if (lfs_tag_type1(tag) == LFS_TYPE_TAIL) { + tempsplit = (lfs_tag_chunk(tag) & 1); + + err = lfs_bd_read(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], off+sizeof(tag), &temptail, 8); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + } + lfs_pair_fromle32(temptail); } - if (res == LFS_CMP_EQ) { - // found a match - tempbesttag = tag; - } else if ((LFS_MKTAG(0x7ff, 0x3ff, 0) & tag) == - (LFS_MKTAG(0x7ff, 0x3ff, 0) & tempbesttag)) { - // found an identical tag, but contents didn't match - // this must mean that our besttag has been overwritten - tempbesttag = -1; - } else if (res == LFS_CMP_GT && - lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { - // found a greater match, keep track to keep things sorted - tempbesttag = tag | 0x80000000; + // found a match for our fetcher? + if ((fmask & tag) == (fmask & ftag)) { + int res = cb(data, tag, &(struct lfs_diskoff){ + dir->pair[0], off+sizeof(tag)}); + if (res < 0) { + if (res == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + return res; + } + + if (res == LFS_CMP_EQ) { + // found a match + tempbesttag = tag; + } else if ((LFS_MKTAG(0x7ff, 0x3ff, 0) & tag) == + (LFS_MKTAG(0x7ff, 0x3ff, 0) & tempbesttag)) { + // found an identical tag, but contents didn't match + // this must mean that our besttag has been overwritten + tempbesttag = -1; + } else if (res == LFS_CMP_GT && + lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { + // found a greater match, keep track to keep + // things sorted + tempbesttag = tag | 0x80000000; + } } } } @@ -1640,7 +1633,6 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { int err = lfs_bd_read(lfs, NULL, &lfs->rcache, esize, commit->block, noff, &etag, sizeof(etag)); - // TODO handle erased-as-corrupt correctly? if (err && err != LFS_ERR_CORRUPT) { return err; } @@ -1650,7 +1642,6 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { err = lfs_bd_crc(lfs, NULL, &lfs->rcache, esize, commit->block, noff, esize, &ecrc); - // TODO handle erased-as-corrupt correctly? if (err && err != LFS_ERR_CORRUPT) { return err; } @@ -1696,44 +1687,34 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { } // successful commit, check checksums to make sure + // + // note that we don't need to check padding commits, worst + // case if they are corrupted we would have had to compact anyways lfs_off_t off = commit->begin; - lfs_off_t noff = off1; - while (off < end) { - // TODO restructure to use lfs_bd_crc? - uint32_t crc = 0xffffffff; - for (lfs_off_t i = off; i < noff+sizeof(uint32_t); i++) { - // check against written crc, may catch blocks that - // become readonly and match our commit size exactly - if (i == off1 && crc != crc1) { - return LFS_ERR_CORRUPT; - } + uint32_t crc = 0xffffffff; + err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, off1+sizeof(uint32_t), + commit->block, off, off1-off, &crc); + if (err) { + return err; + } - uint8_t dat; - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, noff+sizeof(uint32_t)-i, - commit->block, i, &dat, 1); - if (err) { - return err; - } + // check against known crc for non-padding commits + if (crc != crc1) { + return LFS_ERR_CORRUPT; + } - crc = lfs_crc(crc, &dat, 1); - } + // make sure to check crc in case we happened to pick + // up an unrelated crc (frozen block?) + err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, sizeof(uint32_t), + commit->block, off1, sizeof(uint32_t), &crc); + if (err) { + return err; + } - // detected write error? - if (crc != 0) { - return LFS_ERR_CORRUPT; - } - - // skip padding, note that these always contain estate - off = noff - 3*sizeof(uint32_t); - off = lfs_min(end - off, 0x3fe) + off; - if (off < end) { - off = lfs_min(off, end - 2*sizeof(uint32_t)); - } - noff = off + sizeof(uint32_t); - if (noff <= lfs->cfg->block_size - esize) { - noff += 2*sizeof(uint32_t); - } + if (crc != 0) { + return LFS_ERR_CORRUPT; } return 0; diff --git a/tests/test_powerloss.toml b/tests/test_powerloss.toml index 98afe4fe..06f8661d 100644 --- a/tests/test_powerloss.toml +++ b/tests/test_powerloss.toml @@ -90,6 +90,7 @@ code = ''' # partial prog, may not be byte in order! [cases.test_powerloss_partial_prog] +if = "PROG_SIZE < BLOCK_SIZE" defines.BYTE_OFF = ["0", "PROG_SIZE-1", "PROG_SIZE/2"] defines.BYTE_VALUE = [0x33, 0xcc] in = "lfs.c" From b4091c6871c05a5439b70db4db13df39e85f1639 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 6 Dec 2020 23:54:55 -0600 Subject: [PATCH 03/12] Switched to separate-tag encoding of forward-looking CRCs Previously forward-looking CRCs was just two new CRC types, one for commits with forward-looking CRCs, one without. These both contained the CRC needed to complete the current commit (note that the commit CRC must come last!). [-- 32 --|-- 32 --|-- 32 --|-- 32 --] with: [ crc3 tag | nprog size | nprog crc | commit crc ] without: [ crc2 tag | commit crc ] This meant there had to be several checks for the two possible structure sizes, messying up the implementation. [-- 32 --|-- 32 --|-- 32 --|-- 32 --|-- 32 --] with: [nprogcrc tag| nprog size | nprog crc | commit tag | commit crc ] without: [ commit tag | commit crc ] But we already have a mechanism for storing optional metadata! The different metadata tags! So why not use a separate tage for the forward-looking CRC, separate from the commit CRC? I wasn't sure this would actually help that much, there are still necessary conditions for wether or not a forward-looking CRC is there, but in the end it simplified the code quite nicely, and resulted in a ~200 byte code-cost saving. --- lfs.c | 130 +++++++++++++++++++++----------------------- lfs.h | 2 + scripts/readmdir.py | 30 ++++++---- 3 files changed, 82 insertions(+), 80 deletions(-) diff --git a/lfs.c b/lfs.c index c8f87b02..853a646f 100644 --- a/lfs.c +++ b/lfs.c @@ -345,6 +345,10 @@ static inline uint16_t lfs_tag_type1(lfs_tag_t tag) { return (tag & 0x70000000) >> 20; } +static inline uint16_t lfs_tag_type2(lfs_tag_t tag) { + return (tag & 0x78000000) >> 20; +} + static inline uint16_t lfs_tag_type3(lfs_tag_t tag) { return (tag & 0x7ff00000) >> 20; } @@ -1071,6 +1075,9 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, bool tempsplit = false; lfs_stag_t tempbesttag = besttag; + bool hasestate = false; + struct lfs_estate estate; + dir->rev = lfs_tole32(dir->rev); uint32_t crc = lfs_crc(0xffffffff, &dir->rev, sizeof(dir->rev)); dir->rev = lfs_fromle32(dir->rev); @@ -1102,32 +1109,12 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, ptag = tag; - if (lfs_tag_type1(tag) == LFS_TYPE_CRC) { - lfs_off_t noff = off + sizeof(tag); - struct lfs_estate estate; - - if (lfs_tag_chunk(tag) == 3) { - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], noff, &estate, sizeof(estate)); - if (err) { - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } - return err; - } - - crc = lfs_crc(crc, &estate, sizeof(estate)); - lfs_estate_fromle32(&estate); - noff += sizeof(estate); - } - + if (lfs_tag_type2(tag) == LFS_TYPE_CRC) { // check the crc attr uint32_t dcrc; err = lfs_bd_read(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], noff, &dcrc, sizeof(dcrc)); + dir->pair[0], off+sizeof(tag), &dcrc, sizeof(dcrc)); if (err) { if (err == LFS_ERR_CORRUPT) { dir->erased = false; @@ -1157,10 +1144,7 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->tail[1] = temptail[1]; dir->split = tempsplit; - // reset crc - crc = 0xffffffff; - - if (lfs_tag_chunk(tag) == 3) { + if (hasestate) { // check if the next page is erased // // this may look inefficient, but since cache_size is @@ -1196,15 +1180,19 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->erased = true; break; } - } else if (lfs_tag_chunk(tag) == 2) { - // end of block commit - dir->erased = false; - break; - } else { + } else if (lfs_tag_chunk(tag) < 2) { // for backwards compatibility we fall through here on // unrecognized tags, leaving it up to the CRC to reject // bad commits + } else { + // end of block commit + dir->erased = false; + break; } + + // reset crc + crc = 0xffffffff; + hasestate = false; } else { // crc the entry first, hopefully leaving it in the cache err = lfs_bd_crc(lfs, @@ -1240,7 +1228,8 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, err = lfs_bd_read(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), &temptail, 8); + dir->pair[0], off+sizeof(tag), + &temptail, sizeof(temptail)); if (err) { if (err == LFS_ERR_CORRUPT) { dir->erased = false; @@ -1248,6 +1237,19 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, } } lfs_pair_fromle32(temptail); + } else if (lfs_tag_type1(tag) == LFS_TYPE_NPROGCRC) { + err = lfs_bd_read(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], off+sizeof(tag), + &estate, sizeof(estate)); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + } + lfs_estate_fromle32(&estate); + hasestate = true; } // found a match for our fetcher? @@ -1589,13 +1591,9 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // - 4-word crc with estate to check following prog (middle of block) // - 2-word crc with no following prog (end of block) const lfs_off_t end = lfs_alignup( - lfs_min(commit->off + 4*sizeof(uint32_t), lfs->cfg->block_size), + lfs_min(commit->off + 5*sizeof(uint32_t), lfs->cfg->block_size), lfs->cfg->prog_size); - // clamp erase size to tag size, this gives us the full tag as potential - // to intentionally invalidate erase CRCs - const lfs_size_t esize = lfs_max(lfs->cfg->prog_size, sizeof(lfs_tag_t)); - lfs_off_t off1 = 0; uint32_t crc1 = 0; @@ -1609,23 +1607,12 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { noff = lfs_min(noff, end - 2*sizeof(uint32_t)); } - // build crc tag + // clamp erase size to tag size, this gives us the full tag as potential + // to intentionally invalidate erase CRCs + const lfs_size_t esize = lfs_max( + lfs->cfg->prog_size, sizeof(lfs_tag_t)); lfs_tag_t etag = 0; - lfs_tag_t tag = LFS_MKTAG(LFS_TYPE_CRC + 2, 0x3ff, noff - off); - lfs_size_t size = 2*sizeof(uint32_t); - struct { - lfs_tag_t tag; - union { - struct { - uint32_t crc; - } crc2; - struct { - struct lfs_estate estate; - uint32_t crc; - } crc3; - } u; - } data; - + // space for estate? also only emit on last commit in padding commits if (noff <= lfs->cfg->block_size - esize) { // first we get a tag-worth of bits, this is so we can // tweak our current tag to force future writes to be @@ -1646,36 +1633,43 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { return err; } - data.u.crc3.estate.size = esize; - data.u.crc3.estate.crc = ecrc; - lfs_estate_tole32(&data.u.crc3.estate); - - // indicate we include estate - tag |= LFS_MKTAG(1, 0, 0); - size += sizeof(struct lfs_estate); + struct lfs_estate estate = {.size = esize, .crc = ecrc}; + lfs_estate_tole32(&estate); + err = lfs_dir_commitattr(lfs, commit, + LFS_MKTAG(LFS_TYPE_NPROGCRC, 0x3ff, sizeof(estate)), + &estate); + if (err) { + return err; + } } - data.tag = lfs_tobe32(tag ^ commit->ptag); - commit->crc = lfs_crc(commit->crc, &data, size-sizeof(uint32_t)); - ((uint32_t*)&data)[size/sizeof(uint32_t) - 1] = lfs_tole32(commit->crc); + // build crc state + off = commit->off + sizeof(lfs_tag_t); + struct { + lfs_tag_t tag; + uint32_t crc; + } cstate; + lfs_tag_t ctag = LFS_MKTAG(LFS_TYPE_COMMITCRC, 0x3ff, noff-off); + cstate.tag = lfs_tobe32(ctag ^ commit->ptag); + commit->crc = lfs_crc(commit->crc, &cstate.tag, sizeof(cstate.tag)); + cstate.crc = lfs_tole32(commit->crc); int err = lfs_bd_prog(lfs, &lfs->pcache, &lfs->rcache, false, - commit->block, commit->off, &data, size); + commit->block, commit->off, &cstate, sizeof(cstate)); if (err) { return err; } // keep track of non-padding checksum to verify if (off1 == 0) { - //off1 = commit->off + sizeof(uint32_t); - off1 = commit->off + size-sizeof(uint32_t); + off1 = off; crc1 = commit->crc; } - commit->off += sizeof(tag)+lfs_tag_size(tag); + commit->off = noff; // perturb valid bit? - commit->ptag = tag | (0x80000000 & ~lfs_frombe32(etag)); + commit->ptag = ctag | (0x80000000 & ~lfs_frombe32(etag)); // reset crc for next commit commit->crc = 0xffffffff; } diff --git a/lfs.h b/lfs.h index ab534526..c52e62c6 100644 --- a/lfs.h +++ b/lfs.h @@ -112,6 +112,8 @@ enum lfs_type { LFS_TYPE_SOFTTAIL = 0x600, LFS_TYPE_HARDTAIL = 0x601, LFS_TYPE_MOVESTATE = 0x7ff, + LFS_TYPE_COMMITCRC = 0x502, + LFS_TYPE_NPROGCRC = 0x5ff, // internal chip sources LFS_FROM_NOOP = 0x000, diff --git a/scripts/readmdir.py b/scripts/readmdir.py index cf6d9d88..e86b1d97 100755 --- a/scripts/readmdir.py +++ b/scripts/readmdir.py @@ -24,6 +24,7 @@ TAG_TYPES = { 'gstate': (0x700, 0x700), 'movestate': (0x7ff, 0x7ff), 'crc': (0x700, 0x500), + 'nprogcrc': (0x7ff, 0x5ff), } class Tag: @@ -125,9 +126,10 @@ class Tag: return ntag def typerepr(self): - if self.is_('crc') and getattr(self, 'crc', 0xffffffff) != 0xffffffff: + if (self.is_('crc') and not self.is_('nprogcrc') and + getattr(self, 'crc', 0xffffffff) != 0xffffffff): crc_status = ' (bad)' - elif self.is_('crc') and getattr(self, 'erased', False): + elif self.is_('nprogcrc') and getattr(self, 'erased', False): crc_status = ' (era)' else: crc_status = '' @@ -186,6 +188,8 @@ class MetadataPair: self.rev, = struct.unpack(' Date: Wed, 7 Dec 2022 23:02:19 -0600 Subject: [PATCH 04/12] Continued implementation of forward-crcs, adopted new test runners This fixes most of the remaining bugs (except one with multiple padding commits + noop erases in test_badblocks), with some other code tweaks. The biggest change was dropping reliance on end-of-block commits to know when to stop parsing commits. We can just continue to parse tags and rely on the crc for catch bad commits, avoiding a backwards-compatiblity hiccup. So no new commit tag. Also renamed nprogcrc -> fcrc and commitcrc -> ccrc and made naming in the code a bit more consistent. --- bd/lfs_emubd.c | 9 +- lfs.c | 290 ++++++++++++++++++++++---------------------- lfs.h | 4 +- scripts/readmdir.py | 17 ++- 4 files changed, 161 insertions(+), 159 deletions(-) diff --git a/bd/lfs_emubd.c b/bd/lfs_emubd.c index 97bcf035..29925538 100644 --- a/bd/lfs_emubd.c +++ b/bd/lfs_emubd.c @@ -584,13 +584,14 @@ lfs_emubd_swear_t lfs_emubd_wear(const struct lfs_config *cfg, wear = 0; } - LFS_EMUBD_TRACE("lfs_emubd_wear -> %"PRIu32, wear); + LFS_EMUBD_TRACE("lfs_emubd_wear -> %"PRIi32, wear); return wear; } int lfs_emubd_setwear(const struct lfs_config *cfg, lfs_block_t block, lfs_emubd_wear_t wear) { - LFS_EMUBD_TRACE("lfs_emubd_setwear(%p, %"PRIu32")", (void*)cfg, block); + LFS_EMUBD_TRACE("lfs_emubd_setwear(%p, %"PRIu32", %"PRIi32")", + (void*)cfg, block, wear); lfs_emubd_t *bd = cfg->context; // check if block is valid @@ -599,12 +600,12 @@ int lfs_emubd_setwear(const struct lfs_config *cfg, // set the wear lfs_emubd_block_t *b = lfs_emubd_mutblock(cfg, &bd->blocks[block]); if (!b) { - LFS_EMUBD_TRACE("lfs_emubd_setwear -> %"PRIu32, LFS_ERR_NOMEM); + LFS_EMUBD_TRACE("lfs_emubd_setwear -> %d", LFS_ERR_NOMEM); return LFS_ERR_NOMEM; } b->wear = wear; - LFS_EMUBD_TRACE("lfs_emubd_setwear -> %"PRIu32, 0); + LFS_EMUBD_TRACE("lfs_emubd_setwear -> %d", 0); return 0; } diff --git a/lfs.c b/lfs.c index 853a646f..7e9e53be 100644 --- a/lfs.c +++ b/lfs.c @@ -133,6 +133,7 @@ static int lfs_bd_cmp(lfs_t *lfs, for (lfs_off_t i = 0; i < size; i += diff) { uint8_t dat[8]; + diff = lfs_min(size-i, sizeof(dat)); int err = lfs_bd_read(lfs, pcache, rcache, hint-i, @@ -437,21 +438,23 @@ static inline void lfs_gstate_tole32(lfs_gstate_t *a) { } #endif -// operations on estate in CRC tags -struct lfs_estate { +// operations on forward-CRCs used to track erased state +struct lfs_fcrc { lfs_size_t size; uint32_t crc; }; -static void lfs_estate_fromle32(struct lfs_estate *estate) { - estate->size = lfs_fromle32(estate->size); - estate->crc = lfs_fromle32(estate->crc); +static void lfs_fcrc_fromle32(struct lfs_fcrc *fcrc) { + fcrc->size = lfs_fromle32(fcrc->size); + fcrc->crc = lfs_fromle32(fcrc->crc); } -static void lfs_estate_tole32(struct lfs_estate *estate) { - estate->size = lfs_tole32(estate->size); - estate->crc = lfs_tole32(estate->crc); +#ifndef LFS_READONLY +static void lfs_fcrc_tole32(struct lfs_fcrc *fcrc) { + fcrc->size = lfs_tole32(fcrc->size); + fcrc->crc = lfs_tole32(fcrc->crc); } +#endif // other endianness operations static void lfs_ctz_fromle32(struct lfs_ctz *ctz) { @@ -1075,8 +1078,8 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, bool tempsplit = false; lfs_stag_t tempbesttag = besttag; - bool hasestate = false; - struct lfs_estate estate; + bool hasfcrc = false; + struct lfs_fcrc fcrc; dir->rev = lfs_tole32(dir->rev); uint32_t crc = lfs_crc(0xffffffff, &dir->rev, sizeof(dir->rev)); @@ -1144,61 +1147,85 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->tail[1] = temptail[1]; dir->split = tempsplit; - if (hasestate) { - // check if the next page is erased - // + // check for an fcrc matching the next prog's erased state, if + // this failed most likely a previous prog was interrupted, we + // need a new erase + if (hasfcrc) { // this may look inefficient, but since cache_size is // probably > prog_size, the data will always remain in // cache for the next iteration - // first we get a tag-worth of bits, this is so we can - // tweak our current tag to force future writes to be - // different than the erased state - lfs_tag_t etag; + // first read the leading byte, this always contains a bit + // we can perturb to avoid writes that don't change the fcrc + uint8_t eperturb; err = lfs_bd_read(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], dir->off, &etag, sizeof(etag)); + dir->pair[0], dir->off, &eperturb, 1); if (err && err != LFS_ERR_CORRUPT) { return err; } // perturb valid bit? - dir->etag |= 0x80000000 & ~lfs_frombe32(etag); + dir->etag |= (0x80 & ~eperturb) << 24; - // crc the rest the full prog_size, including etag in case - // the actual esize is < tag size (though this shouldn't - // happen normally) - uint32_t tcrc = 0xffffffff; + // crc the full prog_size, don't bother avoiding a reread + // of the eperturb, it should still be in our cache + uint32_t ecrc = 0xffffffff; err = lfs_bd_crc(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], dir->off, estate.size, &tcrc); + dir->pair[0], dir->off, fcrc.size, &ecrc); if (err && err != LFS_ERR_CORRUPT) { return err; } - if (tcrc == estate.crc) { + // found beginning of erased part? + if (ecrc == fcrc.crc) { dir->erased = true; break; } - } else if (lfs_tag_chunk(tag) < 2) { - // for backwards compatibility we fall through here on - // unrecognized tags, leaving it up to the CRC to reject - // bad commits - } else { - // end of block commit - dir->erased = false; - break; } // reset crc crc = 0xffffffff; - hasestate = false; - } else { - // crc the entry first, hopefully leaving it in the cache - err = lfs_bd_crc(lfs, + hasfcrc = false; + continue; + } + + // crc the entry first, hopefully leaving it in the cache + err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], off+sizeof(tag), + lfs_tag_dsize(tag)-sizeof(tag), &crc); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; + } + return err; + } + + // directory modification tags? + if (lfs_tag_type1(tag) == LFS_TYPE_NAME) { + // increase count of files if necessary + if (lfs_tag_id(tag) >= tempcount) { + tempcount = lfs_tag_id(tag) + 1; + } + } else if (lfs_tag_type1(tag) == LFS_TYPE_SPLICE) { + tempcount += lfs_tag_splice(tag); + + if (tag == (LFS_MKTAG(LFS_TYPE_DELETE, 0, 0) | + (LFS_MKTAG(0, 0x3ff, 0) & tempbesttag))) { + tempbesttag |= 0x80000000; + } else if (tempbesttag != -1 && + lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { + tempbesttag += LFS_MKTAG(0, lfs_tag_splice(tag), 0); + } + } else if (lfs_tag_type1(tag) == LFS_TYPE_TAIL) { + tempsplit = (lfs_tag_chunk(tag) & 1); + + err = lfs_bd_read(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), - lfs_tag_dsize(tag)-sizeof(tag), &crc); + dir->pair[0], off+sizeof(tag), &temptail, 8); if (err) { if (err == LFS_ERR_CORRUPT) { dir->erased = false; @@ -1206,78 +1233,47 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, } return err; } - - // directory modification tags? - if (lfs_tag_type1(tag) == LFS_TYPE_NAME) { - // increase count of files if necessary - if (lfs_tag_id(tag) >= tempcount) { - tempcount = lfs_tag_id(tag) + 1; + lfs_pair_fromle32(temptail); + } else if (lfs_tag_type3(tag) == LFS_TYPE_FCRC) { + err = lfs_bd_read(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], off+sizeof(tag), + &fcrc, sizeof(fcrc)); + if (err) { + if (err == LFS_ERR_CORRUPT) { + dir->erased = false; + break; } - } else if (lfs_tag_type1(tag) == LFS_TYPE_SPLICE) { - tempcount += lfs_tag_splice(tag); - - if (tag == (LFS_MKTAG(LFS_TYPE_DELETE, 0, 0) | - (LFS_MKTAG(0, 0x3ff, 0) & tempbesttag))) { - tempbesttag |= 0x80000000; - } else if (tempbesttag != -1 && - lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { - tempbesttag += LFS_MKTAG(0, lfs_tag_splice(tag), 0); - } - } else if (lfs_tag_type1(tag) == LFS_TYPE_TAIL) { - tempsplit = (lfs_tag_chunk(tag) & 1); - - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), - &temptail, sizeof(temptail)); - if (err) { - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } - } - lfs_pair_fromle32(temptail); - } else if (lfs_tag_type1(tag) == LFS_TYPE_NPROGCRC) { - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], off+sizeof(tag), - &estate, sizeof(estate)); - if (err) { - if (err == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } - } - lfs_estate_fromle32(&estate); - hasestate = true; } - // found a match for our fetcher? - if ((fmask & tag) == (fmask & ftag)) { - int res = cb(data, tag, &(struct lfs_diskoff){ - dir->pair[0], off+sizeof(tag)}); - if (res < 0) { - if (res == LFS_ERR_CORRUPT) { - dir->erased = false; - break; - } - return res; - } + lfs_fcrc_fromle32(&fcrc); + hasfcrc = true; + } - if (res == LFS_CMP_EQ) { - // found a match - tempbesttag = tag; - } else if ((LFS_MKTAG(0x7ff, 0x3ff, 0) & tag) == - (LFS_MKTAG(0x7ff, 0x3ff, 0) & tempbesttag)) { - // found an identical tag, but contents didn't match - // this must mean that our besttag has been overwritten - tempbesttag = -1; - } else if (res == LFS_CMP_GT && - lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { - // found a greater match, keep track to keep - // things sorted - tempbesttag = tag | 0x80000000; + // found a match for our fetcher? + if ((fmask & tag) == (fmask & ftag)) { + int res = cb(data, tag, &(struct lfs_diskoff){ + dir->pair[0], off+sizeof(tag)}); + if (res < 0) { + if (res == LFS_ERR_CORRUPT) { + dir->erased = false; + break; } + return res; + } + + if (res == LFS_CMP_EQ) { + // found a match + tempbesttag = tag; + } else if ((LFS_MKTAG(0x7ff, 0x3ff, 0) & tag) == + (LFS_MKTAG(0x7ff, 0x3ff, 0) & tempbesttag)) { + // found an identical tag, but contents didn't match + // this must mean that our besttag has been overwritten + tempbesttag = -1; + } else if (res == LFS_CMP_GT && + lfs_tag_id(tag) <= lfs_tag_id(tempbesttag)) { + // found a greater match, keep track to keep things sorted + tempbesttag = tag | 0x80000000; } } } @@ -1584,11 +1580,12 @@ static int lfs_dir_commitattr(lfs_t *lfs, struct lfs_commit *commit, #endif #ifndef LFS_READONLY + static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // align to program units // // this gets a bit complex as we have two types of crcs: - // - 4-word crc with estate to check following prog (middle of block) + // - 5-word crc with fcrc to check following prog (middle of block) // - 2-word crc with no following prog (end of block) const lfs_off_t end = lfs_alignup( lfs_min(commit->off + 5*sizeof(uint32_t), lfs->cfg->block_size), @@ -1601,75 +1598,77 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // padding is not crced, which lets fetches skip padding but // makes committing a bit more complicated while (commit->off < end) { - lfs_off_t off = commit->off + sizeof(lfs_tag_t); - lfs_off_t noff = lfs_min(end - off, 0x3fe) + off; + lfs_off_t noff = lfs_min(end - (commit->off+sizeof(lfs_tag_t)), 0x3fe) + + (commit->off+sizeof(lfs_tag_t)); + // too large for crc tag? need padding commits if (noff < end) { - noff = lfs_min(noff, end - 2*sizeof(uint32_t)); + noff = lfs_min(noff, end - 5*sizeof(uint32_t)); } - // clamp erase size to tag size, this gives us the full tag as potential - // to intentionally invalidate erase CRCs - const lfs_size_t esize = lfs_max( - lfs->cfg->prog_size, sizeof(lfs_tag_t)); - lfs_tag_t etag = 0; - // space for estate? also only emit on last commit in padding commits - if (noff <= lfs->cfg->block_size - esize) { - // first we get a tag-worth of bits, this is so we can - // tweak our current tag to force future writes to be - // different than the erased state + // space for fcrc? + uint8_t eperturb = -1; + if (noff < lfs->cfg->block_size) { + // first read the leading byte, this always contains a bit + // we can perturb to avoid writes that don't change the fcrc int err = lfs_bd_read(lfs, - NULL, &lfs->rcache, esize, - commit->block, noff, &etag, sizeof(etag)); + NULL, &lfs->rcache, lfs->cfg->prog_size, + commit->block, noff, &eperturb, 1); if (err && err != LFS_ERR_CORRUPT) { return err; } - // find expected erased state - uint32_t ecrc = 0xffffffff; + // find the expected fcrc, don't bother avoiding a reread + // of the eperturb, it should still be in our cache + struct lfs_fcrc fcrc = { + // if our commit is a padding commit, we only care about + // invalidating outdated commits if there is a partial write, + // so we fcrc the minimum amount (1 byte) + .size=(noff < end ? 1 : lfs->cfg->prog_size), + .crc=0xffffffff, + }; err = lfs_bd_crc(lfs, - NULL, &lfs->rcache, esize, - commit->block, noff, esize, &ecrc); + NULL, &lfs->rcache, fcrc.size, + commit->block, noff, fcrc.size, &fcrc.crc); if (err && err != LFS_ERR_CORRUPT) { return err; } - struct lfs_estate estate = {.size = esize, .crc = ecrc}; - lfs_estate_tole32(&estate); + lfs_fcrc_tole32(&fcrc); err = lfs_dir_commitattr(lfs, commit, - LFS_MKTAG(LFS_TYPE_NPROGCRC, 0x3ff, sizeof(estate)), - &estate); + LFS_MKTAG(LFS_TYPE_FCRC, 0x3ff, sizeof(struct lfs_fcrc)), + &fcrc); if (err) { return err; } } - // build crc state - off = commit->off + sizeof(lfs_tag_t); + // build commit crc struct { lfs_tag_t tag; uint32_t crc; - } cstate; - lfs_tag_t ctag = LFS_MKTAG(LFS_TYPE_COMMITCRC, 0x3ff, noff-off); - cstate.tag = lfs_tobe32(ctag ^ commit->ptag); - commit->crc = lfs_crc(commit->crc, &cstate.tag, sizeof(cstate.tag)); - cstate.crc = lfs_tole32(commit->crc); + } ccrc; + lfs_tag_t ntag = LFS_MKTAG(LFS_TYPE_CCRC, 0x3ff, + noff - (commit->off+sizeof(lfs_tag_t))); + ccrc.tag = lfs_tobe32(ntag ^ commit->ptag); + commit->crc = lfs_crc(commit->crc, &ccrc.tag, sizeof(lfs_tag_t)); + ccrc.crc = lfs_tole32(commit->crc); int err = lfs_bd_prog(lfs, &lfs->pcache, &lfs->rcache, false, - commit->block, commit->off, &cstate, sizeof(cstate)); + commit->block, commit->off, &ccrc, sizeof(ccrc)); if (err) { return err; } // keep track of non-padding checksum to verify if (off1 == 0) { - off1 = off; + off1 = commit->off + sizeof(lfs_tag_t); crc1 = commit->crc; } commit->off = noff; // perturb valid bit? - commit->ptag = ctag | (0x80000000 & ~lfs_frombe32(etag)); + commit->ptag = ntag | ((0x80 & ~eperturb) << 24); // reset crc for next commit commit->crc = 0xffffffff; } @@ -1693,12 +1692,12 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { return err; } - // check against known crc for non-padding commits + // check non-padding commits against known crc if (crc != crc1) { return LFS_ERR_CORRUPT; } - // make sure to check crc in case we happened to pick + // make sure to check crc in case we happen to pick // up an unrelated crc (frozen block?) err = lfs_bd_crc(lfs, NULL, &lfs->rcache, sizeof(uint32_t), @@ -4556,7 +4555,8 @@ static lfs_stag_t lfs_fs_parent(lfs_t *lfs, const lfs_block_t pair[2], #ifndef LFS_READONLY static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans) { - LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0 || orphans >= 0); + LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0x000 || orphans >= 0); + LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) < 0x3ff || orphans <= 0); lfs->gstate.tag += orphans; lfs->gstate.tag = ((lfs->gstate.tag & ~LFS_MKTAG(0x800, 0, 0)) | ((uint32_t)lfs_gstate_hasorphans(&lfs->gstate) << 31)); @@ -4587,6 +4587,10 @@ static int lfs_fs_demove(lfs_t *lfs) { lfs->gdisk.pair[1], lfs_tag_id(lfs->gdisk.tag)); + // no other gstate is supported at this time, so if we found something else + // something most likely went wrong in gstate calculation + LFS_ASSERT(lfs_tag_type3(lfs->gdisk.tag) == LFS_TYPE_DELETE); + // fetch and delete the moved entry lfs_mdir_t movedir; int err = lfs_dir_fetch(lfs, &movedir, lfs->gdisk.pair); diff --git a/lfs.h b/lfs.h index c52e62c6..82f69854 100644 --- a/lfs.h +++ b/lfs.h @@ -112,8 +112,8 @@ enum lfs_type { LFS_TYPE_SOFTTAIL = 0x600, LFS_TYPE_HARDTAIL = 0x601, LFS_TYPE_MOVESTATE = 0x7ff, - LFS_TYPE_COMMITCRC = 0x502, - LFS_TYPE_NPROGCRC = 0x5ff, + LFS_TYPE_CCRC = 0x502, + LFS_TYPE_FCRC = 0x5ff, // internal chip sources LFS_FROM_NOOP = 0x000, diff --git a/scripts/readmdir.py b/scripts/readmdir.py index e86b1d97..d4f39c44 100755 --- a/scripts/readmdir.py +++ b/scripts/readmdir.py @@ -23,8 +23,8 @@ TAG_TYPES = { 'hardtail': (0x7ff, 0x601), 'gstate': (0x700, 0x700), 'movestate': (0x7ff, 0x7ff), - 'crc': (0x700, 0x500), - 'nprogcrc': (0x7ff, 0x5ff), + 'crc': (0x780, 0x500), + 'fcrc': (0x7ff, 0x5ff), } class Tag: @@ -126,10 +126,9 @@ class Tag: return ntag def typerepr(self): - if (self.is_('crc') and not self.is_('nprogcrc') and - getattr(self, 'crc', 0xffffffff) != 0xffffffff): + if (self.is_('crc') and getattr(self, 'crc', 0xffffffff) != 0xffffffff): crc_status = ' (bad)' - elif self.is_('nprogcrc') and getattr(self, 'erased', False): + elif self.is_('fcrc') and getattr(self, 'erased', False): crc_status = ' (era)' else: crc_status = '' @@ -203,7 +202,7 @@ class MetadataPair: tag = Tag((int(tag) ^ ntag) & 0x7fffffff) tag.off = off + 4 tag.data = block[off+4:off+tag.dsize] - if tag.is_('crc') and not tag.is_('nprogcrc'): + if tag.is_('crc'): crc = binascii.crc32(block[off:off+2*4], crc) else: crc = binascii.crc32(block[off:off+tag.dsize], crc) @@ -212,7 +211,7 @@ class MetadataPair: self.all_.append(tag) - if tag.is_('nprogcrc') and len(tag.data) == 8: + if tag.is_('fcrc') and len(tag.data) == 8: etag = tag estate = struct.unpack(' Date: Sat, 10 Dec 2022 13:16:52 -0600 Subject: [PATCH 05/12] Reverted removal of 1-bit counter threaded through tags Initially I thought the fcrc would be sufficient for all of the end-of-commit context, since indicating that there is a new commit is a simple as invalidating the fcrc. But it turns out there are cases that make this impossible. The surprising, and actually common, case, is that of an fcrc that will end up containing a full commit. This is common as soon as the prog_size is big, as small commits are padded to the prog_size at minimum. .------------------. \ | metadata | | | | | | | +-. |------------------| | | | foward CRC ------------. |------------------| / | | | commit CRC -----' | |------------------| | | padding | | | | | |------------------| \ \ | | metadata | | | | | | +-. | | | | | | +-' |------------------| / | | | commit CRC --------' | |------------------| | | | / '------------------' When the commit + crc is all contained in the fcrc, something silly happens with the math behind crcs. Everything in the commit gets canceled out: crc(m) = m(x) x^|P|-1 mod P(x) m ++ crc(m) = m(x) x^|P|-1 + (m(x) x^|P|-1 mod P(x)) crc(m ++ crc(m)) = (m(x) x^|P|-1 + (m(x) x^|P|-1 mod P(x))) x^|P|-1 mod P(x) crc(m ++ crc(m)) = (m(x) x^|P|-1 + m(x) x^|P|-1) x^|P|-1 mod P(x) crc(m ++ crc(m)) = 0 * x^|P|-1 mod P(x) This is the reason the crc of a message + naive crc is zero. Even with an initializer/bit-fiddling, the crc of the whole commit ends up as some constant. So no manipulation of the commit can change the fcrc... But even if this did work, or we changed this scheme to use two different checksums, it would still require calculating the fcrc of the whole commit to know if we need to tweak the first bit to invalidate the unlikely-but-problematic case where we happen to match the fcrc. This would add a large amount of complexity to the commit code. It's much simpler and cheaper to keep the 1-bit counter in the tag, even if it adds another moving part to the system. --- lfs.c | 168 ++++++++++++++++++++------------------------ lfs.h | 2 +- scripts/readmdir.py | 38 +++++----- 3 files changed, 98 insertions(+), 110 deletions(-) diff --git a/lfs.c b/lfs.c index 7e9e53be..12864c79 100644 --- a/lfs.c +++ b/lfs.c @@ -1078,6 +1078,8 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, bool tempsplit = false; lfs_stag_t tempbesttag = besttag; + // assume not erased until proven otherwise + bool maybeerased = false; bool hasfcrc = false; struct lfs_fcrc fcrc; @@ -1095,24 +1097,26 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, if (err) { if (err == LFS_ERR_CORRUPT) { // can't continue? - dir->erased = false; break; } return err; } crc = lfs_crc(crc, &tag, sizeof(tag)); - tag = (lfs_frombe32(tag) ^ ptag) & 0x7fffffff; + tag = lfs_frombe32(tag) ^ ptag; + // next commit not yet programmed? + if (!lfs_tag_isvalid(tag)) { + maybeerased = true; + break; // out of range? - if (off + lfs_tag_dsize(tag) > lfs->cfg->block_size) { - dir->erased = false; + } else if (off + lfs_tag_dsize(tag) > lfs->cfg->block_size) { break; } ptag = tag; - if (lfs_tag_type2(tag) == LFS_TYPE_CRC) { + if (lfs_tag_type2(tag) == LFS_TYPE_CCRC) { // check the crc attr uint32_t dcrc; err = lfs_bd_read(lfs, @@ -1120,7 +1124,6 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->pair[0], off+sizeof(tag), &dcrc, sizeof(dcrc)); if (err) { if (err == LFS_ERR_CORRUPT) { - dir->erased = false; break; } return err; @@ -1128,10 +1131,12 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dcrc = lfs_fromle32(dcrc); if (crc != dcrc) { - dir->erased = false; break; } + // reset the next bit if we need to + ptag ^= (lfs_tag_t)(lfs_tag_chunk(tag) & 1U) << 31; + // toss our crc into the filesystem seed for // pseudorandom numbers, note we use another crc here // as a collection function because it is sufficiently @@ -1147,50 +1152,14 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->tail[1] = temptail[1]; dir->split = tempsplit; - // check for an fcrc matching the next prog's erased state, if - // this failed most likely a previous prog was interrupted, we - // need a new erase - if (hasfcrc) { - // this may look inefficient, but since cache_size is - // probably > prog_size, the data will always remain in - // cache for the next iteration - - // first read the leading byte, this always contains a bit - // we can perturb to avoid writes that don't change the fcrc - uint8_t eperturb; - err = lfs_bd_read(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], dir->off, &eperturb, 1); - if (err && err != LFS_ERR_CORRUPT) { - return err; - } - - // perturb valid bit? - dir->etag |= (0x80 & ~eperturb) << 24; - - // crc the full prog_size, don't bother avoiding a reread - // of the eperturb, it should still be in our cache - uint32_t ecrc = 0xffffffff; - err = lfs_bd_crc(lfs, - NULL, &lfs->rcache, lfs->cfg->block_size, - dir->pair[0], dir->off, fcrc.size, &ecrc); - if (err && err != LFS_ERR_CORRUPT) { - return err; - } - - // found beginning of erased part? - if (ecrc == fcrc.crc) { - dir->erased = true; - break; - } - } - // reset crc crc = 0xffffffff; - hasfcrc = false; continue; } + // fcrc is only valid when last tag was a crc + hasfcrc = false; + // crc the entry first, hopefully leaving it in the cache err = lfs_bd_crc(lfs, NULL, &lfs->rcache, lfs->cfg->block_size, @@ -1198,7 +1167,6 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, lfs_tag_dsize(tag)-sizeof(tag), &crc); if (err) { if (err == LFS_ERR_CORRUPT) { - dir->erased = false; break; } return err; @@ -1228,7 +1196,6 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->pair[0], off+sizeof(tag), &temptail, 8); if (err) { if (err == LFS_ERR_CORRUPT) { - dir->erased = false; break; } return err; @@ -1241,7 +1208,6 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, &fcrc, sizeof(fcrc)); if (err) { if (err == LFS_ERR_CORRUPT) { - dir->erased = false; break; } } @@ -1256,7 +1222,6 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, dir->pair[0], off+sizeof(tag)}); if (res < 0) { if (res == LFS_ERR_CORRUPT) { - dir->erased = false; break; } return res; @@ -1278,35 +1243,54 @@ static lfs_stag_t lfs_dir_fetchmatch(lfs_t *lfs, } } - // consider what we have good enough - if (dir->off > 0) { - // synthetic move - if (lfs_gstate_hasmovehere(&lfs->gdisk, dir->pair)) { - if (lfs_tag_id(lfs->gdisk.tag) == lfs_tag_id(besttag)) { - besttag |= 0x80000000; - } else if (besttag != -1 && - lfs_tag_id(lfs->gdisk.tag) < lfs_tag_id(besttag)) { - besttag -= LFS_MKTAG(0, 1, 0); - } + // found no valid commits? + if (dir->off == 0) { + // try the other block? + lfs_pair_swap(dir->pair); + dir->rev = revs[(r+1)%2]; + continue; + } + + // did we end on a valid commit? we may have an erased block + dir->erased = false; + if (maybeerased && hasfcrc && dir->off % lfs->cfg->prog_size == 0) { + // check for an fcrc matching the next prog's erased state, if + // this failed most likely a previous prog was interrupted, we + // need a new erase + uint32_t fcrc_ = 0xffffffff; + int err = lfs_bd_crc(lfs, + NULL, &lfs->rcache, lfs->cfg->block_size, + dir->pair[0], dir->off, fcrc.size, &fcrc_); + if (err && err != LFS_ERR_CORRUPT) { + return err; } - // found tag? or found best id? - if (id) { - *id = lfs_min(lfs_tag_id(besttag), dir->count); - } + // found beginning of erased part? + dir->erased = (fcrc_ == fcrc.crc); + } - if (lfs_tag_isvalid(besttag)) { - return besttag; - } else if (lfs_tag_id(besttag) < dir->count) { - return LFS_ERR_NOENT; - } else { - return 0; + // synthetic move + if (lfs_gstate_hasmovehere(&lfs->gdisk, dir->pair)) { + if (lfs_tag_id(lfs->gdisk.tag) == lfs_tag_id(besttag)) { + besttag |= 0x80000000; + } else if (besttag != -1 && + lfs_tag_id(lfs->gdisk.tag) < lfs_tag_id(besttag)) { + besttag -= LFS_MKTAG(0, 1, 0); } } - // failed, try the other block? - lfs_pair_swap(dir->pair); - dir->rev = revs[(r+1)%2]; + // found tag? or found best id? + if (id) { + *id = lfs_min(lfs_tag_id(besttag), dir->count); + } + + if (lfs_tag_isvalid(besttag)) { + return besttag; + } else if (lfs_tag_id(besttag) < dir->count) { + return LFS_ERR_NOENT; + } else { + return 0; + } } LFS_ERROR("Corrupted dir pair at {0x%"PRIx32", 0x%"PRIx32"}", @@ -1598,8 +1582,9 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // padding is not crced, which lets fetches skip padding but // makes committing a bit more complicated while (commit->off < end) { - lfs_off_t noff = lfs_min(end - (commit->off+sizeof(lfs_tag_t)), 0x3fe) - + (commit->off+sizeof(lfs_tag_t)); + lfs_off_t noff = ( + lfs_min(end - (commit->off+sizeof(lfs_tag_t)), 0x3fe) + + (commit->off+sizeof(lfs_tag_t))); // too large for crc tag? need padding commits if (noff < end) { noff = lfs_min(noff, end - 5*sizeof(uint32_t)); @@ -1607,7 +1592,7 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // space for fcrc? uint8_t eperturb = -1; - if (noff < lfs->cfg->block_size) { + if (noff >= end && noff <= lfs->cfg->block_size - lfs->cfg->prog_size) { // first read the leading byte, this always contains a bit // we can perturb to avoid writes that don't change the fcrc int err = lfs_bd_read(lfs, @@ -1619,15 +1604,9 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // find the expected fcrc, don't bother avoiding a reread // of the eperturb, it should still be in our cache - struct lfs_fcrc fcrc = { - // if our commit is a padding commit, we only care about - // invalidating outdated commits if there is a partial write, - // so we fcrc the minimum amount (1 byte) - .size=(noff < end ? 1 : lfs->cfg->prog_size), - .crc=0xffffffff, - }; + struct lfs_fcrc fcrc = {.size=lfs->cfg->prog_size, .crc=0xffffffff}; err = lfs_bd_crc(lfs, - NULL, &lfs->rcache, fcrc.size, + NULL, &lfs->rcache, lfs->cfg->prog_size, commit->block, noff, fcrc.size, &fcrc.crc); if (err && err != LFS_ERR_CORRUPT) { return err; @@ -1647,7 +1626,8 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { lfs_tag_t tag; uint32_t crc; } ccrc; - lfs_tag_t ntag = LFS_MKTAG(LFS_TYPE_CCRC, 0x3ff, + lfs_tag_t ntag = LFS_MKTAG( + LFS_TYPE_CCRC + (((uint8_t)~eperturb) >> 7), 0x3ff, noff - (commit->off+sizeof(lfs_tag_t))); ccrc.tag = lfs_tobe32(ntag ^ commit->ptag); commit->crc = lfs_crc(commit->crc, &ccrc.tag, sizeof(lfs_tag_t)); @@ -1668,15 +1648,19 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { commit->off = noff; // perturb valid bit? - commit->ptag = ntag | ((0x80 & ~eperturb) << 24); + commit->ptag = ntag ^ ((0x80 & ~eperturb) << 24); // reset crc for next commit commit->crc = 0xffffffff; - } - // flush buffers - int err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false); - if (err) { - return err; + // manually flush here since we don't prog the padding, this confuses + // the caching layer + if (noff >= end || noff >= lfs->pcache.off + lfs->cfg->cache_size) { + // flush buffers + int err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false); + if (err) { + return err; + } + } } // successful commit, check checksums to make sure @@ -1685,7 +1669,7 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) { // case if they are corrupted we would have had to compact anyways lfs_off_t off = commit->begin; uint32_t crc = 0xffffffff; - err = lfs_bd_crc(lfs, + int err = lfs_bd_crc(lfs, NULL, &lfs->rcache, off1+sizeof(uint32_t), commit->block, off, off1-off, &crc); if (err) { diff --git a/lfs.h b/lfs.h index 82f69854..a1de75e8 100644 --- a/lfs.h +++ b/lfs.h @@ -112,7 +112,7 @@ enum lfs_type { LFS_TYPE_SOFTTAIL = 0x600, LFS_TYPE_HARDTAIL = 0x601, LFS_TYPE_MOVESTATE = 0x7ff, - LFS_TYPE_CCRC = 0x502, + LFS_TYPE_CCRC = 0x500, LFS_TYPE_FCRC = 0x5ff, // internal chip sources diff --git a/scripts/readmdir.py b/scripts/readmdir.py index d4f39c44..98816df9 100755 --- a/scripts/readmdir.py +++ b/scripts/readmdir.py @@ -23,7 +23,8 @@ TAG_TYPES = { 'hardtail': (0x7ff, 0x601), 'gstate': (0x700, 0x700), 'movestate': (0x7ff, 0x7ff), - 'crc': (0x780, 0x500), + 'crc': (0x700, 0x500), + 'ccrc': (0x780, 0x500), 'fcrc': (0x7ff, 0x5ff), } @@ -121,12 +122,13 @@ class Tag: ntag = Tag(self.type, nid, self.size) if hasattr(self, 'off'): ntag.off = self.off if hasattr(self, 'data'): ntag.data = self.data - if hasattr(self, 'crc'): ntag.crc = self.crc + if hasattr(self, 'ccrc'): ntag.crc = self.crc if hasattr(self, 'erased'): ntag.erased = self.erased return ntag def typerepr(self): - if (self.is_('crc') and getattr(self, 'crc', 0xffffffff) != 0xffffffff): + if (self.is_('ccrc') + and getattr(self, 'ccrc', 0xffffffff) != 0xffffffff): crc_status = ' (bad)' elif self.is_('fcrc') and getattr(self, 'erased', False): crc_status = ' (era)' @@ -187,8 +189,8 @@ class MetadataPair: self.rev, = struct.unpack(' Date: Sat, 10 Dec 2022 22:38:20 -0600 Subject: [PATCH 06/12] Fixed issue where deorphan could get stuck circling between two half-orphans This of course should never happen normally, two half-orphans requires two parents, which is disallowed in littlefs for this reason. But it can happen if there is an outdated half-orphan later in the metadata linked-list. The two half-orphans can cause the deorphan step to get stuck, constantly "fixing" the first half-orphan before it has a chance to remove the problematic, outdated half-orphan later in the list. The solution here is to do a full check for half-orphans before restarting the half-orphan loop. This strategy has the potential to visit more metadata blocks unnecessarily, but avoids situations where removing a later half-orphan will eventually cause an earlier half-orphan to resolve itself. Found with heuristic powerloss testing with test_relocations_reentrant_renames after 192 nested powerlosses. --- lfs.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lfs.c b/lfs.c index 12864c79..7a4b41a8 100644 --- a/lfs.c +++ b/lfs.c @@ -4603,7 +4603,6 @@ static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss) { int8_t found = 0; -restart: // Check for orphans in two separate passes: // - 1 for half-orphans (relocations) // - 2 for full-orphans (removes/renames) @@ -4612,10 +4611,12 @@ restart: // references to full-orphans, effectively hiding them from the deorphan // search. // - for (int pass = 0; pass < 2; pass++) { + int pass = 0; + while (pass < 2) { // Fix any orphans lfs_mdir_t pdir = {.split = true, .tail = {0, 1}}; lfs_mdir_t dir; + bool moreorphans = false; // iterate over all directory directory entries while (!lfs_pair_isnull(pdir.tail)) { @@ -4676,7 +4677,7 @@ restart: // did our commit create more orphans? if (state == LFS_OK_ORPHANED) { - goto restart; + moreorphans = true; } // refetch tail @@ -4712,7 +4713,7 @@ restart: // did our commit create more orphans? if (state == LFS_OK_ORPHANED) { - goto restart; + moreorphans = true; } // refetch tail @@ -4722,6 +4723,8 @@ restart: pdir = dir; } + + pass = moreorphans ? 0 : pass+1; } // mark orphans as fixed From f0cc1db793b3e1b35ae62a04df666703beb10476 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 19 Apr 2023 15:41:43 -0500 Subject: [PATCH 07/12] Tweaked changeprefix.py to not rename dir component in paths This wasn't implemented correctly anyways, as it would need to recursively rename directories that may not exist. Things would also get a bit complicated if only some files in a directory were renamed. Doable, but not needed for our use case. For now just ignore any directory components. Though this may be worth changing if the source directory structure becomes more complicated in the future (maybe with a -r/--recursive flag?). --- scripts/changeprefix.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/changeprefix.py b/scripts/changeprefix.py index 51844c05..381a4568 100755 --- a/scripts/changeprefix.py +++ b/scripts/changeprefix.py @@ -107,7 +107,10 @@ def main(from_prefix, to_prefix, paths=[], *, elif no_renames: to_path = from_path else: - to_path, _ = changeprefix(from_prefix, to_prefix, from_path) + to_path = os.path.join( + os.path.dirname(from_path), + changeprefix(from_prefix, to_prefix, + os.path.basename(from_path))[0]) # rename contents changefile(from_prefix, to_prefix, from_path, to_path, From 116332d3f7e33e2d4ada3b4896024b1d44567593 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 20 Apr 2023 02:13:58 -0500 Subject: [PATCH 08/12] Added tests for forwards and backwards disk compatibility This is a bit tricky since we need two different version of littlefs in order to test for most compatibility concerns. Fortunately we already have scripts/changeprefix.py for version-specific symbols, so it's not that hard to link in the previous version of littlefs in CI as a separate set of symbols, "lfsp_" in this case. So that we can at least test the compatibility tests locally, I've added an ifdef against the expected define "LFSP" to define a set of aliases mapping "lfsp_" symbols to "lfs_" symbols. This is manual at the moment, and a bit hacky, but gets the job done. --- Also changed BUILDDIR creation to derive subdirectories from a few Makefile variables. This makes the subdirectories less manual and more flexible for things like LFSP. Note this wasn't possible until BUILDDIR was changed to default to "." when omitted. --- Makefile | 27 +- tests/test_compat.toml | 1278 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 1294 insertions(+), 11 deletions(-) create mode 100644 tests/test_compat.toml diff --git a/Makefile b/Makefile index 50167156..24865e5e 100644 --- a/Makefile +++ b/Makefile @@ -1,15 +1,5 @@ -ifdef BUILDDIR -# bit of a hack, but we want to make sure BUILDDIR directory structure -# is correct before any commands -$(if $(findstring n,$(MAKEFLAGS)),, $(shell mkdir -p \ - $(BUILDDIR)/ \ - $(BUILDDIR)/bd \ - $(BUILDDIR)/runners \ - $(BUILDDIR)/tests \ - $(BUILDDIR)/benches)) -endif +# overrideable build dir, default is in-place BUILDDIR ?= . - # overridable target/src/tools/flags/etc ifneq ($(wildcard test.c main.c),) TARGET ?= $(BUILDDIR)/lfs @@ -163,6 +153,18 @@ TESTFLAGS += --perf-path="$(PERF)" BENCHFLAGS += --perf-path="$(PERF)" endif +# this is a bit of a hack, but we want to make sure the BUILDDIR +# directory structure is correct before we run any commands +ifneq ($(BUILDDIR),.) +$(if $(findstring n,$(MAKEFLAGS)),, $(shell mkdir -p \ + $(addprefix $(BUILDDIR)/,$(dir \ + $(SRC) \ + $(TESTS) \ + $(TEST_SRC) \ + $(BENCHES) \ + $(BENCH_SRC))))) +endif + # commands @@ -514,6 +516,9 @@ $(BUILDDIR)/runners/bench_runner: $(BENCH_OBJ) $(BUILDDIR)/%.o $(BUILDDIR)/%.ci: %.c $(CC) -c -MMD $(CFLAGS) $< -o $(BUILDDIR)/$*.o +$(BUILDDIR)/%.o $(BUILDDIR)/%.ci: $(BUILDDIR)/%.c + $(CC) -c -MMD $(CFLAGS) $< -o $(BUILDDIR)/$*.o + $(BUILDDIR)/%.s: %.c $(CC) -S $(CFLAGS) $< -o $@ diff --git a/tests/test_compat.toml b/tests/test_compat.toml new file mode 100644 index 00000000..7af487f5 --- /dev/null +++ b/tests/test_compat.toml @@ -0,0 +1,1278 @@ +# Test for compatibility between different littlefs versions +# +# Note, these tests are a bit special. They expect to be linked against two +# different versions of littlefs: +# - lfs => the new/current version of littlefs +# - lfsp => the previous version of littlefs +# +# If lfsp is not linked, and LFSP is not defined, these tests will alias +# the relevant lfs types/functions as necessary so at least the tests can +# themselves be tested locally. +# +# But to get value from these tests, it's expected that the previous version +# of littlefs be linked in during CI, with the help of scripts/changeprefix.py +# + +# alias littlefs symbols as needed +# +# there may be a better way to do this, but oh well, explicit aliases works +code = ''' +#ifdef LFSP +#define STRINGIZE(x) STRINGIZE_(x) +#define STRINGIZE_(x) #x +#include STRINGIZE(LFSP) +#else +#define LFSP_VERSION LFS_VERSION +#define LFSP_VERSION_MAJOR LFS_VERSION_MAJOR +#define LFSP_VERSION_MINOR LFS_VERSION_MINOR +#define lfsp_t lfs_t +#define lfsp_config lfs_config +#define lfsp_format lfs_format +#define lfsp_mount lfs_mount +#define lfsp_unmount lfs_unmount +#define lfsp_dir_t lfs_dir_t +#define lfsp_info lfs_info +#define LFSP_TYPE_REG LFS_TYPE_REG +#define LFSP_TYPE_DIR LFS_TYPE_DIR +#define lfsp_mkdir lfs_mkdir +#define lfsp_dir_open lfs_dir_open +#define lfsp_dir_read lfs_dir_read +#define lfsp_dir_close lfs_dir_close +#define lfsp_file_t lfs_file_t +#define LFSP_O_RDONLY LFS_O_RDONLY +#define LFSP_O_WRONLY LFS_O_WRONLY +#define LFSP_O_CREAT LFS_O_CREAT +#define LFSP_O_EXCL LFS_O_EXCL +#define LFSP_SEEK_SET LFS_SEEK_SET +#define lfsp_file_open lfs_file_open +#define lfsp_file_write lfs_file_write +#define lfsp_file_read lfs_file_read +#define lfsp_file_seek lfs_file_seek +#define lfsp_file_close lfs_file_close +#endif +''' + + + +## forward-compatibility tests ## + +# test we can mount in a new version +[cases.test_compat_forward_mount] +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // confirm the previous mount works + lfsp_mount(&lfsp, &cfgp) => 0; + lfsp_unmount(&lfsp) => 0; + + + // now test the new mount + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + lfs_unmount(&lfs) => 0; +''' + +# test we can read dirs in a new version +[cases.test_compat_forward_read_dirs] +defines.COUNT = 5 +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // write COUNT dirs + lfsp_mount(&lfsp, &cfgp) => 0; + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfsp_mkdir(&lfsp, name) => 0; + } + lfsp_unmount(&lfsp) => 0; + + + // mount the new version + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + + // can we list the directories? + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfs_dir_read(&lfs, &dir, &info) => 0; + lfs_dir_close(&lfs, &dir) => 0; + + lfs_unmount(&lfs) => 0; +''' + +# test we can read files in a new version +[cases.test_compat_forward_read_files] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 4 +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // write COUNT files + lfsp_mount(&lfsp, &cfgp) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfsp_file_open(&lfsp, &file, name, + LFSP_O_WRONLY | LFSP_O_CREAT | LFSP_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfsp_file_write(&lfsp, &file, chunk, CHUNK) => CHUNK; + } + lfsp_file_close(&lfsp, &file) => 0; + } + lfsp_unmount(&lfsp) => 0; + + + // mount the new version + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + + // can we list the files? + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_REG); + char name[8]; + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + } + + lfs_dir_read(&lfs, &dir, &info) => 0; + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfs_file_open(&lfs, &file, name, LFS_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfs_file_read(&lfs, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfs_file_close(&lfs, &file) => 0; + } + + lfs_unmount(&lfs) => 0; +''' + +# test we can read files in dirs in a new version +[cases.test_compat_forward_read_files_in_dirs] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 4 +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // write COUNT files+dirs + lfsp_mount(&lfsp, &cfgp) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[16]; + sprintf(name, "dir%03d", i); + lfsp_mkdir(&lfsp, name) => 0; + + lfsp_file_t file; + sprintf(name, "dir%03d/file%03d", i, i); + lfsp_file_open(&lfsp, &file, name, + LFSP_O_WRONLY | LFSP_O_CREAT | LFSP_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfsp_file_write(&lfsp, &file, chunk, CHUNK) => CHUNK; + } + lfsp_file_close(&lfsp, &file) => 0; + } + lfsp_unmount(&lfsp) => 0; + + + // mount the new version + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + + // can we list the directories? + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfs_dir_read(&lfs, &dir, &info) => 0; + lfs_dir_close(&lfs, &dir) => 0; + + // can we list the files? + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, name) => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_REG); + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + + lfs_dir_read(&lfs, &dir, &info) => 0; + lfs_dir_close(&lfs, &dir) => 0; + } + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_file_t file; + char name[16]; + sprintf(name, "dir%03d/file%03d", i, i); + lfs_file_open(&lfs, &file, name, LFS_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfs_file_read(&lfs, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfs_file_close(&lfs, &file) => 0; + } + + lfs_unmount(&lfs) => 0; +''' + +# test we can write dirs in a new version +[cases.test_compat_forward_write_dirs] +defines.COUNT = 10 +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // write COUNT/2 dirs + lfsp_mount(&lfsp, &cfgp) => 0; + for (lfs_size_t i = 0; i < COUNT/2; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfsp_mkdir(&lfsp, name) => 0; + } + lfsp_unmount(&lfsp) => 0; + + + // mount the new version + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + + // write another COUNT/2 dirs + for (lfs_size_t i = COUNT/2; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfs_mkdir(&lfs, name) => 0; + } + + // can we list the directories? + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfs_dir_read(&lfs, &dir, &info) => 0; + lfs_dir_close(&lfs, &dir) => 0; + + lfs_unmount(&lfs) => 0; +''' + +# test we can write files in a new version +[cases.test_compat_forward_write_files] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 2 +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // write half COUNT files + lfsp_mount(&lfsp, &cfgp) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + // write half + lfsp_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfsp_file_open(&lfsp, &file, name, + LFSP_O_WRONLY | LFSP_O_CREAT | LFSP_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE/2; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfsp_file_write(&lfsp, &file, chunk, CHUNK) => CHUNK; + } + lfsp_file_close(&lfsp, &file) => 0; + + // skip the other half but keep our prng reproducible + for (lfs_size_t j = SIZE/2; j < SIZE; j++) { + TEST_PRNG(&prng); + } + } + lfsp_unmount(&lfsp) => 0; + + + // mount the new version + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + + // write half COUNT files + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + // skip half but keep our prng reproducible + for (lfs_size_t j = 0; j < SIZE/2; j++) { + TEST_PRNG(&prng); + } + + // write the other half + lfs_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfs_file_open(&lfs, &file, name, LFS_O_WRONLY) => 0; + lfs_file_seek(&lfs, &file, SIZE/2, LFS_SEEK_SET) => SIZE/2; + + for (lfs_size_t j = SIZE/2; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfs_file_write(&lfs, &file, chunk, CHUNK) => CHUNK; + } + lfs_file_close(&lfs, &file) => 0; + } + + // can we list the files? + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_REG); + char name[8]; + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + } + + lfs_dir_read(&lfs, &dir, &info) => 0; + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfs_file_open(&lfs, &file, name, LFS_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfs_file_read(&lfs, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfs_file_close(&lfs, &file) => 0; + } + + lfs_unmount(&lfs) => 0; +''' + +# test we can write files in dirs in a new version +[cases.test_compat_forward_write_files_in_dirs] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 2 +if = 'LFS_VERSION_MAJOR == LFSP_VERSION_MAJOR' +code = ''' + // create the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_format(&lfsp, &cfgp) => 0; + + // write half COUNT files + lfsp_mount(&lfsp, &cfgp) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[16]; + sprintf(name, "dir%03d", i); + lfsp_mkdir(&lfsp, name) => 0; + + // write half + lfsp_file_t file; + sprintf(name, "dir%03d/file%03d", i, i); + lfsp_file_open(&lfsp, &file, name, + LFSP_O_WRONLY | LFSP_O_CREAT | LFSP_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE/2; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfsp_file_write(&lfsp, &file, chunk, CHUNK) => CHUNK; + } + lfsp_file_close(&lfsp, &file) => 0; + + // skip the other half but keep our prng reproducible + for (lfs_size_t j = SIZE/2; j < SIZE; j++) { + TEST_PRNG(&prng); + } + } + lfsp_unmount(&lfsp) => 0; + + + // mount the new version + lfs_t lfs; + lfs_mount(&lfs, cfg) => 0; + + // write half COUNT files + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + // skip half but keep our prng reproducible + for (lfs_size_t j = 0; j < SIZE/2; j++) { + TEST_PRNG(&prng); + } + + // write the other half + lfs_file_t file; + char name[16]; + sprintf(name, "dir%03d/file%03d", i, i); + lfs_file_open(&lfs, &file, name, LFS_O_WRONLY) => 0; + lfs_file_seek(&lfs, &file, SIZE/2, LFS_SEEK_SET) => SIZE/2; + + for (lfs_size_t j = SIZE/2; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfs_file_write(&lfs, &file, chunk, CHUNK) => CHUNK; + } + lfs_file_close(&lfs, &file) => 0; + } + + // can we list the directories? + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, "/") => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfs_dir_read(&lfs, &dir, &info) => 0; + lfs_dir_close(&lfs, &dir) => 0; + + // can we list the files? + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfs_dir_t dir; + lfs_dir_open(&lfs, &dir, name) => 0; + struct lfs_info info; + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + lfs_dir_read(&lfs, &dir, &info) => 1; + assert(info.type == LFS_TYPE_REG); + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + + lfs_dir_read(&lfs, &dir, &info) => 0; + lfs_dir_close(&lfs, &dir) => 0; + } + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_file_t file; + char name[16]; + sprintf(name, "dir%03d/file%03d", i, i); + lfs_file_open(&lfs, &file, name, LFS_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfs_file_read(&lfs, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfs_file_close(&lfs, &file) => 0; + } + + lfs_unmount(&lfs) => 0; +''' + + + +## backwards-compatibility tests ## + +# test we can mount in an old version +[cases.test_compat_backward_mount] +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the new version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // confirm the new mount works + lfs_mount(&lfs, cfg) => 0; + lfs_unmount(&lfs) => 0; + + // now test the previous mount + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + lfsp_unmount(&lfsp) => 0; +''' + +# test we can read dirs in an old version +[cases.test_compat_backward_read_dirs] +defines.COUNT = 5 +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the new version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // write COUNT dirs + lfs_mount(&lfs, cfg) => 0; + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfs_mkdir(&lfs, name) => 0; + } + lfs_unmount(&lfs) => 0; + + + // mount the new version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + + // can we list the directories? + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, "/") => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + lfsp_dir_close(&lfsp, &dir) => 0; + + lfsp_unmount(&lfsp) => 0; +''' + +# test we can read files in an old version +[cases.test_compat_backward_read_files] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 4 +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the new version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // write COUNT files + lfs_mount(&lfs, cfg) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfs_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfs_file_open(&lfs, &file, name, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfs_file_write(&lfs, &file, chunk, CHUNK) => CHUNK; + } + lfs_file_close(&lfs, &file) => 0; + } + lfs_unmount(&lfs) => 0; + + + // mount the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + + // can we list the files? + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, "/") => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_REG); + char name[8]; + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + } + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfsp_file_open(&lfsp, &file, name, LFSP_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfsp_file_read(&lfsp, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfsp_file_close(&lfsp, &file) => 0; + } + + lfsp_unmount(&lfsp) => 0; +''' + +# test we can read files in dirs in an old version +[cases.test_compat_backward_read_files_in_dirs] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 4 +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the new version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // write COUNT files+dirs + lfs_mount(&lfs, cfg) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[16]; + sprintf(name, "dir%03d", i); + lfs_mkdir(&lfs, name) => 0; + + lfs_file_t file; + sprintf(name, "dir%03d/file%03d", i, i); + lfs_file_open(&lfs, &file, name, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfs_file_write(&lfs, &file, chunk, CHUNK) => CHUNK; + } + lfs_file_close(&lfs, &file) => 0; + } + lfs_unmount(&lfs) => 0; + + + // mount the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + + // can we list the directories? + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, "/") => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + lfsp_dir_close(&lfsp, &dir) => 0; + + // can we list the files? + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, name) => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_REG); + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + lfsp_dir_close(&lfsp, &dir) => 0; + } + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_file_t file; + char name[16]; + sprintf(name, "dir%03d/file%03d", i, i); + lfsp_file_open(&lfsp, &file, name, LFSP_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfsp_file_read(&lfsp, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfsp_file_close(&lfsp, &file) => 0; + } + + lfsp_unmount(&lfsp) => 0; +''' + +# test we can write dirs in an old version +[cases.test_compat_backward_write_dirs] +defines.COUNT = 10 +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the new version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // write COUNT/2 dirs + lfs_mount(&lfs, cfg) => 0; + for (lfs_size_t i = 0; i < COUNT/2; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfs_mkdir(&lfs, name) => 0; + } + lfs_unmount(&lfs) => 0; + + + // mount the previous version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + + // write another COUNT/2 dirs + for (lfs_size_t i = COUNT/2; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfsp_mkdir(&lfsp, name) => 0; + } + + // can we list the directories? + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, "/") => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + lfsp_dir_close(&lfsp, &dir) => 0; + + lfsp_unmount(&lfsp) => 0; +''' + +# test we can write files in an old version +[cases.test_compat_backward_write_files] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 2 +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the previous version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // write half COUNT files + lfs_mount(&lfs, cfg) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + // write half + lfs_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfs_file_open(&lfs, &file, name, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE/2; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfs_file_write(&lfs, &file, chunk, CHUNK) => CHUNK; + } + lfs_file_close(&lfs, &file) => 0; + + // skip the other half but keep our prng reproducible + for (lfs_size_t j = SIZE/2; j < SIZE; j++) { + TEST_PRNG(&prng); + } + } + lfs_unmount(&lfs) => 0; + + + // mount the new version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + + // write half COUNT files + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + // skip half but keep our prng reproducible + for (lfs_size_t j = 0; j < SIZE/2; j++) { + TEST_PRNG(&prng); + } + + // write the other half + lfsp_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfsp_file_open(&lfsp, &file, name, LFSP_O_WRONLY) => 0; + lfsp_file_seek(&lfsp, &file, SIZE/2, LFSP_SEEK_SET) => SIZE/2; + + for (lfs_size_t j = SIZE/2; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfsp_file_write(&lfsp, &file, chunk, CHUNK) => CHUNK; + } + lfsp_file_close(&lfsp, &file) => 0; + } + + // can we list the files? + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, "/") => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_REG); + char name[8]; + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + } + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_file_t file; + char name[8]; + sprintf(name, "file%03d", i); + lfsp_file_open(&lfsp, &file, name, LFSP_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfsp_file_read(&lfsp, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfsp_file_close(&lfsp, &file) => 0; + } + + lfsp_unmount(&lfsp) => 0; +''' + +# test we can write files in dirs in an old version +[cases.test_compat_backward_write_files_in_dirs] +defines.COUNT = 5 +defines.SIZE = [4, 32, 512, 8192] +defines.CHUNK = 2 +if = 'LFS_VERSION == LFSP_VERSION' +code = ''' + // create the previous version + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // write half COUNT files + lfs_mount(&lfs, cfg) => 0; + uint32_t prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[16]; + sprintf(name, "dir%03d", i); + lfs_mkdir(&lfs, name) => 0; + + // write half + lfs_file_t file; + sprintf(name, "dir%03d/file%03d", i, i); + lfs_file_open(&lfs, &file, name, + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + for (lfs_size_t j = 0; j < SIZE/2; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfs_file_write(&lfs, &file, chunk, CHUNK) => CHUNK; + } + lfs_file_close(&lfs, &file) => 0; + + // skip the other half but keep our prng reproducible + for (lfs_size_t j = SIZE/2; j < SIZE; j++) { + TEST_PRNG(&prng); + } + } + lfs_unmount(&lfs) => 0; + + + // mount the new version + struct lfsp_config cfgp; + memcpy(&cfgp, cfg, sizeof(cfgp)); + lfsp_t lfsp; + lfsp_mount(&lfsp, &cfgp) => 0; + + // write half COUNT files + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + // skip half but keep our prng reproducible + for (lfs_size_t j = 0; j < SIZE/2; j++) { + TEST_PRNG(&prng); + } + + // write the other half + lfsp_file_t file; + char name[16]; + sprintf(name, "dir%03d/file%03d", i, i); + lfsp_file_open(&lfsp, &file, name, LFSP_O_WRONLY) => 0; + lfsp_file_seek(&lfsp, &file, SIZE/2, LFSP_SEEK_SET) => SIZE/2; + + for (lfs_size_t j = SIZE/2; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + for (lfs_size_t k = 0; k < CHUNK; k++) { + chunk[k] = TEST_PRNG(&prng) & 0xff; + } + + lfsp_file_write(&lfsp, &file, chunk, CHUNK) => CHUNK; + } + lfsp_file_close(&lfsp, &file) => 0; + } + + // can we list the directories? + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, "/") => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + char name[8]; + sprintf(name, "dir%03d", i); + assert(strcmp(info.name, name) == 0); + } + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + lfsp_dir_close(&lfsp, &dir) => 0; + + // can we list the files? + for (lfs_size_t i = 0; i < COUNT; i++) { + char name[8]; + sprintf(name, "dir%03d", i); + lfsp_dir_t dir; + lfsp_dir_open(&lfsp, &dir, name) => 0; + struct lfsp_info info; + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, ".") == 0); + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_DIR); + assert(strcmp(info.name, "..") == 0); + + lfsp_dir_read(&lfsp, &dir, &info) => 1; + assert(info.type == LFSP_TYPE_REG); + sprintf(name, "file%03d", i); + assert(strcmp(info.name, name) == 0); + assert(info.size == SIZE); + + lfsp_dir_read(&lfsp, &dir, &info) => 0; + lfsp_dir_close(&lfsp, &dir) => 0; + } + + // now can we read the files? + prng = 42; + for (lfs_size_t i = 0; i < COUNT; i++) { + lfsp_file_t file; + char name[16]; + sprintf(name, "dir%03d/file%03d", i, i); + lfsp_file_open(&lfsp, &file, name, LFSP_O_RDONLY) => 0; + for (lfs_size_t j = 0; j < SIZE; j += CHUNK) { + uint8_t chunk[CHUNK]; + lfsp_file_read(&lfsp, &file, chunk, CHUNK) => CHUNK; + + for (lfs_size_t k = 0; k < CHUNK; k++) { + assert(chunk[k] == TEST_PRNG(&prng) & 0xff); + } + } + lfsp_file_close(&lfsp, &file) => 0; + } + + lfsp_unmount(&lfsp) => 0; +''' + + + +## incompatiblity tests ## + +# test that we fail to mount after a major version bump +[cases.test_compat_major_incompat] +in = 'lfs.c' +code = ''' + // create a superblock + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // bump the major version + // + // note we're messing around with internals to do this! this + // is not a user API + lfs_mount(&lfs, cfg) => 0; + lfs_mdir_t mdir; + lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0; + lfs_superblock_t superblock = { + .version = LFS_DISK_VERSION + 0x00010000, + .block_size = lfs.cfg->block_size, + .block_count = lfs.cfg->block_count, + .name_max = lfs.name_max, + .file_max = lfs.file_max, + .attr_max = lfs.attr_max, + }; + lfs_superblock_tole32(&superblock); + lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS( + {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock})) => 0; + lfs_unmount(&lfs) => 0; + + // mount should now fail + lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; +''' + +# test that we fail to mount after a minor version bump +[cases.test_compat_minor_incompat] +in = 'lfs.c' +code = ''' + // create a superblock + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + + // bump the minor version + // + // note we're messing around with internals to do this! this + // is not a user API + lfs_mount(&lfs, cfg) => 0; + lfs_mdir_t mdir; + lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0; + lfs_superblock_t superblock = { + .version = LFS_DISK_VERSION + 0x00000001, + .block_size = lfs.cfg->block_size, + .block_count = lfs.cfg->block_count, + .name_max = lfs.name_max, + .file_max = lfs.file_max, + .attr_max = lfs.attr_max, + }; + lfs_superblock_tole32(&superblock); + lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS( + {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock})) => 0; + lfs_unmount(&lfs) => 0; + + // mount should now fail + lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; +''' From ca0da3d4907df931636b8fa46aeb17d8ee799aa4 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 20 Apr 2023 03:54:08 -0500 Subject: [PATCH 09/12] Added compatibility testing on pull-request to GitHub test action This uses the "github.event.pull_request.base.ref" variable as the "lfsp" target for compatibility testing. --- .github/workflows/test.yml | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6ebe4a63..2cee3528 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -473,6 +473,42 @@ jobs: path: status retention-days: 1 + # run compatibility tests using the current master as the previous version + test-compat: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v2 + if: ${{github.event_name == 'pull_request'}} + # checkout the current pr target into lfsp + - uses: actions/checkout@v2 + if: ${{github.event_name == 'pull_request'}} + with: + ref: ${{github.event.pull_request.base.ref}} + path: lfsp + - name: install + if: ${{github.event_name == 'pull_request'}} + run: | + # need a few things + sudo apt-get update -qq + sudo apt-get install -qq gcc python3 python3-pip + pip3 install toml + gcc --version + python3 --version + # adjust prefix of lfsp + - name: changeprefix + if: ${{github.event_name == 'pull_request'}} + run: | + ./scripts/changeprefix.py lfs lfsp lfsp/*.h lfsp/*.c + - name: test-compat + if: ${{github.event_name == 'pull_request'}} + run: | + TESTS=tests/test_compat.toml \ + SRC="$(find . lfsp -name '*.c' -maxdepth 1 \ + -and -not -name '*.t.*' \ + -and -not -name '*.b.*')" \ + CFLAGS="-DLFSP=lfsp/lfsp.h" \ + make test + # self-host with littlefs-fuse for a fuzz-like test fuse: runs-on: ubuntu-22.04 From 4c9360020ed8b6db5d943fd61155a09553e8f30f Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 20 Apr 2023 16:42:19 -0500 Subject: [PATCH 10/12] Added ability to bump on-disk minor version This just means a rewrite of the superblock entry with the new minor version. Though it's interesting to note, we don't need to rewrite the superblock entry until the first write operation in the filesystem, an optimization that is already in use for the fixing of orphans and in-flight moves. To keep track of any outdated minor version found during lfs_mount, we can carve out a bit from the reserved bits in our gstate. These are currently used for a counter tracking the number of orphans in the filesystem, but this is usually a very small number so this hopefully won't be an issue. In-device gstate tag: [-- 32 --] [1|- 11 -| 10 |1| 9 ] ^----^-----^--^--^-- 1-bit has orphans '-----|--|--|-- 11-bit move type '--|--|-- 10-bit move id '--|-- 1-bit needs superblock '-- 9-bit orphan count --- lfs.c | 83 +++++++++++++++++++++++++++++++++++++++--- tests/test_compat.toml | 82 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 5 deletions(-) diff --git a/lfs.c b/lfs.c index 7a4b41a8..5ad8a1fe 100644 --- a/lfs.c +++ b/lfs.c @@ -411,12 +411,16 @@ static inline bool lfs_gstate_hasorphans(const lfs_gstate_t *a) { } static inline uint8_t lfs_gstate_getorphans(const lfs_gstate_t *a) { - return lfs_tag_size(a->tag); + return lfs_tag_size(a->tag) & 0x1ff; } static inline bool lfs_gstate_hasmove(const lfs_gstate_t *a) { return lfs_tag_type1(a->tag); } + +static inline bool lfs_gstate_needssuperblock(const lfs_gstate_t *a) { + return lfs_tag_size(a->tag) >> 9; +} #endif static inline bool lfs_gstate_hasmovehere(const lfs_gstate_t *a, @@ -533,6 +537,7 @@ static int lfs_file_outline(lfs_t *lfs, lfs_file_t *file); static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file); static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss); +static void lfs_fs_prepsuperblock(lfs_t *lfs, bool needssuperblock); static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans); static void lfs_fs_prepmove(lfs_t *lfs, uint16_t id, const lfs_block_t pair[2]); @@ -4258,12 +4263,29 @@ static int lfs_rawmount(lfs_t *lfs, const struct lfs_config *cfg) { uint16_t minor_version = (0xffff & (superblock.version >> 0)); if ((major_version != LFS_DISK_VERSION_MAJOR || minor_version > LFS_DISK_VERSION_MINOR)) { - LFS_ERROR("Invalid version v%"PRIu16".%"PRIu16, - major_version, minor_version); + LFS_ERROR("Invalid version " + "v%"PRIu16".%"PRIu16" != v%"PRIu16".%"PRIu16, + major_version, minor_version, + LFS_DISK_VERSION_MAJOR, LFS_DISK_VERSION_MINOR); err = LFS_ERR_INVAL; goto cleanup; } + // found older minor version? set an in-device only bit in the + // gstate so we know we need to rewrite the superblock before + // the first write + if (minor_version < LFS_DISK_VERSION_MINOR) { + LFS_DEBUG("Found older minor version " + "v%"PRIu16".%"PRIu16" < v%"PRIu16".%"PRIu16, + major_version, minor_version, + LFS_DISK_VERSION_MAJOR, LFS_DISK_VERSION_MINOR); + #ifndef LFS_READONLY + // note this bit is reserved on disk, so fetching more gstate + // will not interfere here + lfs_fs_prepsuperblock(lfs, true); + #endif + } + // check superblock configuration if (superblock.name_max) { if (superblock.name_max > lfs->name_max) { @@ -4537,10 +4559,17 @@ static lfs_stag_t lfs_fs_parent(lfs_t *lfs, const lfs_block_t pair[2], } #endif +#ifndef LFS_READONLY +static void lfs_fs_prepsuperblock(lfs_t *lfs, bool needssuperblock) { + lfs->gstate.tag = (lfs->gstate.tag & ~LFS_MKTAG(0, 0, 0x200)) + | (uint32_t)needssuperblock << 9; +} +#endif + #ifndef LFS_READONLY static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans) { LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0x000 || orphans >= 0); - LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) < 0x3ff || orphans <= 0); + LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) < 0x1ff || orphans <= 0); lfs->gstate.tag += orphans; lfs->gstate.tag = ((lfs->gstate.tag & ~LFS_MKTAG(0x800, 0, 0)) | ((uint32_t)lfs_gstate_hasorphans(&lfs->gstate) << 31)); @@ -4559,6 +4588,45 @@ static void lfs_fs_prepmove(lfs_t *lfs, } #endif +#ifndef LFS_READONLY +static int lfs_fs_desuperblock(lfs_t *lfs) { + if (!lfs_gstate_needssuperblock(&lfs->gstate)) { + return 0; + } + + LFS_DEBUG("Rewriting superblock {0x%"PRIx32", 0x%"PRIx32"}", + lfs->root[0], + lfs->root[1]); + + lfs_mdir_t root; + int err = lfs_dir_fetch(lfs, &root, lfs->root); + if (err) { + return err; + } + + // write a new superblock + lfs_superblock_t superblock = { + .version = LFS_DISK_VERSION, + .block_size = lfs->cfg->block_size, + .block_count = lfs->cfg->block_count, + .name_max = lfs->name_max, + .file_max = lfs->file_max, + .attr_max = lfs->attr_max, + }; + + lfs_superblock_tole32(&superblock); + err = lfs_dir_commit(lfs, &root, LFS_MKATTRS( + {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock})); + if (err) { + return err; + } + + lfs_fs_prepsuperblock(lfs, false); + return 0; +} +#endif + #ifndef LFS_READONLY static int lfs_fs_demove(lfs_t *lfs) { if (!lfs_gstate_hasmove(&lfs->gdisk)) { @@ -4736,7 +4804,12 @@ static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss) { #ifndef LFS_READONLY static int lfs_fs_forceconsistency(lfs_t *lfs) { - int err = lfs_fs_demove(lfs); + int err = lfs_fs_desuperblock(lfs); + if (err) { + return err; + } + + err = lfs_fs_demove(lfs); if (err) { return err; } diff --git a/tests/test_compat.toml b/tests/test_compat.toml index 7af487f5..a36c38a4 100644 --- a/tests/test_compat.toml +++ b/tests/test_compat.toml @@ -1276,3 +1276,85 @@ code = ''' // mount should now fail lfs_mount(&lfs, cfg) => LFS_ERR_INVAL; ''' + +# test that we correctly bump the minor version +[cases.test_compat_minor_bump] +in = 'lfs.c' +if = 'LFS_DISK_VERSION_MINOR > 0' +code = ''' + // create a superblock + lfs_t lfs; + lfs_format(&lfs, cfg) => 0; + lfs_mount(&lfs, cfg) => 0; + lfs_file_t file; + lfs_file_open(&lfs, &file, "test", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + lfs_file_write(&lfs, &file, "testtest", 8) => 8; + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; + + // write an old minor version + // + // note we're messing around with internals to do this! this + // is not a user API + lfs_mount(&lfs, cfg) => 0; + lfs_mdir_t mdir; + lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0; + lfs_superblock_t superblock = { + .version = LFS_DISK_VERSION - 0x00000001, + .block_size = lfs.cfg->block_size, + .block_count = lfs.cfg->block_count, + .name_max = lfs.name_max, + .file_max = lfs.file_max, + .attr_max = lfs.attr_max, + }; + lfs_superblock_tole32(&superblock); + lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS( + {LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock})) => 0; + lfs_unmount(&lfs) => 0; + + // mount should still work + lfs_mount(&lfs, cfg) => 0; + lfs_file_open(&lfs, &file, "test", LFS_O_RDONLY) => 0; + uint8_t buffer[8]; + lfs_file_read(&lfs, &file, buffer, 8) => 8; + assert(memcmp(buffer, "testtest", 8) == 0); + lfs_file_close(&lfs, &file) => 0; + lfs_unmount(&lfs) => 0; + + // if we write, we need to bump the minor version + lfs_mount(&lfs, cfg) => 0; + lfs_file_open(&lfs, &file, "test", LFS_O_WRONLY | LFS_O_TRUNC) => 0; + lfs_file_write(&lfs, &file, "teeeeest", 8) => 8; + lfs_file_close(&lfs, &file) => 0; + + // minor version should have changed + lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0; + lfs_dir_get(&lfs, &mdir, LFS_MKTAG(0x7ff, 0x3ff, 0), + LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock) + => LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)); + lfs_superblock_fromle32(&superblock); + assert((superblock.version >> 16) & 0xffff == LFS_DISK_VERSION_MAJOR); + assert((superblock.version >> 0) & 0xffff == LFS_DISK_VERSION_MINOR); + lfs_unmount(&lfs) => 0; + + // and of course mount should still work + lfs_mount(&lfs, cfg) => 0; + lfs_file_open(&lfs, &file, "test", LFS_O_RDONLY) => 0; + lfs_file_read(&lfs, &file, buffer, 8) => 8; + assert(memcmp(buffer, "teeeeest", 8) == 0); + lfs_file_close(&lfs, &file) => 0; + + // minor version should have changed + lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0; + lfs_dir_get(&lfs, &mdir, LFS_MKTAG(0x7ff, 0x3ff, 0), + LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)), + &superblock) + => LFS_MKTAG(LFS_TYPE_INLINESTRUCT, 0, sizeof(superblock)); + lfs_superblock_fromle32(&superblock); + assert((superblock.version >> 16) & 0xffff == LFS_DISK_VERSION_MAJOR); + assert((superblock.version >> 0) & 0xffff == LFS_DISK_VERSION_MINOR); + lfs_unmount(&lfs) => 0; +''' From 9e28c754823e27f835b6cbd02c8a925fd3d52ea5 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Thu, 20 Apr 2023 16:42:58 -0500 Subject: [PATCH 11/12] Bumped minor version to v2.6 and on-disk minor version to lfs2.1 --- lfs.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lfs.h b/lfs.h index a1de75e8..f539f6af 100644 --- a/lfs.h +++ b/lfs.h @@ -21,14 +21,14 @@ extern "C" // Software library version // Major (top-nibble), incremented on backwards incompatible changes // Minor (bottom-nibble), incremented on feature additions -#define LFS_VERSION 0x00020005 +#define LFS_VERSION 0x00020006 #define LFS_VERSION_MAJOR (0xffff & (LFS_VERSION >> 16)) #define LFS_VERSION_MINOR (0xffff & (LFS_VERSION >> 0)) // Version of On-disk data structures // Major (top-nibble), incremented on backwards incompatible changes // Minor (bottom-nibble), incremented on feature additions -#define LFS_DISK_VERSION 0x00020000 +#define LFS_DISK_VERSION 0x00020001 #define LFS_DISK_VERSION_MAJOR (0xffff & (LFS_DISK_VERSION >> 16)) #define LFS_DISK_VERSION_MINOR (0xffff & (LFS_DISK_VERSION >> 0)) From 3e25dfc16cbb8aabed1cd80319cc7be964e56c5a Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 21 Apr 2023 14:47:23 -0500 Subject: [PATCH 12/12] Added FCRC tags and an explanation of how FCRCs work to SPEC.md See SPEC.md for more info. Also considered adding an explanation to DESIGN.md, but there's not a great place for it. Maybe FCRCs are too low-level for the high-level design document. Though may be worth reconsidering if DESIGN.md gets revisited. --- SPEC.md | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 90 insertions(+), 11 deletions(-) diff --git a/SPEC.md b/SPEC.md index 3663ea54..2370ea6d 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1,10 +1,10 @@ ## littlefs technical specification -This is the technical specification of the little filesystem. This document -covers the technical details of how the littlefs is stored on disk for -introspection and tooling. This document assumes you are familiar with the -design of the littlefs, for more info on how littlefs works check -out [DESIGN.md](DESIGN.md). +This is the technical specification of the little filesystem with on-disk +version lfs2.1. This document covers the technical details of how the littlefs +is stored on disk for introspection and tooling. This document assumes you are +familiar with the design of the littlefs, for more info on how littlefs works +check out [DESIGN.md](DESIGN.md). ``` | | | .---._____ @@ -133,12 +133,6 @@ tags XORed together, starting with `0xffffffff`. '-------------------' '-------------------' ``` -One last thing to note before we get into the details around tag encoding. Each -tag contains a valid bit used to indicate if the tag and containing commit is -valid. This valid bit is the first bit found in the tag and the commit and can -be used to tell if we've attempted to write to the remaining space in the -block. - Here's a more complete example of metadata block containing 4 entries: ``` @@ -191,6 +185,53 @@ Here's a more complete example of metadata block containing 4 entries: '---- most recent D ``` +Two things to note before we get into the details around tag encoding: + +1. Each tag contains a valid bit used to indicate if the tag and containing + commit is valid. After XORing, this bit should always be zero. + + At the end of each commit, the valid bit of the previous tag is XORed + with the lowest bit in the type field of the CRC tag. This allows + the CRC tag to force the next commit to fail the valid bit test if it + has not yet been written to. + +2. The valid bit alone is not enough info to know if the next commit has been + erased. We don't know the order bits will be programmed in a program block, + so it's possible that the next commit had an attempted program that left the + valid bit unchanged. + + To ensure we only ever program erased bytes, each commit can contain an + optional forward-CRC (FCRC). An FCRC contains a checksum of some amount of + bytes in the next commit at the time it was erased. + + ``` + .-------------------. \ \ + | revision count | | | + |-------------------| | | + | metadata | | | + | | +---. +-- current commit + | | | | | + |-------------------| | | | + | FCRC ---|-. | | + |-------------------| / | | | + | CRC -----|-' / + |-------------------| | + | padding | | padding (does't need CRC) + | | | + |-------------------| \ | \ + | erased? | +-' | + | | | | +-- next commit + | v | / | + | | / + | | + '-------------------' + ``` + + If the FCRC is missing or the checksum does not match, we must assume a + commit was attempted but failed due to power-loss. + + Note that end-of-block commits do not need an FCRC. + ## Metadata tags So in littlefs, 32-bit tags describe every type of metadata. And this means @@ -785,3 +826,41 @@ CRC fields: are made about the contents. --- +#### `0x5ff` LFS_TYPE_FCRC + +Added in lfs2.1, the optional FCRC tag contains a checksum of some amount of +bytes in the next commit at the time it was erased. This allows us to ensure +that we only ever program erased bytes, even if a previous commit failed due +to power-loss. + +When programming a commit, the FCRC size must be at least as large as the +program block size. However, the program block is not saved on disk, and can +change between mounts, so the FCRC size on disk may be different than the +current program block size. + +If the FCRC is missing or the checksum does not match, we must assume a +commit was attempted but failed due to power-loss. + +Layout of the FCRC tag: + +``` + tag data +[-- 32 --][-- 32 --|-- 32 --] +[1|- 11 -| 10 | 10 ][-- 32 --|-- 32 --] + ^ ^ ^ ^ ^- fcrc size ^- fcrc + | | | '- size (8) + | | '------ id (0x3ff) + | '------------ type (0x5ff) + '----------------- valid bit +``` + +FCRC fields: + +1. **FCRC size (32-bits)** - Number of bytes after this commit's CRC tag's + padding to include in the FCRC. + +2. **FCRC (32-bits)** - CRC of the bytes after this commit's CRC tag's padding + when erased. Like the CRC tag, this uses a CRC-32 with a polynomial of + `0x04c11db7` initialized with `0xffffffff`. + +---