Browse Source

Added a better solution for large prog sizes

A current limitation of the lfs tag is the 10-bit (1024) length field.
This field is used to indicate padding for commits and effectively
limits the size of commits to 1KiB. Because commits must be prog size
aligned, this is a problem on devices with prog size > 1024.

[----                   6KiB erase block                   ----]
[-- 2KiB prog size --|-- 2KiB prog size --|-- 2KiB prog size --]
[ 1KiB commit |  ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ]

This can be increased to 12-bit (4096), but for NAND devices this is
still to small to completely solve the issue.

The previous workaround was to just create unaligned commits. This can
occur naturally if littlefs is used on portable media as the prog size
does not have to be consistent on different drivers. If littlefs sees
an unaligned commit, it treats the dir as unerased and must compact the
dir if it creates any new commits.

Unfortunately this isn't great. It effectively means that every small
commit forced an erase on devices with prog size > 1024. This is pretty
terrible.

[----                   6KiB erase block                   ----]
[-- 2KiB prog size --|-- 2KiB prog size --|-- 2KiB prog size --]
[ 1KiB commit |------------------- wasted ---------------------]

A different solution, implemented here, is to use multiple crc tags
to pad the commit until the remaining space fits in the padding. This
effectively looks like multiple empty commits and has a small runtime
cost to parse these tags, but otherwise does no harm.

[----                   6KiB erase block                   ----]
[-- 2KiB prog size --|-- 2KiB prog size --|-- 2KiB prog size --]
[ 1KiB commit | noop | 1KiB commit | noop | 1KiB commit | noop ]

It was a bit tricky to implement, but now we can effectively support
unlimited prog sizes since there's no limit to the number of commits
in a block.

found by kazink and joicetm
Christopher Haster 6 years ago
parent
commit
312326c4e4
1 changed files with 66 additions and 47 deletions
  1. 66 47
      lfs.c

+ 66 - 47
lfs.c

@@ -1232,65 +1232,85 @@ 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);
 
-    // read erased state from next program unit
-    lfs_tag_t tag;
-    int err = lfs_bd_read(lfs,
-            NULL, &lfs->rcache, sizeof(tag),
-            commit->block, off, &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,
-            lfs_min(off - (commit->off+sizeof(lfs_tag_t)), 0x3fe));
+    // 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));
+        }
 
-    // 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,
-            &lfs->pcache, &lfs->rcache, false,
-            commit->block, commit->off, &footer, sizeof(footer));
-    if (err) {
-        return err;
-    }
-    commit->off += sizeof(tag)+lfs_tag_size(tag);
-    commit->ptag = tag ^ (reset << 31);
+        // 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;
+        }
 
-    // flush buffers
-    err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false);
-    if (err) {
-        return err;
-    }
+        // build crc tag
+        bool reset = ~lfs_frombe32(tag) >> 31;
+        tag = LFS_MKTAG(LFS_TYPE_CRC + reset, 0x3ff, noff - off);
 
-    // successful commit, check checksum to make sure
-    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++) {
-        // 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);
+        // 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,
+                &lfs->pcache, &lfs->rcache, false,
+                commit->block, commit->off, &footer, sizeof(footer));
         if (err) {
             return err;
         }
 
-        crc = lfs_crc(crc, &dat, 1);
+        commit->off += sizeof(tag)+lfs_tag_size(tag);
+        commit->ptag = tag ^ (reset << 31);
+        commit->crc = 0xffffffff; // reset crc for next "commit"
     }
 
+    // flush buffers
+    int err = lfs_bd_sync(lfs, &lfs->pcache, &lfs->rcache, false);
     if (err) {
         return err;
     }
 
-    if (crc != commit->crc) {
-        return LFS_ERR_CORRUPT;
+    // 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;
+        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, noff+sizeof(uint32_t)-i,
+                    commit->block, i, &dat, 1);
+            if (err) {
+                return err;
+            }
+
+            crc = lfs_crc(crc, &dat, 1);
+        }
+
+        // detected write error?
+        if (crc != 0) {
+            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;
@@ -1583,11 +1603,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,
@@ -1758,10 +1778,9 @@ 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;
-        // workaround to handle prog_size >1024
-        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)) {