From e3bca2bb7c0c9d5a291e4e4656ec3f65b75f1474 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Mon, 29 Dec 2025 17:11:34 -0600 Subject: [PATCH] preerase: Made REVPERTURB required to use pre-erased blocks With REVPERTURB implemented, I think this is all we need to finish up preerase allocations. Though not quite tested yet. As noted when implementing lfs3_allocclaim, lfs3_allocclaim is only half the solution for preerase allocations. If we tried to use lfs3_allocclaim everywhere, our mdir commit path would quickly end up a recursive mess. This is where our ecksums kick in. In theory, ecksums (erased-state checksums), let us detect attempted progs. Unfortunately, in practice it's not so simple. If we tried to detect a failed data block write, for example, it's entirely possible the attempted write matches the erased-state exactly, making attempted prog detection impossible. Imagine if users couldn't write all 0xffs to a file, that'd be a weird constraint. To work around this, we also require at least one bit flip during progs. This ensures an ecksum failure requires a non-trivial checksum collision. This is where REVPERTURB comes in (and in rbyd logs, the valid bits). --- Humorously, now that REVPERTURBs are implemented, I think the only change required for preerased allocations is to error if REVPERTURB is disabled. Extra humorously, this is surprisingly tricky because REVPERTURB is a mount flag and PREERASE a gc flag. The solution is sort of best-effort. We error if trying to preerase without REVPERTURB, but _don't_ error if trying to allocate when the gbmap contains preerased blocks. This wastes the preerase cycles, but allows disk compatibility between filesystems in different modes. No code changes: code stack ctx before: 35144 2136 660 after: 35144 (+0.0%) 2136 (+0.0%) 660 (+0.0%) code stack ctx gbmap+np before: 38272 2144 776 gbmap+np after: 38272 (+0.0%) 2144 (+0.0%) 776 (+0.0%) code stack ctx gbmap+yp before: 38908 2168 796 gbmap+yp after: 38908 (+0.0%) 2168 (+0.0%) 796 (+0.0%) --- lfs3.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/lfs3.c b/lfs3.c index 6a6259da..17a1fe17 100644 --- a/lfs3.c +++ b/lfs3.c @@ -10995,11 +10995,12 @@ static lfs3_sblock_t lfs3_alloc_findfree(lfs3_t *lfs3, // free? erased? // - // well, we can only use erased if pre-erase support is - // enabled + // well, we can only use erased if pre-erase and + // revperturb is enabled if (tag == LFS3_TAG_BMFREE || LFS3_IFDEF_PREERASE( - tag == LFS3_TAG_BMERASED, + tag == LFS3_TAG_BMERASED + && lfs3_m_isrevperturb(lfs3->flags), false)) { lfs3->gbmap.next = lfs3_min( (block+1) - lfs3->gbmap.window, @@ -15964,6 +15965,13 @@ int lfs3_mount(lfs3_t *lfs3, uint32_t flags, #endif LFS3_ASSERT(!lfs3_m_isrdonly(flags) || !lfs3_t_compact(flags)); #endif + // we can't use preerased blocks without revperturb, so this is + // likely a mistake + #if !defined(LFS3_RDONLY) \ + && defined(LFS3_GBMAP) \ + && !defined(LFS3_NO_PREERASE) + LFS3_ASSERT(lfs3_m_isrevperturb(flags) || !lfs3_t_ispreerase(flags)); + #endif int err = lfs3_init(lfs3, flags & ( @@ -16270,6 +16278,13 @@ int lfs3_format(lfs3_t *lfs3, uint32_t flags, | LFS3_F_COMPACT | LFS3_F_CKMETA | LFS3_F_CKDATA)) == 0); + // we can't use preerased blocks without revperturb, so this is + // likely a mistake + #if !defined(LFS3_RDONLY) \ + && defined(LFS3_GBMAP) \ + && !defined(LFS3_NO_PREERASE) + LFS3_ASSERT(lfs3_m_isrevperturb(flags) || !lfs3_t_ispreerase(flags)); + #endif int err = lfs3_init(lfs3, flags & ( @@ -16733,6 +16748,14 @@ int lfs3_fs_ck(lfs3_t *lfs3, uint32_t flags) { LFS3_ASSERT(!lfs3_m_isrdonly(lfs3->flags) || !lfs3_t_compact(flags)); #endif + // we can't use preerased blocks without revperturb, so this is + // likely a mistake + #if !defined(LFS3_RDONLY) \ + && defined(LFS3_GBMAP) \ + && !defined(LFS3_NO_PREERASE) + LFS3_ASSERT(lfs3_m_isrevperturb(lfs3->flags) + || !lfs3_t_ispreerase(flags)); + #endif // set needs-ck flags, this has the side-effect of signaling ck work // is incomplete if we encounter an error, which is probably a good @@ -16748,6 +16771,7 @@ int lfs3_fs_ck(lfs3_t *lfs3, uint32_t flags) { // perform any pending janitorial work #ifdef LFS3_GC int lfs3_fs_gc(lfs3_t *lfs3) { + // TODO should we actually assert on these in lfs3_init? // unknown gc flags? LFS3_ASSERT((lfs3->cfg->gc_flags & ~( LFS3_IFDEF_RDONLY(0, LFS3_GC_MKCONSISTENT) @@ -16771,6 +16795,14 @@ int lfs3_fs_gc(lfs3_t *lfs3) { #endif LFS3_ASSERT(!lfs3_m_isrdonly(lfs3->flags) || !lfs3_t_compact(lfs3->cfg->gc_flags)); + // we can't use preerased blocks without revperturb, so this is + // likely a mistake + #if !defined(LFS3_RDONLY) \ + && defined(LFS3_GBMAP) \ + && !defined(LFS3_NO_PREERASE) + LFS3_ASSERT(lfs3_m_isrevperturb(lfs3->flags) + || !lfs3_t_ispreerase(lfs3->cfg->gc_flags)); + #endif // run gc a configurable number of steps return lfs3_fs_gc_(lfs3, &lfs3->gc, @@ -17052,6 +17084,13 @@ int lfs3_trv_open(lfs3_t *lfs3, lfs3_trv_t *trv, uint32_t flags) { LFS3_ASSERT(!lfs3_t_ismtreeonly(flags) || !lfs3_t_islookahead(flags)); #endif LFS3_ASSERT(!lfs3_t_ismtreeonly(flags) || !lfs3_t_isckdata(flags)); + // we can't use preerased blocks without revperturb, so this is + // likely a mistake + #if !defined(LFS3_RDONLY) \ + && defined(LFS3_GBMAP) \ + && !defined(LFS3_NO_PREERASE) + LFS3_ASSERT(lfs3_m_isrevperturb(flags) || !lfs3_t_ispreerase(flags)); + #endif // setup traversal state trv->gc.t.h.flags = flags | lfs3_o_typeflags(LFS3_type_TRV);