From 14c369af938d4eab4b88479ed7ba65b897e167a5 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 8 Nov 2025 01:40:01 -0600 Subject: [PATCH] trv: Adopted LFS3_t_STALE for marking block queue as stale This solves the previous gc-needs-block-queue-so-we-can-clobber-block- queue issue by adding an additional LFS3_t_STALE flag to indicate when any block queues would be invalid. So instead of clearing block queues in lfs3_alloc_ckpoint, we just set LFS3_t_STALE, and any lfs3_trv_ts can clear their block queues in lfs3_trv_read. This allows lfs3_mgc_ts to be allocated without a block queue when doing any LFS3_M_*/LFS3_F_*/LFS3_GC_* work. LFS3_t_STALE is set at the same time as LFS3_t_CKPOINT and LFS3_t_DIRTY, but we need a separate bit so lfs3_trv_read can clear the flag after flushing without losing ckpoint/dirty information. --- Unfortunately, none of the stack-allocated lfs3_mgc_ts are on the stack hot-path, so we don't immediate savings. But note the 2-words saved in ctx when compiling in LFS3_GC mode: code stack ctx before: 35940 2280 660 after: 35944 (+0.0%) 2280 (+0.0%) 660 (+0.0%) code stack ctx gbmap before: 38916 2296 772 gbmap after: 38916 (+0.0%) 2296 (+0.0%) 772 (+0.0%) code stack ctx gc before: 36012 2280 776 gc after: 36016 (+0.0%) 2280 (+0.0%) 768 (-1.0%) --- lfs3.c | 96 +++++++++++++++++++++------------------------ lfs3.h | 7 ++-- scripts/dbgflags.py | 5 ++- tests/test_gc.toml | 36 ++++++++--------- 4 files changed, 69 insertions(+), 75 deletions(-) diff --git a/lfs3.c b/lfs3.c index 0385af8e..82da2eae 100644 --- a/lfs3.c +++ b/lfs3.c @@ -7434,12 +7434,16 @@ static inline void lfs3_t_setbtype(uint32_t *flags, uint8_t btype) { *flags = (*flags & ~LFS3_t_BTYPE) | lfs3_t_btypeflags(btype); } +static inline bool lfs3_t_isckpointed(uint32_t flags) { + return flags & LFS3_t_CKPOINTED; +} + static inline bool lfs3_t_isdirty(uint32_t flags) { return flags & LFS3_t_DIRTY; } -static inline bool lfs3_t_isckpointed(uint32_t flags) { - return flags & LFS3_t_CKPOINTED; +static inline bool lfs3_t_isstale(uint32_t flags) { + return flags & LFS3_t_STALE; } // mount flags @@ -10052,7 +10056,7 @@ static void lfs3_mtrv_init(lfs3_mtrv_t *mtrv, uint32_t flags) { static void lfs3_mtrv_ckpoint(lfs3_mtrv_t *mtrv) { // mark as ckpointed and dirty - mtrv->h.flags |= LFS3_t_CKPOINTED | LFS3_t_DIRTY; + mtrv->h.flags |= LFS3_t_CKPOINTED | LFS3_t_DIRTY | LFS3_t_STALE; // when tracked, our mdir should be kept in-sync, but we need to // discard any btrees/bshrubs that may fall out-of-date @@ -10920,10 +10924,10 @@ static inline void lfs3_alloc_ckpoint_(lfs3_t *lfs3) { lfs3->lookahead.ckpoint = lfs3->block_count; // ckpoint traversals, marking them as ckpointed + dirty and - // reseting aany btrv state + // reseting any btrv state for (lfs3_handle_t *h = lfs3->handles; h; h = h->next) { if (lfs3_o_type(h->flags) == LFS3_type_TRV) { - lfs3_trv_ckpoint_(lfs3, (lfs3_trv_t*)h); + lfs3_mgc_ckpoint(&((lfs3_trv_t*)h)->gc); } } } @@ -15872,7 +15876,7 @@ static int lfs3_mountinited(lfs3_t *lfs3) { } // needed in lfs3_mount -static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_trv_t *trv, +static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_mgc_t *mgc, uint32_t flags, lfs3_soff_t steps); int lfs3_mount(lfs3_t *lfs3, uint32_t flags, @@ -15979,8 +15983,8 @@ int lfs3_mount(lfs3_t *lfs3, uint32_t flags, | LFS3_IFDEF_RDONLY(0, LFS3_M_COMPACTMETA) | LFS3_M_CKMETA | LFS3_M_CKDATA)) { - lfs3_trv_t trv; - err = lfs3_fs_gc_(lfs3, &trv, + lfs3_mgc_t mgc; + err = lfs3_fs_gc_(lfs3, &mgc, flags & ( LFS3_IFDEF_RDONLY(0, LFS3_M_MKCONSISTENT) | LFS3_IFDEF_RDONLY(0, LFS3_M_RELOOKAHEAD) @@ -16023,8 +16027,8 @@ int lfs3_unmount(lfs3_t *lfs3) { LFS3_ASSERT(lfs3->handles == NULL // special case for our gc traversal handle || LFS3_IFDEF_GC( - (lfs3->handles == &lfs3->gc.gc.t.h - && lfs3->gc.gc.t.h.next == NULL), + (lfs3->handles == &lfs3->gc.t.h + && lfs3->gc.t.h.next == NULL), false)); return lfs3_deinit(lfs3); @@ -16290,8 +16294,8 @@ int lfs3_format(lfs3_t *lfs3, uint32_t flags, if (flags & ( LFS3_F_CKMETA | LFS3_F_CKDATA)) { - lfs3_trv_t trv; - err = lfs3_fs_gc_(lfs3, &trv, + lfs3_mgc_t mgc; + err = lfs3_fs_gc_(lfs3, &mgc, flags & ( LFS3_F_CKMETA | LFS3_F_CKDATA), @@ -16587,10 +16591,7 @@ int lfs3_fs_cksum(lfs3_t *lfs3, uint32_t *cksum) { // // runs the traversal until all work is completed, which may take // multiple passes -// -// TODO can we reduce this to just lfs3_mgc_t? in theory we don't -// need the block queue here, do we need to track lfs3_mgc_t? -static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_trv_t *trv, +static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_mgc_t *mgc, uint32_t flags, lfs3_soff_t steps) { // unknown gc flags? LFS3_ASSERT((flags & ~LFS3_GC_ALL) == 0); @@ -16620,40 +16621,40 @@ static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_trv_t *trv, while (pending && (lfs3_off_t)steps > 0) { // start a new traversal? - if (!lfs3_handle_isopen(lfs3, &trv->gc.t.h)) { - lfs3_mgc_init(&trv->gc, pending); - lfs3_handle_open(lfs3, &trv->gc.t.h); + if (!lfs3_handle_isopen(lfs3, &mgc->t.h)) { + lfs3_mgc_init(mgc, pending); + lfs3_handle_open(lfs3, &mgc->t.h); } // don't bother with lookahead/gbmap if we've ckpointed #ifndef LFS3_RDONLY - if (lfs3_t_isckpointed(trv->gc.t.h.flags)) { - trv->gc.t.h.flags &= ~LFS3_T_RELOOKAHEAD; + if (lfs3_t_isckpointed(mgc->t.h.flags)) { + mgc->t.h.flags &= ~LFS3_T_RELOOKAHEAD; #ifdef LFS3_GBMAP - trv->gc.t.h.flags &= ~LFS3_T_REGBMAP; + mgc->t.h.flags &= ~LFS3_T_REGBMAP; #endif } #endif // will this traversal still make progress? no? start over - if (!(trv->gc.t.h.flags & LFS3_GC_ALL)) { - lfs3_handle_close(lfs3, &trv->gc.t.h); + if (!(mgc->t.h.flags & LFS3_GC_ALL)) { + lfs3_handle_close(lfs3, &mgc->t.h); continue; } // do we really need a full traversal? - if (!(trv->gc.t.h.flags & ( + if (!(mgc->t.h.flags & ( LFS3_IFDEF_RDONLY(0, LFS3_GC_RELOOKAHEAD) | LFS3_IFDEF_RDONLY(0, LFS3_IFDEF_GBMAP(LFS3_GC_REGBMAP, 0)) | LFS3_GC_CKMETA | LFS3_GC_CKDATA))) { - trv->gc.t.h.flags |= LFS3_T_MTREEONLY; + mgc->t.h.flags |= LFS3_T_MTREEONLY; } // progress gc lfs3_bptr_t bptr; - lfs3_stag_t tag = lfs3_mtree_gc(lfs3, &trv->gc, + lfs3_stag_t tag = lfs3_mtree_gc(lfs3, mgc, &bptr); if (tag < 0 && tag != LFS3_ERR_NOENT) { return tag; @@ -16661,7 +16662,7 @@ static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_trv_t *trv, // end of traversal? if (tag == LFS3_ERR_NOENT) { - lfs3_handle_close(lfs3, &trv->gc.t.h); + lfs3_handle_close(lfs3, &mgc->t.h); // clear any pending flags we make progress on pending &= lfs3->flags & LFS3_GC_ALL; @@ -16709,7 +16710,7 @@ int lfs3_fs_unck(lfs3_t *lfs3, uint32_t flags) { // lfs3_fs_gc will terminate early if it discovers it can no longer // make progress #ifdef LFS3_GC - lfs3->gc.gc.t.h.flags &= ~flags; + lfs3->gc.t.h.flags &= ~flags; #endif return 0; @@ -16985,6 +16986,13 @@ int lfs3_trv_read(lfs3_t *lfs3, lfs3_trv_t *trv, } #endif + // discard current block queue? + if (lfs3_t_isstale(trv->gc.t.h.flags)) { + trv->blocks[0] = -1; + trv->blocks[1] = -1; + trv->gc.t.h.flags &= ~LFS3_t_STALE; + } + while (true) { // some redund blocks left over? if (trv->blocks[0] != -1) { @@ -17005,6 +17013,9 @@ int lfs3_trv_read(lfs3_t *lfs3, lfs3_trv_t *trv, return tag; } + // ignore new stale flags + trv->gc.t.h.flags &= ~LFS3_t_STALE; + // figure out type/blocks if (tag == LFS3_TAG_MDIR) { lfs3_mdir_t *mdir = (lfs3_mdir_t*)bptr.d.u.buffer; @@ -17029,38 +17040,19 @@ int lfs3_trv_read(lfs3_t *lfs3, lfs3_trv_t *trv, } } -#ifndef LFS3_RDONLY -static void lfs3_trv_ckpoint_(lfs3_t *lfs3, lfs3_trv_t *trv) { - (void)lfs3; - - // clobber traversal - lfs3_mgc_ckpoint(&trv->gc); - - // and clear any pending blocks - trv->blocks[0] = -1; - trv->blocks[1] = -1; -} -#endif - static int lfs3_trv_rewind_(lfs3_t *lfs3, lfs3_trv_t *trv) { (void)lfs3; - // reset traversal lfs3_mgc_init(&trv->gc, - trv->gc.t.h.flags - & ~LFS3_t_DIRTY - & ~LFS3_t_CKPOINTED); - - // and clear any pending blocks - trv->blocks[0] = -1; - trv->blocks[1] = -1; - + (trv->gc.t.h.flags + & ~LFS3_t_DIRTY + & ~LFS3_t_CKPOINTED) + | LFS3_t_STALE); return 0; } int lfs3_trv_rewind(lfs3_t *lfs3, lfs3_trv_t *trv) { LFS3_ASSERT(lfs3_handle_isopen(lfs3, &trv->gc.t.h)); - return lfs3_trv_rewind_(lfs3, trv); } diff --git a/lfs3.h b/lfs3.h index b5a1ea82..656bd831 100644 --- a/lfs3.h +++ b/lfs3.h @@ -340,9 +340,10 @@ enum lfs3_btype { #define LFS3_t_TYPE 0xf0000000 // The traversal's type #define LFS3_t_BTYPE 0x00f00000 // The current block type #define LFS3_t_ZOMBIE 0x08000000 // File has been removed -#define LFS3_t_DIRTY 0x04000000 // Filesystem ckpointed outside traversal #define LFS3_t_CKPOINTED \ - 0x02000000 // Filesystem ckpointed during traversal + 0x04000000 // Filesystem ckpointed during traversal +#define LFS3_t_DIRTY 0x02000000 // Filesystem ckpointed outside traversal +#define LFS3_t_STALE 0x01000000 // Block queue probably out-of-date // GC flags #ifndef LFS3_RDONLY @@ -1022,7 +1023,7 @@ typedef struct lfs3 { // optional incremental gc state #ifdef LFS3_GC - lfs3_trv_t gc; + lfs3_mgc_t gc; #endif } lfs3_t; diff --git a/scripts/dbgflags.py b/scripts/dbgflags.py index 959265a0..f4da8fdc 100755 --- a/scripts/dbgflags.py +++ b/scripts/dbgflags.py @@ -159,8 +159,9 @@ t_MDIR = 0x00100000 # i^ Btype = mdir t_BTREE = 0x00200000 # i^ Btype = btree t_DATA = 0x00300000 # i^ Btype = data t_ZOMBIE = 0x08000000 # i- File has been removed -t_DIRTY = 0x04000000 # i- Filesystem ckpointed outside traversal -t_CKPOINTED = 0x02000000 # i- Filesystem ckpointed during traversal +t_CKPOINTED = 0x04000000 # i- Filesystem ckpointed during traversal +t_DIRTY = 0x02000000 # i- Filesystem ckpointed outside traversal +t_STALE = 0x01000000 # i- Block queue probably out-of-date # Block allocator flags alloc_ERASE = 0x00000001 # i- Please erase the block diff --git a/tests/test_gc.toml b/tests/test_gc.toml index f6ea1f4b..f8efa1fe 100644 --- a/tests/test_gc.toml +++ b/tests/test_gc.toml @@ -57,7 +57,7 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_RELOOKAHEAD); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC until we make progress for (lfs3_block_t i = 0;; i++) { @@ -127,11 +127,11 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_RELOOKAHEAD); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC one step lfs3_fs_gc(&lfs3) => 0; - assert(lfs3.handles == &lfs3.gc.gc.t.h); + assert(lfs3.handles == &lfs3.gc.t.h); // mutate the filesystem lfs3_file_open(&lfs3, &file, "spider", @@ -143,7 +143,7 @@ code = ''' lfs3_file_close(&lfs3, &file) => 0; // run GC until our traversal is done - while (lfs3.handles == &lfs3.gc.gc.t.h) { + while (lfs3.handles == &lfs3.gc.t.h) { lfs3_fs_gc(&lfs3) => 0; } @@ -318,7 +318,7 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_REGBMAP); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC until we make progress for (lfs3_block_t i = 0;; i++) { @@ -391,11 +391,11 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_REGBMAP); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC one step lfs3_fs_gc(&lfs3) => 0; - assert(lfs3.handles == &lfs3.gc.gc.t.h); + assert(lfs3.handles == &lfs3.gc.t.h); // mutate the filesystem lfs3_file_open(&lfs3, &file, "spider", @@ -407,7 +407,7 @@ code = ''' lfs3_file_close(&lfs3, &file) => 0; // run GC until our traversal is done - while (lfs3.handles == &lfs3.gc.gc.t.h) { + while (lfs3.handles == &lfs3.gc.t.h) { lfs3_fs_gc(&lfs3) => 0; } @@ -592,7 +592,7 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_COMPACTMETA); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC until we make progress for (lfs3_block_t i = 0;; i++) { @@ -684,19 +684,19 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_COMPACTMETA); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC one traversal + one step while (true) { lfs3_fs_gc(&lfs3) => 0; // internal traversal done? - if (lfs3.handles != &lfs3.gc.gc.t.h) { + if (lfs3.handles != &lfs3.gc.t.h) { break; } } lfs3_fs_gc(&lfs3) => 0; - assert(lfs3.handles == &lfs3.gc.gc.t.h); + assert(lfs3.handles == &lfs3.gc.t.h); // mutate the filesystem lfs3_file_rewind(&lfs3, &file) => 0; @@ -707,7 +707,7 @@ code = ''' lfs3_file_sync(&lfs3, &file) => 0; // run GC until our traversal is done (twice for compactmeta) - while (lfs3.handles == &lfs3.gc.gc.t.h) { + while (lfs3.handles == &lfs3.gc.t.h) { lfs3_fs_gc(&lfs3) => 0; } @@ -811,7 +811,7 @@ code = ''' struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_MKCONSISTENT); - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); // run GC until we make progress for (lfs3_block_t i = 0;; i++) { @@ -917,7 +917,7 @@ code = ''' lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags & LFS3_I_MKCONSISTENT); #ifdef LFS3_GC - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); #endif // call lfs3_fs_mkconsistent @@ -1021,9 +1021,9 @@ code = ''' } // run GC one step - assert(lfs3.handles != &lfs3.gc.gc.t.h); + assert(lfs3.handles != &lfs3.gc.t.h); lfs3_fs_gc(&lfs3) => 0; - assert(lfs3.handles == &lfs3.gc.gc.t.h); + assert(lfs3.handles == &lfs3.gc.t.h); // create the rest of the orphans after GC has started for (lfs3_size_t i = 0; i < ORPHANS; i++) { @@ -1045,7 +1045,7 @@ code = ''' assert(fsinfo.flags & LFS3_I_MKCONSISTENT); // run GC until our traversal is done - while (lfs3.handles == &lfs3.gc.gc.t.h) { + while (lfs3.handles == &lfs3.gc.t.h) { lfs3_fs_gc(&lfs3) => 0; }