From c34da290a580e4cd665531a9ae66402afb2c97de Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 5 Dec 2025 01:47:44 -0600 Subject: [PATCH] gc: Simplified gc heuristics, prioritize lookahead work This is a number of tweaks intended to (1) minimize unexpected gc latency due to last-minute lookahead scans, while (2) keeping the gc logic simple and easy to reason about: - Switched to using lfs3->flags directly for the is-work-done predicate. This ensures lfs3_fs_gc_ never terminates until the requested work is done, at the risk of, well, never terminating. But the previous "pending" variables had the same risk (set gc_compact_thresh=0 for example), it just removed the risk of non-termination due to conflicting gc requests. In both cases gc_compact_thresh has the biggest risk of non-termination. - Prioritize lookahead scans before anything that can allocate (MKCONSISTENT, COMPACT, etc). - Dropped aborting useless traversals (ckpointed lookaheads mainly). It's a good idea, but surprisingly complicated to decide when we should abort for all traversals. COMPACT, CKMETA, for example, are still useful to continue even if they can't prove anything about the system. Though now that I'm writing this, I'm wondering what the argument against LOOKAHEAD aborting is. Maybe this should be reverted for LOOKAHEAD as a special case... Code changes minimal: code stack ctx before: 35152 2136 660 after: 35124 (-0.1%) 2136 (+0.0%) 660 (+0.0%) code stack ctx gbmap before: 38076 2136 776 gbmap after: 38048 (-0.1%) 2136 (+0.0%) 776 (+0.0%) --- lfs3.c | 42 +++++++++++++++++------------------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/lfs3.c b/lfs3.c index 264a9b78..4dedb3b7 100644 --- a/lfs3.c +++ b/lfs3.c @@ -16355,34 +16355,28 @@ static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_mgc_t *mgc, #endif // do we have any pending work? - uint32_t pending = flags & lfs3->flags & LFS3_GC_ALL; - - while (pending && (lfs3_off_t)steps > 0) { + while ((flags & (lfs3->flags & LFS3_GC_ALL)) + && (lfs3_off_t)steps > 0) { // start a new traversal? if (!lfs3_handle_isopen(lfs3, &mgc->t.h)) { - lfs3_mgc_init(mgc, pending); + lfs3_mgc_init(mgc, flags & (lfs3->flags & LFS3_GC_ALL)); lfs3_handle_open(lfs3, &mgc->t.h); - } - // don't bother with lookahead/gbmap if we've ckpointed - #ifndef LFS3_RDONLY - if (lfs3_t_isckpointed(mgc->t.h.flags)) { - mgc->t.h.flags &= ~LFS3_T_LOOKAHEAD; - } - #endif + // prioritize lookahead/gbmap before any work that may need to + // allocate + if (lfs3_t_islookahead(mgc->t.h.flags)) { + mgc->t.h.flags &= ~( + LFS3_IFDEF_RDONLY(0, LFS3_GC_MKCONSISTENT) + | LFS3_IFDEF_RDONLY(0, LFS3_GC_COMPACT)); + } - // will this traversal still make progress? no? start over - if (!(mgc->t.h.flags & LFS3_GC_ALL)) { - lfs3_handle_close(lfs3, &mgc->t.h); - continue; - } - - // do we really need a full traversal? - if (!(mgc->t.h.flags & ( - LFS3_IFDEF_RDONLY(0, LFS3_GC_LOOKAHEAD) - | LFS3_GC_CKMETA - | LFS3_GC_CKDATA))) { - mgc->t.h.flags |= LFS3_T_MTREEONLY; + // do we really need a full traversal? + if (!(mgc->t.h.flags & ( + LFS3_IFDEF_RDONLY(0, LFS3_GC_LOOKAHEAD) + | LFS3_GC_CKMETA + | LFS3_GC_CKDATA))) { + mgc->t.h.flags |= LFS3_T_MTREEONLY; + } } // progress gc @@ -16397,8 +16391,6 @@ static int lfs3_fs_gc_(lfs3_t *lfs3, lfs3_mgc_t *mgc, // end of traversal? if (tag == LFS3_ERR_NOENT) { lfs3_handle_close(lfs3, &mgc->t.h); - // clear any pending flags we make progress on - pending &= lfs3->flags & LFS3_GC_ALL; } // decrement steps