From 9bd44aec1226d844339ef658e01e0b0e54e8f248 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sat, 3 Jan 2026 01:18:54 -0600 Subject: [PATCH] preerase: Added test_mount_t_preerase, fixed several more bugs - Fixed lfs3_alloc_cansyncgbmap ignoring known window changes. Being able to just call lfs3_btree_cmp(b, b_p) would be nice, but this ignores known window changes! Fixed by comparing the on-disk encoding, which is heavy-handed, but probably the safest approach. lfs3_alloc_cansyncgbmap will probably never be on the stack hot-path, and the added code is roughly one function call. The main cost is CPU-cycles, but fortunately(?) that's not something we really care about? - Fixed lfs3_allocclaim accidentally returning lfs3_mdir_commit's return value instead of the allocated block! Probably caused by a copy-paste, resulted in lfs3_allocclaim returning block 0, which is really not good! - Fixed assert typo in lfs3_trv_open where we assert REVPERTURB in flags instead of lfs3->flags. Code changes: code stack ctx before: 35260 2136 660 after: 35260 (+0.0%) 2136 (+0.0%) 660 (+0.0%) code stack ctx gbmap before: 38560 2144 776 gbmap after: 38616 (+0.1%) 2144 (+0.0%) 776 (+0.0%) code stack ctx preerase before: 39168 2168 796 preerase after: 39232 (+0.2%) 2168 (+0.0%) 796 (+0.0%) --- lfs3.c | 18 +++++-- tests/test_gc.toml | 97 ++++++++++++++++++++++++++++++++----- tests/test_mount.toml | 110 +++++++++++++++++++++++++++++++++++++++++- tests/test_trvs.toml | 22 +++++++-- 4 files changed, 227 insertions(+), 20 deletions(-) diff --git a/lfs3.c b/lfs3.c index 78b237bd..1195d91f 100644 --- a/lfs3.c +++ b/lfs3.c @@ -10868,7 +10868,15 @@ static inline bool lfs3_alloc_canpreerase(const lfs3_t *lfs3) { // is gbmap out-of-sync with disk? #if !defined(LFS3_RDONLY) && defined(LFS3_GBMAP) static inline bool lfs3_alloc_cansyncgbmap(const lfs3_t *lfs3) { - return lfs3_btree_cmp(&lfs3->gbmap.b, &lfs3->gbmap.b_p) != 0; + // do we even have a gbmap? + if (!lfs3_f_isgbmap(lfs3->flags)) { + return false; + } + + // just compare the on-disk encoding + uint8_t gbmap_[LFS3_GBMAP_DSIZE]; + lfs3_data_fromgbmap(&lfs3->gbmap, gbmap_); + return memcmp(gbmap_, lfs3->gbmap_p, LFS3_GBMAP_DSIZE) != 0; } #endif @@ -11311,7 +11319,10 @@ static lfs3_sblock_t lfs3_allocclaim(lfs3_t *lfs3, lfs3_mdir_t *mdir, if (lfs3_ecksum_isecksum(&ecksum_)) { LFS3_ASSERT(lfs3_alloc_cansyncgbmap(lfs3)); // lfs3_mdir_commit implicitly commits any pending gbmap state - return lfs3_mdir_commit(lfs3, mdir, LFS3_RATTRS(LFS3_RATTR_NULL)); + int err = lfs3_mdir_commit(lfs3, mdir, LFS3_RATTRS(LFS3_RATTR_NULL)); + if (err) { + return err; + } } #endif @@ -17091,7 +17102,8 @@ int lfs3_trv_open(lfs3_t *lfs3, lfs3_trv_t *trv, uint32_t flags) { // we can't use preerased blocks without revperturb, so this is // likely a mistake #if !defined(LFS3_RDONLY) && defined(LFS3_PREERASE) - LFS3_ASSERT(lfs3_m_isrevperturb(flags) || !lfs3_t_ispreerase(flags)); + LFS3_ASSERT(lfs3_m_isrevperturb(lfs3->flags) + || !lfs3_t_ispreerase(flags)); #endif // setup traversal state diff --git a/tests/test_gc.toml b/tests/test_gc.toml index d1e130c6..3875c63f 100644 --- a/tests/test_gc.toml +++ b/tests/test_gc.toml @@ -1853,9 +1853,19 @@ code = ''' lfs3_t lfs3; lfs3_format(&lfs3, LFS3_F_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_F_REVPERTURB, -1) + : 0) | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_F_GBMAP, -1) : 0), CFG) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0), + CFG) => 0; // create an interesting filesystem uint32_t prng = 42; @@ -1877,13 +1887,22 @@ code = ''' // remount to reset flags lfs3_unmount(&lfs3) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0), + CFG) => 0; // check that flags were reset struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | LFS3_I_MKCONSISTENT | ((!GBMAP || (fsinfo.flags & LFS3_I_LOOKAHEAD)) @@ -1982,7 +2001,14 @@ code = ''' // remount with gc flags } else if (AFTER == 3) { lfs3_unmount(&lfs3) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR | GC_FLAGS, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0) + | GC_FLAGS, + CFG) => 0; } else { assert(false); @@ -1991,7 +2017,10 @@ code = ''' // did these clear the right flags? lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | ((!MKCONSISTENT) ? LFS3_I_MKCONSISTENT : 0) | ((!LOOKAHEAD @@ -2054,9 +2083,19 @@ code = ''' lfs3_t lfs3; lfs3_format(&lfs3, LFS3_F_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_F_REVPERTURB, -1) + : 0) | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_F_GBMAP, -1) : 0), CFG) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0), + CFG) => 0; // create an interesting filesystem uint32_t prng = 42; @@ -2078,13 +2117,22 @@ code = ''' // remount to reset flags lfs3_unmount(&lfs3) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0), + CFG) => 0; // check that flags were reset struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | LFS3_I_MKCONSISTENT | ((!GBMAP || (fsinfo.flags & LFS3_I_LOOKAHEAD)) @@ -2183,7 +2231,14 @@ code = ''' // remount with gc flags } else if (AFTER == 3) { lfs3_unmount(&lfs3) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR | GC_FLAGS, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0) + | GC_FLAGS, + CFG) => 0; } else { assert(false); @@ -2192,7 +2247,10 @@ code = ''' // did these clear the right flags? lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | ((!MKCONSISTENT) ? LFS3_I_MKCONSISTENT : 0) | ((!LOOKAHEAD @@ -2214,7 +2272,10 @@ code = ''' // check that flags were reset lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | LFS3_I_MKCONSISTENT | ((!GBMAP || (fsinfo.flags & LFS3_I_LOOKAHEAD)) @@ -2317,7 +2378,14 @@ code = ''' // remount with gc flags } else if (AFTER == 3) { lfs3_unmount(&lfs3) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR | GC_FLAGS, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0) + | GC_FLAGS, + CFG) => 0; } else { assert(false); @@ -2326,7 +2394,10 @@ code = ''' // did these clear the right flags? lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | ((!MKCONSISTENT) ? LFS3_I_MKCONSISTENT : 0) | ((!LOOKAHEAD diff --git a/tests/test_mount.toml b/tests/test_mount.toml index 6431f2a3..3ca3ff8b 100644 --- a/tests/test_mount.toml +++ b/tests/test_mount.toml @@ -47,6 +47,7 @@ if = [ '!RDONLY || !LOOKAHEAD', 'LFS3_IFDEF_PREERASE(true, !PREERASE)', 'GBMAP || !PREERASE', + 'REVPERTURB || !PREERASE', '!RDONLY || !PREERASE', '!RDONLY || !COMPACT', ] @@ -140,6 +141,7 @@ if = [ 'LFS3_IFDEF_CKDATACKSUMS(true, !CKDATACKSUMS)', 'LFS3_IFDEF_PREERASE(true, !PREERASE)', 'GBMAP || !PREERASE', + 'REVPERTURB || !PREERASE', ] code = ''' lfs3_t lfs3; @@ -259,7 +261,7 @@ code = ''' uint32_t prng = 42; - // gbmap is persistant, so by default we _don't_ need a gbmap scan + // gbmap is persistent, so by default we _don't_ need a gbmap scan lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0; struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; @@ -320,6 +322,112 @@ code = ''' lfs3_unmount(&lfs3) => 0; ''' +[cases.test_mount_t_preerase] +defines.LOOKAHEAD = [false, true] +defines.CKMETA = [false, true] +defines.CKDATA = [false, true] +defines.SIZE = [ + 'BLOCK_SIZE/2', + 'BLOCK_SIZE', + '2*BLOCK_SIZE', + '8*BLOCK_SIZE', +] +# REMOUNT=0 => don't remount +# REMOUNT=1 => remount with preerase +# REMOUNT=2 => remount without preerase +defines.REMOUNT = [0, 1, 2] +ifdef = 'LFS3_GBMAP && LFS3_REVPERTURB && LFS3_PREERASE' +if = 'GBMAP' +code = ''' + lfs3_t lfs3; + lfs3_format(&lfs3, + LFS3_F_RDWR + // note preerasing needs revperturb + | LFS3_F_REVPERTURB + | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_F_GBMAP, -1) : 0), + CFG) => 0; + + uint32_t prng = 42; + + // by default, we don't preerase + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | LFS3_M_REVPERTURB, + CFG) => 0; + struct lfs3_fsinfo fsinfo; + lfs3_fs_stat(&lfs3, &fsinfo) => 0; + assert(fsinfo.flags == ( + LFS3_I_REVPERTURB + | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) + | LFS3_I_MKCONSISTENT + | LFS3_I_PREERASE + | LFS3_I_COMPACT + | LFS3_I_CKMETA + | LFS3_I_CKDATA + | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_I_GBMAP, -1) : 0))); + lfs3_unmount(&lfs3) => 0; + + // with LFS3_M_PREERASE, mount preerases a configurable number of + // blocks + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | LFS3_M_REVPERTURB + | ((LOOKAHEAD) ? LFS3_M_LOOKAHEAD : 0) + | LFS3_M_PREERASE, + CFG) => 0; + lfs3_fs_stat(&lfs3, &fsinfo) => 0; + assert(fsinfo.flags == ( + LFS3_I_REVPERTURB + | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) + | LFS3_I_MKCONSISTENT + | ((!LOOKAHEAD) ? LFS3_I_LOOKAHEAD : 0) + | LFS3_I_COMPACT + | LFS3_I_CKMETA + | LFS3_I_CKDATA + | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_I_GBMAP, -1) : 0))); + + // remount? + // + // preerasing is persistent, but our preeraser doesn't know the + // gbmap state until triggered at least once + if (REMOUNT) { + lfs3_unmount(&lfs3) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | LFS3_M_REVPERTURB + | ((REMOUNT == 1 && LOOKAHEAD) ? LFS3_M_LOOKAHEAD : 0) + | ((REMOUNT == 1) ? LFS3_M_PREERASE : 0), + CFG) => 0; + lfs3_fs_stat(&lfs3, &fsinfo) => 0; + assert(fsinfo.flags == ( + LFS3_I_REVPERTURB + | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) + | LFS3_I_MKCONSISTENT + | ((!LOOKAHEAD) ? LFS3_I_LOOKAHEAD : 0) + | ((REMOUNT != 1) ? LFS3_I_PREERASE : 0) + | LFS3_I_COMPACT + | LFS3_I_CKMETA + | LFS3_I_CKDATA + | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_I_GBMAP, -1) : 0))); + } + + // write to a file to try to make sure nothing obvious is broken + lfs3_file_t file; + lfs3_file_open(&lfs3, &file, "jellyfish", + LFS3_O_WRONLY | LFS3_O_CREAT | LFS3_O_EXCL) => 0; + uint8_t wbuf[SIZE]; + for (lfs3_size_t j = 0; j < SIZE; j++) { + wbuf[j] = 'a' + (TEST_PRNG(&prng) % 26); + } + lfs3_file_write(&lfs3, &file, wbuf, SIZE) => SIZE; + lfs3_file_close(&lfs3, &file) => 0; + + lfs3_unmount(&lfs3) => 0; +''' + [cases.test_mount_t_compact] defines.LOOKAHEAD = [false, true] defines.CKMETA = [false, true] diff --git a/tests/test_trvs.toml b/tests/test_trvs.toml index 37e3d7fc..23f49f3b 100644 --- a/tests/test_trvs.toml +++ b/tests/test_trvs.toml @@ -1772,15 +1772,28 @@ code = ''' lfs3_t lfs3; lfs3_format(&lfs3, LFS3_F_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_F_REVPERTURB, -1) + : 0) | ((GBMAP) ? LFS3_IFDEF_GBMAP(LFS3_F_GBMAP, -1) : 0), CFG) => 0; - lfs3_mount(&lfs3, LFS3_M_RDWR, CFG) => 0; + lfs3_mount(&lfs3, + LFS3_M_RDWR + // note preerasing needs revperturb + | ((PREERASE) + ? LFS3_IFDEF_PREERASE(LFS3_M_REVPERTURB, -1) + : 0), + CFG) => 0; // check flags before struct lfs3_fsinfo fsinfo; lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | LFS3_I_MKCONSISTENT | ((!GBMAP) ? LFS3_I_LOOKAHEAD : 0) @@ -1821,7 +1834,10 @@ code = ''' // check flags after lfs3_fs_stat(&lfs3, &fsinfo) => 0; assert(fsinfo.flags == ( - LFS3_IFYES_REVPERTURB(LFS3_I_REVPERTURB, 0, 0) + LFS3_IFYES_REVPERTURB( + LFS3_I_REVPERTURB, + (PREERASE) ? LFS3_I_REVPERTURB : 0, + 0) | LFS3_IFYES_REVNOISE(LFS3_I_REVNOISE, 0, 0) | ((!MKCONSISTENT) ? LFS3_I_MKCONSISTENT : 0) | ((!LOOKAHEAD && !GBMAP) ? LFS3_I_LOOKAHEAD : 0)