From 395eff49ade91561653a772dea0caf0461c9c8c3 Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Sun, 14 May 2023 02:58:07 -0500
Subject: [PATCH] Changed mdir weight->0 to drop caches instead of introducing
 a temporary commit

This was actually a bug, and would have eventually been caught when we resume
power-loss testing.

In our current implementation of the mtree, we immediately drop mdirs
when their weight goes to zero, since at this point there's no route to
write new commits to the mdir. This was implemented by 1. writing out the
commit, and then 2. removing the mdir from the mtree if its weight is zero.

But this has a problem analogous to why we can't salvage failed compacts
during mdir split: If we allow a valid commit to be written to an mdir
before we update its position in the mtree, that commit becomes immediately
visible in the case of a power-loss.

This is a bit tricky to fix since we rely entirely on appending tags to
the on-disk rbyd to determine weight changes. We tried simulating weight
changes previously, but that was a mistake that created complexity.

The solution here is separate the appending of tags from the commit
finalization:

1. Append any pending tags.
2. If weight->0, drop caches, abort the commit.
3. Otherwise, write the checksum, finalizing the commit.

This has the new side-effect of intentionally leaving unfinalized commits
on-disk, but since we have no way to reclaim the erased bytes in these
mdirs, that is probably ok.
---
 lfs.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/lfs.c b/lfs.c
index cf4eba5f..b9919017 100644
--- a/lfs.c
+++ b/lfs.c
@@ -4661,11 +4661,17 @@ static int lfsr_mdir_compact_(lfs_t *lfs, lfsr_mdir_t *mdir,
         return err;
     }
 
+    // drop commit if weight goes to zero
+    if (mdir->mid >= 0 && mdir->rbyd.weight == 0) {
+        lfs_cache_drop(lfs, &lfs->pcache);
+
     // finalize commit
-    err = lfsr_rbyd_commit(lfs, &mdir->rbyd, NULL, 0);
-    if (err) {
-        LFS_ASSERT(err != LFS_ERR_RANGE);
-        return err;
+    } else {
+        err = lfsr_rbyd_commit(lfs, &mdir->rbyd, NULL, 0);
+        if (err) {
+            LFS_ASSERT(err != LFS_ERR_RANGE);
+            return err;
+        }
     }
 
     return 0;
@@ -4676,7 +4682,8 @@ static int lfsr_mdir_commit_(lfs_t *lfs, lfsr_mdir_t *mdir,
         lfs_size_t *lower_id_, lfs_size_t *lower_dsize_,
         const lfsr_attr_t *attrs, lfs_size_t attr_count) {
     // try to append a commit
-    int err = lfsr_rbyd_commit(lfs, &mdir->rbyd, attrs, attr_count);
+    lfsr_mdir_t mdir_ = *mdir;
+    int err = lfsr_rbyd_appendall(lfs, &mdir_.rbyd, -1, -1, attrs, attr_count);
     if (err && err != LFS_ERR_RANGE) {
         return err;
     }
@@ -4684,6 +4691,23 @@ static int lfsr_mdir_commit_(lfs_t *lfs, lfsr_mdir_t *mdir,
         goto compact;
     }
 
+    // drop commit if weight goes to zero
+    if (mdir_.mid >= 0 && mdir_.rbyd.weight == 0) {
+        lfs_cache_drop(lfs, &lfs->pcache);
+
+    // finalize commit
+    } else {
+        err = lfsr_rbyd_commit(lfs, &mdir_.rbyd, NULL, 0);
+        if (err && err != LFS_ERR_RANGE) {
+            return err;
+        }
+        if (err == LFS_ERR_RANGE) {
+            goto compact;
+        }
+    }
+
+    // update our mdir
+    *mdir = mdir_;
     return 0;
 
 compact:;
@@ -4704,7 +4728,6 @@ compact:;
 
     // if we've compacted this mdir block_cycles number of times, trigger
     // a relocation
-    lfsr_mdir_t mdir_ = *mdir;
     if (lfs->cfg->block_cycles > 0
                 && (mdir->rbyd.rev+1) % lfs->cfg->block_cycles == 0) {
         // allocate a new mdir for relocation