Thinking about use case a bit, most lfsr_fs_gc will be to perform
background work, and can benefit from being incremental.
We already support incremental gc and all the mess associated with
traversal invalidation via the traversal API, so we might as well expose
this through lfsr_fs_gc.
The main downside is that we need to store an lfsr_traversal_t object
somewhere, which is not exactly a cheap struct. I was originally
considering limiting incremental gc to the traversal API for this
reason, but I think the value add of an incremental lfsr_fs_gc is too
compelling... Though we really should add a compile-time option
(LFS_NO_GC? LFS_NO_INCRGC?) to allow users to opt-out of this RAM cost
if they're never going to call this function.
Oh, and lfs_t also becomes self-referential, which might become a
problem for higher-level language users...
---
The incremental behavior of lfsr_fs_gc can be controlled by the new
gc_steps config option. This allows more than one step to be performed
at a time, which may allow for more progress when intermixed with
write-heavy filesystem operations. Setting gc_steps=-1 performs a full
traversal every call, which guarantees always making some amount of
progress.
This adds a bit of code, since we now need to check for/resume existing
traversals. But the real cost is the added RAM to lfs_t, which is
unfortunately wasted if you never call lfsr_fs_gc:
code stack lfs_t
before: 35708 2672 164
after: 35756 (+0.1%) 2672 (+0.0%) 296 (+80.5%)
This just provides a simple, easy-to-call, wrapper over the new
traversal API:
int lfsr_fs_gc(lfs_t *lfs, uint32_t flags);
The main difference from its previous incarnation, is that lfsr_fs_gc
now takes a flags argument to indicate exactly what gc operations to
perform. This gives the user more control, and may also make the API
more robust towards adding new features:
LFS_GC_MTREEONLY = 0x0010, // Only traverse the mtree
LFS_GC_MKCONSISTENT = 0x0020, // Make the filesystem consistent
LFS_GC_LOOKAHEAD = 0x0040, // Populate lookahead buffer
LFS_GC_COMPACT = 0x0080, // Compact metadata logs
LFS_GC_CKMETA = 0x0100, // Check metadata checksums
LFS_GC_CKDATA = 0x0200, // Check metadata + data checksums
LFS_GC_REPAIRMETA+ = 0x0400, // Repair metadata blocks
LFS_GC_REPAIRDATA+ = 0x0800, // Repair metadata + data blocks
+ Planned
Alternatively, gc_flags could have been added as a config option. But
making gc_flags a function argument matches other flag APIs (open
mainly), and is slightly more flexible in that it allows a system to do
different gc operations in different system states (though this could
also be accomplished with the hypothetical lfsr_fs_gccfg, which would
probably be good to add anyways).
Worst case, defining a system-wide define that you always pass to
lfsr_fs_gc accomplishes roughly the same thing.
---
This adds a bit more code, mainly to check if we actually need to
traverse, and to make sure traversals accomplish all of the requested
work.
code stack
before: 35448 2680
after: 35708 (+0.7%) 2672 (-0.3%)
Curiously it also saved a bit of stack, which is a bit silly given this
commit is purely code addition. Apparently something in lfs_alloc and
lfsr_fs_gc is shared, getting uninlined, and messing with the stack
measurement. lfs_alloc is quite sensitive to stack changes after all.
Christopher Haster