瀏覽代碼

Fixed lfs_fs_size doubling metadata-pairs

This was caused by the previous fix for allocations during
lfs_fs_deorphan in this branch. To catch half-orphans during block
allocations we needed to duplicate all metadata-pairs reported to
lfs_fs_traverse. Unfortunately this causes lfs_fs_size to report 2x the
number of metadata-pairs, which would undoubtably confuse users.

The fix here is inelegantly simple, just do a different traversale for
allocations and size measurements. It reuses the same code but touches
slightly different sets of blocks.

Unfortunately, this causes the public lfs_fs_traverse and lfs_fs_size
functions to split in how they report blocks. This is technically
allowed, since lfs_fs_traverse may report blocks multiple times due to
CoW behavior, however it's undesirable and I'm sure there will be some
confusion.

But I don't have a better solution, so from this point lfs_fs_traverse
will be reporting 2x metadata-blocks and shouldn't be used for finding
the number of available blocks on the filesystem.
Christopher Haster 5 年之前
父節點
當前提交
6530cb3a61
共有 2 個文件被更改,包括 20 次插入20 次删除
  1. 18 18
      lfs.c
  2. 2 2
      tests/test_orphans.toml

+ 18 - 18
lfs.c

@@ -414,6 +414,9 @@ static lfs_stag_t lfs_fs_parent(lfs_t *lfs, const lfs_block_t dir[2],
         lfs_mdir_t *parent);
 static int lfs_fs_relocate(lfs_t *lfs,
         const lfs_block_t oldpair[2], lfs_block_t newpair[2]);
+int lfs_fs_traverseraw(lfs_t *lfs,
+        int (*cb)(void *data, lfs_block_t block), void *data,
+        bool includeorphans);
 static int lfs_fs_forceconsistency(lfs_t *lfs);
 static int lfs_deinit(lfs_t *lfs);
 #ifdef LFS_MIGRATE
@@ -472,7 +475,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
 
         // find mask of free blocks from tree
         memset(lfs->free.buffer, 0, lfs->cfg->lookahead_size);
-        int err = lfs_fs_traverse(lfs, lfs_alloc_lookahead, lfs);
+        int err = lfs_fs_traverseraw(lfs, lfs_alloc_lookahead, lfs, true);
         if (err) {
             return err;
         }
@@ -3798,10 +3801,9 @@ int lfs_unmount(lfs_t *lfs) {
 
 
 /// Filesystem filesystem operations ///
-int lfs_fs_traverse(lfs_t *lfs,
-        int (*cb)(void *data, lfs_block_t block), void *data) {
-    LFS_TRACE("lfs_fs_traverse(%p, %p, %p)",
-            (void*)lfs, (void*)(uintptr_t)cb, data);
+int lfs_fs_traverseraw(lfs_t *lfs,
+        int (*cb)(void *data, lfs_block_t block), void *data,
+        bool includeorphans) {
     // iterate over metadata pairs
     lfs_mdir_t dir = {.tail = {0, 1}};
 
@@ -3810,7 +3812,6 @@ int lfs_fs_traverse(lfs_t *lfs,
     if (lfs->lfs1) {
         int err = lfs1_traverse(lfs, cb, data);
         if (err) {
-            LFS_TRACE("lfs_fs_traverse -> %d", err);
             return err;
         }
 
@@ -3823,7 +3824,6 @@ int lfs_fs_traverse(lfs_t *lfs,
         for (int i = 0; i < 2; i++) {
             int err = cb(data, dir.tail[i]);
             if (err) {
-                LFS_TRACE("lfs_fs_traverse -> %d", err);
                 return err;
             }
         }
@@ -3831,7 +3831,6 @@ int lfs_fs_traverse(lfs_t *lfs,
         // iterate through ids in directory
         int err = lfs_dir_fetch(lfs, &dir, dir.tail);
         if (err) {
-            LFS_TRACE("lfs_fs_traverse -> %d", err);
             return err;
         }
 
@@ -3843,7 +3842,6 @@ int lfs_fs_traverse(lfs_t *lfs,
                 if (tag == LFS_ERR_NOENT) {
                     continue;
                 }
-                LFS_TRACE("lfs_fs_traverse -> %"PRId32, tag);
                 return tag;
             }
             lfs_ctz_fromle32(&ctz);
@@ -3852,17 +3850,13 @@ int lfs_fs_traverse(lfs_t *lfs,
                 err = lfs_ctz_traverse(lfs, NULL, &lfs->rcache,
                         ctz.head, ctz.size, cb, data);
                 if (err) {
-                    LFS_TRACE("lfs_fs_traverse -> %d", err);
                     return err;
                 }
-            } else if (/*lfs_gstate_hasorphans(&lfs->gstate) && TODO maybe report size-dirs/2 ? */
+            } else if (includeorphans && 
                     lfs_tag_type3(tag) == LFS_TYPE_DIRSTRUCT) {
-                // TODO HMMMMMM HMMMMMMMMMMMMMMMMMMM
                 for (int i = 0; i < 2; i++) {
-                    //printf("HMM %x\n", (&ctz.head)[i]);
                     err = cb(data, (&ctz.head)[i]);
                     if (err) {
-                        LFS_TRACE("lfs_fs_traverse -> %d", err);
                         return err;
                     }
                 }
@@ -3880,7 +3874,6 @@ int lfs_fs_traverse(lfs_t *lfs,
             int err = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache,
                     f->ctz.head, f->ctz.size, cb, data);
             if (err) {
-                LFS_TRACE("lfs_fs_traverse -> %d", err);
                 return err;
             }
         }
@@ -3889,16 +3882,23 @@ int lfs_fs_traverse(lfs_t *lfs,
             int err = lfs_ctz_traverse(lfs, &f->cache, &lfs->rcache,
                     f->block, f->pos, cb, data);
             if (err) {
-                LFS_TRACE("lfs_fs_traverse -> %d", err);
                 return err;
             }
         }
     }
 
-    LFS_TRACE("lfs_fs_traverse -> %d", 0);
     return 0;
 }
 
+int lfs_fs_traverse(lfs_t *lfs,
+        int (*cb)(void *data, lfs_block_t block), void *data) {
+    LFS_TRACE("lfs_fs_traverse(%p, %p, %p)",
+            (void*)lfs, (void*)(uintptr_t)cb, data);
+    int err = lfs_fs_traverseraw(lfs, cb, data, true);
+    LFS_TRACE("lfs_fs_traverse -> %d", 0);
+    return err;
+}
+
 static int lfs_fs_pred(lfs_t *lfs,
         const lfs_block_t pair[2], lfs_mdir_t *pdir) {
     // iterate over all directory directory entries
@@ -4207,7 +4207,7 @@ static int lfs_fs_size_count(void *p, lfs_block_t block) {
 lfs_ssize_t lfs_fs_size(lfs_t *lfs) {
     LFS_TRACE("lfs_fs_size(%p)", (void*)lfs);
     lfs_size_t size = 0;
-    int err = lfs_fs_traverse(lfs, lfs_fs_size_count, &size);
+    int err = lfs_fs_traverseraw(lfs, lfs_fs_size_count, &size, false);
     if (err) {
         LFS_TRACE("lfs_fs_size -> %d", err);
         return err;

+ 2 - 2
tests/test_orphans.toml

@@ -31,13 +31,13 @@ code = '''
     lfs_mount(&lfs, &cfg) => 0;
     lfs_stat(&lfs, "parent/orphan", &info) => LFS_ERR_NOENT;
     lfs_stat(&lfs, "parent/child", &info) => 0;
-    lfs_fs_size(&lfs) => 12;
+    lfs_fs_size(&lfs) => 8;
     lfs_unmount(&lfs) => 0;
 
     lfs_mount(&lfs, &cfg) => 0;
     lfs_stat(&lfs, "parent/orphan", &info) => LFS_ERR_NOENT;
     lfs_stat(&lfs, "parent/child", &info) => 0;
-    lfs_fs_size(&lfs) => 12;
+    lfs_fs_size(&lfs) => 8;
     // this mkdir should both create a dir and deorphan, so size
     // should be unchanged
     lfs_mkdir(&lfs, "parent/otherchild") => 0;