From ab26437e462c3adbb39fec42b518c6878ead7164 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Tue, 21 Jan 2025 16:47:20 -0600 Subject: [PATCH] Fixed issue where ckfetches hid cached btree nodes from ckmeta In lfsr_mtree_traverse, we were explicitly not checking btree nodes when ckfetches was enabled. This was an attempt to avoid double-checking btree nodes when using both ckfetches and ckmeta. Unfortunately this misses errors introduced _after_ fetch in btrees we keep around in RAM. We still want to find these when doing an explicit ckmeta scan! --- Removing this special case _does_ mean we will be back to redundantly checking non-cached btree nodes, but I think this is a case of better safe than sorry. In the future we may cache btree nodes more aggressively depending on available RAM, and it would be a shame if that accidentally leads to a weaker filesystem. Worst case we can always revisit this ckfetches/ckmeta double-fetching in the future if it become a bottleneck. Found by our test_ck_spam_fwrite_fuzz test. This actually saves a tiny bit of code, but at the cost of redundantly checking non-cached btree nodes: code stack ctx default before: 38512 2624 640 default after: 38512 (+0.0%) 2624 (+0.0%) 640 (+0.0%) ckfetches before: 38766 2656 640 ckfetches after: 38762 (-0.0%) 2656 (+0.0%) 640 (+0.0%) --- lfs.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lfs.c b/lfs.c index 29c42a60..30edc9b2 100644 --- a/lfs.c +++ b/lfs.c @@ -9541,13 +9541,13 @@ static int lfsr_mtree_traverse(lfs_t *lfs, lfsr_traversal_t *t, } // validate btree nodes? + // + // this may end up revalidating some btree nodes when ckfetches is + // enabled, but we need to revalidate cached btree nodes or we risk + // missing errors in ckmeta scans if ((lfsr_t_isckmeta(t->o.o.flags) || lfsr_t_isckdata(t->o.o.flags)) - && tag == LFSR_TAG_BRANCH - // note ckfetches already validates btree nodes - && LFS_IFDEF_CKFETCHES( - !lfsr_m_isckfetches(lfs->flags), - true)) { + && tag == LFSR_TAG_BRANCH) { lfsr_rbyd_t *rbyd = (lfsr_rbyd_t*)bptr.data.u.buffer; err = lfsr_rbyd_fetchck(lfs, rbyd, rbyd->blocks[0], rbyd->trunk,