Fixed new bshrub roots and related bug fixing

It turned out by implicitly handling root allocation in
lfsr_btree_commit_, we were never allowing lfsr_bshrub_commit to
intercept new roots as new bshrubs. Fixing this required moving the
root allocation logic up into lfsr_btree_commit.

This resulted in quite a bit of small bug fixing because it turns out if
you can never create non-inlined bshrubs you never test non-inlined
bshrubs:

- Our previous rbyd.weight == btree.weight check for if we've reached
  the root no longer works, changed to an explicit check that the blocks
  match. Fortunately, now that new roots set trunk=0 new roots are no
  longer a problematic case.

- We need to only evict when we calculate an accurate estimate, the
  previous code had a bug where eviction occurred early based only on the
  progged-since-last-estimate.

- We need to manually set bshrub.block=mdir.block on new bshrubs,
  otherwise the lfsr_bshrub_isbshrub check fails in mdir commit staging.

Also updated btree/bshrub following code in the dbg scripts, which
mostly meant making them accept both BRANCH and SHRUBBRANCH tags as
btree/bshrub branches. Conveniently very little code needs to change
to extend btree read operations to support bshrubs.

lfs.c

@@ -4016,9 +4016,10 @@ static lfs_ssize_t lfsr_btree_commit_(lfs_t *lfs,
         lfsr_rbyd_t parent = {.trunk=0, .weight=0};
         lfsr_srid_t rid;
         // are we root?
-        if (rbyd.trunk == 0 || rbyd.weight == btree->weight) {
+        if (rbyd.block == btree->block || rbyd.trunk == 0) {
-            // are we root and shrub? yield root updates to shrub commit
+            // new root? shrub root? yield creation of new roots to
-            if (shrub) {
+            // higher-level bshrub/btree logic
+            if (shrub || rbyd.trunk == 0) {
                 *btree = rbyd;
                 if (attrs_) {
                     *attrs_ = attrs;
@@ -4029,14 +4030,6 @@ static lfs_ssize_t lfsr_btree_commit_(lfs_t *lfs,
                 return 0;
             }
 
-            // need a new root? this happens if we split
-            if (rbyd.trunk == 0) {
-                int err = lfsr_rbyd_alloc(lfs, &rbyd);
-                if (err) {
-                    return err;
-                }
-            }
-
             // mark btree as unerased in case of failure, our btree rbyd and
             // root rbyd can diverge if there's a split, but we would have
             // marked the old root as unerased earlier anyways
@@ -4512,15 +4505,42 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
     lfsr_attr_t scratch_attrs[4];
     uint8_t scratch_buf[2*LFSR_BRANCH_DSIZE];
 
-    lfs_ssize_t attr_count_ = lfsr_btree_commit_(lfs, btree, false,
+    // try to commit to the btree
+    int err = lfsr_btree_commit_(lfs, btree, false,
             scratch_attrs, scratch_buf,
             attrs, attr_count,
-            NULL, NULL);
+            &attrs, &attr_count);
-    if (attr_count_ < 0) {
+    if (err) {
-        return attr_count_;
+        return err;
+    }
+
+    // needs a new root?
+    if (attr_count > 0) {
+        // TODO do we need to be this careful with backup copies?
+        lfsr_rbyd_t rbyd;
+        err = lfsr_rbyd_alloc(lfs, &rbyd);
+        if (err) {
+            return err;
+        }
+
+        // TODO should we just use rbyd commit? it allocates _another_ 
+        // redundant copy which is a bit much...
+        err = lfsr_rbyd_appendattrs(lfs, &rbyd, -1, -1,
+                attrs, attr_count);
+        if (err) {
+            LFS_ASSERT(err != LFS_ERR_RANGE);
+            return err;
+        }
+
+        err = lfsr_rbyd_appendcksum(lfs, &rbyd);
+        if (err) {
+            LFS_ASSERT(err != LFS_ERR_RANGE);
+            return err;
+        }
+
+        *btree = rbyd;
     }
 
-    LFS_ASSERT(attr_count_ == 0);
     LFS_ASSERT(btree->trunk != 0);
     return 0;
 }
@@ -6762,65 +6782,76 @@ static int lfsr_bshrub_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
                 return estimate;
             }
             bshrub->progged = estimate;
+
+            // do we overflow shrub_size/2? the 1/2 here prevents runaway
+            // performance when the shrub is near full
+            if (bshrub->progged > lfs->cfg->shrub_size/2) {
+                goto evict;
+            }
         }
 
-        // do we overflow shrub_size/2? the 1/2 here prevents runaway
+        // if our shrub is a new root, we need to set the correct block
-        // performance when the shrub is near full
+        LFS_ASSERT(bshrub->rbyd.trunk == 0
-        if (bshrub->progged > lfs->cfg->shrub_size/2) {
+                || bshrub->rbyd.block == mdir->u.rbyd.block);
-            // TODO am I missing a simpler function here? at least use
+        if (bshrub->rbyd.trunk == 0) {
-            // lfsr_rbyd_commit once it doesn't maintain a copy...
+            bshrub->rbyd.block = mdir->u.rbyd.block;
-
-            // convert to btree
-            err = lfsr_rbyd_alloc(lfs, &bshrub->rbyd_);
-            if (err) {
-                return err;
-            }
-
-            err = lfsr_rbyd_appendcompactrbyd(lfs, &bshrub->rbyd_, false,
-                    -1, -1, &bshrub->rbyd);
-            if (err) {
-                LFS_ASSERT(err != LFS_ERR_RANGE);
-                return err;
-            }
-
-            err = lfsr_rbyd_compact(lfs, &bshrub->rbyd_, false,
-                    sizeof(uint32_t));
-            if (err) {
-                LFS_ASSERT(err != LFS_ERR_RANGE);
-                return err;
-            }
-
-            err = lfsr_rbyd_appendattrs(lfs, &bshrub->rbyd_, -1, -1,
-                    attrs, attr_count);
-            if (err) {
-                LFS_ASSERT(err != LFS_ERR_RANGE);
-                return err;
-            }
-
-            err = lfsr_rbyd_appendcksum(lfs, &bshrub->rbyd_);
-            if (err) {
-                LFS_ASSERT(err != LFS_ERR_RANGE);
-                return err;
-            }
-
-            bshrub->rbyd = bshrub->rbyd_;
-
-        // otherwise commit to shrub like normal
-        } else {
-            int err = lfsr_mdir_commit(lfs, mdir, LFSR_ATTRS(
-                    LFSR_ATTR(mdir->mid,
-                        BSHRUBCOMMIT, 0, BSHRUBCOMMIT(
-                            bshrub, attrs, attr_count))));
-            if (err) {
-                return err;
-            }
-
-            bshrub->progged += progged;
         }
+
+        // commit to shrub
+        err = lfsr_mdir_commit(lfs, mdir, LFSR_ATTRS(
+                LFSR_ATTR(mdir->mid,
+                    BSHRUBCOMMIT, 0, BSHRUBCOMMIT(
+                        bshrub, attrs, attr_count))));
+        if (err) {
+            return err;
+        }
+
+        bshrub->progged += progged;
     }
 
     LFS_ASSERT(bshrub->rbyd.trunk != 0);
     return 0;
+
+evict:;
+    // TODO am I missing a simpler function here? at least use
+    // lfsr_rbyd_commit once it doesn't maintain a copy...
+
+    // convert to btree
+    err = lfsr_rbyd_alloc(lfs, &bshrub->rbyd_);
+    if (err) {
+        return err;
+    }
+
+    err = lfsr_rbyd_appendcompactrbyd(lfs, &bshrub->rbyd_, false,
+            -1, -1, &bshrub->rbyd);
+    if (err) {
+        LFS_ASSERT(err != LFS_ERR_RANGE);
+        return err;
+    }
+
+    err = lfsr_rbyd_compact(lfs, &bshrub->rbyd_, false,
+            sizeof(uint32_t));
+    if (err) {
+        LFS_ASSERT(err != LFS_ERR_RANGE);
+        return err;
+    }
+
+    err = lfsr_rbyd_appendattrs(lfs, &bshrub->rbyd_, -1, -1,
+            attrs, attr_count);
+    if (err) {
+        LFS_ASSERT(err != LFS_ERR_RANGE);
+        return err;
+    }
+
+    err = lfsr_rbyd_appendcksum(lfs, &bshrub->rbyd_);
+    if (err) {
+        LFS_ASSERT(err != LFS_ERR_RANGE);
+        return err;
+    }
+
+    bshrub->rbyd = bshrub->rbyd_;
+    LFS_ASSERT(bshrub->rbyd.trunk != 0);
+    return 0;
 }
 
 static lfs_scmp_t lfsr_bshrub_namelookup(lfs_t *lfs, const lfsr_mdir_t *mdir,

scripts/dbgbmap.py

@@ -910,7 +910,7 @@ class Rbyd:
                     rid_, w = rid__, w_
 
                 # catch any branches
-                if tag == TAG_BRANCH:
+                if tag & 0xfff == TAG_BRANCH:
                     branch = (tag, j, d, data)
 
                 tags.append((tag, j, d, data))
@@ -1017,7 +1017,10 @@ class Rbyd:
                     ))
 
                 d_ += max(bdepths.get(d, 0), 1)
-                leaf = (bid-(w-1), d, rid-(w-1), TAG_BRANCH)
+                leaf = (bid-(w-1), d, rid-(w-1),
+                    next((tag for tag, _, _, _ in tags
+                            if tag & 0xfff == TAG_BRANCH),
+                        TAG_BRANCH))
 
         # remap branches to leaves if we aren't showing inner branches
         if not inner:

scripts/dbgbtree.py

@@ -619,7 +619,7 @@ def main(disk, roots=None, *,
                         rid_, w = rid__, w_
 
                     # catch any branches
-                    if tag == TAG_BRANCH:
+                    if tag & 0xfff == TAG_BRANCH:
                         branch = (tag, j, d, data)
 
                     tags.append((tag, j, d, data))
@@ -725,7 +725,10 @@ def main(disk, roots=None, *,
                         ))
 
                     d_ += max(bdepths.get(d, 0), 1)
-                    leaf = (bid-(w-1), d, rid-(w-1), TAG_BRANCH)
+                    leaf = (bid-(w-1), d, rid-(w-1),
+                        next((tag for tag, _, _, _ in tags
+                                if tag & 0xfff == TAG_BRANCH),
+                            TAG_BRANCH))
 
             # remap branches to leaves if we aren't showing inner branches
             if not args.get('inner'):

scripts/dbglfs.py

@@ -610,7 +610,7 @@ class Rbyd:
                     rid_, w = rid__, w_
 
                 # catch any branches
-                if tag == TAG_BRANCH:
+                if tag & 0xfff == TAG_BRANCH:
                     branch = (tag, j, d, data)
 
                 tags.append((tag, j, d, data))
@@ -717,7 +717,10 @@ class Rbyd:
                     ))
 
                 d_ += max(bdepths.get(d, 0), 1)
-                leaf = (bid-(w-1), d, rid-(w-1), TAG_BRANCH)
+                leaf = (bid-(w-1), d, rid-(w-1),
+                    next((tag for tag, _, _, _ in tags
+                            if tag & 0xfff == TAG_BRANCH),
+                        TAG_BRANCH))
 
         # remap branches to leaves if we aren't showing inner branches
         if not inner:
@@ -914,7 +917,7 @@ class Rbyd:
             done, rid_, tag_, w_, j, d, data, _ = rbyd.lookup(rid, TAG_STRUCT)
 
             # found another branch
-            if tag_ == TAG_BRANCH:
+            if tag_ & 0xfff == TAG_BRANCH:
                 # update our bid
                 bid += rid - (w-1)
 

scripts/dbgmtree.py

@@ -596,7 +596,7 @@ class Rbyd:
                     rid_, w = rid__, w_
 
                 # catch any branches
-                if tag == TAG_BRANCH:
+                if tag & 0xfff == TAG_BRANCH:
                     branch = (tag, j, d, data)
 
                 tags.append((tag, j, d, data))
@@ -703,7 +703,10 @@ class Rbyd:
                     ))
 
                 d_ += max(bdepths.get(d, 0), 1)
-                leaf = (bid-(w-1), d, rid-(w-1), TAG_BRANCH)
+                leaf = (bid-(w-1), d, rid-(w-1),
+                    next((tag for tag, _, _, _ in tags
+                            if tag & 0xfff == TAG_BRANCH),
+                        TAG_BRANCH))
 
         # remap branches to leaves if we aren't showing inner branches
         if not inner: