]> git.sesse.net Git - bcachefs-tools-debian/blobdiff - libbcachefs/subvolume.c
Update bcachefs sources to 6a20aede29 bcachefs: Fix quotas + snapshots
[bcachefs-tools-debian] / libbcachefs / subvolume.c
index 60b60de83f3e5cf3188e7309a5c2cde519541e5f..68121df2db669ddef47c34ce2aac0196ac7fdda3 100644 (file)
@@ -3,21 +3,86 @@
 #include "bcachefs.h"
 #include "btree_key_cache.h"
 #include "btree_update.h"
+#include "errcode.h"
 #include "error.h"
 #include "fs.h"
 #include "subvolume.h"
 
+static int bch2_subvolume_delete(struct btree_trans *, u32);
+
 /* Snapshot tree: */
 
-static void bch2_delete_dead_snapshots_work(struct work_struct *);
-static void bch2_delete_dead_snapshots(struct bch_fs *);
+void bch2_snapshot_tree_to_text(struct printbuf *out, struct bch_fs *c,
+                               struct bkey_s_c k)
+{
+       struct bkey_s_c_snapshot_tree t = bkey_s_c_to_snapshot_tree(k);
+
+       prt_printf(out, "subvol %u root snapshot %u",
+                  le32_to_cpu(t.v->master_subvol),
+                  le32_to_cpu(t.v->root_snapshot));
+}
+
+int bch2_snapshot_tree_invalid(const struct bch_fs *c, struct bkey_s_c k,
+                              unsigned flags, struct printbuf *err)
+{
+       if (bkey_gt(k.k->p, POS(0, U32_MAX)) ||
+           bkey_lt(k.k->p, POS(0, 1))) {
+               prt_printf(err, "bad pos");
+               return -BCH_ERR_invalid_bkey;
+       }
+
+       return 0;
+}
+
+int bch2_snapshot_tree_lookup(struct btree_trans *trans, u32 id,
+                             struct bch_snapshot_tree *s)
+{
+       return bch2_bkey_get_val_typed(trans, BTREE_ID_snapshot_trees, POS(0, id),
+                                      BTREE_ITER_WITH_UPDATES, snapshot_tree, s);
+}
+
+static struct bkey_i_snapshot_tree *
+__snapshot_tree_create(struct btree_trans *trans)
+{
+       struct btree_iter iter;
+       int ret = bch2_bkey_get_empty_slot(trans, &iter,
+                       BTREE_ID_snapshot_trees, POS(0, U32_MAX));
+       struct bkey_i_snapshot_tree *s_t;
+
+       if (ret == -BCH_ERR_ENOSPC_btree_slot)
+               ret = -BCH_ERR_ENOSPC_snapshot_tree;
+       if (ret)
+               return ERR_PTR(ret);
+
+       s_t = bch2_bkey_alloc(trans, &iter, 0, snapshot_tree);
+       ret = PTR_ERR_OR_ZERO(s_t);
+       bch2_trans_iter_exit(trans, &iter);
+       return ret ? ERR_PTR(ret) : s_t;
+}
+
+static int snapshot_tree_create(struct btree_trans *trans,
+                               u32 root_id, u32 subvol_id, u32 *tree_id)
+{
+       struct bkey_i_snapshot_tree *n_tree =
+               __snapshot_tree_create(trans);
+
+       if (IS_ERR(n_tree))
+               return PTR_ERR(n_tree);
+
+       n_tree->v.master_subvol = cpu_to_le32(subvol_id);
+       n_tree->v.root_snapshot = cpu_to_le32(root_id);
+       *tree_id = n_tree->k.p.offset;
+       return 0;
+}
+
+/* Snapshot nodes: */
 
 void bch2_snapshot_to_text(struct printbuf *out, struct bch_fs *c,
                           struct bkey_s_c k)
 {
        struct bkey_s_c_snapshot s = bkey_s_c_to_snapshot(k);
 
-       prt_printf(out, "is_subvol %llu deleted %llu parent %u children %u %u subvol %u",
+       prt_printf(out, "is_subvol %llu deleted %llu parent %10u children %10u %10u subvol %u",
               BCH_SNAPSHOT_SUBVOL(s.v),
               BCH_SNAPSHOT_DELETED(s.v),
               le32_to_cpu(s.v->parent),
@@ -27,21 +92,15 @@ void bch2_snapshot_to_text(struct printbuf *out, struct bch_fs *c,
 }
 
 int bch2_snapshot_invalid(const struct bch_fs *c, struct bkey_s_c k,
-                         int rw, struct printbuf *err)
+                         unsigned flags, struct printbuf *err)
 {
        struct bkey_s_c_snapshot s;
        u32 i, id;
 
-       if (bkey_cmp(k.k->p, POS(0, U32_MAX)) > 0 ||
-           bkey_cmp(k.k->p, POS(0, 1)) < 0) {
+       if (bkey_gt(k.k->p, POS(0, U32_MAX)) ||
+           bkey_lt(k.k->p, POS(0, 1))) {
                prt_printf(err, "bad pos");
-               return -EINVAL;
-       }
-
-       if (bkey_val_bytes(k.k) != sizeof(struct bch_snapshot)) {
-               prt_printf(err, "bad val size (%zu != %zu)",
-                      bkey_val_bytes(k.k), sizeof(struct bch_snapshot));
-               return -EINVAL;
+               return -BCH_ERR_invalid_bkey;
        }
 
        s = bkey_s_c_to_snapshot(k);
@@ -50,18 +109,18 @@ int bch2_snapshot_invalid(const struct bch_fs *c, struct bkey_s_c k,
        if (id && id <= k.k->p.offset) {
                prt_printf(err, "bad parent node (%u <= %llu)",
                       id, k.k->p.offset);
-               return -EINVAL;
+               return -BCH_ERR_invalid_bkey;
        }
 
        if (le32_to_cpu(s.v->children[0]) < le32_to_cpu(s.v->children[1])) {
                prt_printf(err, "children not normalized");
-               return -EINVAL;
+               return -BCH_ERR_invalid_bkey;
        }
 
        if (s.v->children[0] &&
            s.v->children[0] == s.v->children[1]) {
                prt_printf(err, "duplicate child nodes");
-               return -EINVAL;
+               return -BCH_ERR_invalid_bkey;
        }
 
        for (i = 0; i < 2; i++) {
@@ -70,7 +129,7 @@ int bch2_snapshot_invalid(const struct bch_fs *c, struct bkey_s_c k,
                if (id >= k.k->p.offset) {
                        prt_printf(err, "bad child node (%u >= %llu)",
                               id, k.k->p.offset);
-                       return -EINVAL;
+                       return -BCH_ERR_invalid_bkey;
                }
        }
 
@@ -78,6 +137,7 @@ int bch2_snapshot_invalid(const struct bch_fs *c, struct bkey_s_c k,
 }
 
 int bch2_mark_snapshot(struct btree_trans *trans,
+                      enum btree_id btree, unsigned level,
                       struct bkey_s_c old, struct bkey_s_c new,
                       unsigned flags)
 {
@@ -88,7 +148,7 @@ int bch2_mark_snapshot(struct btree_trans *trans,
                               U32_MAX - new.k->p.offset,
                               GFP_KERNEL);
        if (!t)
-               return -ENOMEM;
+               return -BCH_ERR_ENOMEM_mark_snapshot;
 
        if (new.k->type == KEY_TYPE_snapshot) {
                struct bkey_s_c_snapshot s = bkey_s_c_to_snapshot(new);
@@ -97,11 +157,13 @@ int bch2_mark_snapshot(struct btree_trans *trans,
                t->children[0]  = le32_to_cpu(s.v->children[0]);
                t->children[1]  = le32_to_cpu(s.v->children[1]);
                t->subvol       = BCH_SNAPSHOT_SUBVOL(s.v) ? le32_to_cpu(s.v->subvol) : 0;
+               t->tree         = le32_to_cpu(s.v->tree);
        } else {
                t->parent       = 0;
                t->children[0]  = 0;
                t->children[1]  = 0;
                t->subvol       = 0;
+               t->tree         = 0;
        }
 
        return 0;
@@ -110,20 +172,8 @@ int bch2_mark_snapshot(struct btree_trans *trans,
 static int snapshot_lookup(struct btree_trans *trans, u32 id,
                           struct bch_snapshot *s)
 {
-       struct btree_iter iter;
-       struct bkey_s_c k;
-       int ret;
-
-       bch2_trans_iter_init(trans, &iter, BTREE_ID_snapshots, POS(0, id),
-                            BTREE_ITER_WITH_UPDATES);
-       k = bch2_btree_iter_peek_slot(&iter);
-       ret = bkey_err(k) ?: k.k->type == KEY_TYPE_snapshot ? 0 : -ENOENT;
-
-       if (!ret)
-               *s = *bkey_s_c_to_snapshot(k).v;
-
-       bch2_trans_iter_exit(trans, &iter);
-       return ret;
+       return bch2_bkey_get_val_typed(trans, BTREE_ID_snapshots, POS(0, id),
+                                      BTREE_ITER_WITH_UPDATES, snapshot, s);
 }
 
 static int snapshot_live(struct btree_trans *trans, u32 id)
@@ -134,7 +184,7 @@ static int snapshot_live(struct btree_trans *trans, u32 id)
        if (!id)
                return 0;
 
-       ret = lockrestart_do(trans, snapshot_lookup(trans, id, &v));
+       ret = snapshot_lookup(trans, id, &v);
        if (ret == -ENOENT)
                bch_err(trans->c, "snapshot node %u not found", id);
        if (ret)
@@ -143,157 +193,500 @@ static int snapshot_live(struct btree_trans *trans, u32 id)
        return !BCH_SNAPSHOT_DELETED(&v);
 }
 
-static int bch2_snapshots_set_equiv(struct btree_trans *trans)
+static int bch2_snapshot_set_equiv(struct btree_trans *trans, struct bkey_s_c k)
+{
+       struct bch_fs *c = trans->c;
+       unsigned i, nr_live = 0, live_idx = 0;
+       struct bkey_s_c_snapshot snap;
+       u32 id = k.k->p.offset, child[2];
+
+       if (k.k->type != KEY_TYPE_snapshot)
+               return 0;
+
+       snap = bkey_s_c_to_snapshot(k);
+
+       child[0] = le32_to_cpu(snap.v->children[0]);
+       child[1] = le32_to_cpu(snap.v->children[1]);
+
+       for (i = 0; i < 2; i++) {
+               int ret = snapshot_live(trans, child[i]);
+
+               if (ret < 0)
+                       return ret;
+
+               if (ret)
+                       live_idx = i;
+               nr_live += ret;
+       }
+
+       snapshot_t(c, id)->equiv = nr_live == 1
+               ? snapshot_t(c, child[live_idx])->equiv
+               : id;
+       return 0;
+}
+
+/* fsck: */
+
+static u32 bch2_snapshot_child(struct bch_fs *c, u32 id, unsigned child)
+{
+       return snapshot_t(c, id)->children[child];
+}
+
+static u32 bch2_snapshot_left_child(struct bch_fs *c, u32 id)
+{
+       return bch2_snapshot_child(c, id, 0);
+}
+
+static u32 bch2_snapshot_right_child(struct bch_fs *c, u32 id)
+{
+       return bch2_snapshot_child(c, id, 1);
+}
+
+static u32 bch2_snapshot_tree_next(struct bch_fs *c, u32 id)
+{
+       u32 n, parent;
+
+       n = bch2_snapshot_left_child(c, id);
+       if (n)
+               return n;
+
+       while ((parent = bch2_snapshot_parent(c, id))) {
+               n = bch2_snapshot_right_child(c, parent);
+               if (n && n != id)
+                       return n;
+               id = parent;
+       }
+
+       return 0;
+}
+
+static u32 bch2_snapshot_tree_oldest_subvol(struct bch_fs *c, u32 snapshot_root)
+{
+       u32 id = snapshot_root;
+       u32 subvol = 0, s;
+
+       while (id) {
+               s = snapshot_t(c, id)->subvol;
+
+               if (s && (!subvol || s < subvol))
+                       subvol = s;
+
+               id = bch2_snapshot_tree_next(c, id);
+       }
+
+       return subvol;
+}
+
+static int bch2_snapshot_tree_master_subvol(struct btree_trans *trans,
+                                           u32 snapshot_root, u32 *subvol_id)
 {
        struct bch_fs *c = trans->c;
        struct btree_iter iter;
        struct bkey_s_c k;
-       struct bkey_s_c_snapshot snap;
-       unsigned i;
+       struct bkey_s_c_subvolume s;
        int ret;
 
-       for_each_btree_key(trans, iter, BTREE_ID_snapshots,
-                          POS_MIN, 0, k, ret) {
-               u32 id = k.k->p.offset, child[2];
-               unsigned nr_live = 0, live_idx = 0;
+       for_each_btree_key_norestart(trans, iter, BTREE_ID_subvolumes, POS_MIN,
+                                    0, k, ret) {
+               if (k.k->type != KEY_TYPE_subvolume)
+                       continue;
 
-               if (k.k->type != KEY_TYPE_snapshot)
+               s = bkey_s_c_to_subvolume(k);
+               if (!bch2_snapshot_is_ancestor(c, le32_to_cpu(s.v->snapshot), snapshot_root))
                        continue;
+               if (!BCH_SUBVOLUME_SNAP(s.v)) {
+                       *subvol_id = s.k->p.offset;
+                       goto found;
+               }
+       }
+       ret = ret ?: -ENOENT;
+found:
+       bch2_trans_iter_exit(trans, &iter);
 
-               snap = bkey_s_c_to_snapshot(k);
-               child[0] = le32_to_cpu(snap.v->children[0]);
-               child[1] = le32_to_cpu(snap.v->children[1]);
+       if (ret == -ENOENT) {
+               struct bkey_i_subvolume *s;
 
-               for (i = 0; i < 2; i++) {
-                       ret = snapshot_live(trans, child[i]);
-                       if (ret < 0)
-                               goto err;
+               *subvol_id = bch2_snapshot_tree_oldest_subvol(c, snapshot_root);
 
-                       if (ret)
-                               live_idx = i;
-                       nr_live += ret;
-               }
+               s = bch2_bkey_get_mut_typed(trans, &iter,
+                                           BTREE_ID_subvolumes, POS(0, *subvol_id),
+                                           0, subvolume);
+               ret = PTR_ERR_OR_ZERO(s);
+               if (ret)
+                       return ret;
+
+               SET_BCH_SUBVOLUME_SNAP(&s->v, false);
+       }
+
+       return ret;
+}
+
+static int check_snapshot_tree(struct btree_trans *trans,
+                              struct btree_iter *iter,
+                              struct bkey_s_c k)
+{
+       struct bch_fs *c = trans->c;
+       struct bkey_s_c_snapshot_tree st;
+       struct bch_snapshot s;
+       struct bch_subvolume subvol;
+       struct printbuf buf = PRINTBUF;
+       u32 root_id;
+       int ret;
+
+       if (k.k->type != KEY_TYPE_snapshot_tree)
+               return 0;
+
+       st = bkey_s_c_to_snapshot_tree(k);
+       root_id = le32_to_cpu(st.v->root_snapshot);
+
+       ret = snapshot_lookup(trans, root_id, &s);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               goto err;
+
+       if (fsck_err_on(ret ||
+                       root_id != bch2_snapshot_root(c, root_id) ||
+                       st.k->p.offset != le32_to_cpu(s.tree),
+                       c,
+                       "snapshot tree points to missing/incorrect snapshot:\n  %s",
+                       (bch2_bkey_val_to_text(&buf, c, st.s_c), buf.buf))) {
+               ret = bch2_btree_delete_at(trans, iter, 0);
+               goto err;
+       }
+
+       ret = bch2_subvolume_get(trans, le32_to_cpu(st.v->master_subvol),
+                                false, 0, &subvol);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               goto err;
+
+       if (fsck_err_on(ret ||
+                       !bch2_snapshot_is_ancestor(c,
+                                                  le32_to_cpu(subvol.snapshot),
+                                                  root_id) ||
+                       !BCH_SUBVOLUME_SNAP(&subvol), c,
+                       "snapshot tree points to missing/incorrect subvolume:\n  %s",
+                       (printbuf_reset(&buf),
+                        bch2_bkey_val_to_text(&buf, c, st.s_c), buf.buf))) {
+               struct bkey_i_snapshot_tree *u;
+               u32 subvol_id;
+
+               ret = bch2_snapshot_tree_master_subvol(trans, root_id, &subvol_id);
+               if (ret)
+                       goto err;
+
+               u = bch2_bkey_make_mut_typed(trans, iter, k, 0, snapshot_tree);
+               ret = PTR_ERR_OR_ZERO(u);
+               if (ret)
+                       goto err;
 
-               snapshot_t(c, id)->equiv = nr_live == 1
-                       ? snapshot_t(c, child[live_idx])->equiv
-                       : id;
+               u->v.master_subvol = cpu_to_le32(subvol_id);
+               st = snapshot_tree_i_to_s_c(u);
        }
 err:
-       bch2_trans_iter_exit(trans, &iter);
+fsck_err:
+       printbuf_exit(&buf);
+       return ret;
+}
 
-       if (ret)
-               bch_err(c, "error walking snapshots: %i", ret);
+/*
+ * For each snapshot_tree, make sure it points to the root of a snapshot tree
+ * and that snapshot entry points back to it, or delete it.
+ *
+ * And, make sure it points to a subvolume within that snapshot tree, or correct
+ * it to point to the oldest subvolume within that snapshot tree.
+ */
+int bch2_fs_check_snapshot_trees(struct bch_fs *c)
+{
+       struct btree_iter iter;
+       struct bkey_s_c k;
+       int ret;
 
+       ret = bch2_trans_run(c,
+               for_each_btree_key_commit(&trans, iter,
+                       BTREE_ID_snapshot_trees, POS_MIN,
+                       BTREE_ITER_PREFETCH, k,
+                       NULL, NULL, BTREE_INSERT_LAZY_RW|BTREE_INSERT_NOFAIL,
+               check_snapshot_tree(&trans, &iter, k)));
+
+       if (ret)
+               bch_err(c, "error %i checking snapshot trees", ret);
        return ret;
 }
 
-/* fsck: */
-static int bch2_snapshot_check(struct btree_trans *trans,
-                              struct bkey_s_c_snapshot s)
+/*
+ * Look up snapshot tree for @tree_id and find root,
+ * make sure @snap_id is a descendent:
+ */
+static int snapshot_tree_ptr_good(struct btree_trans *trans,
+                                 u32 snap_id, u32 tree_id)
 {
-       struct bch_subvolume subvol;
-       struct bch_snapshot v;
-       u32 i, id;
+       struct bch_snapshot_tree s_t;
+       int ret = bch2_snapshot_tree_lookup(trans, tree_id, &s_t);
+
+       if (ret)
+               return ret;
+
+       return bch2_snapshot_is_ancestor(trans->c, snap_id, le32_to_cpu(s_t.root_snapshot));
+}
+
+/*
+ * snapshot_tree pointer was incorrect: look up root snapshot node, make sure
+ * its snapshot_tree pointer is correct (allocate new one if necessary), then
+ * update this node's pointer to root node's pointer:
+ */
+static int snapshot_tree_ptr_repair(struct btree_trans *trans,
+                                   struct btree_iter *iter,
+                                   struct bkey_s_c_snapshot *s)
+{
+       struct bch_fs *c = trans->c;
+       struct btree_iter root_iter;
+       struct bch_snapshot_tree s_t;
+       struct bkey_s_c_snapshot root;
+       struct bkey_i_snapshot *u;
+       u32 root_id = bch2_snapshot_root(c, s->k->p.offset), tree_id;
        int ret;
 
-       id = le32_to_cpu(s.v->subvol);
-       ret = lockrestart_do(trans, bch2_subvolume_get(trans, id, 0, false, &subvol));
-       if (ret == -ENOENT)
-               bch_err(trans->c, "snapshot node %llu has nonexistent subvolume %u",
-                       s.k->p.offset, id);
+       root = bch2_bkey_get_iter_typed(trans, &root_iter,
+                              BTREE_ID_snapshots, POS(0, root_id),
+                              BTREE_ITER_WITH_UPDATES, snapshot);
+       ret = bkey_err(root);
+       if (ret)
+               goto err;
+
+       tree_id = le32_to_cpu(root.v->tree);
+
+       ret = bch2_snapshot_tree_lookup(trans, tree_id, &s_t);
        if (ret)
                return ret;
 
-       if (BCH_SNAPSHOT_SUBVOL(s.v) != (le32_to_cpu(subvol.snapshot) == s.k->p.offset)) {
-               bch_err(trans->c, "snapshot node %llu has wrong BCH_SNAPSHOT_SUBVOL",
-                       s.k->p.offset);
-               return -EINVAL;
+       if (le32_to_cpu(s_t.root_snapshot) != root_id) {
+               u = bch2_bkey_make_mut_typed(trans, &root_iter, root.s_c, 0, snapshot);
+               ret =   PTR_ERR_OR_ZERO(u) ?:
+                       snapshot_tree_create(trans, root_id,
+                               bch2_snapshot_tree_oldest_subvol(c, root_id),
+                               &tree_id);
+               if (ret)
+                       goto err;
+
+               u->v.tree = cpu_to_le32(tree_id);
+               if (s->k->p.snapshot == root_id)
+                       *s = snapshot_i_to_s_c(u);
        }
 
+       if (s->k->p.snapshot != root_id) {
+               u = bch2_bkey_make_mut_typed(trans, iter, s->s_c, 0, snapshot);
+               ret = PTR_ERR_OR_ZERO(u);
+               if (ret)
+                       goto err;
+
+               u->v.tree = cpu_to_le32(tree_id);
+               *s = snapshot_i_to_s_c(u);
+       }
+err:
+       bch2_trans_iter_exit(trans, &root_iter);
+       return ret;
+}
+
+static int check_snapshot(struct btree_trans *trans,
+                         struct btree_iter *iter,
+                         struct bkey_s_c k)
+{
+       struct bch_fs *c = trans->c;
+       struct bkey_s_c_snapshot s;
+       struct bch_subvolume subvol;
+       struct bch_snapshot v;
+       struct printbuf buf = PRINTBUF;
+       bool should_have_subvol;
+       u32 i, id;
+       int ret = 0;
+
+       if (k.k->type != KEY_TYPE_snapshot)
+               return 0;
+
+       s = bkey_s_c_to_snapshot(k);
        id = le32_to_cpu(s.v->parent);
        if (id) {
-               ret = lockrestart_do(trans, snapshot_lookup(trans, id, &v));
+               ret = snapshot_lookup(trans, id, &v);
                if (ret == -ENOENT)
-                       bch_err(trans->c, "snapshot node %llu has nonexistent parent %u",
-                               s.k->p.offset, id);
+                       bch_err(c, "snapshot with nonexistent parent:\n  %s",
+                               (bch2_bkey_val_to_text(&buf, c, s.s_c), buf.buf));
                if (ret)
-                       return ret;
+                       goto err;
 
                if (le32_to_cpu(v.children[0]) != s.k->p.offset &&
                    le32_to_cpu(v.children[1]) != s.k->p.offset) {
-                       bch_err(trans->c, "snapshot parent %u missing pointer to child %llu",
+                       bch_err(c, "snapshot parent %u missing pointer to child %llu",
                                id, s.k->p.offset);
-                       return -EINVAL;
+                       ret = -EINVAL;
+                       goto err;
                }
        }
 
        for (i = 0; i < 2 && s.v->children[i]; i++) {
                id = le32_to_cpu(s.v->children[i]);
 
-               ret = lockrestart_do(trans, snapshot_lookup(trans, id, &v));
+               ret = snapshot_lookup(trans, id, &v);
                if (ret == -ENOENT)
-                       bch_err(trans->c, "snapshot node %llu has nonexistent child %u",
+                       bch_err(c, "snapshot node %llu has nonexistent child %u",
                                s.k->p.offset, id);
                if (ret)
-                       return ret;
+                       goto err;
 
                if (le32_to_cpu(v.parent) != s.k->p.offset) {
-                       bch_err(trans->c, "snapshot child %u has wrong parent (got %u should be %llu)",
+                       bch_err(c, "snapshot child %u has wrong parent (got %u should be %llu)",
                                id, le32_to_cpu(v.parent), s.k->p.offset);
-                       return -EINVAL;
+                       ret = -EINVAL;
+                       goto err;
                }
        }
 
-       return 0;
+       should_have_subvol = BCH_SNAPSHOT_SUBVOL(s.v) &&
+               !BCH_SNAPSHOT_DELETED(s.v);
+
+       if (should_have_subvol) {
+               id = le32_to_cpu(s.v->subvol);
+               ret = bch2_subvolume_get(trans, id, 0, false, &subvol);
+               if (ret == -ENOENT)
+                       bch_err(c, "snapshot points to nonexistent subvolume:\n  %s",
+                               (bch2_bkey_val_to_text(&buf, c, s.s_c), buf.buf));
+               if (ret)
+                       goto err;
+
+               if (BCH_SNAPSHOT_SUBVOL(s.v) != (le32_to_cpu(subvol.snapshot) == s.k->p.offset)) {
+                       bch_err(c, "snapshot node %llu has wrong BCH_SNAPSHOT_SUBVOL",
+                               s.k->p.offset);
+                       ret = -EINVAL;
+                       goto err;
+               }
+       } else {
+               if (fsck_err_on(s.v->subvol, c, "snapshot should not point to subvol:\n  %s",
+                               (bch2_bkey_val_to_text(&buf, c, s.s_c), buf.buf))) {
+                       struct bkey_i_snapshot *u = bch2_trans_kmalloc(trans, sizeof(*u));
+
+                       ret = PTR_ERR_OR_ZERO(u);
+                       if (ret)
+                               goto err;
+
+                       bkey_reassemble(&u->k_i, s.s_c);
+                       u->v.subvol = 0;
+                       ret = bch2_trans_update(trans, iter, &u->k_i, 0);
+                       if (ret)
+                               goto err;
+
+                       s = snapshot_i_to_s_c(u);
+               }
+       }
+
+       ret = snapshot_tree_ptr_good(trans, s.k->p.offset, le32_to_cpu(s.v->tree));
+       if (ret < 0)
+               goto err;
+
+       if (fsck_err_on(!ret, c, "snapshot points to missing/incorrect tree:\n  %s",
+                       (bch2_bkey_val_to_text(&buf, c, s.s_c), buf.buf))) {
+               ret = snapshot_tree_ptr_repair(trans, iter, &s);
+               if (ret)
+                       goto err;
+       }
+
+       if (BCH_SNAPSHOT_DELETED(s.v))
+               set_bit(BCH_FS_HAVE_DELETED_SNAPSHOTS, &c->flags);
+err:
+fsck_err:
+       printbuf_exit(&buf);
+       return ret;
 }
 
-int bch2_fs_snapshots_check(struct bch_fs *c)
+int bch2_fs_check_snapshots(struct bch_fs *c)
 {
-       struct btree_trans trans;
        struct btree_iter iter;
        struct bkey_s_c k;
-       struct bch_snapshot s;
-       unsigned id;
        int ret;
 
-       bch2_trans_init(&trans, c, 0, 0);
+       ret = bch2_trans_run(c,
+               for_each_btree_key_commit(&trans, iter,
+                       BTREE_ID_snapshots, POS_MIN,
+                       BTREE_ITER_PREFETCH, k,
+                       NULL, NULL, BTREE_INSERT_LAZY_RW|BTREE_INSERT_NOFAIL,
+               check_snapshot(&trans, &iter, k)));
 
-       for_each_btree_key(&trans, iter, BTREE_ID_snapshots,
-                          POS_MIN, 0, k, ret) {
-               if (k.k->type != KEY_TYPE_snapshot)
-                       continue;
+       if (ret)
+               bch_err(c, "error %i checking snapshots", ret);
+       return ret;
+}
+
+static int check_subvol(struct btree_trans *trans,
+                       struct btree_iter *iter,
+                       struct bkey_s_c k)
+{
+       struct bch_fs *c = trans->c;
+       struct bkey_s_c_subvolume subvol;
+       struct bch_snapshot snapshot;
+       unsigned snapid;
+       int ret = 0;
+
+       if (k.k->type != KEY_TYPE_subvolume)
+               return 0;
+
+       subvol = bkey_s_c_to_subvolume(k);
+       snapid = le32_to_cpu(subvol.v->snapshot);
+       ret = snapshot_lookup(trans, snapid, &snapshot);
+
+       if (ret == -ENOENT)
+               bch_err(c, "subvolume %llu points to nonexistent snapshot %u",
+                       k.k->p.offset, snapid);
+       if (ret)
+               return ret;
 
-               ret = bch2_snapshot_check(&trans, bkey_s_c_to_snapshot(k));
+       if (BCH_SUBVOLUME_UNLINKED(subvol.v)) {
+               bch2_fs_lazy_rw(c);
+
+               ret = bch2_subvolume_delete(trans, iter->pos.offset);
                if (ret)
-                       break;
+                       bch_err(c, "error deleting subvolume %llu: %s",
+                               iter->pos.offset, bch2_err_str(ret));
+               return ret ?: -BCH_ERR_transaction_restart_nested;
        }
-       bch2_trans_iter_exit(&trans, &iter);
 
-       if (ret) {
-               bch_err(c, "error %i checking snapshots", ret);
-               goto err;
-       }
+       if (!BCH_SUBVOLUME_SNAP(subvol.v)) {
+               u32 snapshot_root = bch2_snapshot_root(c, le32_to_cpu(subvol.v->snapshot));
+               struct bch_snapshot_tree st;
 
-       for_each_btree_key(&trans, iter, BTREE_ID_subvolumes,
-                          POS_MIN, 0, k, ret) {
-               if (k.k->type != KEY_TYPE_subvolume)
-                       continue;
-again_2:
-               id = le32_to_cpu(bkey_s_c_to_subvolume(k).v->snapshot);
-               ret = snapshot_lookup(&trans, id, &s);
-
-               if (ret == -EINTR) {
-                       k = bch2_btree_iter_peek(&iter);
-                       goto again_2;
-               } else if (ret == -ENOENT)
-                       bch_err(c, "subvolume %llu points to nonexistent snapshot %u",
-                               k.k->p.offset, id);
-               else if (ret)
-                       break;
+               ret = bch2_snapshot_tree_lookup(trans, snapshot_root, &st);
+               if (ret)
+                       return ret;
+
+               if (fsck_err_on(le32_to_cpu(st.master_subvol) != subvol.k->p.offset, c,
+                               "subvolume %llu is not set as snapshot but is not master subvolume",
+                               k.k->p.offset)) {
+                       struct bkey_i_subvolume *s =
+                               bch2_bkey_make_mut_typed(trans, iter, subvol.s_c, 0, subvolume);
+                       ret = PTR_ERR_OR_ZERO(s);
+                       if (ret)
+                               return ret;
+
+                       SET_BCH_SUBVOLUME_SNAP(&s->v, true);
+               }
        }
-       bch2_trans_iter_exit(&trans, &iter);
-err:
+
+fsck_err:
+       return ret;
+}
+
+int bch2_fs_check_subvols(struct bch_fs *c)
+{
+       struct btree_trans trans;
+       struct btree_iter iter;
+       struct bkey_s_c k;
+       int ret;
+
+       bch2_trans_init(&trans, c, 0, 0);
+
+       ret = for_each_btree_key_commit(&trans, iter,
+                       BTREE_ID_subvolumes, POS_MIN, BTREE_ITER_PREFETCH, k,
+                       NULL, NULL, BTREE_INSERT_LAZY_RW|BTREE_INSERT_NOFAIL,
+               check_subvol(&trans, &iter, k));
+
        bch2_trans_exit(&trans);
+
        return ret;
 }
 
@@ -307,49 +700,19 @@ int bch2_fs_snapshots_start(struct bch_fs *c)
        struct btree_trans trans;
        struct btree_iter iter;
        struct bkey_s_c k;
-       bool have_deleted = false;
        int ret = 0;
 
        bch2_trans_init(&trans, c, 0, 0);
 
-       for_each_btree_key(&trans, iter, BTREE_ID_snapshots,
-                          POS_MIN, 0, k, ret) {
-              if (bkey_cmp(k.k->p, POS(0, U32_MAX)) > 0)
-                      break;
-
-               if (k.k->type != KEY_TYPE_snapshot) {
-                       bch_err(c, "found wrong key type %u in snapshot node table",
-                               k.k->type);
-                       continue;
-               }
-
-               if (BCH_SNAPSHOT_DELETED(bkey_s_c_to_snapshot(k).v))
-                       have_deleted = true;
-
-               ret = bch2_mark_snapshot(&trans, bkey_s_c_null, k, 0);
-               if (ret)
-                       break;
-       }
-       bch2_trans_iter_exit(&trans, &iter);
-
-       if (ret)
-               goto err;
+       for_each_btree_key2(&trans, iter, BTREE_ID_snapshots,
+                          POS_MIN, 0, k,
+               bch2_mark_snapshot(&trans, BTREE_ID_snapshots, 0, bkey_s_c_null, k, 0) ?:
+               bch2_snapshot_set_equiv(&trans, k));
 
-       ret = bch2_snapshots_set_equiv(&trans);
-       if (ret)
-               goto err;
-err:
        bch2_trans_exit(&trans);
 
-       if (!ret && have_deleted) {
-               bch_info(c, "restarting deletion of dead snapshots");
-               if (c->opts.fsck) {
-                       bch2_delete_dead_snapshots_work(&c->snapshot_delete_work);
-               } else {
-                       bch2_delete_dead_snapshots(c);
-               }
-       }
-
+       if (ret)
+               bch_err(c, "error starting snapshots: %s", bch2_err_str(ret));
        return ret;
 }
 
@@ -359,38 +722,25 @@ err:
 static int bch2_snapshot_node_set_deleted(struct btree_trans *trans, u32 id)
 {
        struct btree_iter iter;
-       struct bkey_s_c k;
        struct bkey_i_snapshot *s;
        int ret = 0;
 
-       bch2_trans_iter_init(trans, &iter, BTREE_ID_snapshots, POS(0, id),
-                            BTREE_ITER_INTENT);
-       k = bch2_btree_iter_peek_slot(&iter);
-       ret = bkey_err(k);
-       if (ret)
-               goto err;
-
-       if (k.k->type != KEY_TYPE_snapshot) {
-               bch2_fs_inconsistent(trans->c, "missing snapshot %u", id);
-               ret = -ENOENT;
-               goto err;
+       s = bch2_bkey_get_mut_typed(trans, &iter,
+                                   BTREE_ID_snapshots, POS(0, id),
+                                   0, snapshot);
+       ret = PTR_ERR_OR_ZERO(s);
+       if (unlikely(ret)) {
+               bch2_fs_inconsistent_on(ret == -ENOENT, trans->c, "missing snapshot %u", id);
+               return ret;
        }
 
        /* already deleted? */
-       if (BCH_SNAPSHOT_DELETED(bkey_s_c_to_snapshot(k).v))
+       if (BCH_SNAPSHOT_DELETED(&s->v))
                goto err;
 
-       s = bch2_trans_kmalloc(trans, sizeof(*s));
-       ret = PTR_ERR_OR_ZERO(s);
-       if (ret)
-               goto err;
-
-       bkey_reassemble(&s->k_i, k);
-
        SET_BCH_SNAPSHOT_DELETED(&s->v, true);
-       ret = bch2_trans_update(trans, &iter, &s->k_i, 0);
-       if (ret)
-               goto err;
+       SET_BCH_SNAPSHOT_SUBVOL(&s->v, false);
+       s->v.subvol = 0;
 err:
        bch2_trans_iter_exit(trans, &iter);
        return ret;
@@ -398,60 +748,43 @@ err:
 
 static int bch2_snapshot_node_delete(struct btree_trans *trans, u32 id)
 {
+       struct bch_fs *c = trans->c;
        struct btree_iter iter, p_iter = (struct btree_iter) { NULL };
-       struct bkey_s_c k;
+       struct btree_iter tree_iter = (struct btree_iter) { NULL };
        struct bkey_s_c_snapshot s;
-       struct bkey_i_snapshot *parent;
        u32 parent_id;
        unsigned i;
        int ret = 0;
 
-       bch2_trans_iter_init(trans, &iter, BTREE_ID_snapshots, POS(0, id),
-                            BTREE_ITER_INTENT);
-       k = bch2_btree_iter_peek_slot(&iter);
-       ret = bkey_err(k);
-       if (ret)
-               goto err;
+       s = bch2_bkey_get_iter_typed(trans, &iter, BTREE_ID_snapshots, POS(0, id),
+                                    BTREE_ITER_INTENT, snapshot);
+       ret = bkey_err(s);
+       bch2_fs_inconsistent_on(ret == -ENOENT, c, "missing snapshot %u", id);
 
-       if (k.k->type != KEY_TYPE_snapshot) {
-               bch2_fs_inconsistent(trans->c, "missing snapshot %u", id);
-               ret = -ENOENT;
+       if (ret)
                goto err;
-       }
-
-       s = bkey_s_c_to_snapshot(k);
 
        BUG_ON(!BCH_SNAPSHOT_DELETED(s.v));
        parent_id = le32_to_cpu(s.v->parent);
 
-       if (parent_id) {
-               bch2_trans_iter_init(trans, &p_iter, BTREE_ID_snapshots,
-                                    POS(0, parent_id),
-                                    BTREE_ITER_INTENT);
-               k = bch2_btree_iter_peek_slot(&p_iter);
-               ret = bkey_err(k);
-               if (ret)
-                       goto err;
-
-               if (k.k->type != KEY_TYPE_snapshot) {
-                       bch2_fs_inconsistent(trans->c, "missing snapshot %u", parent_id);
-                       ret = -ENOENT;
-                       goto err;
-               }
+       if (parent_id) {
+               struct bkey_i_snapshot *parent;
 
-               parent = bch2_trans_kmalloc(trans, sizeof(*parent));
+               parent = bch2_bkey_get_mut_typed(trans, &p_iter,
+                                    BTREE_ID_snapshots, POS(0, parent_id),
+                                    0, snapshot);
                ret = PTR_ERR_OR_ZERO(parent);
-               if (ret)
+               if (unlikely(ret)) {
+                       bch2_fs_inconsistent_on(ret == -ENOENT, c, "missing snapshot %u", parent_id);
                        goto err;
-
-               bkey_reassemble(&parent->k_i, k);
+               }
 
                for (i = 0; i < 2; i++)
                        if (le32_to_cpu(parent->v.children[i]) == id)
                                break;
 
                if (i == 2)
-                       bch_err(trans->c, "snapshot %u missing child pointer to %u",
+                       bch_err(c, "snapshot %u missing child pointer to %u",
                                parent_id, id);
                else
                        parent->v.children[i] = 0;
@@ -460,29 +793,49 @@ static int bch2_snapshot_node_delete(struct btree_trans *trans, u32 id)
                    le32_to_cpu(parent->v.children[1]))
                        swap(parent->v.children[0],
                             parent->v.children[1]);
+       } else {
+               /*
+                * We're deleting the root of a snapshot tree: update the
+                * snapshot_tree entry to point to the new root, or delete it if
+                * this is the last snapshot ID in this tree:
+                */
+               struct bkey_i_snapshot_tree *s_t;
+
+               BUG_ON(s.v->children[1]);
 
-               ret = bch2_trans_update(trans, &p_iter, &parent->k_i, 0);
+               s_t = bch2_bkey_get_mut_typed(trans, &tree_iter,
+                               BTREE_ID_snapshot_trees, POS(0, le32_to_cpu(s.v->tree)),
+                               0, snapshot_tree);
+               ret = PTR_ERR_OR_ZERO(s_t);
                if (ret)
                        goto err;
+
+               if (s.v->children[0]) {
+                       s_t->v.root_snapshot = cpu_to_le32(s.v->children[0]);
+               } else {
+                       s_t->k.type = KEY_TYPE_deleted;
+                       set_bkey_val_u64s(&s_t->k, 0);
+               }
        }
 
        ret = bch2_btree_delete_at(trans, &iter, 0);
 err:
+       bch2_trans_iter_exit(trans, &tree_iter);
        bch2_trans_iter_exit(trans, &p_iter);
        bch2_trans_iter_exit(trans, &iter);
        return ret;
 }
 
-int bch2_snapshot_node_create(struct btree_trans *trans, u32 parent,
-                             u32 *new_snapids,
-                             u32 *snapshot_subvols,
-                             unsigned nr_snapids)
+static int create_snapids(struct btree_trans *trans, u32 parent, u32 tree,
+                         u32 *new_snapids,
+                         u32 *snapshot_subvols,
+                         unsigned nr_snapids)
 {
        struct btree_iter iter;
        struct bkey_i_snapshot *n;
        struct bkey_s_c k;
        unsigned i;
-       int ret = 0;
+       int ret;
 
        bch2_trans_iter_init(trans, &iter, BTREE_ID_snapshots,
                             POS_MIN, BTREE_ITER_INTENT);
@@ -498,189 +851,208 @@ int bch2_snapshot_node_create(struct btree_trans *trans, u32 parent,
                        goto err;
 
                if (!k.k || !k.k->p.offset) {
-                       ret = -ENOSPC;
+                       ret = -BCH_ERR_ENOSPC_snapshot_create;
                        goto err;
                }
 
-               n = bch2_trans_kmalloc(trans, sizeof(*n));
+               n = bch2_bkey_alloc(trans, &iter, 0, snapshot);
                ret = PTR_ERR_OR_ZERO(n);
                if (ret)
                        goto err;
 
-               bkey_snapshot_init(&n->k_i);
-               n->k.p          = iter.pos;
                n->v.flags      = 0;
                n->v.parent     = cpu_to_le32(parent);
                n->v.subvol     = cpu_to_le32(snapshot_subvols[i]);
-               n->v.pad        = 0;
+               n->v.tree       = cpu_to_le32(tree);
                SET_BCH_SNAPSHOT_SUBVOL(&n->v, true);
 
-               ret   = bch2_trans_update(trans, &iter, &n->k_i, 0) ?:
-                       bch2_mark_snapshot(trans, bkey_s_c_null, bkey_i_to_s_c(&n->k_i), 0);
+               ret = bch2_mark_snapshot(trans, BTREE_ID_snapshots, 0,
+                                        bkey_s_c_null, bkey_i_to_s_c(&n->k_i), 0);
                if (ret)
                        goto err;
 
                new_snapids[i]  = iter.pos.offset;
        }
+err:
+       bch2_trans_iter_exit(trans, &iter);
+       return ret;
+}
 
-       if (parent) {
-               bch2_btree_iter_set_pos(&iter, POS(0, parent));
-               k = bch2_btree_iter_peek(&iter);
-               ret = bkey_err(k);
-               if (ret)
-                       goto err;
+/*
+ * Create new snapshot IDs as children of an existing snapshot ID:
+ */
+static int bch2_snapshot_node_create_children(struct btree_trans *trans, u32 parent,
+                             u32 *new_snapids,
+                             u32 *snapshot_subvols,
+                             unsigned nr_snapids)
+{
+       struct btree_iter iter;
+       struct bkey_i_snapshot *n_parent;
+       int ret = 0;
 
-               if (k.k->type != KEY_TYPE_snapshot) {
+       n_parent = bch2_bkey_get_mut_typed(trans, &iter,
+                       BTREE_ID_snapshots, POS(0, parent),
+                       0, snapshot);
+       ret = PTR_ERR_OR_ZERO(n_parent);
+       if (unlikely(ret)) {
+               if (ret == -ENOENT)
                        bch_err(trans->c, "snapshot %u not found", parent);
-                       ret = -ENOENT;
-                       goto err;
-               }
-
-               n = bch2_trans_kmalloc(trans, sizeof(*n));
-               ret = PTR_ERR_OR_ZERO(n);
-               if (ret)
-                       goto err;
+               return ret;
+       }
 
-               bkey_reassemble(&n->k_i, k);
+       if (n_parent->v.children[0] || n_parent->v.children[1]) {
+               bch_err(trans->c, "Trying to add child snapshot nodes to parent that already has children");
+               ret = -EINVAL;
+               goto err;
+       }
 
-               if (n->v.children[0] || n->v.children[1]) {
-                       bch_err(trans->c, "Trying to add child snapshot nodes to parent that already has children");
-                       ret = -EINVAL;
-                       goto err;
-               }
+       ret = create_snapids(trans, parent, le32_to_cpu(n_parent->v.tree),
+                            new_snapids, snapshot_subvols, nr_snapids);
+       if (ret)
+               goto err;
 
-               n->v.children[0] = cpu_to_le32(new_snapids[0]);
-               n->v.children[1] = cpu_to_le32(new_snapids[1]);
-               SET_BCH_SNAPSHOT_SUBVOL(&n->v, false);
-               ret = bch2_trans_update(trans, &iter, &n->k_i, 0);
-               if (ret)
-                       goto err;
-       }
+       n_parent->v.children[0] = cpu_to_le32(new_snapids[0]);
+       n_parent->v.children[1] = cpu_to_le32(new_snapids[1]);
+       n_parent->v.subvol = 0;
+       SET_BCH_SNAPSHOT_SUBVOL(&n_parent->v, false);
 err:
        bch2_trans_iter_exit(trans, &iter);
        return ret;
 }
 
-static int snapshot_id_add(snapshot_id_list *s, u32 id)
+/*
+ * Create a snapshot node that is the root of a new tree:
+ */
+static int bch2_snapshot_node_create_tree(struct btree_trans *trans,
+                             u32 *new_snapids,
+                             u32 *snapshot_subvols,
+                             unsigned nr_snapids)
+{
+       struct bkey_i_snapshot_tree *n_tree;
+       int ret;
+
+       n_tree = __snapshot_tree_create(trans);
+       ret =   PTR_ERR_OR_ZERO(n_tree) ?:
+               create_snapids(trans, 0, n_tree->k.p.offset,
+                            new_snapids, snapshot_subvols, nr_snapids);
+       if (ret)
+               return ret;
+
+       n_tree->v.master_subvol = cpu_to_le32(snapshot_subvols[0]);
+       n_tree->v.root_snapshot = cpu_to_le32(new_snapids[0]);
+       return 0;
+}
+
+int bch2_snapshot_node_create(struct btree_trans *trans, u32 parent,
+                             u32 *new_snapids,
+                             u32 *snapshot_subvols,
+                             unsigned nr_snapids)
 {
-       BUG_ON(snapshot_list_has_id(s, id));
+       BUG_ON((parent == 0) != (nr_snapids == 1));
+       BUG_ON((parent != 0) != (nr_snapids == 2));
+
+       return parent
+               ? bch2_snapshot_node_create_children(trans, parent,
+                               new_snapids, snapshot_subvols, nr_snapids)
+               : bch2_snapshot_node_create_tree(trans,
+                               new_snapids, snapshot_subvols, nr_snapids);
 
-       return darray_push(s, id);
 }
 
-static int bch2_snapshot_delete_keys_btree(struct btree_trans *trans,
-                                          snapshot_id_list *deleted,
-                                          enum btree_id btree_id)
+static int snapshot_delete_key(struct btree_trans *trans,
+                              struct btree_iter *iter,
+                              struct bkey_s_c k,
+                              snapshot_id_list *deleted,
+                              snapshot_id_list *equiv_seen,
+                              struct bpos *last_pos)
 {
        struct bch_fs *c = trans->c;
-       struct btree_iter iter;
-       struct bkey_s_c k;
-       snapshot_id_list equiv_seen = { 0 };
-       struct bpos last_pos = POS_MIN;
-       int ret = 0;
-
-       /*
-        * XXX: We should also delete whiteouts that no longer overwrite
-        * anything
-        */
+       u32 equiv = snapshot_t(c, k.k->p.snapshot)->equiv;
 
-       bch2_trans_iter_init(trans, &iter, btree_id, POS_MIN,
-                            BTREE_ITER_INTENT|
-                            BTREE_ITER_PREFETCH|
-                            BTREE_ITER_NOT_EXTENTS|
-                            BTREE_ITER_ALL_SNAPSHOTS);
-
-       while ((bch2_trans_begin(trans),
-               (k = bch2_btree_iter_peek(&iter)).k) &&
-              !(ret = bkey_err(k))) {
-               u32 equiv = snapshot_t(c, k.k->p.snapshot)->equiv;
-
-               if (bkey_cmp(k.k->p, last_pos))
-                       equiv_seen.nr = 0;
-               last_pos = k.k->p;
-
-               if (snapshot_list_has_id(deleted, k.k->p.snapshot) ||
-                   snapshot_list_has_id(&equiv_seen, equiv)) {
-                       if (btree_id == BTREE_ID_inodes &&
-                           bch2_btree_key_cache_flush(trans, btree_id, iter.pos))
-                               continue;
-
-                       ret = __bch2_trans_do(trans, NULL, NULL,
-                                             BTREE_INSERT_NOFAIL,
-                               bch2_btree_iter_traverse(&iter) ?:
-                               bch2_btree_delete_at(trans, &iter,
-                                       BTREE_UPDATE_INTERNAL_SNAPSHOT_NODE));
-                       if (ret)
-                               break;
-               } else {
-                       ret = snapshot_id_add(&equiv_seen, equiv);
-                       if (ret)
-                               break;
-               }
+       if (!bkey_eq(k.k->p, *last_pos))
+               equiv_seen->nr = 0;
+       *last_pos = k.k->p;
 
-               bch2_btree_iter_advance(&iter);
+       if (snapshot_list_has_id(deleted, k.k->p.snapshot) ||
+           snapshot_list_has_id(equiv_seen, equiv)) {
+               return bch2_btree_delete_at(trans, iter,
+                                           BTREE_UPDATE_INTERNAL_SNAPSHOT_NODE);
+       } else {
+               return snapshot_list_add(c, equiv_seen, equiv);
        }
-       bch2_trans_iter_exit(trans, &iter);
+}
+
+static int bch2_delete_redundant_snapshot(struct btree_trans *trans, struct btree_iter *iter,
+                                         struct bkey_s_c k)
+{
+       struct bkey_s_c_snapshot snap;
+       u32 children[2];
+       int ret;
+
+       if (k.k->type != KEY_TYPE_snapshot)
+               return 0;
+
+       snap = bkey_s_c_to_snapshot(k);
+       if (BCH_SNAPSHOT_DELETED(snap.v) ||
+           BCH_SNAPSHOT_SUBVOL(snap.v))
+               return 0;
 
-       darray_exit(&equiv_seen);
+       children[0] = le32_to_cpu(snap.v->children[0]);
+       children[1] = le32_to_cpu(snap.v->children[1]);
 
-       return ret;
+       ret   = snapshot_live(trans, children[0]) ?:
+               snapshot_live(trans, children[1]);
+       if (ret < 0)
+               return ret;
+
+       if (!ret)
+               return bch2_snapshot_node_set_deleted(trans, k.k->p.offset);
+       return 0;
 }
 
-static void bch2_delete_dead_snapshots_work(struct work_struct *work)
+int bch2_delete_dead_snapshots(struct bch_fs *c)
 {
-       struct bch_fs *c = container_of(work, struct bch_fs, snapshot_delete_work);
        struct btree_trans trans;
        struct btree_iter iter;
        struct bkey_s_c k;
        struct bkey_s_c_snapshot snap;
        snapshot_id_list deleted = { 0 };
-       u32 i, id, children[2];
+       u32 i, id;
        int ret = 0;
 
+       if (!test_bit(BCH_FS_HAVE_DELETED_SNAPSHOTS, &c->flags))
+               return 0;
+
+       if (!test_bit(BCH_FS_STARTED, &c->flags)) {
+               ret = bch2_fs_read_write_early(c);
+               if (ret) {
+                       bch_err(c, "error deleleting dead snapshots: error going rw: %s", bch2_err_str(ret));
+                       return ret;
+               }
+       }
+
        bch2_trans_init(&trans, c, 0, 0);
 
        /*
         * For every snapshot node: If we have no live children and it's not
         * pointed to by a subvolume, delete it:
         */
-       for_each_btree_key(&trans, iter, BTREE_ID_snapshots,
-                          POS_MIN, 0, k, ret) {
-               if (k.k->type != KEY_TYPE_snapshot)
-                       continue;
-
-               snap = bkey_s_c_to_snapshot(k);
-               if (BCH_SNAPSHOT_DELETED(snap.v) ||
-                   BCH_SNAPSHOT_SUBVOL(snap.v))
-                       continue;
-
-               children[0] = le32_to_cpu(snap.v->children[0]);
-               children[1] = le32_to_cpu(snap.v->children[1]);
-
-               ret   = snapshot_live(&trans, children[0]) ?:
-                       snapshot_live(&trans, children[1]);
-               if (ret < 0)
-                       break;
-               if (ret)
-                       continue;
-
-               ret = __bch2_trans_do(&trans, NULL, NULL, 0,
-                       bch2_snapshot_node_set_deleted(&trans, iter.pos.offset));
-               if (ret) {
-                       bch_err(c, "error deleting snapshot %llu: %i", iter.pos.offset, ret);
-                       break;
-               }
-       }
-       bch2_trans_iter_exit(&trans, &iter);
-
+       ret = for_each_btree_key_commit(&trans, iter, BTREE_ID_snapshots,
+                       POS_MIN, 0, k,
+                       NULL, NULL, 0,
+               bch2_delete_redundant_snapshot(&trans, &iter, k));
        if (ret) {
-               bch_err(c, "error walking snapshots: %i", ret);
+               bch_err(c, "error deleting redundant snapshots: %s", bch2_err_str(ret));
                goto err;
        }
 
-       ret = bch2_snapshots_set_equiv(&trans);
-       if (ret)
+       for_each_btree_key2(&trans, iter, BTREE_ID_snapshots,
+                          POS_MIN, 0, k,
+               bch2_snapshot_set_equiv(&trans, k));
+       if (ret) {
+               bch_err(c, "error in bch2_snapshots_set_equiv: %s", bch2_err_str(ret));
                goto err;
+       }
 
        for_each_btree_key(&trans, iter, BTREE_ID_snapshots,
                           POS_MIN, 0, k, ret) {
@@ -689,7 +1061,7 @@ static void bch2_delete_dead_snapshots_work(struct work_struct *work)
 
                snap = bkey_s_c_to_snapshot(k);
                if (BCH_SNAPSHOT_DELETED(snap.v)) {
-                       ret = snapshot_id_add(&deleted, k.k->p.offset);
+                       ret = snapshot_list_add(c, &deleted, k.k->p.offset);
                        if (ret)
                                break;
                }
@@ -697,67 +1069,86 @@ static void bch2_delete_dead_snapshots_work(struct work_struct *work)
        bch2_trans_iter_exit(&trans, &iter);
 
        if (ret) {
-               bch_err(c, "error walking snapshots: %i", ret);
+               bch_err(c, "error walking snapshots: %s", bch2_err_str(ret));
                goto err;
        }
 
        for (id = 0; id < BTREE_ID_NR; id++) {
+               struct bpos last_pos = POS_MIN;
+               snapshot_id_list equiv_seen = { 0 };
+
                if (!btree_type_has_snapshots(id))
                        continue;
 
-               ret = bch2_snapshot_delete_keys_btree(&trans, &deleted, id);
+               ret = for_each_btree_key_commit(&trans, iter,
+                               id, POS_MIN,
+                               BTREE_ITER_PREFETCH|BTREE_ITER_ALL_SNAPSHOTS, k,
+                               NULL, NULL, BTREE_INSERT_NOFAIL,
+                       snapshot_delete_key(&trans, &iter, k, &deleted, &equiv_seen, &last_pos));
+
+               darray_exit(&equiv_seen);
+
                if (ret) {
-                       bch_err(c, "error deleting snapshot keys: %i", ret);
+                       bch_err(c, "error deleting snapshot keys: %s", bch2_err_str(ret));
                        goto err;
                }
        }
 
        for (i = 0; i < deleted.nr; i++) {
-               ret = __bch2_trans_do(&trans, NULL, NULL, 0,
+               ret = commit_do(&trans, NULL, NULL, 0,
                        bch2_snapshot_node_delete(&trans, deleted.data[i]));
                if (ret) {
-                       bch_err(c, "error deleting snapshot %u: %i",
-                               deleted.data[i], ret);
+                       bch_err(c, "error deleting snapshot %u: %s",
+                               deleted.data[i], bch2_err_str(ret));
                        goto err;
                }
        }
+
+       clear_bit(BCH_FS_HAVE_DELETED_SNAPSHOTS, &c->flags);
 err:
        darray_exit(&deleted);
        bch2_trans_exit(&trans);
-       percpu_ref_put(&c->writes);
+       return ret;
 }
 
-static void bch2_delete_dead_snapshots(struct bch_fs *c)
+static void bch2_delete_dead_snapshots_work(struct work_struct *work)
 {
-       if (unlikely(!percpu_ref_tryget_live(&c->writes)))
-               return;
+       struct bch_fs *c = container_of(work, struct bch_fs, snapshot_delete_work);
 
-       if (!queue_work(system_long_wq, &c->snapshot_delete_work))
-               percpu_ref_put(&c->writes);
+       bch2_delete_dead_snapshots(c);
+       bch2_write_ref_put(c, BCH_WRITE_REF_delete_dead_snapshots);
+}
+
+void bch2_delete_dead_snapshots_async(struct bch_fs *c)
+{
+       if (bch2_write_ref_tryget(c, BCH_WRITE_REF_delete_dead_snapshots) &&
+           !queue_work(c->write_ref_wq, &c->snapshot_delete_work))
+               bch2_write_ref_put(c, BCH_WRITE_REF_delete_dead_snapshots);
 }
 
 static int bch2_delete_dead_snapshots_hook(struct btree_trans *trans,
                                           struct btree_trans_commit_hook *h)
 {
-       bch2_delete_dead_snapshots(trans->c);
+       struct bch_fs *c = trans->c;
+
+       set_bit(BCH_FS_HAVE_DELETED_SNAPSHOTS, &c->flags);
+
+       if (!test_bit(BCH_FS_FSCK_DONE, &c->flags))
+               return 0;
+
+       bch2_delete_dead_snapshots_async(c);
        return 0;
 }
 
 /* Subvolumes: */
 
 int bch2_subvolume_invalid(const struct bch_fs *c, struct bkey_s_c k,
-                          int rw, struct printbuf *err)
+                          unsigned flags, struct printbuf *err)
 {
-       if (bkey_cmp(k.k->p, SUBVOL_POS_MIN) < 0 ||
-           bkey_cmp(k.k->p, SUBVOL_POS_MAX) > 0) {
+       if (bkey_lt(k.k->p, SUBVOL_POS_MIN) ||
+           bkey_gt(k.k->p, SUBVOL_POS_MAX)) {
                prt_printf(err, "invalid pos");
-               return -EINVAL;
-       }
-
-       if (bkey_val_bytes(k.k) != sizeof(struct bch_subvolume)) {
-               prt_printf(err, "incorrect value size (%zu != %zu)",
-                      bkey_val_bytes(k.k), sizeof(struct bch_subvolume));
-               return -EINVAL;
+               return -BCH_ERR_invalid_bkey;
        }
 
        return 0;
@@ -769,8 +1160,24 @@ void bch2_subvolume_to_text(struct printbuf *out, struct bch_fs *c,
        struct bkey_s_c_subvolume s = bkey_s_c_to_subvolume(k);
 
        prt_printf(out, "root %llu snapshot id %u",
-              le64_to_cpu(s.v->inode),
-              le32_to_cpu(s.v->snapshot));
+                  le64_to_cpu(s.v->inode),
+                  le32_to_cpu(s.v->snapshot));
+
+       if (bkey_val_bytes(s.k) > offsetof(struct bch_subvolume, parent))
+               prt_printf(out, " parent %u", le32_to_cpu(s.v->parent));
+}
+
+static __always_inline int
+bch2_subvolume_get_inlined(struct btree_trans *trans, unsigned subvol,
+                          bool inconsistent_if_not_found,
+                          int iter_flags,
+                          struct bch_subvolume *s)
+{
+       int ret = bch2_bkey_get_val_typed(trans, BTREE_ID_subvolumes, POS(0, subvol),
+                                         iter_flags, subvolume, s);
+       bch2_fs_inconsistent_on(ret == -ENOENT && inconsistent_if_not_found,
+                               trans->c, "missing subvolume %u", subvol);
+       return ret;
 }
 
 int bch2_subvolume_get(struct btree_trans *trans, unsigned subvol,
@@ -778,22 +1185,7 @@ int bch2_subvolume_get(struct btree_trans *trans, unsigned subvol,
                       int iter_flags,
                       struct bch_subvolume *s)
 {
-       struct btree_iter iter;
-       struct bkey_s_c k;
-       int ret;
-
-       bch2_trans_iter_init(trans, &iter, BTREE_ID_subvolumes, POS(0, subvol),
-                            iter_flags);
-       k = bch2_btree_iter_peek_slot(&iter);
-       ret = bkey_err(k) ?: k.k->type == KEY_TYPE_subvolume ? 0 : -ENOENT;
-
-       if (ret == -ENOENT && inconsistent_if_not_found)
-               bch2_fs_inconsistent(trans->c, "missing subvolume %u", subvol);
-       if (!ret)
-               *s = *bkey_s_c_to_subvolume(k).v;
-
-       bch2_trans_iter_exit(trans, &iter);
-       return ret;
+       return bch2_subvolume_get_inlined(trans, subvol, inconsistent_if_not_found, iter_flags, s);
 }
 
 int bch2_snapshot_get_subvol(struct btree_trans *trans, u32 snapshot,
@@ -808,63 +1200,97 @@ int bch2_snapshot_get_subvol(struct btree_trans *trans, u32 snapshot,
 int bch2_subvolume_get_snapshot(struct btree_trans *trans, u32 subvol,
                                u32 *snapid)
 {
-       struct bch_subvolume s;
+       struct btree_iter iter;
+       struct bkey_s_c k;
        int ret;
 
-       ret = bch2_subvolume_get(trans, subvol, true,
-                                BTREE_ITER_CACHED|
-                                BTREE_ITER_WITH_UPDATES,
-                                &s);
+       k = bch2_bkey_get_iter(trans, &iter, BTREE_ID_subvolumes, POS(0, subvol),
+                              BTREE_ITER_CACHED|
+                              BTREE_ITER_WITH_UPDATES);
+       ret = bkey_err(k) ?: k.k->type == KEY_TYPE_subvolume ? 0 : -ENOENT;
 
-       *snapid = le32_to_cpu(s.snapshot);
+       if (likely(!ret))
+               *snapid = le32_to_cpu(bkey_s_c_to_subvolume(k).v->snapshot);
+       else if (ret == -ENOENT)
+               bch2_fs_inconsistent(trans->c, "missing subvolume %u", subvol);
+       bch2_trans_iter_exit(trans, &iter);
        return ret;
 }
 
+static int bch2_subvolume_reparent(struct btree_trans *trans,
+                                  struct btree_iter *iter,
+                                  struct bkey_s_c k,
+                                  u32 old_parent, u32 new_parent)
+{
+       struct bkey_i_subvolume *s;
+       int ret;
+
+       if (k.k->type != KEY_TYPE_subvolume)
+               return 0;
+
+       if (bkey_val_bytes(k.k) > offsetof(struct bch_subvolume, parent) &&
+           le32_to_cpu(bkey_s_c_to_subvolume(k).v->parent) != old_parent)
+               return 0;
+
+       s = bch2_bkey_make_mut_typed(trans, iter, k, 0, subvolume);
+       ret = PTR_ERR_OR_ZERO(s);
+       if (ret)
+               return ret;
+
+       s->v.parent = cpu_to_le32(new_parent);
+       return 0;
+}
+
+/*
+ * Scan for subvolumes with parent @subvolid_to_delete, reparent:
+ */
+static int bch2_subvolumes_reparent(struct btree_trans *trans, u32 subvolid_to_delete)
+{
+       struct btree_iter iter;
+       struct bkey_s_c k;
+       struct bch_subvolume s;
+
+       return lockrestart_do(trans,
+                       bch2_subvolume_get(trans, subvolid_to_delete, true,
+                                  BTREE_ITER_CACHED, &s)) ?:
+               for_each_btree_key_commit(trans, iter,
+                               BTREE_ID_subvolumes, POS_MIN, BTREE_ITER_PREFETCH, k,
+                               NULL, NULL, BTREE_INSERT_NOFAIL,
+                       bch2_subvolume_reparent(trans, &iter, k,
+                                       subvolid_to_delete, le32_to_cpu(s.parent)));
+}
+
 /*
  * Delete subvolume, mark snapshot ID as deleted, queue up snapshot
  * deletion/cleanup:
  */
-int bch2_subvolume_delete(struct btree_trans *trans, u32 subvolid)
+static int __bch2_subvolume_delete(struct btree_trans *trans, u32 subvolid)
 {
        struct btree_iter iter;
-       struct bkey_s_c k;
        struct bkey_s_c_subvolume subvol;
        struct btree_trans_commit_hook *h;
-       struct bkey_i *delete;
        u32 snapid;
        int ret = 0;
 
-       bch2_trans_iter_init(trans, &iter, BTREE_ID_subvolumes,
-                            POS(0, subvolid),
-                            BTREE_ITER_CACHED|
-                            BTREE_ITER_INTENT);
-       k = bch2_btree_iter_peek_slot(&iter);
-       ret = bkey_err(k);
+       subvol = bch2_bkey_get_iter_typed(trans, &iter,
+                               BTREE_ID_subvolumes, POS(0, subvolid),
+                               BTREE_ITER_CACHED|BTREE_ITER_INTENT,
+                               subvolume);
+       ret = bkey_err(subvol);
+       bch2_fs_inconsistent_on(ret == -ENOENT, trans->c, "missing subvolume %u", subvolid);
        if (ret)
-               goto err;
-
-       if (k.k->type != KEY_TYPE_subvolume) {
-               bch2_fs_inconsistent(trans->c, "missing subvolume %u", subvolid);
-               ret = -EIO;
-               goto err;
-       }
+               return ret;
 
-       subvol = bkey_s_c_to_subvolume(k);
        snapid = le32_to_cpu(subvol.v->snapshot);
 
-       delete = bch2_trans_kmalloc(trans, sizeof(*delete));
-       ret = PTR_ERR_OR_ZERO(delete);
+       ret = bch2_btree_delete_at(trans, &iter, 0);
        if (ret)
                goto err;
 
-       bkey_init(&delete->k);
-       delete->k.p = iter.pos;
-       ret = bch2_trans_update(trans, &iter, delete, 0);
+       ret = bch2_snapshot_node_set_deleted(trans, snapid);
        if (ret)
                goto err;
 
-       ret = bch2_snapshot_node_set_deleted(trans, snapid);
-
        h = bch2_trans_kmalloc(trans, sizeof(*h));
        ret = PTR_ERR_OR_ZERO(h);
        if (ret)
@@ -877,6 +1303,13 @@ err:
        return ret;
 }
 
+static int bch2_subvolume_delete(struct btree_trans *trans, u32 subvolid)
+{
+       return bch2_subvolumes_reparent(trans, subvolid) ?:
+               commit_do(trans, NULL, NULL, BTREE_INSERT_NOFAIL,
+                         __bch2_subvolume_delete(trans, subvolid));
+}
+
 void bch2_subvolume_wait_for_pagecache_and_delete(struct work_struct *work)
 {
        struct bch_fs *c = container_of(work, struct bch_fs,
@@ -897,10 +1330,9 @@ void bch2_subvolume_wait_for_pagecache_and_delete(struct work_struct *work)
                bch2_evict_subvolume_inodes(c, &s);
 
                for (id = s.data; id < s.data + s.nr; id++) {
-                       ret = bch2_trans_do(c, NULL, NULL, BTREE_INSERT_NOFAIL,
-                                     bch2_subvolume_delete(&trans, *id));
+                       ret = bch2_trans_run(c, bch2_subvolume_delete(&trans, *id));
                        if (ret) {
-                               bch_err(c, "error %i deleting subvolume %u", ret, *id);
+                               bch_err(c, "error deleting subvolume %u: %s", *id, bch2_err_str(ret));
                                break;
                        }
                }
@@ -908,7 +1340,7 @@ void bch2_subvolume_wait_for_pagecache_and_delete(struct work_struct *work)
                darray_exit(&s);
        }
 
-       percpu_ref_put(&c->writes);
+       bch2_write_ref_put(c, BCH_WRITE_REF_snapshot_delete_pagecache);
 }
 
 struct subvolume_unlink_hook {
@@ -925,64 +1357,46 @@ int bch2_subvolume_wait_for_pagecache_and_delete_hook(struct btree_trans *trans,
 
        mutex_lock(&c->snapshots_unlinked_lock);
        if (!snapshot_list_has_id(&c->snapshots_unlinked, h->subvol))
-               ret = snapshot_id_add(&c->snapshots_unlinked, h->subvol);
+               ret = snapshot_list_add(c, &c->snapshots_unlinked, h->subvol);
        mutex_unlock(&c->snapshots_unlinked_lock);
 
        if (ret)
                return ret;
 
-       if (unlikely(!percpu_ref_tryget_live(&c->writes)))
+       if (!bch2_write_ref_tryget(c, BCH_WRITE_REF_snapshot_delete_pagecache))
                return -EROFS;
 
-       if (!queue_work(system_long_wq, &c->snapshot_wait_for_pagecache_and_delete_work))
-               percpu_ref_put(&c->writes);
+       if (!queue_work(c->write_ref_wq, &c->snapshot_wait_for_pagecache_and_delete_work))
+               bch2_write_ref_put(c, BCH_WRITE_REF_snapshot_delete_pagecache);
        return 0;
 }
 
 int bch2_subvolume_unlink(struct btree_trans *trans, u32 subvolid)
 {
        struct btree_iter iter;
-       struct bkey_s_c k;
        struct bkey_i_subvolume *n;
        struct subvolume_unlink_hook *h;
        int ret = 0;
 
-       bch2_trans_iter_init(trans, &iter, BTREE_ID_subvolumes,
-                            POS(0, subvolid),
-                            BTREE_ITER_CACHED|
-                            BTREE_ITER_INTENT);
-       k = bch2_btree_iter_peek_slot(&iter);
-       ret = bkey_err(k);
-       if (ret)
-               goto err;
-
-       if (k.k->type != KEY_TYPE_subvolume) {
-               bch2_fs_inconsistent(trans->c, "missing subvolume %u", subvolid);
-               ret = -EIO;
-               goto err;
-       }
-
-       n = bch2_trans_kmalloc(trans, sizeof(*n));
-       ret = PTR_ERR_OR_ZERO(n);
-       if (ret)
-               goto err;
-
-       bkey_reassemble(&n->k_i, k);
-       SET_BCH_SUBVOLUME_UNLINKED(&n->v, true);
-
-       ret = bch2_trans_update(trans, &iter, &n->k_i, 0);
-       if (ret)
-               goto err;
-
        h = bch2_trans_kmalloc(trans, sizeof(*h));
        ret = PTR_ERR_OR_ZERO(h);
        if (ret)
-               goto err;
+               return ret;
 
        h->h.fn         = bch2_subvolume_wait_for_pagecache_and_delete_hook;
        h->subvol       = subvolid;
        bch2_trans_commit_hook(trans, &h->h);
-err:
+
+       n = bch2_bkey_get_mut_typed(trans, &iter,
+                       BTREE_ID_subvolumes, POS(0, subvolid),
+                       BTREE_ITER_CACHED, subvolume);
+       ret = PTR_ERR_OR_ZERO(n);
+       if (unlikely(ret)) {
+               bch2_fs_inconsistent_on(ret == -ENOENT, trans->c, "missing subvolume %u", subvolid);
+               return ret;
+       }
+
+       SET_BCH_SUBVOLUME_UNLINKED(&n->v, true);
        bch2_trans_iter_exit(trans, &iter);
        return ret;
 }
@@ -997,54 +1411,32 @@ int bch2_subvolume_create(struct btree_trans *trans, u64 inode,
        struct btree_iter dst_iter, src_iter = (struct btree_iter) { NULL };
        struct bkey_i_subvolume *new_subvol = NULL;
        struct bkey_i_subvolume *src_subvol = NULL;
-       struct bkey_s_c k;
        u32 parent = 0, new_nodes[2], snapshot_subvols[2];
        int ret = 0;
 
-       for_each_btree_key(trans, dst_iter, BTREE_ID_subvolumes, SUBVOL_POS_MIN,
-                          BTREE_ITER_SLOTS|BTREE_ITER_INTENT, k, ret) {
-               if (bkey_cmp(k.k->p, SUBVOL_POS_MAX) > 0)
-                       break;
-
-               /*
-                * bch2_subvolume_delete() doesn't flush the btree key cache -
-                * ideally it would but that's tricky
-                */
-               if (bkey_deleted(k.k) &&
-                   !bch2_btree_key_cache_find(c, BTREE_ID_subvolumes, dst_iter.pos))
-                       goto found_slot;
-       }
+       ret = bch2_bkey_get_empty_slot(trans, &dst_iter,
+                               BTREE_ID_subvolumes, POS(0, U32_MAX));
+       if (ret == -BCH_ERR_ENOSPC_btree_slot)
+               ret = -BCH_ERR_ENOSPC_subvolume_create;
+       if (ret)
+               return ret;
 
-       if (!ret)
-               ret = -ENOSPC;
-       goto err;
-found_slot:
        snapshot_subvols[0] = dst_iter.pos.offset;
        snapshot_subvols[1] = src_subvolid;
 
        if (src_subvolid) {
                /* Creating a snapshot: */
-               src_subvol = bch2_trans_kmalloc(trans, sizeof(*src_subvol));
-               ret = PTR_ERR_OR_ZERO(src_subvol);
-               if (ret)
-                       goto err;
 
-               bch2_trans_iter_init(trans, &src_iter, BTREE_ID_subvolumes,
-                                    POS(0, src_subvolid),
-                                    BTREE_ITER_CACHED|
-                                    BTREE_ITER_INTENT);
-               k = bch2_btree_iter_peek_slot(&src_iter);
-               ret = bkey_err(k);
-               if (ret)
-                       goto err;
-
-               if (k.k->type != KEY_TYPE_subvolume) {
-                       bch_err(c, "subvolume %u not found", src_subvolid);
-                       ret = -ENOENT;
+               src_subvol = bch2_bkey_get_mut_typed(trans, &src_iter,
+                               BTREE_ID_subvolumes, POS(0, src_subvolid),
+                               BTREE_ITER_CACHED, subvolume);
+               ret = PTR_ERR_OR_ZERO(src_subvol);
+               if (unlikely(ret)) {
+                       bch2_fs_inconsistent_on(ret == -ENOENT, c,
+                                               "subvolume %u not found", src_subvolid);
                        goto err;
                }
 
-               bkey_reassemble(&src_subvol->k_i, k);
                parent = le32_to_cpu(src_subvol->v.snapshot);
        }
 
@@ -1061,21 +1453,20 @@ found_slot:
                        goto err;
        }
 
-       new_subvol = bch2_trans_kmalloc(trans, sizeof(*new_subvol));
+       new_subvol = bch2_bkey_alloc(trans, &dst_iter, 0, subvolume);
        ret = PTR_ERR_OR_ZERO(new_subvol);
        if (ret)
                goto err;
 
-       bkey_subvolume_init(&new_subvol->k_i);
        new_subvol->v.flags     = 0;
        new_subvol->v.snapshot  = cpu_to_le32(new_nodes[0]);
        new_subvol->v.inode     = cpu_to_le64(inode);
+       new_subvol->v.parent    = cpu_to_le32(src_subvolid);
+       new_subvol->v.otime.lo  = cpu_to_le64(bch2_current_time(c));
+       new_subvol->v.otime.hi  = 0;
+
        SET_BCH_SUBVOLUME_RO(&new_subvol->v, ro);
        SET_BCH_SUBVOLUME_SNAP(&new_subvol->v, src_subvolid != 0);
-       new_subvol->k.p         = dst_iter.pos;
-       ret = bch2_trans_update(trans, &dst_iter, &new_subvol->k_i, 0);
-       if (ret)
-               goto err;
 
        *new_subvolid   = new_subvol->k.p.offset;
        *new_snapshotid = new_nodes[0];