Merge pull request #242 from ARMmbed/fix-2048-erase-size

Fix issues with large prog sizes (prog_size > 1KiB)

.travis.yml

@@ -23,8 +23,20 @@ script:
   - make test QUIET=1 CFLAGS+="-DLFS_BLOCK_COUNT=1023 -DLFS_LOOKAHEAD_SIZE=256"
 
   - make clean test QUIET=1 CFLAGS+="-DLFS_INLINE_MAX=0"
+  - make clean test QUIET=1 CFLAGS+="-DLFS_EMUBD_ERASE_VALUE=0xff"
   - make clean test QUIET=1 CFLAGS+="-DLFS_NO_INTRINSICS"
 
+  # additional configurations that don't support all tests (this should be
+  # fixed but at the moment it is what it is)
+  - make test_files QUIET=1
+        CFLAGS+="-DLFS_READ_SIZE=1 -DLFS_BLOCK_SIZE=4096"
+  - make test_files QUIET=1
+        CFLAGS+="-DLFS_READ_SIZE=\(2*1024\) -DLFS_BLOCK_SIZE=\(64*1024\)"
+  - make test_files QUIET=1
+        CFLAGS+="-DLFS_READ_SIZE=\(8*1024\) -DLFS_BLOCK_SIZE=\(64*1024\)"
+  - make test_files QUIET=1
+        CFLAGS+="-DLFS_READ_SIZE=11 -DLFS_BLOCK_SIZE=704"
+
   # compile and find the code size with the smallest configuration
   - make clean size
         OBJ="$(ls lfs*.o | tr '\n' ' ')"

Makefile

@@ -46,7 +46,7 @@ test: test_format test_dirs test_files test_seek test_truncate \
 test_%: tests/test_%.sh
 
 ifdef QUIET
-	@./$< | sed -n '/^[-=]/p'
+	@./$< | sed -nu '/^[-=]/p'
 else
 	./$<
 endif

emubd/lfs_emubd.c

@@ -82,7 +82,7 @@ int lfs_emubd_create(const struct lfs_config *cfg, const char *path) {
     snprintf(emu->child, LFS_NAME_MAX, ".stats");
     FILE *f = fopen(emu->path, "r");
     if (!f) {
-        memset(&emu->stats, 0, sizeof(emu->stats));
+        memset(&emu->stats, LFS_EMUBD_ERASE_VALUE, sizeof(emu->stats));
     } else {
         size_t res = fread(&emu->stats, sizeof(emu->stats), 1, f);
         lfs_emubd_fromle32(emu);

emubd/lfs_emubd.h

@@ -17,20 +17,8 @@ extern "C"
 
 
 // Config options
-#ifndef LFS_EMUBD_READ_SIZE
-#define LFS_EMUBD_READ_SIZE 1
-#endif
-
-#ifndef LFS_EMUBD_PROG_SIZE
-#define LFS_EMUBD_PROG_SIZE 1
-#endif
-
-#ifndef LFS_EMUBD_ERASE_SIZE
-#define LFS_EMUBD_ERASE_SIZE 512
-#endif
-
-#ifndef LFS_EMUBD_TOTAL_SIZE
-#define LFS_EMUBD_TOTAL_SIZE 524288
+#ifndef LFS_EMUBD_ERASE_VALUE
+#define LFS_EMUBD_ERASE_VALUE 0x00
 #endif
 
 

lfs.c

@@ -1227,22 +1227,32 @@ static int lfs_dir_commitattr(lfs_t *lfs, struct lfs_commit *commit,
 
 static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) {
     // align to program units
-    lfs_off_t off = lfs_alignup(commit->off + 2*sizeof(uint32_t),
+    const lfs_off_t off1 = commit->off + sizeof(lfs_tag_t);
+    const lfs_off_t end = lfs_alignup(off1 + sizeof(uint32_t),
             lfs->cfg->prog_size);
 
+    // create crc tags to fill up remainder of commit, note that
+    // padding is not crcd, 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;
+        if (noff < end) {
+            noff = lfs_min(noff, end - 2*sizeof(uint32_t));
+        }
+
         // read erased state from next program unit
-    lfs_tag_t tag;
+        lfs_tag_t tag = 0xffffffff;
         int err = lfs_bd_read(lfs,
                 NULL, &lfs->rcache, sizeof(tag),
-            commit->block, off, &tag, sizeof(tag));
+                commit->block, noff, &tag, sizeof(tag));
         if (err && err != LFS_ERR_CORRUPT) {
             return err;
         }
 
         // build crc tag
         bool reset = ~lfs_frombe32(tag) >> 31;
-    tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff,
-        off - (commit->off+sizeof(lfs_tag_t)));
+        tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff, noff - off);
 
         // write out crc
         uint32_t footer[2];
@@ -1255,24 +1265,29 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) {
         if (err) {
             return err;
         }
+
         commit->off += sizeof(tag)+lfs_tag_size(tag);
         commit->ptag = tag ^ (reset << 31);
+        commit->crc = 0xffffffff; // reset crc for next "commit"
+    }
 
     // flush buffers
-    err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false);
+    int err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false);
     if (err) {
         return err;
     }
 
-    // successful commit, check checksum to make sure
+    // successful commit, check checksums to make sure
+    lfs_off_t off = commit->begin;
+    lfs_off_t noff = off1;
+    while (off < end) {
         uint32_t crc = 0xffffffff;
-    lfs_size_t size = commit->off - lfs_tag_size(tag) - commit->begin;
-    for (lfs_off_t i = 0; i < size; i++) {
+        for (lfs_off_t i = off; i < noff+sizeof(uint32_t); i++) {
             // leave it up to caching to make this efficient
             uint8_t dat;
             err = lfs_bd_read(lfs,
-                NULL, &lfs->rcache, size-i,
-                commit->block, commit->begin+i, &dat, 1);
+                    NULL, &lfs->rcache, noff+sizeof(uint32_t)-i,
+                    commit->block, i, &dat, 1);
             if (err) {
                 return err;
             }
@@ -1280,12 +1295,17 @@ static int lfs_dir_commitcrc(lfs_t *lfs, struct lfs_commit *commit) {
             crc = lfs_crc(crc, &dat, 1);
         }
 
-    if (err) {
-        return err;
+        // detected write error?
+        if (crc != 0) {
+            return LFS_ERR_CORRUPT;
         }
 
-    if (crc != commit->crc) {
-        return LFS_ERR_CORRUPT;
+        // skip padding
+        off = lfs_min(end - noff, 0x3fe) + noff;
+        if (off < end) {
+            off = lfs_min(off, end - 2*sizeof(uint32_t));
+        }
+        noff = off + sizeof(uint32_t);
     }
 
     return 0;
@@ -1578,11 +1598,11 @@ static int lfs_dir_compact(lfs_t *lfs,
             }
 
             // successful compaction, swap dir pair to indicate most recent
+            LFS_ASSERT(commit.off % lfs->cfg->prog_size == 0);
             lfs_pair_swap(dir->pair);
             dir->count = end - begin;
             dir->off = commit.off;
             dir->etag = commit.ptag;
-            dir->erased = (dir->off % lfs->cfg->prog_size == 0);
             // note we able to have already handled move here
             if (lfs_gstate_hasmovehere(&lfs->gpending, dir->pair)) {
                 lfs_gstate_xormove(&lfs->gpending,
@@ -1749,6 +1769,7 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir,
         }
 
         // successful commit, update dir
+        LFS_ASSERT(commit.off % lfs->cfg->prog_size == 0);
         dir->off = commit.off;
         dir->etag = commit.ptag;
 

tests/template.fmt

@@ -82,7 +82,7 @@ uintmax_t test;
 #endif
 
 #ifndef LFS_CACHE_SIZE
-#define LFS_CACHE_SIZE 64
+#define LFS_CACHE_SIZE (64 % LFS_PROG_SIZE == 0 ? 64 : LFS_PROG_SIZE)
 #endif
 
 #ifndef LFS_LOOKAHEAD_SIZE

tests/test_files.sh

@@ -135,20 +135,79 @@ tests/test.py << TEST
     lfs_unmount(&lfs) => 0;
 TEST
 
-echo "--- Many file test ---"
+echo "--- Many files test ---"
 tests/test.py << TEST
     lfs_format(&lfs, &cfg) => 0;
 TEST
 tests/test.py << TEST
-    // Create 300 files of 6 bytes
+    // Create 300 files of 7 bytes
     lfs_mount(&lfs, &cfg) => 0;
-    lfs_mkdir(&lfs, "directory") => 0;
     for (unsigned i = 0; i < 300; i++) {
         snprintf((char*)buffer, sizeof(buffer), "file_%03d", i);
-        lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_WRONLY | LFS_O_CREAT) => 0;
-        size = 6;
-        memcpy(wbuffer, "Hello", size);
+        lfs_file_open(&lfs, &file[0], (char*)buffer,
+                LFS_O_RDWR | LFS_O_CREAT | LFS_O_EXCL) => 0;
+        size = 7;
+        snprintf((char*)wbuffer, size, "Hi %03d", i);
         lfs_file_write(&lfs, &file[0], wbuffer, size) => size;
+        lfs_file_rewind(&lfs, &file[0]) => 0;
+        lfs_file_read(&lfs, &file[0], rbuffer, size) => size;
+        memcmp(wbuffer, rbuffer, size) => 0;
+        lfs_file_close(&lfs, &file[0]) => 0;
+    }
+    lfs_unmount(&lfs) => 0;
+TEST
+
+echo "--- Many files with flush test ---"
+tests/test.py << TEST
+    lfs_format(&lfs, &cfg) => 0;
+TEST
+tests/test.py << TEST
+    // Create 300 files of 7 bytes
+    lfs_mount(&lfs, &cfg) => 0;
+    for (unsigned i = 0; i < 300; i++) {
+        snprintf((char*)buffer, sizeof(buffer), "file_%03d", i);
+        lfs_file_open(&lfs, &file[0], (char*)buffer,
+                LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
+        size = 7;
+        snprintf((char*)wbuffer, size, "Hi %03d", i);
+        lfs_file_write(&lfs, &file[0], wbuffer, size) => size;
+        lfs_file_close(&lfs, &file[0]) => 0;
+
+        snprintf((char*)buffer, sizeof(buffer), "file_%03d", i);
+        lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_RDONLY) => 0;
+        size = 7;
+        snprintf((char*)wbuffer, size, "Hi %03d", i);
+        lfs_file_read(&lfs, &file[0], rbuffer, size) => size;
+        memcmp(wbuffer, rbuffer, size) => 0;
+        lfs_file_close(&lfs, &file[0]) => 0;
+    }
+    lfs_unmount(&lfs) => 0;
+TEST
+
+echo "--- Many files with power cycle test ---"
+tests/test.py << TEST
+    lfs_format(&lfs, &cfg) => 0;
+TEST
+tests/test.py << TEST
+    // Create 300 files of 7 bytes
+    lfs_mount(&lfs, &cfg) => 0;
+    for (unsigned i = 0; i < 300; i++) {
+        snprintf((char*)buffer, sizeof(buffer), "file_%03d", i);
+        lfs_file_open(&lfs, &file[0], (char*)buffer,
+                LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
+        size = 7;
+        snprintf((char*)wbuffer, size, "Hi %03d", i);
+        lfs_file_write(&lfs, &file[0], wbuffer, size) => size;
+        lfs_file_close(&lfs, &file[0]) => 0;
+        lfs_unmount(&lfs) => 0;
+
+        lfs_mount(&lfs, &cfg) => 0;
+        snprintf((char*)buffer, sizeof(buffer), "file_%03d", i);
+        lfs_file_open(&lfs, &file[0], (char*)buffer, LFS_O_RDONLY) => 0;
+        size = 7;
+        snprintf((char*)wbuffer, size, "Hi %03d", i);
+        lfs_file_read(&lfs, &file[0], rbuffer, size) => size;
+        memcmp(wbuffer, rbuffer, size) => 0;
         lfs_file_close(&lfs, &file[0]) => 0;
     }
     lfs_unmount(&lfs) => 0;