Github/littlefs
1
Fork 0
Code Actions Activity

Dropped the HASORPHAN scan in lfsr_mount

Browse Source 
The motivation here is to simplify lfsr_mount, but there's a number of
knock-on effects.

For one, lfsr_mount should now be faster on filesystems with large
blocks:

  O(nb(log b)(log_b n)) -> O(nb(log_b n))

But we now no longer check if our filesystem contains orphaned
stickynotes or unknown filetypes:

- Orphaned stickynotes turned out to not be a big deal. If we find
  orphans we'd need to do a second traversal to remove them anyways (no
  mutation allowed in lfsr_mount), so this actually ends up a net
  improvement in the found-orphan case.

  If anything, doing a traversal on first write sets user expectations
  correctly, and can be offloaded with lfsr_fs_mkconsistent or
  lfsr_fs_gc.

- Unknown filetypes are a bit more annoying (I actually forgot about
  this check), but unknown filetypes that require special care should
  probably set WCOMPAT/RCOMPAT flags.

  Allowing unknown filetypes is a bit more flexible in cases where a
  filesystem image is being shared between drivers with different
  features (bootloader + app for example).

  Though we should probably add more checks/tests that we're handling
  these correctly now that we no longer just bail during mount...

Also renamed LFS_I_HASORPHANS -> LFS_I_UNTIDY.

Not doing something is cheaper than doing something, so this saves a bit
of code:

           code          stack          ctx
  before: 38120           2624          752
  after:  38020 (-0.3%)   2624 (+0.0%)  752 (+0.0%)
This commit is contained in:
Christopher Haster
2024-12-30 20:26:06 -06:00
parent 5302213ec9
commit 6e63920338
5 changed files with 45 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+17 -54
View File
@@ -7257,8 +7257,8 @@ static inline bool lfsr_m_issync(uint32_t flags) {
}
// internal fs flags
static inline bool lfsr_i_hasorphans(uint32_t flags) {
return flags & LFS_I_HASORPHANS;
static inline bool lfsr_i_isuntidy(uint32_t flags) {
return flags & LFS_I_UNTIDY;
}
static inline bool lfsr_i_isuncompacted(uint32_t flags) {
@@ -7271,7 +7271,7 @@ static inline bool lfsr_i_isuncompacted(uint32_t flags) {
static inline uint8_t lfsr_grm_count(const lfs_t *lfs);
static bool lfsr_fs_isinconsistent(const lfs_t *lfs) {
return lfsr_grm_count(lfs) > 0 || lfsr_i_hasorphans(lfs->flags);
return lfsr_grm_count(lfs) > 0 || lfsr_i_isuntidy(lfs->flags);
}
static bool lfsr_fs_canlookahead(const lfs_t *lfs) {
@@ -10048,7 +10048,7 @@ static int lfsr_mtree_traverse(lfs_t *lfs, lfsr_traversal_t *t,
}
// needed in lfsr_mtree_gc
static int lfsr_fs_fixorphans_(lfs_t *lfs, lfsr_mdir_t *mdir);
static int lfsr_fs_mktidy_(lfs_t *lfs, lfsr_mdir_t *mdir);
static void lfs_alloc_ckpoint(lfs_t *lfs);
static void lfs_alloc_markfree(lfs_t *lfs);
@@ -10079,10 +10079,10 @@ dropped:;
// mkconsistencing mdirs?
if (lfsr_t_ismkconsistent(t->o.o.flags)
&& lfsr_i_hasorphans(lfs->flags)
&& lfsr_i_isuntidy(lfs->flags)
&& tag == LFSR_TAG_MDIR) {
lfsr_mdir_t *mdir = (lfsr_mdir_t*)bptr.data.u.buffer;
err = lfsr_fs_fixorphans_(lfs, mdir);
err = lfsr_fs_mktidy_(lfs, mdir);
if (err) {
goto failed;
}
@@ -10156,7 +10156,7 @@ eot:;
// was mkconsistent successful?
if (lfsr_t_ismkconsistent(t->o.o.flags)
&& !lfsr_t_isdirty(t->o.o.flags)) {
lfs->flags &= ~LFS_I_HASORPHANS;
lfs->flags &= ~LFS_I_UNTIDY;
}
// was compaction successful? note we may need multiple passes if
@@ -11685,9 +11685,9 @@ static void lfsr_file_close_(lfs_t *lfs, const lfsr_file_t *file) {
if (lfsr_grm_count(lfs) < 2) {
lfsr_grm_push(lfs, file->o.o.mdir.mid);
// fallback to just marking the filesystem as orphaned
// fallback to just marking the filesystem as untidy
} else {
lfs->flags |= LFS_I_HASORPHANS;
lfs->flags |= LFS_I_UNTIDY;
}
}
}
@@ -13697,6 +13697,8 @@ static int lfs_init(lfs_t *lfs, uint32_t flags,
// setup flags
lfs->flags = flags
// assume we may contain orphans until proven otherwise
| LFS_I_UNTIDY
// default to assuming we need compaction somewhere, worst case
// this just makes lfsr_fs_gc read more than is strictly needed
| LFS_I_UNCOMPACTED;
@@ -14350,43 +14352,6 @@ static int lfsr_mountinited(lfs_t *lfs) {
return err;
}
// check for any orphaned stickynotes, note we only need
// this if filesystem will be writable
if (!lfsr_m_isrdonly(lfs->flags)) {
for (lfs_size_t rid = 0;
rid < mdir->rbyd.weight;
rid++) {
lfsr_tag_t tag;
err = lfsr_rbyd_sublookup(lfs, &mdir->rbyd,
rid, LFSR_TAG_NAME,
&tag, NULL);
if (err) {
LFS_ASSERT(err != LFS_ERR_NOENT);
return err;
}
// name 0 should be reserved
LFS_ASSERT(tag != (LFSR_TAG_NAME + 0));
// found an orphaned stickynote?
if (tag == LFSR_TAG_STICKYNOTE) {
LFS_DEBUG("Found orphaned stickynote "
"%"PRId32".%"PRId32,
lfsr_mid_bid(lfs, mdir->mid) >> lfs->mdir_bits,
rid);
lfs->flags |= LFS_I_HASORPHANS;
// found an unknown file type?
} else if (lfsr_tag_isunknown(tag)) {
LFS_WARN("Found unknown file type "
"%"PRId32".%"PRId32" 0x%"PRIx16,
lfsr_mid_bid(lfs, mdir->mid) >> lfs->mdir_bits,
rid,
lfsr_tag_subtype(tag));
return LFS_ERR_NOTSUP;
}
}
}
// found an mtree inner-node?
} else if (tag == LFSR_TAG_BRANCH) {
lfsr_rbyd_t *rbyd = (lfsr_rbyd_t*)bptr.data.u.buffer;
@@ -14775,7 +14740,7 @@ static int lfsr_fs_fixgrm(lfs_t *lfs) {
return 0;
}
static int lfsr_fs_fixorphans_(lfs_t *lfs, lfsr_mdir_t *mdir) {
static int lfsr_fs_mktidy_(lfs_t *lfs, lfsr_mdir_t *mdir) {
// save the current mid
lfsr_mid_t mid = mdir->mid;
@@ -14823,7 +14788,7 @@ failed:;
return err;
}
static int lfsr_fs_fixorphans(lfs_t *lfs) {
static int lfsr_fs_mktidy(lfs_t *lfs) {
// LFS_T_MKCONSISTENT really just removes orphans
lfsr_traversal_t t = LFSR_TRAVERSAL(
LFS_T_MTREEONLY | LFS_T_MKCONSISTENT);
@@ -14871,10 +14836,8 @@ int lfsr_fs_mkconsistent(lfs_t *lfs) {
// this must happen after fixgrm, since removing orphaned
// stickynotes risks outdating the grm
//
if (lfsr_i_hasorphans(lfs->flags)) {
LFS_DEBUG("Fixing orphans...");
int err = lfsr_fs_fixorphans(lfs);
if (lfsr_i_isuntidy(lfs->flags)) {
int err = lfsr_fs_mktidy(lfs);
if (err) {
return err;
}
@@ -14951,7 +14914,7 @@ int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags) {
// do we have any pending work?
uint32_t pending = flags & (
(lfs->flags & (
LFS_I_HASORPHANS
LFS_I_UNTIDY
| LFS_I_UNCOMPACTED))
| ((lfsr_fs_canlookahead(lfs)) ? LFS_GC_LOOKAHEAD : 0)
| LFS_GC_CKMETA
@@ -15016,7 +14979,7 @@ int lfsr_fs_gc(lfs_t *lfs, lfs_soff_t steps, uint32_t flags) {
// clear any pending flags we make progress on
pending &= (
(lfs->flags & (
LFS_I_HASORPHANS
LFS_I_UNTIDY
| LFS_I_UNCOMPACTED))
| ((lfsr_fs_canlookahead(lfs)) ? LFS_GC_LOOKAHEAD : 0)
// only consider our filesystem checked if we
lfs.h
+1 -2
View File
@@ -228,8 +228,7 @@ enum lfs_type {
0x00008000 // Filesystem may have uncompacted metadata
// internally used flags, don't use these
#define LFS_I_HASORPHANS \
0x00001000 // Filesystem may have untracked orphans
#define LFS_I_UNTIDY 0x00001000 // Filesystem may have orphaned stickynotes
// Block types
tests/test_forphans.toml
+3 -3
View File
@@ -4701,7 +4701,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
@@ -4837,7 +4837,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
@@ -4987,7 +4987,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
tests/test_mount.toml
+12 -53
View File
@@ -68,6 +68,7 @@ code = '''
| ((CKFETCHES) ? LFS_IFDEF_CKFETCHES(LFS_I_CKFETCHES, -1) : 0)
| ((CKPARITY) ? LFS_IFDEF_CKPARITY(LFS_I_CKPARITY, -1) : 0)
| ((CKCKSUMS) ? LFS_IFDEF_CKCKSUMS(LFS_I_CKCKSUMS, -1) : 0)
| ((!MKCONSISTENT) ? LFS_I_INCONSISTENT : 0)
| ((!LOOKAHEAD) ? LFS_I_CANLOOKAHEAD : 0)
| ((!COMPACT) ? LFS_I_UNCOMPACTED : 0)));
@@ -126,7 +127,8 @@ code = '''
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == (
LFS_I_CANLOOKAHEAD
LFS_I_INCONSISTENT
| LFS_I_CANLOOKAHEAD
| LFS_I_UNCOMPACTED));
lfsr_unmount(&lfs) => 0;
@@ -138,7 +140,9 @@ code = '''
| ((CKDATA) ? LFS_M_CKDATA : 0),
CFG) => 0;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == LFS_I_UNCOMPACTED);
assert(fsinfo.flags == (
LFS_I_INCONSISTENT
| LFS_I_UNCOMPACTED));
lfsr_unmount(&lfs) => 0;
'''
@@ -189,7 +193,8 @@ code = '''
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == (
LFS_I_CANLOOKAHEAD
LFS_I_INCONSISTENT
| LFS_I_CANLOOKAHEAD
| LFS_I_UNCOMPACTED));
lfsr_unmount(&lfs) => 0;
@@ -202,7 +207,9 @@ code = '''
| ((CKDATA) ? LFS_M_CKDATA : 0),
CFG) => 0;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == ((!LOOKAHEAD) ? LFS_I_CANLOOKAHEAD : 0));
assert(fsinfo.flags == (
LFS_I_INCONSISTENT
| ((!LOOKAHEAD) ? LFS_I_CANLOOKAHEAD : 0)));
// mdir should have been compacted
lfsr_file_open(&lfs, &file, "jellyfish", LFS_O_RDONLY) => 0;
@@ -280,7 +287,7 @@ code = '''
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == (
((ORPHANS > 0) ? LFS_I_INCONSISTENT : 0)
LFS_I_INCONSISTENT
| LFS_I_CANLOOKAHEAD
| LFS_I_UNCOMPACTED));
lfsr_unmount(&lfs) => 0;
@@ -895,51 +902,3 @@ code = '''
lfsr_mount(&lfs, LFS_M_RDONLY, CFG) => LFS_ERR_NOTSUP;
'''
# test what happens if we find an unknown file type
[cases.test_mount_incompat_unknown_type]
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
// create some files
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "a",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_write(&lfs, &file,
"hi a!", strlen("hi a!")) => strlen("hi a!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_file_open(&lfs, &file, "b",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_write(&lfs, &file,
"oh no!", strlen("oh no!")) => strlen("oh no!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_file_open(&lfs, &file, "c",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_write(&lfs, &file,
"hi c!", strlen("hi c!")) => strlen("hi c!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
// change a file's type to something unknown
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
const char *path = "b";
lfsr_mdir_t mdir;
lfsr_did_t did;
lfsr_mtree_pathlookup(&lfs, &path,
&mdir, NULL, &did) => 0;
lfsr_mdir_commit(&lfs, &mdir, LFSR_RATS(
LFSR_RAT_NAME(
LFSR_TAG_SUB | (LFSR_TAG_NAME + 0x13), 0,
did, path, lfsr_path_namelen(path)))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => LFS_ERR_NOTSUP;
// but we _can_ mount readonly
lfsr_mount(&lfs, LFS_M_RDONLY, CFG) => 0;
lfsr_unmount(&lfs) => 0;
'''
tests/test_traversal.toml
+12 -10
View File
@@ -1680,7 +1680,8 @@ code = '''
struct lfs_fsinfo fsinfo;
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == (
LFS_I_CANLOOKAHEAD
LFS_I_INCONSISTENT
| LFS_I_CANLOOKAHEAD
| LFS_I_UNCOMPACTED));
// try traversing
@@ -1704,7 +1705,8 @@ code = '''
// check flags after
lfsr_fs_stat(&lfs, &fsinfo) => 0;
assert(fsinfo.flags == (
((!LOOKAHEAD) ? LFS_I_CANLOOKAHEAD : 0)
((!MKCONSISTENT) ? LFS_I_INCONSISTENT : 0)
| ((!LOOKAHEAD) ? LFS_I_CANLOOKAHEAD : 0)
| ((!COMPACT) ? LFS_I_UNCOMPACTED : 0)));
lfsr_unmount(&lfs) => 0;
@@ -6327,7 +6329,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
// which means there shouldn't be that many files left
assert((lfs.mtree.u.weight & 0x7fffffff) <= (2 << lfs.mdir_bits));
@@ -6466,7 +6468,7 @@ code = '''
assert(lfs.grm.mids[1] == -1);
// if we introduce actual orphans, me _must not_ clear the orphan flag
if (ORPHANS >= 3) {
assert(lfs.flags & LFS_I_HASORPHANS);
assert(lfs.flags & LFS_I_UNTIDY);
}
// if we introduced actual orphans, we _must_ be marked as inconsistent
@@ -6618,7 +6620,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
// which means there shouldn't be that many files left
assert((lfs.mtree.u.weight & 0x7fffffff) <= (2 << lfs.mdir_bits));
@@ -6771,7 +6773,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
// which means there shouldn't be that many files left
assert((lfs.mtree.u.weight & 0x7fffffff) <= (2 << lfs.mdir_bits));
@@ -6935,7 +6937,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
// which means there shouldn't be that many files left
assert((lfs.mtree.u.weight & 0x7fffffff) <= (2 << lfs.mdir_bits));
@@ -7097,7 +7099,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
// which means there shouldn't be that many files left
assert((lfs.mtree.u.weight & 0x7fffffff) <= (2 << lfs.mdir_bits));
@@ -7257,7 +7259,7 @@ code = '''
// we should have cleaned up all grms/orphans
assert(lfs.grm.mids[0] == -1);
assert(lfs.grm.mids[1] == -1);
assert(!(lfs.flags & LFS_I_HASORPHANS));
assert(!(lfs.flags & LFS_I_UNTIDY));
// which means there shouldn't be that many files left
assert((lfs.mtree.u.weight & 0x7fffffff) <= (2 << lfs.mdir_bits));
@@ -7445,7 +7447,7 @@ code = '''
assert(lfs.grm.mids[1] == -1);
// if we introduce actual orphans, me _must not_ clear the orphan flag
if (ORPHANS >= 3) {
assert(lfs.flags & LFS_I_HASORPHANS);
assert(lfs.flags & LFS_I_UNTIDY);
}
// mdirs should have been compacted