From 0e2a909148df9f7e2b6ecf750f3402ecfb151ee9 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 8 Jul 2024 23:04:04 -0500 Subject: [PATCH] t: Reverted reverted most of LFS_T_MKCONSISTENT After thinking about this for a bit, there are some compelling motivations for including an incremental LFS_T_MKCONSISTENT: - Being able to run incremental LFS_T_MKCONSISTENT traversals in parallel with read-only operations is actually quite enticing. The only complicated part is maintaining the invalidatable traversal state, which already exists with lfsr_traversal_t (except the annoying LFS_F_MUTATED bit). - While it's not really effective to combine LFS_T_MKCONSISTENT and LFS_T_LOOKAHEAD traversals, it _is_ possible to combine LFS_T_MKCONSISTENT with LFS_T_COMPACT, LFS_T_CKMETA, LFS_T_REPAIRMETA (future), etc. Really, LFS_T_LOOKAHEAD is the odd one out. - Making LFS_T_MKCONSISTENT incremental means all filesystem-level traversals (except lfsr_mount) can be run incrementally. Which is a nice feature to have when O(n = entire fs) risks being very long running. The main downside of LFS_T_MKCONSISTENT (and LFS_T_COMPACT, etc) is that attempting to run it immediately after mount will likely recursively trigger a lookahead scan to satisfy block allocation requests -- which will block the current thread for the duration of the lookahead scan. But this seems to be more a problem of LFS_T_LOOKAHEAD interacting with other traversals poorly. Fortunately, long term, the current plan is to replace the lookahead buffer with an on-disk block map on disks where the lookahead scan is a bottleneck. If this gets implemented the problem goes away. So re-reverting this for now. Worst case we can always re-re-revert this again in the future. There is already a working implementation, so might as well see where it goes... Supporting incremental LFS_T_MKCONSISTENT does add a bit of a code cost, but there is still some room for deduplicating lfsr_mtree_gc + lfsr_fs_mkconsistent, which may be interesting: code stack before: 35232 2680 after: 35480 (+0.7%) 2680 (+0.0%) --- lfs.c | 173 ++++++++++++++++++++++++++++---------- lfs.h | 3 +- tests/test_traversal.toml | 14 ++- 3 files changed, 140 insertions(+), 50 deletions(-) diff --git a/lfs.c b/lfs.c index 3ac52d03..a1af9d15 100644 --- a/lfs.c +++ b/lfs.c @@ -8265,6 +8265,10 @@ static inline bool lfsr_f_isdirty(uint32_t flags) { return flags & LFS_F_DIRTY; } +static inline bool lfsr_f_ismutated(uint32_t flags) { + return flags & LFS_F_MUTATED; +} + // needed in lfsr_mtree_traverse_ @@ -8648,6 +8652,7 @@ static int lfsr_mtree_gc(lfs_t *lfs, lfsr_traversal_t *t, // lfsr_mtree_gc to work correctly LFS_ASSERT(lfsr_omdir_isopen(lfs, &t->o.o)); +again:; lfsr_tag_t tag; lfsr_bptr_t bptr; int err = lfsr_mtree_traverse(lfs, t, @@ -8656,6 +8661,32 @@ static int lfsr_mtree_gc(lfs_t *lfs, lfsr_traversal_t *t, return err; } + // keep track of dirty flag before mutation + bool dirty = t->o.o.flags & LFS_F_DIRTY; + + // mkconsistencing mdirs? + if (lfsr_t_ismkconsistent(t->o.o.flags) + && tag == LFSR_TAG_MDIR + && lfs->hasorphans) { + lfsr_mdir_t *mdir = (lfsr_mdir_t*)bptr.data.u.buffer; + int err = lfsr_mdir_fixorphans(lfs, mdir); + if (err) { + return err; + } + + // did this drop our mdir? + if (mdir->mid != -1 && mdir->rbyd.weight == 0) { + t->o.o.flags &= ~LFS_F_ZOMBIE; + t->o.o.state = LFSR_TSTATE_MDIRS; + + // downgrade any new dirty flags + t->o.o.flags |= (t->o.o.flags & LFS_F_DIRTY) ? LFS_F_MUTATED : 0; + t->o.o.flags &= ~LFS_F_DIRTY; + t->o.o.flags |= (dirty) ? LFS_F_DIRTY : 0; + goto again; + } + } + // compacting mdirs? if (lfsr_t_iscompact(t->o.o.flags) && tag == LFSR_TAG_MDIR @@ -8757,6 +8788,11 @@ static int lfsr_mtree_gc(lfs_t *lfs, lfsr_traversal_t *t, t->u.bt.rid = t->u.bt.bid; } + // downgrade any new dirty flags + t->o.o.flags |= (t->o.o.flags & LFS_F_DIRTY) ? LFS_F_MUTATED : 0; + t->o.o.flags &= ~LFS_F_DIRTY; + t->o.o.flags |= (dirty) ? LFS_F_DIRTY : 0; + if (tag_) { *tag_ = tag; } @@ -12664,54 +12700,77 @@ static int lfsr_fs_fixgrm(lfs_t *lfs) { return 0; } +static int lfsr_mdir_fixorphans(lfs_t *lfs, lfsr_mdir_t *mdir) { + // save the current mid + lfsr_mid_t mid = mdir->mid; + + // iterate through mids looking for orphans + mdir->mid = LFSR_MID(lfs, mdir->mid, 0); + int err; + while (lfsr_mid_rid(lfs, mdir->mid) < (lfsr_srid_t)mdir->rbyd.weight) { + // is this mid open? well we're not an orphan then, skip + if (lfsr_omdir_ismidopen(lfs, mdir->mid)) { + mdir->mid += 1; + continue; + } + + // is this mid marked as an orphan? + err = lfsr_mdir_lookup(lfs, mdir, LFSR_TAG_ORPHAN, + NULL); + if (err) { + if (err == LFS_ERR_NOENT) { + mdir->mid += 1; + continue; + } + goto failed; + } + + // we found an orphaned file, remove + LFS_DEBUG("Fixing orphan %"PRId32".%"PRId32, + lfsr_mid_bid(lfs, mdir->mid) >> lfs->mdir_bits, + lfsr_mid_rid(lfs, mdir->mid)); + + err = lfsr_mdir_commit(lfs, mdir, LFSR_ATTRS( + LFSR_ATTR(LFSR_TAG_RM, -1, LFSR_DATA_NULL()))); + if (err) { + goto failed; + } + } + + // restore the current mid + mdir->mid = mid; + return 0; + +failed:; + // restore the current mid + mdir->mid = mid; + return err; +} + static int lfsr_fs_fixorphans(lfs_t *lfs) { // iterate through the filesystem and remove any orphaned files // // note this never takes longer than lfsr_mount // - lfsr_mdir_t mdir = {.mid=0}; - while (mdir.mid < (lfsr_srid_t)lfsr_mtree_weight(lfs)) { - int err = lfsr_mtree_lookup(lfs, LFSR_MID(lfs, mdir.mid, 0), + lfsr_mid_t mid = 0; + while (mid < lfsr_mtree_weight(lfs)) { + lfsr_mdir_t mdir; + int err = lfsr_mtree_lookup(lfs, mid, &mdir); if (err) { LFS_ASSERT(err != LFS_ERR_NOENT); return err; } - while (lfsr_mid_rid(lfs, mdir.mid) - < (lfsr_srid_t)mdir.rbyd.weight) { - // is this mid open? well we're not an orphan then, skip - if (lfsr_omdir_ismidopen(lfs, mdir.mid)) { - mdir.mid += 1; - continue; - } - - // is this mid marked as an orphan? - err = lfsr_mdir_lookup(lfs, &mdir, LFSR_TAG_ORPHAN, - NULL); - if (err) { - if (err == LFS_ERR_NOENT) { - mdir.mid += 1; - continue; - } - return err; - } - - // we found an orphaned file, remove - LFS_DEBUG("Fixing orphan %"PRId32".%"PRId32, - lfsr_mid_bid(lfs, mdir.mid) >> lfs->mdir_bits, - lfsr_mid_rid(lfs, mdir.mid)); - - err = lfsr_mdir_commit(lfs, &mdir, LFSR_ATTRS( - LFSR_ATTR(LFSR_TAG_RM, -1, LFSR_DATA_NULL()))); - if (err) { - return err; - } + // clean up orphans + err = lfsr_mdir_fixorphans(lfs, &mdir); + if (err) { + return err; } - // increment mid unless we dropped the mdir + // incremend mid unless we dropped the mdir if (mdir.rbyd.weight > 0) { - mdir.mid = lfsr_mid_bid(lfs, mdir.mid) + 1; + mid += 1 << lfs->mdir_bits; } } @@ -12856,14 +12915,34 @@ int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *t, struct lfs_tinfo *tinfo) { LFS_ASSERT(lfsr_omdir_isopen(lfs, &t->o.o)); - // mkconsistencing? - // - // check every step in case some other operation introduced a grm/orphan - if (lfsr_t_ismkconsistent(t->o.o.flags)) { - int err = lfsr_fs_mkconsistent(lfs); + // check for pending grms every step, just in case some other + // operation introduced new grms + if (lfsr_t_ismkconsistent(t->o.o.flags) + && lfsr_grm_count(lfs) > 0) { + if (lfsr_grm_count(lfs) == 2) { + LFS_DEBUG("Fixing grm %"PRId32".%"PRId32" %"PRId32".%"PRId32, + lfsr_mid_bid(lfs, lfs->grm.mids[0]) >> lfs->mdir_bits, + lfsr_mid_rid(lfs, lfs->grm.mids[0]), + lfsr_mid_bid(lfs, lfs->grm.mids[1]) >> lfs->mdir_bits, + lfsr_mid_rid(lfs, lfs->grm.mids[1])); + } else if (lfsr_grm_count(lfs) == 1) { + LFS_DEBUG("Fixing grm %"PRId32".%"PRId32, + lfsr_mid_bid(lfs, lfs->grm.mids[0]) >> lfs->mdir_bits, + lfsr_mid_rid(lfs, lfs->grm.mids[0])); + } + + // keep track of dirty flag before mutation + bool dirty = t->o.o.flags & LFS_F_DIRTY; + + int err = lfsr_fs_fixgrm(lfs); if (err) { return err; } + + // downgrade any new dirty flags + t->o.o.flags |= (t->o.o.flags & LFS_F_DIRTY) ? LFS_F_MUTATED : 0; + t->o.o.flags &= ~LFS_F_DIRTY; + t->o.o.flags |= (dirty) ? LFS_F_DIRTY : 0; } while (true) { @@ -12915,9 +12994,16 @@ int lfsr_traversal_read(lfs_t *lfs, lfsr_traversal_t *t, } done:; + // was mkconsistent successful? + if (lfsr_t_ismkconsistent(t->o.o.flags) + && !lfsr_f_isdirty(t->o.o.flags)) { + lfs->hasorphans = false; + } + // was a lookahead scan successful? if (lfsr_t_islookahead(t->o.o.flags) - && !lfsr_f_isdirty(t->o.o.flags)) { + && !lfsr_f_isdirty(t->o.o.flags) + && !lfsr_f_ismutated(t->o.o.flags)) { lfs_alloc_markfree(lfs); } @@ -12926,11 +13012,8 @@ done:; static void lfsr_traversal_clobber(lfs_t *lfs, lfsr_traversal_t *t) { (void)lfs; - // not yet started? - if (t->o.o.state < LFSR_TSTATE_MROOTCHAIN) { - // do nothing // mroot/mtree? transition to mdir iteration - } else if (t->o.o.state < LFSR_TSTATE_MDIRS) { + if (t->o.o.state < LFSR_TSTATE_MDIRS) { t->o.o.state = LFSR_TSTATE_MDIRS; t->o.o.mdir.mid = 0; t->o.bshrub.u.bshrub.weight = 0; @@ -12963,7 +13046,7 @@ static void lfsr_traversal_clobber(lfs_t *lfs, lfsr_traversal_t *t) { static int lfsr_traversal_rewind_(lfs_t *lfs, lfsr_traversal_t *t) { (void)lfs; // reset traversal - t->o.o.flags &= ~LFS_F_DIRTY; + t->o.o.flags &= ~LFS_F_DIRTY & ~LFS_F_MUTATED; t->o.o.state = LFSR_TSTATE_MROOTANCHOR; t->o.o.mdir.mid = -1; t->o.o.mdir.rbyd.weight = 0; diff --git a/lfs.h b/lfs.h index 7a75496d..ff0eab9e 100644 --- a/lfs.h +++ b/lfs.h @@ -179,7 +179,8 @@ enum lfs_traversal_flags { // LFS_T_REPAIRDATA = 0x0c00, // Repair metadata + data blocks // internally used flags - LFS_F_DIRTY = 0x1000, // Filesystem has been modified + LFS_F_DIRTY = 0x1000, // Filesystem modified during traversal + LFS_F_MUTATED = 0x4000, // Filesystem modified by traversal }; diff --git a/tests/test_traversal.toml b/tests/test_traversal.toml index 5199871d..e2390f80 100644 --- a/tests/test_traversal.toml +++ b/tests/test_traversal.toml @@ -6964,10 +6964,13 @@ code = ''' lfsr_traversal_read(&lfs, &t, &tinfo) => LFS_ERR_NOENT; lfsr_traversal_close(&lfs, &t) => 0; - // we should be able to clean up grms and orphans + // we should be able to clean up grms assert(lfs.grm.mids[0] == -1); assert(lfs.grm.mids[1] == -1); - assert(lfs.hasorphans == false); + // if we introduce actual orphans, me _must not_ clear the orphan flag + if (ORPHANS > 3) { + assert(lfs.hasorphans == true); + } // check we can still read the files for (int remount = 0; remount < 2; remount++) { @@ -7836,10 +7839,13 @@ code = ''' lfsr_traversal_read(&lfs, &t, &tinfo) => LFS_ERR_NOENT; lfsr_traversal_close(&lfs, &t) => 0; - // we should be able to clean up grms and orphans + // we should be able to clean up grms assert(lfs.grm.mids[0] == -1); assert(lfs.grm.mids[1] == -1); - assert(lfs.hasorphans == false); + // if we introduce actual orphans, me _must not_ clear the orphan flag + if (ORPHANS > 3) { + assert(lfs.hasorphans == true); + } // mdirs should have been compacted assert((file1.o.o.mdir.rbyd.eoff & 0x7fffffff) <= GC_COMPACT_THRESH);