]> git.sesse.net Git - bcachefs-tools-debian/blobdiff - libbcachefs/btree_gc.c
New upstream release
[bcachefs-tools-debian] / libbcachefs / btree_gc.c
index 20e804ecb104727bc3d6bccfc8ae59cc5c5d407c..49e9822dda1dd8d07b7be885aefdefe649345d34 100644 (file)
@@ -27,6 +27,7 @@
 #include "reflink.h"
 #include "replicas.h"
 #include "super-io.h"
+#include "trace.h"
 
 #include <linux/slab.h>
 #include <linux/bitops.h>
 #include <linux/preempt.h>
 #include <linux/rcupdate.h>
 #include <linux/sched/task.h>
-#include <trace/events/bcachefs.h>
 
 #define DROP_THIS_NODE         10
 #define DROP_PREV_NODE         11
 
+static bool should_restart_for_topology_repair(struct bch_fs *c)
+{
+       return c->opts.fix_errors != FSCK_FIX_no &&
+               !(c->recovery_passes_explicit & BIT_ULL(BCH_RECOVERY_PASS_check_topology));
+}
+
 static inline void __gc_pos_set(struct bch_fs *c, struct gc_pos new_pos)
 {
        preempt_disable();
@@ -76,7 +82,7 @@ static int bch2_gc_check_topology(struct bch_fs *c,
        if (cur.k->k.type == KEY_TYPE_btree_ptr_v2) {
                struct bkey_i_btree_ptr_v2 *bp = bkey_i_to_btree_ptr_v2(cur.k);
 
-               if (bpos_cmp(expected_start, bp->v.min_key)) {
+               if (!bpos_eq(expected_start, bp->v.min_key)) {
                        bch2_topology_error(c);
 
                        if (bkey_deleted(&prev->k->k)) {
@@ -96,9 +102,9 @@ static int bch2_gc_check_topology(struct bch_fs *c,
                                  "  cur %s",
                                  bch2_btree_ids[b->c.btree_id], b->c.level,
                                  buf1.buf, buf2.buf) &&
-                           !test_bit(BCH_FS_TOPOLOGY_REPAIR_DONE, &c->flags)) {
+                           should_restart_for_topology_repair(c)) {
                                bch_info(c, "Halting mark and sweep to start topology repair pass");
-                               ret = -BCH_ERR_need_topology_repair;
+                               ret = bch2_run_explicit_recovery_pass(c, BCH_RECOVERY_PASS_check_topology);
                                goto err;
                        } else {
                                set_bit(BCH_FS_INITIAL_GC_UNFIXED, &c->flags);
@@ -106,7 +112,7 @@ static int bch2_gc_check_topology(struct bch_fs *c,
                }
        }
 
-       if (is_last && bpos_cmp(cur.k->k.p, node_end)) {
+       if (is_last && !bpos_eq(cur.k->k.p, node_end)) {
                bch2_topology_error(c);
 
                printbuf_reset(&buf1);
@@ -124,9 +130,9 @@ static int bch2_gc_check_topology(struct bch_fs *c,
                          "  expected %s",
                          bch2_btree_ids[b->c.btree_id], b->c.level,
                          buf1.buf, buf2.buf) &&
-                   !test_bit(BCH_FS_TOPOLOGY_REPAIR_DONE, &c->flags)) {
+                   should_restart_for_topology_repair(c)) {
                        bch_info(c, "Halting mark and sweep to start topology repair pass");
-                       ret = -BCH_ERR_need_topology_repair;
+                       ret = bch2_run_explicit_recovery_pass(c, BCH_RECOVERY_PASS_check_topology);
                        goto err;
                } else {
                        set_bit(BCH_FS_INITIAL_GC_UNFIXED, &c->flags);
@@ -201,7 +207,7 @@ static int set_node_min(struct bch_fs *c, struct btree *b, struct bpos new_min)
 
        new = kmalloc_array(BKEY_BTREE_PTR_U64s_MAX, sizeof(u64), GFP_KERNEL);
        if (!new)
-               return -ENOMEM;
+               return -BCH_ERR_ENOMEM_gc_repair_key;
 
        btree_ptr_to_v2(b, new);
        b->data->min_key        = new_min;
@@ -230,7 +236,7 @@ static int set_node_max(struct bch_fs *c, struct btree *b, struct bpos new_max)
 
        new = kmalloc_array(BKEY_BTREE_PTR_U64s_MAX, sizeof(u64), GFP_KERNEL);
        if (!new)
-               return -ENOMEM;
+               return -BCH_ERR_ENOMEM_gc_repair_key;
 
        btree_ptr_to_v2(b, new);
        b->data->max_key        = new_max;
@@ -274,12 +280,12 @@ static int btree_repair_node_boundaries(struct bch_fs *c, struct btree *b,
        bch2_bkey_val_to_text(&buf2, c, bkey_i_to_s_c(&cur->key));
 
        if (prev &&
-           bpos_cmp(expected_start, cur->data->min_key) > 0 &&
+           bpos_gt(expected_start, cur->data->min_key) &&
            BTREE_NODE_SEQ(cur->data) > BTREE_NODE_SEQ(prev->data)) {
                /* cur overwrites prev: */
 
-               if (mustfix_fsck_err_on(bpos_cmp(prev->data->min_key,
-                                                cur->data->min_key) >= 0, c,
+               if (mustfix_fsck_err_on(bpos_ge(prev->data->min_key,
+                                               cur->data->min_key), c,
                                "btree node overwritten by next node at btree %s level %u:\n"
                                "  node %s\n"
                                "  next %s",
@@ -289,7 +295,7 @@ static int btree_repair_node_boundaries(struct bch_fs *c, struct btree *b,
                        goto out;
                }
 
-               if (mustfix_fsck_err_on(bpos_cmp(prev->key.k.p,
+               if (mustfix_fsck_err_on(!bpos_eq(prev->key.k.p,
                                                 bpos_predecessor(cur->data->min_key)), c,
                                "btree node with incorrect max_key at btree %s level %u:\n"
                                "  node %s\n"
@@ -301,8 +307,8 @@ static int btree_repair_node_boundaries(struct bch_fs *c, struct btree *b,
        } else {
                /* prev overwrites cur: */
 
-               if (mustfix_fsck_err_on(bpos_cmp(expected_start,
-                                                cur->data->max_key) >= 0, c,
+               if (mustfix_fsck_err_on(bpos_ge(expected_start,
+                                               cur->data->max_key), c,
                                "btree node overwritten by prev node at btree %s level %u:\n"
                                "  prev %s\n"
                                "  node %s",
@@ -312,7 +318,7 @@ static int btree_repair_node_boundaries(struct bch_fs *c, struct btree *b,
                        goto out;
                }
 
-               if (mustfix_fsck_err_on(bpos_cmp(expected_start, cur->data->min_key), c,
+               if (mustfix_fsck_err_on(!bpos_eq(expected_start, cur->data->min_key), c,
                                "btree node with incorrect min_key at btree %s level %u:\n"
                                "  prev %s\n"
                                "  node %s",
@@ -336,7 +342,7 @@ static int btree_repair_node_end(struct bch_fs *c, struct btree *b,
        bch2_bkey_val_to_text(&buf1, c, bkey_i_to_s_c(&child->key));
        bch2_bpos_to_text(&buf2, b->key.k.p);
 
-       if (mustfix_fsck_err_on(bpos_cmp(child->key.k.p, b->key.k.p), c,
+       if (mustfix_fsck_err_on(!bpos_eq(child->key.k.p, b->key.k.p), c,
                        "btree node with incorrect max_key at btree %s level %u:\n"
                        "  %s\n"
                        "  expected %s",
@@ -374,8 +380,8 @@ again:
        bch2_btree_and_journal_iter_init_node_iter(&iter, c, b);
 
        while ((k = bch2_btree_and_journal_iter_peek(&iter)).k) {
-               BUG_ON(bpos_cmp(k.k->p, b->data->min_key) < 0);
-               BUG_ON(bpos_cmp(k.k->p, b->data->max_key) > 0);
+               BUG_ON(bpos_lt(k.k->p, b->data->min_key));
+               BUG_ON(bpos_gt(k.k->p, b->data->max_key));
 
                bch2_btree_and_journal_iter_advance(&iter);
                bch2_bkey_buf_reassemble(&cur_k, c, k);
@@ -404,8 +410,7 @@ again:
                }
 
                if (ret) {
-                       bch_err(c, "%s: error getting btree node: %s",
-                               __func__, bch2_err_str(ret));
+                       bch_err_msg(c, ret, "getting btree node");
                        break;
                }
 
@@ -473,8 +478,7 @@ again:
                ret = PTR_ERR_OR_ZERO(cur);
 
                if (ret) {
-                       bch_err(c, "%s: error getting btree node: %s",
-                               __func__, bch2_err_str(ret));
+                       bch_err_msg(c, ret, "getting btree node");
                        goto err;
                }
 
@@ -522,7 +526,7 @@ fsck_err:
        return ret;
 }
 
-static int bch2_repair_topology(struct bch_fs *c)
+int bch2_check_topology(struct bch_fs *c)
 {
        struct btree_trans trans;
        struct btree *b;
@@ -531,8 +535,13 @@ static int bch2_repair_topology(struct bch_fs *c)
 
        bch2_trans_init(&trans, c, 0, 0);
 
-       for (i = 0; i < BTREE_ID_NR && !ret; i++) {
-               b = c->btree_roots[i].b;
+       for (i = 0; i < btree_id_nr_alive(c)&& !ret; i++) {
+               struct btree_root *r = bch2_btree_id_root(c, i);
+
+               if (!r->alive)
+                       continue;
+
+               b = r->b;
                if (btree_node_fake(b))
                        continue;
 
@@ -572,15 +581,15 @@ static int bch2_check_fix_ptrs(struct btree_trans *trans, enum btree_id btree_id
                struct bucket *g = PTR_GC_BUCKET(ca, &p.ptr);
                enum bch_data_type data_type = bch2_bkey_ptr_data_type(*k, &entry->ptr);
 
-               if (c->opts.reconstruct_alloc ||
-                   fsck_err_on(!g->gen_valid, c,
-                               "bucket %u:%zu data type %s ptr gen %u missing in alloc btree\n"
-                               "while marking %s",
-                               p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
-                               bch2_data_types[ptr_data_type(k->k, &p.ptr)],
-                               p.ptr.gen,
-                               (printbuf_reset(&buf),
-                                bch2_bkey_val_to_text(&buf, c, *k), buf.buf))) {
+               if (!g->gen_valid &&
+                   (c->opts.reconstruct_alloc ||
+                    fsck_err(c, "bucket %u:%zu data type %s ptr gen %u missing in alloc btree\n"
+                             "while marking %s",
+                             p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
+                             bch2_data_types[ptr_data_type(k->k, &p.ptr)],
+                             p.ptr.gen,
+                             (printbuf_reset(&buf),
+                              bch2_bkey_val_to_text(&buf, c, *k), buf.buf)))) {
                        if (!p.ptr.cached) {
                                g->gen_valid            = true;
                                g->gen                  = p.ptr.gen;
@@ -589,14 +598,15 @@ static int bch2_check_fix_ptrs(struct btree_trans *trans, enum btree_id btree_id
                        }
                }
 
-               if (fsck_err_on(gen_cmp(p.ptr.gen, g->gen) > 0, c,
-                               "bucket %u:%zu data type %s ptr gen in the future: %u > %u\n"
-                               "while marking %s",
-                               p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
-                               bch2_data_types[ptr_data_type(k->k, &p.ptr)],
-                               p.ptr.gen, g->gen,
-                               (printbuf_reset(&buf),
-                                bch2_bkey_val_to_text(&buf, c, *k), buf.buf))) {
+               if (gen_cmp(p.ptr.gen, g->gen) > 0 &&
+                   (c->opts.reconstruct_alloc ||
+                    fsck_err(c, "bucket %u:%zu data type %s ptr gen in the future: %u > %u\n"
+                             "while marking %s",
+                             p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
+                             bch2_data_types[ptr_data_type(k->k, &p.ptr)],
+                             p.ptr.gen, g->gen,
+                             (printbuf_reset(&buf),
+                              bch2_bkey_val_to_text(&buf, c, *k), buf.buf)))) {
                        if (!p.ptr.cached) {
                                g->gen_valid            = true;
                                g->gen                  = p.ptr.gen;
@@ -609,32 +619,33 @@ static int bch2_check_fix_ptrs(struct btree_trans *trans, enum btree_id btree_id
                        }
                }
 
-               if (fsck_err_on(gen_cmp(g->gen, p.ptr.gen) > BUCKET_GC_GEN_MAX, c,
-                               "bucket %u:%zu gen %u data type %s: ptr gen %u too stale\n"
-                               "while marking %s",
-                               p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), g->gen,
-                               bch2_data_types[ptr_data_type(k->k, &p.ptr)],
-                               p.ptr.gen,
-                               (printbuf_reset(&buf),
-                                bch2_bkey_val_to_text(&buf, c, *k), buf.buf)))
+               if (gen_cmp(g->gen, p.ptr.gen) > BUCKET_GC_GEN_MAX &&
+                   (c->opts.reconstruct_alloc ||
+                    fsck_err(c, "bucket %u:%zu gen %u data type %s: ptr gen %u too stale\n"
+                             "while marking %s",
+                             p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr), g->gen,
+                             bch2_data_types[ptr_data_type(k->k, &p.ptr)],
+                             p.ptr.gen,
+                             (printbuf_reset(&buf),
+                              bch2_bkey_val_to_text(&buf, c, *k), buf.buf))))
                        do_update = true;
 
-               if (fsck_err_on(!p.ptr.cached &&
-                               gen_cmp(p.ptr.gen, g->gen) < 0, c,
-                               "bucket %u:%zu data type %s stale dirty ptr: %u < %u\n"
-                               "while marking %s",
-                               p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
-                               bch2_data_types[ptr_data_type(k->k, &p.ptr)],
-                               p.ptr.gen, g->gen,
-                               (printbuf_reset(&buf),
-                                bch2_bkey_val_to_text(&buf, c, *k), buf.buf)))
+               if (!p.ptr.cached && gen_cmp(p.ptr.gen, g->gen) < 0 &&
+                   (c->opts.reconstruct_alloc ||
+                    fsck_err(c, "bucket %u:%zu data type %s stale dirty ptr: %u < %u\n"
+                             "while marking %s",
+                             p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
+                             bch2_data_types[ptr_data_type(k->k, &p.ptr)],
+                             p.ptr.gen, g->gen,
+                             (printbuf_reset(&buf),
+                              bch2_bkey_val_to_text(&buf, c, *k), buf.buf))))
                        do_update = true;
 
                if (data_type != BCH_DATA_btree && p.ptr.gen != g->gen)
                        continue;
 
-               if (fsck_err_on(g->data_type &&
-                               g->data_type != data_type, c,
+               if (fsck_err_on(bucket_data_type(g->data_type) &&
+                               bucket_data_type(g->data_type) != data_type, c,
                                "bucket %u:%zu different types of data in same bucket: %s, %s\n"
                                "while marking %s",
                                p.ptr.dev, PTR_BUCKET_NR(ca, &p.ptr),
@@ -661,7 +672,7 @@ static int bch2_check_fix_ptrs(struct btree_trans *trans, enum btree_id btree_id
                                         bch2_bkey_val_to_text(&buf, c, *k), buf.buf)))
                                do_update = true;
 
-                       if (fsck_err_on(!bch2_ptr_matches_stripe_m(m, p), c,
+                       if (fsck_err_on(m && m->alive && !bch2_ptr_matches_stripe_m(m, p), c,
                                        "pointer does not match stripe %llu\n"
                                        "while marking %s",
                                        (u64) p.ec.idx,
@@ -685,8 +696,8 @@ static int bch2_check_fix_ptrs(struct btree_trans *trans, enum btree_id btree_id
 
                new = kmalloc(bkey_bytes(k->k), GFP_KERNEL);
                if (!new) {
-                       bch_err(c, "%s: error allocating new key", __func__);
-                       ret = -ENOMEM;
+                       bch_err_msg(c, ret, "allocating new key");
+                       ret = -BCH_ERR_ENOMEM_gc_repair_key;
                        goto err;
                }
 
@@ -757,7 +768,7 @@ found:
                if (level)
                        bch2_btree_node_update_key_early(trans, btree_id, level - 1, *k, new);
 
-               if (c->opts.verbose) {
+               if (0) {
                        printbuf_reset(&buf);
                        bch2_bkey_val_to_text(&buf, c, *k);
                        bch_info(c, "updated %s", buf.buf);
@@ -808,11 +819,11 @@ static int bch2_gc_mark_key(struct btree_trans *trans, enum btree_id btree_id,
        }
 
        ret = commit_do(trans, NULL, NULL, 0,
-                       bch2_mark_key(trans, old, *k, flags));
+                       bch2_mark_key(trans, btree_id, level, old, *k, flags));
 fsck_err:
 err:
        if (ret)
-               bch_err(c, "error from %s(): %s", __func__, bch2_err_str(ret));
+               bch_err_fn(c, ret);
        return ret;
 }
 
@@ -883,11 +894,11 @@ static int bch2_gc_btree(struct btree_trans *trans, enum btree_id btree_id,
                return ret;
 
        mutex_lock(&c->btree_root_lock);
-       b = c->btree_roots[btree_id].b;
+       b = bch2_btree_id_root(c, btree_id)->b;
        if (!btree_node_fake(b)) {
                struct bkey_s_c k = bkey_i_to_s_c(&b->key);
 
-               ret = bch2_gc_mark_key(trans, b->c.btree_id, b->c.level,
+               ret = bch2_gc_mark_key(trans, b->c.btree_id, b->c.level + 1,
                                       true, &k, initial);
        }
        gc_pos_set(c, gc_pos_btree_root(b->c.btree_id));
@@ -912,16 +923,13 @@ static int bch2_gc_btree_init_recurse(struct btree_trans *trans, struct btree *b
        bkey_init(&prev.k->k);
 
        while ((k = bch2_btree_and_journal_iter_peek(&iter)).k) {
-               BUG_ON(bpos_cmp(k.k->p, b->data->min_key) < 0);
-               BUG_ON(bpos_cmp(k.k->p, b->data->max_key) > 0);
+               BUG_ON(bpos_lt(k.k->p, b->data->min_key));
+               BUG_ON(bpos_gt(k.k->p, b->data->max_key));
 
                ret = bch2_gc_mark_key(trans, b->c.btree_id, b->c.level,
                                       false, &k, true);
-               if (ret) {
-                       bch_err(c, "%s: error from bch2_gc_mark_key: %s",
-                               __func__, bch2_err_str(ret));
+               if (ret)
                        goto fsck_err;
-               }
 
                if (b->c.level) {
                        bch2_bkey_buf_reassemble(&cur, c, k);
@@ -967,9 +975,9 @@ static int bch2_gc_btree_init_recurse(struct btree_trans *trans, struct btree *b
                                          b->c.level - 1,
                                          (printbuf_reset(&buf),
                                           bch2_bkey_val_to_text(&buf, c, bkey_i_to_s_c(cur.k)), buf.buf)) &&
-                                   !test_bit(BCH_FS_TOPOLOGY_REPAIR_DONE, &c->flags)) {
-                                       ret = -BCH_ERR_need_topology_repair;
+                                   should_restart_for_topology_repair(c)) {
                                        bch_info(c, "Halting mark and sweep to start topology repair pass");
+                                       ret = bch2_run_explicit_recovery_pass(c, BCH_RECOVERY_PASS_check_topology);
                                        goto fsck_err;
                                } else {
                                        /* Continue marking when opted to not
@@ -979,8 +987,7 @@ static int bch2_gc_btree_init_recurse(struct btree_trans *trans, struct btree *b
                                        continue;
                                }
                        } else if (ret) {
-                               bch_err(c, "%s: error getting btree node: %s",
-                                       __func__, bch2_err_str(ret));
+                               bch_err_msg(c, ret, "getting btree node");
                                break;
                        }
 
@@ -1010,7 +1017,7 @@ static int bch2_gc_btree_init(struct btree_trans *trans,
        struct printbuf buf = PRINTBUF;
        int ret = 0;
 
-       b = c->btree_roots[btree_id].b;
+       b = bch2_btree_id_root(c, btree_id)->b;
 
        if (btree_node_fake(b))
                return 0;
@@ -1018,7 +1025,7 @@ static int bch2_gc_btree_init(struct btree_trans *trans,
        six_lock_read(&b->c.lock, NULL, NULL);
        printbuf_reset(&buf);
        bch2_bpos_to_text(&buf, b->data->min_key);
-       if (mustfix_fsck_err_on(bpos_cmp(b->data->min_key, POS_MIN), c,
+       if (mustfix_fsck_err_on(!bpos_eq(b->data->min_key, POS_MIN), c,
                        "btree root with incorrect min_key: %s", buf.buf)) {
                bch_err(c, "repair unimplemented");
                ret = -BCH_ERR_fsck_repair_unimplemented;
@@ -1027,7 +1034,7 @@ static int bch2_gc_btree_init(struct btree_trans *trans,
 
        printbuf_reset(&buf);
        bch2_bpos_to_text(&buf, b->data->max_key);
-       if (mustfix_fsck_err_on(bpos_cmp(b->data->max_key, SPOS_MAX), c,
+       if (mustfix_fsck_err_on(!bpos_eq(b->data->max_key, SPOS_MAX), c,
                        "btree root with incorrect max_key: %s", buf.buf)) {
                bch_err(c, "repair unimplemented");
                ret = -BCH_ERR_fsck_repair_unimplemented;
@@ -1040,14 +1047,14 @@ static int bch2_gc_btree_init(struct btree_trans *trans,
        if (!ret) {
                struct bkey_s_c k = bkey_i_to_s_c(&b->key);
 
-               ret = bch2_gc_mark_key(trans, b->c.btree_id, b->c.level, true,
+               ret = bch2_gc_mark_key(trans, b->c.btree_id, b->c.level + 1, true,
                                       &k, true);
        }
 fsck_err:
        six_unlock_read(&b->c.lock);
 
        if (ret < 0)
-               bch_err(c, "error from %s(): %s", __func__, bch2_err_str(ret));
+               bch_err_fn(c, ret);
        printbuf_exit(&buf);
        return ret;
 }
@@ -1079,8 +1086,17 @@ static int bch2_gc_btrees(struct bch_fs *c, bool initial, bool metadata_only)
                        ? bch2_gc_btree_init(&trans, ids[i], metadata_only)
                        : bch2_gc_btree(&trans, ids[i], initial, metadata_only);
 
+       for (i = BTREE_ID_NR; i < btree_id_nr_alive(c) && !ret; i++) {
+               if (!bch2_btree_id_root(c, i)->alive)
+                       continue;
+
+               ret = initial
+                       ? bch2_gc_btree_init(&trans, i, metadata_only)
+                       : bch2_gc_btree(&trans, i, initial, metadata_only);
+       }
+
        if (ret < 0)
-               bch_err(c, "error from %s(): %s", __func__, bch2_err_str(ret));
+               bch_err_fn(c, ret);
 
        bch2_trans_exit(&trans);
        return ret;
@@ -1222,7 +1238,7 @@ static int bch2_gc_done(struct bch_fs *c,
        for_each_member_device(ca, c, dev) {
                struct bch_dev_usage *dst = ca->usage_base;
                struct bch_dev_usage *src = (void *)
-                       bch2_acc_percpu_u64s((void *) ca->usage_gc,
+                       bch2_acc_percpu_u64s((u64 __percpu *) ca->usage_gc,
                                             dev_usage_u64s());
 
                copy_dev_field(buckets_ec,              "buckets_ec");
@@ -1238,7 +1254,7 @@ static int bch2_gc_done(struct bch_fs *c,
                unsigned nr = fs_usage_u64s(c);
                struct bch_fs_usage *dst = c->usage_base;
                struct bch_fs_usage *src = (void *)
-                       bch2_acc_percpu_u64s((void *) c->usage_gc, nr);
+                       bch2_acc_percpu_u64s((u64 __percpu *) c->usage_gc, nr);
 
                copy_fs_field(hidden,           "hidden");
                copy_fs_field(btree,            "btree");
@@ -1278,15 +1294,14 @@ fsck_err:
        if (ca)
                percpu_ref_put(&ca->ref);
        if (ret)
-               bch_err(c, "error from %s(): %s", __func__, bch2_err_str(ret));
+               bch_err_fn(c, ret);
 
        percpu_up_write(&c->mark_lock);
        printbuf_exit(&buf);
        return ret;
 }
 
-static int bch2_gc_start(struct bch_fs *c,
-                        bool metadata_only)
+static int bch2_gc_start(struct bch_fs *c)
 {
        struct bch_dev *ca = NULL;
        unsigned i;
@@ -1297,18 +1312,17 @@ static int bch2_gc_start(struct bch_fs *c,
                                         sizeof(u64), GFP_KERNEL);
        if (!c->usage_gc) {
                bch_err(c, "error allocating c->usage_gc");
-               return -ENOMEM;
+               return -BCH_ERR_ENOMEM_gc_start;
        }
 
        for_each_member_device(ca, c, i) {
-               BUG_ON(ca->buckets_gc);
                BUG_ON(ca->usage_gc);
 
                ca->usage_gc = alloc_percpu(struct bch_dev_usage);
                if (!ca->usage_gc) {
                        bch_err(c, "error allocating ca->usage_gc");
                        percpu_ref_put(&ca->ref);
-                       return -ENOMEM;
+                       return -BCH_ERR_ENOMEM_gc_start;
                }
 
                this_cpu_write(ca->usage_gc->d[BCH_DATA_free].buckets,
@@ -1318,6 +1332,22 @@ static int bch2_gc_start(struct bch_fs *c,
        return 0;
 }
 
+static int bch2_gc_reset(struct bch_fs *c)
+{
+       struct bch_dev *ca;
+       unsigned i;
+
+       for_each_member_device(ca, c, i) {
+               free_percpu(ca->usage_gc);
+               ca->usage_gc = NULL;
+       }
+
+       free_percpu(c->usage_gc);
+       c->usage_gc = NULL;
+
+       return bch2_gc_start(c);
+}
+
 /* returns true if not equal */
 static inline bool bch2_alloc_v4_cmp(struct bch_alloc_v4 l,
                                     struct bch_alloc_v4 r)
@@ -1340,15 +1370,16 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
        struct bch_dev *ca = bch_dev_bkey_exists(c, iter->pos.inode);
        struct bucket gc, *b;
        struct bkey_i_alloc_v4 *a;
-       struct bch_alloc_v4 old, new;
+       struct bch_alloc_v4 old_convert, new;
+       const struct bch_alloc_v4 *old;
        enum bch_data_type type;
        int ret;
 
-       if (bkey_cmp(iter->pos, POS(ca->dev_idx, ca->mi.nbuckets)) >= 0)
+       if (bkey_ge(iter->pos, POS(ca->dev_idx, ca->mi.nbuckets)))
                return 1;
 
-       bch2_alloc_to_v4(k, &old);
-       new = old;
+       old = bch2_alloc_to_v4(k, &old_convert);
+       new = *old;
 
        percpu_down_read(&c->mark_lock);
        b = gc_bucket(ca, iter->pos.offset);
@@ -1360,7 +1391,7 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
        type = __alloc_data_type(b->dirty_sectors,
                                 b->cached_sectors,
                                 b->stripe,
-                                old,
+                                *old,
                                 b->data_type);
        if (b->data_type != type) {
                struct bch_dev_usage *u;
@@ -1382,9 +1413,19 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
            gc.data_type != BCH_DATA_btree)
                return 0;
 
-       if (gen_after(old.gen, gc.gen))
+       if (gen_after(old->gen, gc.gen))
                return 0;
 
+       if (c->opts.reconstruct_alloc ||
+           fsck_err_on(new.data_type != gc.data_type, c,
+                       "bucket %llu:%llu gen %u has wrong data_type"
+                       ": got %s, should be %s",
+                       iter->pos.inode, iter->pos.offset,
+                       gc.gen,
+                       bch2_data_types[new.data_type],
+                       bch2_data_types[gc.data_type]))
+               new.data_type = gc.data_type;
+
 #define copy_bucket_field(_f)                                          \
        if (c->opts.reconstruct_alloc ||                                \
            fsck_err_on(new._f != gc._f, c,                             \
@@ -1397,14 +1438,13 @@ static int bch2_alloc_write_key(struct btree_trans *trans,
                new._f = gc._f;                                         \
 
        copy_bucket_field(gen);
-       copy_bucket_field(data_type);
        copy_bucket_field(dirty_sectors);
        copy_bucket_field(cached_sectors);
        copy_bucket_field(stripe_redundancy);
        copy_bucket_field(stripe);
 #undef copy_bucket_field
 
-       if (!bch2_alloc_v4_cmp(old, new))
+       if (!bch2_alloc_v4_cmp(*old, new))
                return 0;
 
        a = bch2_alloc_to_v4_mut(trans, k);
@@ -1462,7 +1502,8 @@ static int bch2_gc_alloc_start(struct bch_fs *c, bool metadata_only)
        struct btree_iter iter;
        struct bkey_s_c k;
        struct bucket *g;
-       struct bch_alloc_v4 a;
+       struct bch_alloc_v4 a_convert;
+       const struct bch_alloc_v4 *a;
        unsigned i;
        int ret;
 
@@ -1473,7 +1514,7 @@ static int bch2_gc_alloc_start(struct bch_fs *c, bool metadata_only)
                if (!buckets) {
                        percpu_ref_put(&ca->ref);
                        bch_err(c, "error allocating ca->buckets[gc]");
-                       return -ENOMEM;
+                       return -BCH_ERR_ENOMEM_gc_alloc_start;
                }
 
                buckets->first_bucket   = ca->mi.first_bucket;
@@ -1488,20 +1529,20 @@ static int bch2_gc_alloc_start(struct bch_fs *c, bool metadata_only)
                ca = bch_dev_bkey_exists(c, k.k->p.inode);
                g = gc_bucket(ca, k.k->p.offset);
 
-               bch2_alloc_to_v4(k, &a);
+               a = bch2_alloc_to_v4(k, &a_convert);
 
                g->gen_valid    = 1;
-               g->gen          = a.gen;
+               g->gen          = a->gen;
 
                if (metadata_only &&
-                   (a.data_type == BCH_DATA_user ||
-                    a.data_type == BCH_DATA_cached ||
-                    a.data_type == BCH_DATA_parity)) {
-                       g->data_type            = a.data_type;
-                       g->dirty_sectors        = a.dirty_sectors;
-                       g->cached_sectors       = a.cached_sectors;
-                       g->stripe               = a.stripe;
-                       g->stripe_redundancy    = a.stripe_redundancy;
+                   (a->data_type == BCH_DATA_user ||
+                    a->data_type == BCH_DATA_cached ||
+                    a->data_type == BCH_DATA_parity)) {
+                       g->data_type            = a->data_type;
+                       g->dirty_sectors        = a->dirty_sectors;
+                       g->cached_sectors       = a->cached_sectors;
+                       g->stripe               = a->stripe;
+                       g->stripe_redundancy    = a->stripe_redundancy;
                }
        }
        bch2_trans_iter_exit(&trans, &iter);
@@ -1567,21 +1608,16 @@ static int bch2_gc_write_reflink_key(struct btree_trans *trans,
                        "  should be %u",
                        (bch2_bkey_val_to_text(&buf, c, k), buf.buf),
                        r->refcount)) {
-               struct bkey_i *new;
+               struct bkey_i *new = bch2_bkey_make_mut(trans, iter, &k, 0);
 
-               new = bch2_trans_kmalloc(trans, bkey_bytes(k.k));
                ret = PTR_ERR_OR_ZERO(new);
                if (ret)
                        return ret;
 
-               bkey_reassemble(new, k);
-
                if (!r->refcount)
                        new->k.type = KEY_TYPE_deleted;
                else
                        *bkey_refcount(new) = cpu_to_le64(r->refcount);
-
-               ret = bch2_trans_update(trans, iter, new, 0);
        }
 fsck_err:
        printbuf_exit(&buf);
@@ -1637,7 +1673,7 @@ static int bch2_gc_reflink_start(struct bch_fs *c,
                r = genradix_ptr_alloc(&c->reflink_gc_table, c->reflink_gc_nr++,
                                       GFP_KERNEL);
                if (!r) {
-                       ret = -ENOMEM;
+                       ret = -BCH_ERR_ENOMEM_gc_reflink_start;
                        break;
                }
 
@@ -1668,6 +1704,7 @@ static int bch2_gc_write_stripes_key(struct btree_trans *trans,
        struct printbuf buf = PRINTBUF;
        const struct bch_stripe *s;
        struct gc_stripe *m;
+       bool bad = false;
        unsigned i;
        int ret = 0;
 
@@ -1677,18 +1714,21 @@ static int bch2_gc_write_stripes_key(struct btree_trans *trans,
        s = bkey_s_c_to_stripe(k).v;
        m = genradix_ptr(&c->gc_stripes, k.k->p.offset);
 
-       for (i = 0; i < s->nr_blocks; i++)
-               if (stripe_blockcount_get(s, i) != (m ? m->block_sectors[i] : 0))
-                       goto inconsistent;
-       return 0;
-inconsistent:
-       if (fsck_err_on(true, c,
-                       "stripe has wrong block sector count %u:\n"
-                       "  %s\n"
-                       "  should be %u", i,
-                       (printbuf_reset(&buf),
-                        bch2_bkey_val_to_text(&buf, c, k), buf.buf),
-                       m ? m->block_sectors[i] : 0)) {
+       for (i = 0; i < s->nr_blocks; i++) {
+               u32 old = stripe_blockcount_get(s, i);
+               u32 new = (m ? m->block_sectors[i] : 0);
+
+               if (old != new) {
+                       prt_printf(&buf, "stripe block %u has wrong sector count: got %u, should be %u\n",
+                                  i, old, new);
+                       bad = true;
+               }
+       }
+
+       if (bad)
+               bch2_bkey_val_to_text(&buf, c, k);
+
+       if (fsck_err_on(bad, c, "%s", buf.buf)) {
                struct bkey_i_stripe *new;
 
                new = bch2_trans_kmalloc(trans, bkey_bytes(k.k));
@@ -1764,7 +1804,7 @@ int bch2_gc(struct bch_fs *c, bool initial, bool metadata_only)
 
        bch2_btree_interior_updates_flush(c);
 
-       ret   = bch2_gc_start(c, metadata_only) ?:
+       ret   = bch2_gc_start(c) ?:
                bch2_gc_alloc_start(c, metadata_only) ?:
                bch2_gc_reflink_start(c, metadata_only);
        if (ret)
@@ -1774,31 +1814,8 @@ again:
 
        bch2_mark_superblocks(c);
 
-       if (BCH_SB_HAS_TOPOLOGY_ERRORS(c->disk_sb.sb) &&
-           !test_bit(BCH_FS_INITIAL_GC_DONE, &c->flags) &&
-           c->opts.fix_errors != FSCK_OPT_NO) {
-               bch_info(c, "Starting topology repair pass");
-               ret = bch2_repair_topology(c);
-               if (ret)
-                       goto out;
-               bch_info(c, "Topology repair pass done");
-
-               set_bit(BCH_FS_TOPOLOGY_REPAIR_DONE, &c->flags);
-       }
-
        ret = bch2_gc_btrees(c, initial, metadata_only);
 
-       if (ret == -BCH_ERR_need_topology_repair &&
-           !test_bit(BCH_FS_TOPOLOGY_REPAIR_DONE, &c->flags) &&
-           !test_bit(BCH_FS_INITIAL_GC_DONE, &c->flags)) {
-               set_bit(BCH_FS_NEED_ANOTHER_GC, &c->flags);
-               SET_BCH_SB_HAS_TOPOLOGY_ERRORS(c->disk_sb.sb, true);
-               ret = 0;
-       }
-
-       if (ret == -BCH_ERR_need_topology_repair)
-               ret = -BCH_ERR_fsck_errors_not_fixed;
-
        if (ret)
                goto out;
 
@@ -1825,6 +1842,9 @@ again:
                bch2_gc_stripes_reset(c, metadata_only);
                bch2_gc_alloc_reset(c, metadata_only);
                bch2_gc_reflink_reset(c, metadata_only);
+               ret = bch2_gc_reset(c);
+               if (ret)
+                       goto out;
 
                /* flush fsck errors, reset counters */
                bch2_flush_fsck_errs(c);
@@ -1856,6 +1876,9 @@ out:
         * allocator thread - issue wakeup in case they blocked on gc_lock:
         */
        closure_wake_up(&c->freelist_wait);
+
+       if (ret)
+               bch_err_fn(c, ret);
        return ret;
 }
 
@@ -1889,28 +1912,25 @@ static int gc_btree_gens_key(struct btree_trans *trans,
        percpu_up_read(&c->mark_lock);
        return 0;
 update:
-       u = bch2_trans_kmalloc(trans, bkey_bytes(k.k));
+       u = bch2_bkey_make_mut(trans, iter, &k, 0);
        ret = PTR_ERR_OR_ZERO(u);
        if (ret)
                return ret;
 
-       bkey_reassemble(u, k);
-
        bch2_extent_normalize(c, bkey_i_to_s(u));
-       return bch2_trans_update(trans, iter, u, 0);
+       return 0;
 }
 
 static int bch2_alloc_write_oldest_gen(struct btree_trans *trans, struct btree_iter *iter,
                                       struct bkey_s_c k)
 {
        struct bch_dev *ca = bch_dev_bkey_exists(trans->c, iter->pos.inode);
-       struct bch_alloc_v4 a;
+       struct bch_alloc_v4 a_convert;
+       const struct bch_alloc_v4 *a = bch2_alloc_to_v4(k, &a_convert);
        struct bkey_i_alloc_v4 *a_mut;
        int ret;
 
-       bch2_alloc_to_v4(k, &a);
-
-       if (a.oldest_gen == ca->oldest_gen[iter->pos.offset])
+       if (a->oldest_gen == ca->oldest_gen[iter->pos.offset])
                return 0;
 
        a_mut = bch2_alloc_to_v4_mut(trans, k);
@@ -1954,7 +1974,7 @@ int bch2_gc_gens(struct bch_fs *c)
                ca->oldest_gen = kvmalloc(ca->mi.nbuckets, GFP_KERNEL);
                if (!ca->oldest_gen) {
                        percpu_ref_put(&ca->ref);
-                       ret = -ENOMEM;
+                       ret = -BCH_ERR_ENOMEM_gc_gens;
                        goto err;
                }
 
@@ -1979,7 +1999,7 @@ int bch2_gc_gens(struct bch_fs *c)
                                        NULL, NULL,
                                        BTREE_INSERT_NOFAIL,
                                gc_btree_gens_key(&trans, &iter, k));
-                       if (ret && ret != -EROFS)
+                       if (ret && !bch2_err_matches(ret, EROFS))
                                bch_err(c, "error recalculating oldest_gen: %s", bch2_err_str(ret));
                        if (ret)
                                goto err;
@@ -1992,7 +2012,7 @@ int bch2_gc_gens(struct bch_fs *c)
                        NULL, NULL,
                        BTREE_INSERT_NOFAIL,
                bch2_alloc_write_oldest_gen(&trans, &iter, k));
-       if (ret && ret != -EROFS)
+       if (ret && !bch2_err_matches(ret, EROFS))
                bch_err(c, "error writing oldest_gen: %s", bch2_err_str(ret));
        if (ret)
                goto err;