From 6530cb3a6129f28dda117fdcb0231291b0eff680 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 9 Feb 2020 09:05:37 -0600 Subject: [PATCH] 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. --- lfs.c | 36 ++++++++++++++++++------------------ tests/test_orphans.toml | 4 ++-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/lfs.c b/lfs.c index 74fafd9..2f4593a 100644 --- a/lfs.c +++ b/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; diff --git a/tests/test_orphans.toml b/tests/test_orphans.toml index ea4ca50..8d0f3de 100644 --- a/tests/test_orphans.toml +++ b/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;