Commit Graph

27 Commits

Author SHA1 Message Date
Christopher Haster 6eba1180c8 Big rename! Renamed lfs -> lfs3 and lfsr -> lfs3 2025-05-28 15:00:04 -05:00
Christopher Haster f7e17c8aad Added LFS_T_RDONLY, LFS_T_RDWR, etc
These mimic the relevant LFS_O_* flags, and allow users to assert
whether or not a traversal will mutate the filesystem:

  LFS_T_MODE          0x00000001  The traversal's access mode
  LFS_T_RDWR          0x00000000  Open traversal as read and write
  LFS_T_RDONLY        0x00000001  Open traversal as read only

In theory, these could also change internal allocations, but littlefs
doesn't really work that way.

Note we _don't_ add related LFS_GC_RDONLY, LFS_GC_RDWR, etc flags. These
are sort of implied by the relevant LFS_M_* flags.

Adds a bit more code, probably because of the slightly more complicated
internal constants for the internal traversals. But I think the
self-documentingness is worth it:

           code          stack          ctx
  before: 37200           2288          636
  after:  37220 (+0.1%)   2288 (+0.0%)  636 (+0.0%)
2025-05-24 23:27:10 -05:00
Christopher Haster b5e503ca85 Made lfsr_file_sync a noop if zombied
So now calling lfsr_file_sync on zombied files is a noop:

  // create a file
  lfsr_file_t a;
  lfsr_file_open(&lfs, &a, "a",
          LFS_O_RDWR | LFS_O_CREAT | LFS_O_EXCL) => 0;

  // remove, creating a zombie
  lfsr_remove(&lfs, "a") => 0;

  // sync, this is now a noop (previously LFS_ERR_NOENT)
  lfsr_file_sync(&lfs, &a) => 0;

  // close is also a noop
  lfsr_file_close(&lfs, &a) => 0;

I've been on the fence on this for a while, on one hand erroring
provides more information to the user, on the other hand a noop is less
surprising if the user comes from other systems.

Ended up making this a noop. I figured minimizing surprises is good API
design, and the user can always use lfsr_stat to check if the file still
exists.

This also matches POSIX, and, perhaps more importantly, the current
version of littlefs.

---

Note that lfsr_file_resync still errors with LFS_ERR_NOENT. It's hard to
argue the file "matches the state of disk" otherwise.

Code changes minimal:

           code          stack          ctx
  before: 35784           2440          640
  after:  35780 (-0.0%)   2440 (+0.0%)  640 (+0.0%)
2025-04-25 17:29:34 -05:00
Christopher Haster 76493142e7 Reworked stickynote API, exposed LFS_TYPE_STICKYNOTE to users
This adds the LFS_TYPE_STICKYNOTE type, allowing users to interact with
stickynotes as long as they aren't orphaned.

This hopefully solves the long-standing mess that was the LFS_O_EXCL
API.

---

As for what I mean by orphaned vs non-orphaned stickynotes:

Non-orphaned stickynotes represent files that have been "created" (via
LFS_O_CREAT), but not "committed" (via sync/close). You can still close
and convert the stickynote to a reg file, so these aren't orphans. These
are also called "uncreated" files in some parts of the codebase:

- open+O_CREAT -> non-orphaned stickynote (uncreated file)

Orphaned stickynotes are possible by either removing an open file, or
desyncing a file before sync/close. These are still invisible to the
user and will be eventually cleaned up after the last file handle is
closed:

- open+remove               -> orphaned stickynote (zombied file)
- open+O_CREAT+desync+close -> orphaned stickynote (orphaned file)

Desynced files are a bit special. Even though they technically aren't
orphaned, they also behave like orphaned file handles:

- open+O_CREAT+close -> orphaned stickynote (desynced file)

The idea is this mimics the state of files post-close, and allows for
some tricks like using a desync file as a temporary file with no
observable effects on the filesystem.

---

The motivation for this comes from staring at the LFS_O_EXCL API for too
long and realizing the problem is that littlefs's API contradicts itself
when it comes to whether or not uncreated files exist.

This solution is to consistently treat uncreated files as though they
exist (the alternative would make LFS_O_EXCL pretty much useless), but I
really didn't want to do this as having what appears to be normal files
disappear after powerloss risks confusion.

The compromise here is to give these files a special type, repurposing
the internal LFS_TAG_STICKYNOTE, which hopefully hints to the user these
won't behave like normal files.

If the user is more interested in POSIX compatibility, they can always
map these to either LFS_TYPE_REG or LFS_ERR_NOENT, whichever they think
is the least confusing.

As a quirk of littlefs's API, stickynotes should never actually contain
any data, and will always have size 0.

However they can have custom attributes assigned now (which is I guess
ok? also TODO should probably test this).

---

The implementation right now is a bit naive, I mostly just wanted to get
the tests working again in this new model. It may be possible to claw
back some of this code cost:

           code          stack          ctx
  before: 35740           2440          640
  after:  35952 (+0.6%)   2440 (+0.0%)  640 (+0.0%)
2025-04-23 23:22:09 -05:00
Christopher Haster 19a23c7788 Renamed/reverted file->buffer -> file->cache
And the related config options:

- cfg->file_buffer_size -> cfg->file_cache_size
- file->cfg->buffer_size -> file->cfg->cache_size
- file->cfg->buffer -> file->cfg->cache_buffer

The original motivation to rename this to file->buffer was to better
align with what other filesystems call this, but I think this is a case
where internal consistency is more important than external consistency.

file->cache better matches lfs->pcache and lfs->rcache, and makes it
easier to read code involving both file->cache and other user-provided
buffers.

Keeping the upstream name also helps with continuity.
2025-02-13 16:02:46 -06:00
Christopher Haster bc639b03f2 Reworked lfsr_bshrub_t, renamed file.o -> file.b
This moves all of the shrub tracking logic from lfsr_obshrub_t into
lfsr_bshrub_t, completely drops the lfsr_obshrub_t type, and changes all
lfsr_bshrub_* functions to take lfsr_bshrub_t instead of the mdir+shrub
pair.

This makes the lfsr_bshrub_* functions <-> lfsr_bshrub_t relationship
more consistent with other APIs, such as lfsr_btree_t:

  - lfsr_bshrub_lookupnext(lfs, &file->o.o.mdir, &file->o.bshrub, ...)
  + lfsr_bshrub_lookupnext(lfs, &file->b, ...)

I think the reason why this design wasn't obvious before is because, at
least conceptually, having the lfsr_mdir_t live inside the lfsr_bshrub_t
is a bit weird. It's only thanks to lfsr_file_t invasively using the
internal lfsr_mdir_t that we can avoid duplicate lfsr_mdir_t objects.

This also reorganizes the structs in lfs.h a bit, and renames the
related file.o -> file.b fields (much needed because lfs->gc.t.o.o.mdir.
rbyd.blocks was starting to get _real_ confusing).

---

Unfortunately, reducing the number of arguments to lfsr_bshrub_*
functions did not save nearly as much code as I thought it would. It
even ended up with a net _increase_ of code, apparently due to needing
to recalculate the bshrub->shrub offset more often:

           code          stack          ctx
  before: 36476           2608          640
  after:  36484 (+0.0%)   2608 (+0.0%)  640 (+0.0%)

Strange, but this rework is still worthwhile if only for the code
readability.
2025-02-11 02:50:28 -06:00
Christopher Haster 9ed9cf0ccd gc: Added more tests over info flags, dropped gc_flags default
Since we dropped lfsr_gc_setflags/setsteps, it was no longer possible to
set gc_flags to zero (perfectly valid and useful for system bringup/
testing things). Supporting gc_flags=0 means it's not possible to
provide a default, but this is probably ok as users need to opt-in to
LFS_GC anyways.

Note that at least gc_steps=0 doesn't make sense, so the default there
is reasonable.

Fixing this also highlighted that gc_flags/steps are no longer mutable,
making the comment in lfs_init out-of-date. Dropping these saves a bit
of lfs_t size, so that's nice.

And then testing also revealed that LFS_GC_CKDATA implying LFS_GC_CKDATA
means it should probably clear the LFS_I_CKMETA flag as well.

---

And here I thought this was going to be just a simple test-writing
exercise!

Code changes:

                   code          stack          ctx
  default before: 37792           2608          620
  default after:  37792 (-0.0%)   2608 (+0.0%)  620 (+0.0%)

  gc before:      37896           2608          768
  gc after:       37848 (-0.1%)   2608 (+0.0%)  760 (-1.0%)
2025-01-28 14:41:45 -06:00
Christopher Haster a4c74967ec Renamed LFS_I_* flags to match LFS_GC_*
- LFS_I_INCONSISTENT -> LFS_I_MKCONSISTENT
- LFS_I_CANLOOKAHEAD -> LFS_I_LOOKAHEAD
- LFS_I_UNCOMPACTED  -> LFS_I_COMPACT
- LFS_I_CANCKMETA    -> LFS_I_CKMETA
- LFS_I_CANCKDATA    -> LFS_I_CKDATA

This just makes everything easier to read/pattern match, even if it's
a bit inaccurate english-wise. The imperative transformations were also
wildly inconsistent...
2025-01-28 14:41:45 -06:00
Christopher Haster 9c9a23e27b gc: Renamed lfsr_gc -> lfsr_fs_gc, keep lfsr_fs_unck in non-gc
- lfsr_gc -> lfsr_fs_gc
- lfsr_gc_unck -> lfsr_fs_unck

lfsr_fs_unck is surprisingly still useful in non-gc builds, since we
still have ckmeta/ckdata state. These flags can still be queried with
lfsr_fs_stat and cleared with lfsr_fs_ckmeta/ckdata/lfsr_traversal_t, so
it seems useful to keep this function around.

It's also a relatively cheap function.

Though this does mean it deserves a rename. Dropping the gc prefix
hopefully makes it clearer this function is not entirely gc-specific.

And since we no longer have lfsr_gc_setflags/setsteps, it makes sense to
rename lfsr_gc back to lfsr_fs_gc, to be consistent with the other
filesystem-wide utilities.

Code changes, apparently lfsr_fs_unck costs 12 bytes:

                   code          stack          ctx
  default before: 37792           2608          620
  default after:  37804 (+0.0%)   2608 (+0.0%)  620 (+0.0%)

  gc before:      37938           2608          768
  gc after:       37940 (+0.0%)   2608 (+0.0%)  768 (+0.0%)
2025-01-28 14:41:45 -06:00
Christopher Haster 39d488a1ef gc: Made CKMETA/CKDATA progressable, added lfsr_gc_unck
LFS_GC_CKMETA and LFS_GC_CKDATA are a bit unique in that their work is
never really done.

Where LFS_GC_MKCONSISTENT/COMPACT can prove things about the system,
LFS_GC_CKMETA/CKDATA can't, because it's always possible for new
bit-errors to develop. Even _during_ an LFS_GC_CKMETA/CKDATA traversal.

But while this is technically true, it's not a very useful state of
things for our lfsr_gc API...

---

What we really want is some way to know if ckmeta/ckdata has completed
"recently" (for some definition of recently), and to let users indicate
when they need another ckmeta/ckdata scan.

To try to solve this:

1. Added LFS_I_CANCKMETA and LFS_I_CANCKDATA to indicate when lfsr_gc
   has not checked metadata/data.

   These are set during mount (unless mounting with
   LFS_M_CKMETA/CKDATA), and cleared when either lfsr_gc completes or
   lfsr_fs_ckmeta/data is called. Once cleared, littlefs will not reset
   them on its own.

2. Added lfsr_gc_unck to allow users to explicitly reset LFS_I_CKMETA
   and/or LFS_I_CKDATA, which will tell lfsr_gc to check metadata/data
   again on the next call.

   There is some subtlety around clobbering ongoing traversals, but a
   mask and some tests should prevent this from being a problem.

   Currently, lfsr_gc_unck also allows clearing of other gc flags, but
   I'm not sure there's any real use-case for this...

Note that you can still get the previous behavior if you just call
lfsr_gc_unck after every lfsr_gc call.

This also changes info flag behavior slightly in default mode, with
LFS_I_CANCKMETA/CANCKDATA telling you if metadata/data has been checked
since mount. Which does seem useful? Maybe these flags deserve a better
name?

Code changes:

                   code          stack          ctx
  default before: 37796 (+0.0%)   2608 (+0.0%)  620 (+0.0%)
  default after:  37792 (+0.0%)   2608 (+0.0%)  620 (+0.0%)

  gc before:      37896           2608          768
  gc after:       37938 (+0.1%)   2608 (+0.0%)  768 (+0.0.%)
2025-01-28 14:41:45 -06:00
Christopher Haster 0617244aa3 gc: Dropped lfsr_gc_setflags/setsteps
Now that you can provide gc_flags/gc_steps in lfs_config, I think it's a
bit more clear that _mutating_ the flags/steps is a niche feature, and
not worth implementing/testing.

It raises the question why not have a similar lfsr_setflags or
lfsr_file_setflags, and the answer there is it would be a pain-in-the-
ass to make sure all possible corner cases are covered.

It actually already was a pain-in-the-ass to test lfsr_gcsetflags/
setsteps... but just because we already did the work is not a good
reason for keeping complexity around.

---

Note that most of the use cases for lfsr_gc_setflags/setsteps can be
covered by either remounting the filesystem or through the
lfsr_traversal_t APIs directly.

The end result is a bit of code savings when incremental gc is enabled:

                   code          stack          ctx
  default before: 37796           2608          620
  default after:  37796 (+0.0%)   2608 (+0.0%)  620 (+0.0%)

  gc before:      37944           2608          768
  gc after        37896 (-0.1%)   2608 (+0.0%)  768 (+0.0%)
2025-01-28 14:41:45 -06:00
Christopher Haster 1b3054db89 gc: Moved incremental gc behind ifdef LFS_GC
Incremental gc, being stateful and not gc-able (ironic), was always
going to need to be conditionally compilable.

This moves incremental gc behind the LFS_GC define, so that we can focus
on the "default" costs. This cuts lfs_t in nearly half!

  lfs_t with LFS_GC:   308
  lfs_t without LFS_C: 168 (-45.5%)

This does save less code than one might expect though. We still need
most of the internal traversal/gc logic for things like block allocation
and orphan cleanup, so most of the savings is limited to the RAM storing
the incremental state:

                          code          stack          ctx
  before:                37916           2608          768
  after with LFS_CFG:    37944 (+0.1%)   2608 (+0.0%)  768 (+0.0%)
  after without LFS_CFG: 37796 (-0.3%)   2608 (+0.0%)  620 (-19.3%)

On the flip side, this does mean most of the incremental gc
functionality is still availables in the lfsr_traversal_t APIs.

Applications with more advanced gc use-cases may actually benefit from
_not_ enabling the incremental gc APIs, and instead use the
lfsr_traversal_t APIs directly.
2025-01-28 14:41:45 -06:00
Christopher Haster 5d756fe698 gc: Tweaked lfsr_gc API to be more stateful
Before:

  int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags);

After:

  int lfsr_gc(lfs_t *lfs);
  int lfsr_gc_setflags(lfs_t *lfs, uint32_t flags);
  int lfsr_gc_setsteps(lfs_t *lfs, lfs_soff_t steps);

---

The interesting thing about the lfsr_gc API is that the caller will
often be very different from whoever configures the system. One example
being an OS calling lfsr_gc in a background loop, while leaving
configuration up to the user.

The idea here, is instead of forcing the OS to come up with its own
stateful system to pass flags to lfsr_gc, we just embed this state in
littlefs directly. The whole point of lfsr_gc is that it's a stateful
system anyways.

Unfortunately this state does require a bit more logic to maintain,
which adds code/ctx cost:

           code          stack          ctx
  before: 37812           2608          752
  after:  37916 (+0.3%)   2608 (+0.0%)  768 (+2.1%)
2025-01-28 14:41:45 -06:00
Christopher Haster 18190054d9 Trying to better use uncreat/zombie/orphan terms in tests
Renamed a bunch of tests:

- test_forphans_create_* -> test_forphans_uncreat_*
- test_forphans_cleanup_opened -> test_forphans_cleanup_open
- test_forphans_cleanup_orphaned -> test_forphans_cleanup_uncreat
- test_forphans_orphanzombie_fuzz -> test_forphans_uz_fuzz
- test_forphans_orphanzombiedir_fuzz -> test_forphans_uzd_fuzz
- test_*_oz_fuzz -> test_*_uz_fuzz
- test_*_ozd_fuzz -> test_*_uzd_fuzz
- test_traversal_*_orphan_* -> test_traversal_*_uncreat_*
- test_traversal_*_orphaned -> test_traversal_*_uncreat
- test_attrs_fattr_orphan -> test_attrs_fattr_uncreat

And renamed a number of variables and things.
2025-01-28 14:41:45 -06:00
Christopher Haster a53151df1f Renamed high-level spam tests to include *_spam_*
These are our current set of general-purpose high-level tests that can
be turned to when needing to test a wide range of filesystem operations.

They were getting a bit hard to keep track of without a consistent
prefix, especially since no individual test suite can actually use all
of them at the same time.

Now, finding these tests is as simple as: ./scripts/test.py -L *_spam_*

I also renamed a couple because their names were starting to get
ridiculous. I mean just look at
test_badblocks_alternating_spam_orphanzombiedir_fuzz...

- *_spam_orphanzombie_fuzz    -> *_spam_oz_fuzz
- *_spam_orphanzombiedir_fuzz -> *_spam_ozd_fuzz
- *_spam_file_pl_fuzz         -> *_spam_f_pl_fuzz
- *_spam_filedir_pl_fuzz      -> *_spam_fd_pl_fuzz

Here are all of the current spam tests and contexts we use them in:

                traversal               badblocks   relocations
                |     gc    ck    grow  |     powerloss   exhaustion
  dir_many      y     y           y     y     y     y
  dir_fuzz      y     y     y     y     y           y     y
  file_many     y     y           y     y     y     y
  file_fuzz     y     y     y     y     y           y     y
  fwrite_fuzz   y     y     y           y                 y
  oz_fuzz       y     y     y     y     y           y     y
  ozd_fuzz      y     y     y     y     y           y     y
  f_pl_fuzz                       y           y     y
  fd_pl_fuzz                      y           y     y
2024-08-20 00:28:55 -05:00
Christopher Haster acad3a3143 Added format flags to lfsr_format
This is mainly to solve the weird check-hole where passing CKPROGS/
CKREADS as mount flags has no effect on lfsr_format (I mean, it'd be a
bit silly if it did somehow):

  LFS_F_RDWR              0  // Format the filesystem as read and write
  LFS_F_CKPROGS  0x00000010  // Check progs by reading back progged data
  LFS_F_CKREADS  0x00000020  // Check reads via parity bits/checksums

This makes lfsr_format a more cumbersome interface, but I don't know if
this is necessarily a bad thing. There's always risk of data loss when
calling lfsr_format, so maybe it should be a pain to call.

At the very least, format flags may be useful in the future for
enabling/disabling format-time things such as the planned block-map,
parity-tree, etc. Though it's unclear if such significant settings
should be format flags or somehow encoded as fields in our config
struct.

---

The LFS_F_* format flags of course ended up conflicting with our
internal LFS_F_* flags, so I renamed most of the internal flags to match
the closest flag set they participate in:

- LFS_F_TYPE        -> LFS_O_TYPE
- LFS_F_UNFLUSH     -> LFS_O_UNFLUSH
- LFS_F_UNSYNC      -> LFS_O_UNSYNC
- LFS_F_ORPHAN      -> LFS_O_ORPHAN
- LFS_F_ZOMBIE      -> LFS_O_ZOMBIE

- LFS_F_ORPHANS     -> LFS_I_ORPHANS
- LFS_F_UNCOMPACTED -> LFS_I_UNCOMPACTED

- LFS_F_TSTATE      -> LFS_T_TSTATE
- LFS_F_BTYPE       -> LFS_T_BTYPE
- LFS_F_DIRTY       -> LFS_T_DIRTY
- LFS_F_MUTATED     -> LFS_T_MUTATED

This may make it a bit less clear which flags are a part of the public
API, vs intended only for internal use, but at the very least our asserts
in format/mount/open/etc should catch most of these mistakes.

---

Code cost ended up being pretty minimal. Actually negative. This is the
second time we're _adding_ a feature that somehow saves code, though the
reality for this one is we're really just pushing constants up into the
user's stack frame. Still, it's a good indication the cost of format
flags is small:

           code          stack
  before: 36452           2680
  after:  36448 (-0.0%)   2680 (+0.0%)
2024-08-16 01:04:13 -05:00
Christopher Haster e2c238c30d Added lfsr_file_ckmeta/ckdata
These are basically the same as lfsr_fs_ckmeta/ckdata but limited to a
single file. They may be useful when you need to validate a file but
don't want to bother validating the entire filesystem:

  // Check a file for metadata errors
  int lfsr_file_ckmeta(lfs_t *lfs, lfsr_file_t *file);

  // Check a file for metadata + data errors
  int lfsr_file_ckdata(lfs_t *lfs, lfsr_file_t *file);

I've also added test_ck to test these and added some more
lfsr_fs_ckmeta/ckdata tests there. These currently just test simple
full-block clobbering, but we should eventually test more interesting
error patterns.

Unfortunately lfsr_file_ckmeta/ckdata can't reuse the internal
lfsr_mtree_traverse in quite the same way lfsr_fs_ckmeta/ckdata can, so
they're actually a bit more expensive. Though keep in mind with
link-time gc you won't pay the cost unless you call these functions:

           code          stack
  before: 36024           2696
  after:  36368 (+1.0%)   2664 (-1.2%)

Oh, and the multiple calls to lfsr_btree/bshrub_traverse apparently
uninlined it out of lfsr_mtree_traverse, saving the stack cost in the
stack hot-path... Yay?
2024-07-27 00:47:45 -05:00
Christopher Haster 15090e5dcf gc: Also restart gc if lookahead + mutated/dirty
There is really no reason to continue lookahead traversals if our
filesystem has been mutated. Clearing the flag and restarting in this
case is more likely to make progress.

Note that it's worth continuing for all of the other current gc flags:

- LFS_GC_MKCONSISTENT - Except maybe for mkconsistent. We can't actually
  make progress, since we can't prove the filesystem is free of orphans,
  but it's beneficial to keep traversing and clearing orphans in case of
  other traversal flags that mutation would force a second traversal
  anyways.

  Continuing mkconsistent traversals also spreads out orphan cleanup a
  bit better, instead of just repeatedly cleaning up the first couple
  mdirs when under heavy contention.

  But to be honest, the chance of mutation that still leaves the
  filesystem with orphans is just so low that it's not worth doing
  anything. mkconsistent only needs to traverse the mtree anyways...

- LFS_GC_COMPACT - Like mkconsistent, compacting traversals are worth
  continuing for better mtree coverage under heavy contention.

  We will need a second pass to prove we compacted everything anyways,
  so might as well try to get as much mutation done as possible in the
  current traversal.

- LFS_GC_CKMETA/CKDATA - Continuing ckmeta/ckdata traversals provides
  better mtree coverage under heavy contention.

  This is much more important for CKMETA/CKDATA than the others, because
  _eventually_ checking every block for errors is more valuable than
  proving anything.

This adds some code, but the use of flags here is quite valuable for
expressing complex constraints like this cheaply:

           code          stack
  before: 36228           2680
  after:  36240 (+0.0%)   2680 (+0.0%)
2024-07-20 01:27:45 -05:00
Christopher Haster 6cf78527b4 Reverted gc-restart on flag change
Thinking about this more, we probably don't want to entangle
lfsr_fs_mkconsistent/ckmeta/etc and lfsr_fs_gc:

- lfsr_fs_ckmeta/ckdata are readonly and don't need to clobber
  traversals. The system can make more progress if these use separate
  states.

- We already need a bit of code to force traversals to restart for
  lfsr_fs_ckmeta/ckdata, so these already aren't simple wrappers.

- lfsr_fs_mkconsistent should also probably not invalidate gc traversals
  when the filesystem is already consistent. It is called by... checks
  notes... every function that writes to disk.

  This could be fixed in lfsr_fs_mkconsistent, but it'd be pretty close
  to just calling lfsr_mtree_gc...

- We don't really benefit from reusing the gc traversal state.
  lfsr_fs_mkconsistent/ckmeta/etc aren't on the stack hot-path, so the
  stack usage is more-or-less free (though I realize this depends on
  what functions are called in a given system).

- Calling lfsr_fs_gc can actually be a detriment for code size when
  considering link-time-gc (not related to fs-gc), since it will drag in
  the function when we don't need the traversal-invalidation features.

- Calling lfsr_fs_gc vs lfsr_mtree_gc shouldn't really be a significant
  code size difference. We should probably look into lfsr_mtree_gc,
  which is called from many places, instead of tangling everything
  together...

So this commit reverts gc-restarts and brings back gc masking on flag
change.

At the very least, moving all the code around led to a bit of code
savings:

                      code          stack
  before gc-restart: 36316           2680
  gc-restart:        36068 (-0.7%)   2680 (+0.0%)
  after gc-restart:  36240 (-0.2%)   2680 (+0.0%)
2024-07-20 01:27:45 -05:00
Christopher Haster 1cd6a6873a Restart gc on flag change, better dedup mkconsistent/ckmeta/etc
This simplifies lfsr_fs_gc a bit, and allows lfsr_fs_mkconsistent/
ckmeta/etc to call lfsr_fs_gc directly (it would be a bit strange for
these function to finish up unrelated gc traversals).

Unfortunately, this does risk gc getting stuck constantly restarting if
there is contention between two lfsr_fs_gc calls with different flags,
but you could argue this would be a system design mistake...

The deduplication of traversal state leads to some pretty nice code
savings:

           code          stack
  before: 36316           2680
  after:  36068 (-0.7%)   2680 (+0.0%)
2024-07-20 01:26:33 -05:00
Christopher Haster eced943685 Changed gc_steps into a runtime parameter, better dedup mount gc
So instead of configuring gc_steps at mount time (or eventually compile
time), lfsr_fs_gc now takes a steps parameter that controls how much gc
work to attempt:

  int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags);

This API was needed internally to better deduplicate on-mount gc, and I
figured it might also be useful for users to be able to easily change
gc_steps per lfsr_fs_gc call.

I realize this could also be accomplished with the theoretical
lfsr_fs_gccfg, but it's a bit easier to not need a struct every call.

Most likely, depending on project/system, users will always call
lfsr_fs_gc with either 1 (minimal work) or -1 (maximal work), or, worst
case, can define a system-wide GC_STEPS somewhere.

---

Deduplicating on-mount gc work better saved some code, though it's worth
noting this could have been done internally and not exposed to users:

           code          stack
  before: 36476           2680 (+0.0%)
  after:  36316 (-0.4%)   2680 (+0.0%)
2024-07-18 20:46:58 -05:00
Christopher Haster 4fe46a983f Added simple lfsr_fs_ckmeta/ckdata functions
These functions provide an easy API for checking all metadata/data
checksums in the filesystem:

  // Check the filesystem for metadata errors
  int lfsr_fs_ckmeta(lfs_t *lfs);

  // Check the filesystem for metadata + data errors
  int lfsr_fs_ckdata(lfs_t *lfs);

These are more-or-less the same as calling lfsr_fs_gc with
LFS_GC_CKMETA/CKDATA, but don't involve the gc/traversal-invalidation
machinery, and may be a bit easier for users to pick up.

---

Unfortunately, for simple wrappers, we're again hit with a somewhat
surprising code cost:

           code          stack
  before: 36288           2680
  after:  36472 (+0.5%)   2680 (+0.0%)

But I think we can again blame the high overhead of LFS_TRAVERSAL/
lfsr_mtree_gc. We should look into reducing/deduplicating this logic...
2024-07-17 22:15:08 -05:00
Christopher Haster 2f08662fb9 Added on-mount traversal flags: LFS_M_MKCONSISTENT/CKMETA/CKDATA/etc
These tell littlefs to do the relevant gc work during mount, which may
be more convenient than calling lfsr_mount and then lfsr_fs_gc.

It also implicitly tears down the filesystem on error, which you can
imagine would be quite useful for LFS_M_CKMETA/LFS_M_CKDATA.

Some flags are more useful here than other (is LFS_M_LOOKAHEAD/COMPACT
really useful?), but since we just pass these directly to our traversal
APIs, we might as well support all of them for consistency.

Also note that since these only change mount's behavior, and have no
effect on the rest of the filesystem, these LFS_M_* flags don't have
related LFS_I_* flags and are not returned by lfsr_fs_stat.

---

This added quite a chunk of code, considering that this is entirely for
convenience:

           code          stack
  before: 35932           2680
  after:  36280 (+1.0%)   2680 (+0.0%)

But I think this is mostly because our low-level traversal state is
relatively costly to manage. It may be possible to deduplicate this a
bit better...
2024-07-17 21:40:37 -05:00
Christopher Haster acfae9e072 Extended lfsr_mount to accept mount flags
This has been a long-time coming, mount flags are just too useful for
configuring a filesystem at runtime.

Currently this is limited to LFS_M_RDONLY and LFS_M_CKPROGS, but there
are a few more planned in the future:

  LFS_M_RDWR     = 0x0000, // Mount the filesystem as read and write
  LFS_M_RDONLY   = 0x0001, // Mount the filesystem as readonly
  LFS_M_STRICT*  = 0x0002, // Error if on-disk config does not match
  LFS_M_FORCE*   = 0x0004, // Ignore compat flags, mount readonly
  LFS_M_FORCEWITHRECKLESSABANDON*
                 = 0x0008, // Ignore compat flags, mount read write

  LFS_M_CKPROGS  = 0x0010, // Check progs by reading back progged data
  LFS_M_CKREADS* = 0x0020, // Check reads via checksums

  * Hypothetical

As a convenience, we also return mount flags in the struct lfs_fsinfo's
flags field as their relevant LFS_I_* variants. Though only to match
statvfs, and only because it's cheap, littlefs's API is low-level and we
should expect users to know what flags they passed to lfsr_mount.

As for the new mount flags:

- LFS_M_RDONLY - For consistency with existing APIs, this just asserts
  on write operations, which makes it a bit useless... But the info flag
  LFS_I_RDONLY may be useful for falling back to a readonly mode if
  we encounter on-disk compat issues.

  At least if implement the theoretical LFS_UNTRUSTED_USER mode
  LFS_M_RDONLY could become a runtime error.

- LFS_M_RDWR - This really just exists to compliment LFS_M_RDONLY and to
  match LFS_O_RDONLY/LFS_O_RDWR. It's just an alias for 0, and I don't
  think there will ever be a reason to make it non-0 (but I can always
  be wrong!).

- LFS_M_CKPROGS - This replaces the check_progs config option and avoids
  using a full byte to store a bool.

  We should probably also have a compile-time option to compile this out
  (LFS_NO_CKPROGS?), but that's a future thing to do.

This ended up adding a surprising bit of code, considering we're just
moving flags around, and noise in lfs_alloc added a bit of stack again:

           code          stack
  before: 35880           2672
  after:  35932 (+0.1%)   2680 (+0.3%)
2024-07-17 20:39:31 -05:00
Christopher Haster 0a3cb2dd3a Added filesystem-level info flags to lfsr_fs_stat
Thinking again of use cases, lfsr_fs_gc provides the perfect API to call
in the background to perform any pending filesystem work. But what if
there's no work to be done? Sure we could just spin forever, but that's
a waste. Especially on devices that can turn on sleep modes to save
power.

To help with this, this commit adds a set of flags to struct lfs_fsinfo
that signals when lfsr_fs_gc can accomplish work:

  LFS_I_INCONSISTENT     = 0x01, // Filesystem needs mkconsistent to write
  LFS_I_NEEDSUPGRADE*    = 0x02, // Filesystem needs an upgrade to write
  LFS_I_CANLOOKAHEAD     = 0x04, // Lookahead buffer is not full
  LFS_I_CANPREERASE+     = 0x08, // Pre-erase buffer is not full
  LFS_I_UNCOMPACTED      = 0x10, // Filesystem may have uncompacted metadata
  LFS_I_NEEDSREPAIRMETA+ = 0x20, // Filesystem contains damaged metadata
  LFS_I_NEEDSREPAIRDATA+ = 0x40, // Filesystem contains damaged data

  *Hypothetical
  +Planned

This flags field also provides a useful place internally to store other
filesystem-related flags, currently LFS_F_ORPHANS, though this may be
expanded in the future.

These flags allow users to know exactly what work can/needs to be done
for the filesystem to make progress:

- LFS_I_INCONSISTENT => LFS_GC_MKCONSISTENT or lfsr_fs_mkconsistent
- LFS_I_CANLOOKAHEAD => LFS_GC_LOOKAHEAD

- LFS_I_UNCOMPACTED => LFS_GC_COMPACT

  The one is new!

  If we complete a compaction-traversal without any mutation, we know
  all mdirs/btree nodes have been compacted and future traversals won't
  accomplish anything. Of course, we need to clear this bit on
  filesystem mutation.

  Right now we just pessimistically assume the filesystem is uncompacted
  during mount, but in theory we can also figure this out during our
  initial mount traversal.

- LFS_GC_CKMETA/CKDATA?

  LFS_GC_CKMETA and LFS_GC_CKDATA are a bit trickier. In theory,
  LFS_GC_CKMETA/CKDATA will always accomplish something, since time is
  the only ingredient necessary to introduce bit errors.

  So there isn't really a reasonable flag here. It's entirely up to the
  user to decide when to do an LFS_GC_CKMETA/CKDATA traversal.

Code changes:

           code          stack
  before: 35740           2672
  after:  35880 (+0.4%)   2672 (+0.0%)
2024-07-17 18:58:06 -05:00
Christopher Haster fc486ca4f7 Reworked lfsr_fs_gc to be incremental
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%)
2024-07-17 18:08:32 -05:00
Christopher Haster 0ee6d73560 (Re)implemented lfsr_fs_gc
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.
2024-07-17 17:10:20 -05:00