]> git.sesse.net Git - bcachefs-tools-debian/blobdiff - libbcachefs/subvolume.c
Update bcachefs sources to 8dbfede1d9 fixup! bcachefs: More info on check_bucket_ref...
[bcachefs-tools-debian] / libbcachefs / subvolume.c
index 1a212bac2a04624709fde0d92aaa53a47af6b117..1805c8542d65381605a5506a5c82554102524587 100644 (file)
@@ -3,6 +3,7 @@
 #include "bcachefs.h"
 #include "btree_key_cache.h"
 #include "btree_update.h"
+#include "errcode.h"
 #include "error.h"
 #include "fs.h"
 #include "subvolume.h"
@@ -24,21 +25,21 @@ 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;
+               return -BCH_ERR_invalid_bkey;
        }
 
        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);
@@ -47,18 +48,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++) {
@@ -67,7 +68,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;
                }
        }
 
@@ -157,6 +158,7 @@ static int bch2_snapshot_set_equiv(struct btree_trans *trans, struct bkey_s_c k)
 
        for (i = 0; i < 2; i++) {
                int ret = snapshot_live(trans, child[i]);
+
                if (ret < 0)
                        return ret;
 
@@ -277,8 +279,8 @@ int bch2_fs_check_snapshots(struct bch_fs *c)
 
        bch2_trans_init(&trans, c, 0, 0);
 
-       ret = for_each_btree_key_commit(&trans, iter, BTREE_ID_snapshots,
-                       POS(BCACHEFS_ROOT_INO, 0),
+       ret = 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));
@@ -291,22 +293,14 @@ int bch2_fs_check_snapshots(struct bch_fs *c)
 }
 
 static int check_subvol(struct btree_trans *trans,
-                       struct btree_iter *iter)
+                       struct btree_iter *iter,
+                       struct bkey_s_c k)
 {
-       struct bkey_s_c k;
        struct bkey_s_c_subvolume subvol;
        struct bch_snapshot snapshot;
        unsigned snapid;
        int ret;
 
-       k = bch2_btree_iter_peek(iter);
-       if (!k.k)
-               return 0;
-
-       ret = bkey_err(k);
-       if (ret)
-               return ret;
-
        if (k.k->type != KEY_TYPE_subvolume)
                return 0;
 
@@ -322,9 +316,9 @@ static int check_subvol(struct btree_trans *trans,
 
        if (BCH_SUBVOLUME_UNLINKED(subvol.v)) {
                ret = bch2_subvolume_delete(trans, iter->pos.offset);
-               if (ret && ret != -EINTR)
-                       bch_err(trans->c, "error deleting subvolume %llu: %i",
-                               iter->pos.offset, ret);
+               if (ret && !bch2_err_matches(ret, BCH_ERR_transaction_restart))
+                       bch_err(trans->c, "error deleting subvolume %llu: %s",
+                               iter->pos.offset, bch2_err_str(ret));
                if (ret)
                        return ret;
        }
@@ -336,22 +330,15 @@ 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);
 
-       bch2_trans_iter_init(&trans, &iter, BTREE_ID_subvolumes,
-                            POS_MIN, BTREE_ITER_PREFETCH);
-
-       do {
-               ret = commit_do(&trans, NULL, NULL,
-                                     BTREE_INSERT_LAZY_RW|
-                                     BTREE_INSERT_NOFAIL,
-                                     check_subvol(&trans, &iter));
-               if (ret)
-                       break;
-       } while (bch2_btree_iter_advance(&iter));
-       bch2_trans_iter_exit(&trans, &iter);
+       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);
 
@@ -380,7 +367,7 @@ int bch2_fs_snapshots_start(struct bch_fs *c)
        bch2_trans_exit(&trans);
 
        if (ret)
-               bch_err(c, "error starting snapshots: %i", ret);
+               bch_err(c, "error starting snapshots: %s", bch2_err_str(ret));
        return ret;
 }
 
@@ -390,33 +377,22 @@ int bch2_fs_snapshots_start(struct bch_fs *c)
 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;
+       s = bch2_bkey_get_mut_typed(trans, &iter, snapshot);
+       ret = PTR_ERR_OR_ZERO(s);
+       if (unlikely(ret)) {
+               bch2_fs_inconsistent_on(ret == -ENOENT, trans->c, "missing snapshot %u", id);
                goto err;
        }
 
        /* already deleted? */
-       if (BCH_SNAPSHOT_DELETED(bkey_s_c_to_snapshot(k).v))
-               goto err;
-
-       s = bch2_trans_kmalloc(trans, sizeof(*s));
-       ret = PTR_ERR_OR_ZERO(s);
-       if (ret)
+       if (BCH_SNAPSHOT_DELETED(&s->v))
                goto err;
 
-       bkey_reassemble(&s->k_i, k);
        SET_BCH_SNAPSHOT_DELETED(&s->v, true);
        SET_BCH_SNAPSHOT_SUBVOL(&s->v, false);
        s->v.subvol = 0;
@@ -434,7 +410,6 @@ static int bch2_snapshot_node_delete(struct btree_trans *trans, u32 id)
        struct btree_iter iter, p_iter = (struct btree_iter) { NULL };
        struct bkey_s_c k;
        struct bkey_s_c_snapshot s;
-       struct bkey_i_snapshot *parent;
        u32 parent_id;
        unsigned i;
        int ret = 0;
@@ -458,26 +433,17 @@ static int bch2_snapshot_node_delete(struct btree_trans *trans, u32 id)
        parent_id = le32_to_cpu(s.v->parent);
 
        if (parent_id) {
+               struct bkey_i_snapshot *parent;
+
                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;
-               }
-
-               parent = bch2_trans_kmalloc(trans, sizeof(*parent));
+               parent = bch2_bkey_get_mut_typed(trans, &p_iter, snapshot);
                ret = PTR_ERR_OR_ZERO(parent);
-               if (ret)
+               if (unlikely(ret)) {
+                       bch2_fs_inconsistent_on(ret == -ENOENT, trans->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)
@@ -531,17 +497,15 @@ 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, 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]);
@@ -558,23 +522,13 @@ int bch2_snapshot_node_create(struct btree_trans *trans, u32 parent,
 
        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;
-
-               if (k.k->type != KEY_TYPE_snapshot) {
-                       bch_err(trans->c, "snapshot %u not found", parent);
-                       ret = -ENOENT;
-                       goto err;
-               }
-
-               n = bch2_trans_kmalloc(trans, sizeof(*n));
+               n = bch2_bkey_get_mut_typed(trans, &iter, snapshot);
                ret = PTR_ERR_OR_ZERO(n);
-               if (ret)
+               if (unlikely(ret)) {
+                       if (ret == -ENOENT)
+                               bch_err(trans->c, "snapshot %u not found", parent);
                        goto err;
-
-               bkey_reassemble(&n->k_i, k);
+               }
 
                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");
@@ -595,59 +549,27 @@ err:
        return ret;
 }
 
-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;
+       u32 equiv = snapshot_t(c, k.k->p.snapshot)->equiv;
 
-       /*
-        * XXX: We should also delete whiteouts that no longer overwrite
-        * anything
-        */
-
-       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)) {
-                       ret = commit_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_list_add(c, &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);
-
-       darray_exit(&equiv_seen);
-
-       return ret;
 }
 
 static int bch2_delete_redundant_snapshot(struct btree_trans *trans, struct btree_iter *iter,
@@ -694,7 +616,7 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
        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: %i", ret);
+                       bch_err(c, "error deleleting dead snapshots: error going rw: %s", bch2_err_str(ret));
                        return ret;
                }
        }
@@ -710,7 +632,7 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
                        NULL, NULL, 0,
                bch2_delete_redundant_snapshot(&trans, &iter, k));
        if (ret) {
-               bch_err(c, "error deleting redundant snapshots: %i", ret);
+               bch_err(c, "error deleting redundant snapshots: %s", bch2_err_str(ret));
                goto err;
        }
 
@@ -718,7 +640,7 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
                           POS_MIN, 0, k,
                bch2_snapshot_set_equiv(&trans, k));
        if (ret) {
-               bch_err(c, "error in bch2_snapshots_set_equiv: %i", ret);
+               bch_err(c, "error in bch2_snapshots_set_equiv: %s", bch2_err_str(ret));
                goto err;
        }
 
@@ -737,17 +659,27 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
        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;
                }
        }
@@ -756,8 +688,8 @@ int bch2_delete_dead_snapshots(struct bch_fs *c)
                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;
                }
        }
@@ -774,16 +706,14 @@ static void bch2_delete_dead_snapshots_work(struct work_struct *work)
        struct bch_fs *c = container_of(work, struct bch_fs, snapshot_delete_work);
 
        bch2_delete_dead_snapshots(c);
-       percpu_ref_put(&c->writes);
+       bch2_write_ref_put(c, BCH_WRITE_REF_delete_dead_snapshots);
 }
 
 void bch2_delete_dead_snapshots_async(struct bch_fs *c)
 {
-       if (!percpu_ref_tryget_live(&c->writes))
-               return;
-
-       if (!queue_work(system_long_wq, &c->snapshot_delete_work))
-               percpu_ref_put(&c->writes);
+       if (bch2_write_ref_tryget(c, BCH_WRITE_REF_delete_dead_snapshots) &&
+           !queue_work(system_long_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,
@@ -803,18 +733,18 @@ static int bch2_delete_dead_snapshots_hook(struct btree_trans *trans,
 /* 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;
+               return -BCH_ERR_invalid_bkey;
        }
 
        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;
@@ -830,10 +760,11 @@ void bch2_subvolume_to_text(struct printbuf *out, struct bch_fs *c,
               le32_to_cpu(s.v->snapshot));
 }
 
-int bch2_subvolume_get(struct btree_trans *trans, unsigned subvol,
-                      bool inconsistent_if_not_found,
-                      int iter_flags,
-                      struct bch_subvolume *s)
+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)
 {
        struct btree_iter iter;
        struct bkey_s_c k;
@@ -853,6 +784,14 @@ int bch2_subvolume_get(struct btree_trans *trans, unsigned subvol,
        return ret;
 }
 
+int bch2_subvolume_get(struct btree_trans *trans, unsigned subvol,
+                      bool inconsistent_if_not_found,
+                      int iter_flags,
+                      struct bch_subvolume *s)
+{
+       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,
                             struct bch_subvolume *subvol)
 {
@@ -868,12 +807,12 @@ int bch2_subvolume_get_snapshot(struct btree_trans *trans, u32 subvol,
        struct bch_subvolume s;
        int ret;
 
-       ret = bch2_subvolume_get(trans, subvol, true,
-                                BTREE_ITER_CACHED|
-                                BTREE_ITER_WITH_UPDATES,
-                                &s);
-
-       *snapid = le32_to_cpu(s.snapshot);
+       ret = bch2_subvolume_get_inlined(trans, subvol, true,
+                                        BTREE_ITER_CACHED|
+                                        BTREE_ITER_WITH_UPDATES,
+                                        &s);
+       if (!ret)
+               *snapid = le32_to_cpu(s.snapshot);
        return ret;
 }
 
@@ -913,6 +852,8 @@ int bch2_subvolume_delete(struct btree_trans *trans, u32 subvolid)
                goto err;
 
        ret = bch2_snapshot_node_set_deleted(trans, snapid);
+       if (ret)
+               goto err;
 
        h = bch2_trans_kmalloc(trans, sizeof(*h));
        ret = PTR_ERR_OR_ZERO(h);
@@ -949,7 +890,7 @@ void bch2_subvolume_wait_for_pagecache_and_delete(struct work_struct *work)
                        ret = bch2_trans_do(c, NULL, NULL, BTREE_INSERT_NOFAIL,
                                      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;
                        }
                }
@@ -957,7 +898,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 {
@@ -980,18 +921,17 @@ int bch2_subvolume_wait_for_pagecache_and_delete_hook(struct btree_trans *trans,
        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);
+               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;
@@ -1000,23 +940,13 @@ int bch2_subvolume_unlink(struct btree_trans *trans, u32 subvolid)
                             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));
+       n = bch2_bkey_get_mut_typed(trans, &iter, subvolume);
        ret = PTR_ERR_OR_ZERO(n);
-       if (ret)
+       if (unlikely(ret)) {
+               bch2_fs_inconsistent_on(ret == -ENOENT, trans->c, "missing subvolume %u", subvolid);
                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);
@@ -1052,7 +982,7 @@ int bch2_subvolume_create(struct btree_trans *trans, u64 inode,
 
        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)
+               if (bkey_gt(k.k->p, SUBVOL_POS_MAX))
                        break;
 
                /*
@@ -1065,7 +995,7 @@ int bch2_subvolume_create(struct btree_trans *trans, u64 inode,
        }
 
        if (!ret)
-               ret = -ENOSPC;
+               ret = -BCH_ERR_ENOSPC_subvolume_create;
        goto err;
 found_slot:
        snapshot_subvols[0] = dst_iter.pos.offset;
@@ -1073,27 +1003,19 @@ found_slot:
 
        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, subvolume);
+               ret = PTR_ERR_OR_ZERO(src_subvol);
+               if (unlikely(ret)) {
+                       bch2_fs_inconsistent_on(ret == -ENOENT, trans->c,
+                                               "subvolume %u not found", src_subvolid);
                        goto err;
                }
 
-               bkey_reassemble(&src_subvol->k_i, k);
                parent = le32_to_cpu(src_subvol->v.snapshot);
        }
 
@@ -1110,18 +1032,16 @@ found_slot:
                        goto err;
        }
 
-       new_subvol = bch2_trans_kmalloc(trans, sizeof(*new_subvol));
+       new_subvol = bch2_bkey_alloc(trans, &dst_iter, 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);
        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;