Github/littlefs
1
Fork 0
Code Actions Activity

Implemented ckfetches

Browse Source 
Ckfetches implements what might be your first idea on how to check
checksums in a filesystem: Check each block/mdir on first access
(fetch) to make sure the data is sound.

Unfortunately, there are two problems with this approach, both which
come from the fact that blocks are big and can't fit in RAM:

1. We still have a checksum-read hole.

   We can't keep a whole block around in RAM, so reads after a fetch may
   need to reread from disk, at which point new bit-errors may slip in
   undetected.

   This is especially problematic for traversing our rbyds, which
   involves a lot of small reads in a block.

2. Ckfetches may have a surprisingly negative performance impact.

   Consider the case of reading a large file with a bunch of small
   reads. Because we don't cache blocks, each read may need a btree
   lookup, and a full block fetch. On paper this can quickly end up
   O(b^2), which is not great.

   Though this is helped by the file buffer. It will be interesting to
   benchmark and see if this theoretical O(b^2) translates to poor
   performance in practice.

   Note ckreads has this same performance issue.

Still, despite these problems, ckfetches may be useful for cases where
you just want an extra layer of safety, or don't care about the tiny
chance an error is introduced between a fetch an subsequent read.

---

Like ckprogs/ckreads, ckfetches is an opt-in feature, and requires both
1. defining LFS_CKFETCHES, and 2. passing LFS_M_CKFETCHES during mount.

This is a bit of a quick implementation to get testing in place, so the
code cost is probably higher than strictly necessary. If we can refactor
the code internally to avoid all the duplicate lfsr_rbyd_fetchck/
lfsr_bptr_ck calls, we can probably bring this down a bit:

                  code          stack
  before:        36428           2680
  yes-ckfetches: 36848 (+1.2%)   2680 (+0.0%)
  no-ckfetches:  36428 (+0.0%)   2680 (+0.0%)

Oh, and also added lfs_emubd_flipbit to allow tests to manually flip
bits themselves. LFS_EMUBD_BADBLOCK_PROGFLIP is quick to find the above
mentioned checksum-read hole.

This could be done manually with read+erase+prog, but no reason to make
it harder than it needs to be.
This commit is contained in:
Christopher Haster
2024-08-13 01:17:28 -05:00
parent 10feccf18c
commit 5502fe55ab
7 changed files with 539 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+182 -1
View File
@@ -317,7 +317,7 @@ static int lfsr_bd_prog_(lfs_t *lfs, lfs_block_t block, lfs_size_t off,
}
#ifdef LFS_CKPROGS
// check progs?
// checking progs?
if (lfsr_m_isckprogs(lfs->flags)) {
// pcache should have been dropped at this point
LFS_ASSERT(lfs->pcache.size == 0);
@@ -4794,6 +4794,11 @@ static int lfsr_data_readbtree(lfs_t *lfs, lfsr_data_t *data,
// core btree operations
// needed in lfsr_btree_lookupnext_
#ifdef LFS_CKFETCHES
static inline bool lfsr_m_isckfetches(uint32_t flags);
#endif
static int lfsr_btree_lookupnext_(lfs_t *lfs, const lfsr_btree_t *btree,
lfsr_bid_t bid,
lfsr_bid_t *bid_, lfsr_rbyd_t *rbyd_, lfsr_srid_t *rid_,
@@ -4833,6 +4838,18 @@ static int lfsr_btree_lookupnext_(lfs_t *lfs, const lfsr_btree_t *btree,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &branch,
branch.blocks[0], lfsr_rbyd_trunk(&branch),
branch.cksum);
if (err) {
return err;
}
}
#endif
// found our bid
} else {
// TODO how many of these should be conditional?
@@ -4946,6 +4963,18 @@ static int lfsr_btree_parent(lfs_t *lfs, const lfsr_btree_t *btree,
return 0;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &branch_,
branch_.blocks[0], lfsr_rbyd_trunk(&branch_),
branch_.cksum);
if (err) {
return err;
}
}
#endif
branch = branch_;
}
}
@@ -5117,6 +5146,18 @@ static int lfsr_btree_commit__(lfs_t *lfs, lfsr_btree_t *btree,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &sibling,
sibling.blocks[0], lfsr_rbyd_trunk(&sibling),
sibling.cksum);
if (err) {
return err;
}
}
#endif
// estimate if our sibling will fit
lfs_ssize_t sibling_estimate = lfsr_rbyd_estimate(lfs,
&sibling, -1, -1,
@@ -5165,6 +5206,18 @@ static int lfsr_btree_commit__(lfs_t *lfs, lfsr_btree_t *btree,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &sibling,
sibling.blocks[0], lfsr_rbyd_trunk(&sibling),
sibling.cksum);
if (err) {
return err;
}
}
#endif
// estimate if our sibling will fit
lfs_ssize_t sibling_estimate = lfsr_rbyd_estimate(lfs,
&sibling, -1, -1,
@@ -5607,6 +5660,18 @@ static lfs_scmp_t lfsr_btree_namelookup(lfs_t *lfs, const lfsr_btree_t *btree,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &branch,
branch.blocks[0], lfsr_rbyd_trunk(&branch),
branch.cksum);
if (err < 0) {
return err;
}
}
#endif
// found our rid
} else {
// TODO how many of these should be conditional?
@@ -5703,6 +5768,19 @@ static int lfsr_btree_traverse(lfs_t *lfs, const lfsr_btree_t *btree,
if (err) {
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &bt->rbyd,
bt->rbyd.blocks[0], lfsr_rbyd_trunk(&bt->rbyd),
bt->rbyd.cksum);
if (err) {
return err;
}
}
#endif
LFS_ASSERT((lfsr_bid_t)bt->rbyd.weight == weight__);
bt->branch = &bt->rbyd;
@@ -6697,6 +6775,12 @@ static inline bool lfsr_m_isckreads(uint32_t flags) {
}
#endif
#ifdef LFS_CKFETCHES
static inline bool lfsr_m_isckfetches(uint32_t flags) {
return flags & LFS_M_CKFETCHES;
}
#endif
static inline bool lfsr_m_isflush(uint32_t flags) {
return flags & LFS_M_FLUSH;
}
@@ -9185,6 +9269,19 @@ static int lfsr_mtree_traverse_(lfs_t *lfs, lfsr_traversal_t *t,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &t->o.bshrub.u.btree,
t->o.bshrub.u.btree.blocks[0],
lfsr_rbyd_trunk(&t->o.bshrub.u.btree),
t->o.bshrub.u.btree.cksum);
if (err) {
return err;
}
}
#endif
// transition to traversing the mtree
t->u.bt = LFSR_BTRAVERSAL();
t->o.o.flags = lfsr_t_settstate(t->o.o.flags,
@@ -9266,6 +9363,19 @@ static int lfsr_mtree_traverse_(lfs_t *lfs, lfsr_traversal_t *t,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &t->o.bshrub.u.btree,
t->o.bshrub.u.btree.blocks[0],
lfsr_rbyd_trunk(&t->o.bshrub.u.btree),
t->o.bshrub.u.btree.cksum);
if (err) {
return err;
}
}
#endif
// no? next we need to check any opened files
} else {
t->ot = lfs->omdirs;
@@ -10726,6 +10836,19 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
if (err) {
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)) {
err = lfsr_rbyd_fetchck(lfs, &file->o.bshrub.u.btree,
file->o.bshrub.u.btree.blocks[0],
lfsr_rbyd_trunk(&file->o.bshrub.u.btree),
file->o.bshrub.u.btree.cksum);
if (err) {
return err;
}
}
#endif
}
}
}
@@ -10847,6 +10970,17 @@ static lfs_ssize_t lfsr_file_readnext(lfs_t *lfs, const lfsr_file_t *file,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)
&& tag == LFSR_TAG_BLOCK) {
err = lfsr_bptr_ck(lfs, &bptr);
if (err) {
return err;
}
}
#endif
// any data on disk?
if (pos_ < bid-(weight-1) + lfsr_data_size(bptr.data)) {
// note one important side-effect here is a strict
@@ -11089,6 +11223,17 @@ static int lfsr_file_carve(lfs_t *lfs, lfsr_file_t *file,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)
&& tag_ == LFSR_TAG_BLOCK) {
err = lfsr_bptr_ck(lfs, &bptr_);
if (err) {
return err;
}
}
#endif
// note, an entry can be both a left and right sibling
lfsr_data_t left_slice_ = lfsr_data_slice(bptr_.data,
-1,
@@ -11556,6 +11701,17 @@ static int lfsr_file_flush_(lfs_t *lfs, lfsr_file_t *file,
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)
&& tag_ == LFSR_TAG_BLOCK) {
err = lfsr_bptr_ck(lfs, &bptr_);
if (err) {
return err;
}
}
#endif
// make sure to include all of our crystal, or else this
// loop may never terminate
if (bid_-(weight_-1) >= crystal_end
@@ -11718,6 +11874,17 @@ fragment:;
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)
&& tag == LFSR_TAG_BLOCK) {
err = lfsr_bptr_ck(lfs, &bptr);
if (err) {
return err;
}
}
#endif
// can we coalesce?
if (bid-(weight-1) + lfsr_data_size(bptr.data) >= fragment_start
&& fragment_end - (bid-(weight-1))
@@ -11756,6 +11923,17 @@ fragment:;
return err;
}
#ifdef LFS_CKFETCHES
// checking fetches?
if (lfsr_m_isckfetches(lfs->flags)
&& tag == LFSR_TAG_BLOCK) {
err = lfsr_bptr_ck(lfs, &bptr);
if (err) {
return err;
}
}
#endif
// can we coalesce?
if (fragment_end < bid-(weight-1) + lfsr_data_size(bptr.data)
&& bid-(weight-1) + lfsr_data_size(bptr.data)
@@ -13353,6 +13531,7 @@ int lfsr_mount(lfs_t *lfs, uint32_t flags,
| LFS_M_RDONLY
| LFS_IFDEF_CKPROGS(LFS_M_CKPROGS, 0)
| LFS_IFDEF_CKREADS(LFS_M_CKREADS, 0)
| LFS_IFDEF_CKFETCHES(LFS_M_CKFETCHES, 0)
| LFS_M_FLUSH
| LFS_M_SYNC
| LFS_M_MTREEONLY
@@ -13509,6 +13688,7 @@ int lfsr_format(lfs_t *lfs, uint32_t flags,
LFS_F_RDWR
| LFS_IFDEF_CKPROGS(LFS_F_CKPROGS, 0)
| LFS_IFDEF_CKREADS(LFS_F_CKREADS, 0)
| LFS_IFDEF_CKFETCHES(LFS_F_CKFETCHES, 0)
| LFS_F_MTREEONLY
| LFS_F_COMPACT
| LFS_F_CKMETA
@@ -13574,6 +13754,7 @@ int lfsr_fs_stat(lfs_t *lfs, struct lfs_fsinfo *fsinfo) {
LFS_I_RDONLY
| LFS_IFDEF_CKPROGS(LFS_I_CKPROGS, 0)
| LFS_IFDEF_CKREADS(LFS_I_CKREADS, 0)
| LFS_IFDEF_CKFETCHES(LFS_I_CKFETCHES, 0)
| LFS_I_FLUSH
| LFS_I_SYNC
| LFS_I_UNCOMPACTED);