Commit Graph

4 Commits

Author SHA1 Message Date
Christopher Haster
f9324d1443 Exploring inlined mutable configuration
Currently littlefs uses a separate mutable state struct and immutable
config struct. This lets users place the config struct in ROM where
possible.

However the recent addition of LFS_STATICCFG raises the question of if
this split is still valuable.

If the config is copied into the mutable struct at runtime, this allows
a couple things:
1. Easier user interface, config can be stack allocated, no need to store
   the config struct for the lifetime of littlefs in OSs.
2. Avoids duplication when littlefs would need to change config based on
   observed superblocks, such as LFS_NAME_MAX limits
3. In theory, access to a single struct is faster/smaller as it avoids
   an additional load instruction.

Unfortunately, inlining the dynamic config runs into several issues:

1. The code size actually increases with this change! From digging into
   this it's for a couple reasons:

   - Copying the config over takes code.

   - C has notorious problems with pointer aliasing, accessing
     constants from a const struct actually allows C to assume the
     values aren't going to change in more situations.

     This suggests it may be possible to reduce the code size more by
     loading the config pointer into a variable, but I haven't explored
     this probably not-worth-it optimization.

   - Even assuming deduplication of superblock-dependent configuration,
     the config struct is significantly larger than the mutable struct,
     and it turns out combining these two exceeds the limits of
     immediate-relative-loads, discarding the possible code size
     improvement from avoiding a second dereference.

2. The implementation of dynamic configuration differs significantly from
   the static configuration. This adds mess into the compile-time #ifdefs
   needed to support both options.
2020-11-28 20:13:32 -06:00
Christopher Haster
a7cdd563f6 Changed callbacks to take user-provided context directly
This is a style change to make littlefs's callbacks consistent with most
callback declarations found in C. That is, taking in a user-provided
`void*`.

Previously, these callbacks took a pointer to the config struct itself,
which indirectly contained a user provided context, and this gets the
job done, but taking in a callback with a `void*` is arguably more
expected, has a better chance of integrating with C++/OS-specific code,
and is more likely to be optimized out by a clever compiler.

---

As a part of these changes, the geometry for the test bds needed to be
moved into bd specific configuration objects. This is a good change as
it also allows for testing situations where littlefs's geometry does not
match the underlying bd.
2020-11-28 20:02:18 -06:00
Christopher Haster
c44427f9ec Exploring ideas for static configuration
As an embedded library, littlefs's configuration straddles two worlds.
In most cases the configuration is usually constant at build time, but
when integrated into OSs, the configuration needs to be dynamically
configurable.

To help with this, littlefs has a separate lfs_config struct that can be
placed into ROM when possible.

But you know what's better than ROM configuration? Truely inlinable
static configuration known at compile-time. In addition to avoiding the
RAM cost, compile-time configuration allows for additional compiler
optimizations, such as constexpr-elimination and removal of unused
code-paths.

So how to enable static configuration?

1. define LFS_STATICCFG
2. implement callbacks as global functions:
   - lfs_read
   - lfs_prog
   - lfs_erase
   - lfs_sync
2. define the now-required constants that configure littlefs:
   - LFS_READ_SIZE
   - LFS_PROG_SIZE
   - LFS_BLOCK_SIZE
   - LFS_BLOCK_COUNT
   - LFS_BLOCK_CYCLES
   - LFS_CACHE_SIZE
   - LFS_LOOKAHEAD_SIZE
   - LFS_READ_BUFFER (optional)
   - LFS_PROG_BUFFER (optional)
   - LFS_LOOKAHEAD_BUFFER (optional)
   - LFS_NAME_MAX (optional)
   - LFS_FILE_MAX (optional)
   - LFS_ATTR_MAX (optional)

Note, there is a separate configuration for the file configuration, this
can be enabled/disabled independently of LFS_STATICCFG. You will likely
want to define this as well if you are looking for the smallest code
size.

In order to avoid a mess of #ifdefs, the internals of littlefs use a
simple macro that redirects to either the dynamic or static config at
compile time:

    #ifdef LFS_STATICCFG
    #define LFS_CFG_READ_SIZE(lfs) ((void)lfs, LFS_READ_SIZE)
    #else
    #define LFS_CFG_READ_SIZE(lfs) lfs->cfg->read_size
    #endif

Unfortunately it does look like there still may be a lot of issues
related to warnings of comparisons against constants... If only C had
a way to ignore warnings on individual statements...

Original idea by apmorton
2020-11-28 19:15:24 -06:00
Christopher Haster
4677421aba Added "evil" tests and detecion/recovery from bad pointers and infinite loops
These two features have been much requested by users, and have even had
several PRs proposed to fix these in several cases. Before this, these
error conditions usually were caught by internal asserts, however
asserts prevented users from implementing their own workarounds.

It's taken me a while to provide/accept a useful recovery mechanism
(returning LFS_ERR_CORRUPT instead of asserting) because my original thinking
was that these error conditions only occur due to bugs in the filesystem, and
these bugs should be fixed properly.

While I still think this is mostly true, the point has been made clear
that being able to recover from these conditions is definitely worth the
code cost. Hopefully this new behaviour helps the longevity of devices
even if the storage code fails.

Another, less important, reason I didn't want to accept fixes for these
situations was the lack of tests that prove the code's value. This has
been fixed with the new testing framework thanks to the additional of
"internal tests" which can call C static functions and really take
advantage of the internal information of the filesystem.
2020-03-20 09:26:07 -05:00