]> git.sesse.net Git - bcachefs-tools-debian/blobdiff - libbcachefs/fsck.c
Update bcachefs sources to bee7b5a4fa21 bcachefs: Pin btree cache in ram for random...
[bcachefs-tools-debian] / libbcachefs / fsck.c
index 796e1a8447d6110dffc6742638dd0308de82fd66..dfd54708d2a0212f4c7263654c849057886d2d2a 100644 (file)
@@ -5,7 +5,6 @@
 #include "btree_cache.h"
 #include "btree_update.h"
 #include "buckets.h"
-#include "darray.h"
 #include "dirent.h"
 #include "error.h"
 #include "fs-common.h"
@@ -18,6 +17,7 @@
 #include "xattr.h"
 
 #include <linux/bsearch.h>
+#include <linux/darray.h>
 #include <linux/dcache.h> /* struct qstr */
 
 /*
@@ -59,23 +59,8 @@ static s64 bch2_count_subdirs(struct btree_trans *trans, u64 inum,
        return ret ?: subdirs;
 }
 
-static int __snapshot_lookup_subvol(struct btree_trans *trans, u32 snapshot,
-                                   u32 *subvol)
-{
-       struct bch_snapshot s;
-       int ret = bch2_bkey_get_val_typed(trans, BTREE_ID_snapshots,
-                                         POS(0, snapshot), 0,
-                                         snapshot, &s);
-       if (!ret)
-               *subvol = le32_to_cpu(s.subvol);
-       else if (bch2_err_matches(ret, ENOENT))
-               bch_err(trans->c, "snapshot %u not found", snapshot);
-       return ret;
-
-}
-
-static int __subvol_lookup(struct btree_trans *trans, u32 subvol,
-                          u32 *snapshot, u64 *inum)
+static int subvol_lookup(struct btree_trans *trans, u32 subvol,
+                        u32 *snapshot, u64 *inum)
 {
        struct bch_subvolume s;
        int ret;
@@ -87,12 +72,6 @@ static int __subvol_lookup(struct btree_trans *trans, u32 subvol,
        return ret;
 }
 
-static int subvol_lookup(struct btree_trans *trans, u32 subvol,
-                        u32 *snapshot, u64 *inum)
-{
-       return lockrestart_do(trans, __subvol_lookup(trans, subvol, snapshot, inum));
-}
-
 static int lookup_first_inode(struct btree_trans *trans, u64 inode_nr,
                              struct bch_inode_unpacked *inode)
 {
@@ -120,9 +99,9 @@ err:
        return ret;
 }
 
-static int __lookup_inode(struct btree_trans *trans, u64 inode_nr,
-                         struct bch_inode_unpacked *inode,
-                         u32 *snapshot)
+static int lookup_inode(struct btree_trans *trans, u64 inode_nr,
+                       struct bch_inode_unpacked *inode,
+                       u32 *snapshot)
 {
        struct btree_iter iter;
        struct bkey_s_c k;
@@ -140,29 +119,19 @@ static int __lookup_inode(struct btree_trans *trans, u64 inode_nr,
        if (!ret)
                *snapshot = iter.pos.snapshot;
 err:
-       bch_err_msg(trans->c, ret, "fetching inode %llu:%u", inode_nr, *snapshot);
        bch2_trans_iter_exit(trans, &iter);
        return ret;
 }
 
-static int lookup_inode(struct btree_trans *trans, u64 inode_nr,
-                       struct bch_inode_unpacked *inode,
-                       u32 *snapshot)
-{
-       return lockrestart_do(trans, __lookup_inode(trans, inode_nr, inode, snapshot));
-}
-
-static int __lookup_dirent(struct btree_trans *trans,
+static int lookup_dirent_in_snapshot(struct btree_trans *trans,
                           struct bch_hash_info hash_info,
                           subvol_inum dir, struct qstr *name,
-                          u64 *target, unsigned *type)
+                          u64 *target, unsigned *type, u32 snapshot)
 {
        struct btree_iter iter;
        struct bkey_s_c_dirent d;
-       int ret;
-
-       ret = bch2_hash_lookup(trans, &iter, bch2_dirent_hash_desc,
-                              &hash_info, dir, name, 0);
+       int ret = bch2_hash_lookup_in_snapshot(trans, &iter, bch2_dirent_hash_desc,
+                              &hash_info, dir, name, 0, snapshot);
        if (ret)
                return ret;
 
@@ -173,34 +142,6 @@ static int __lookup_dirent(struct btree_trans *trans,
        return 0;
 }
 
-static int __write_inode(struct btree_trans *trans,
-                        struct bch_inode_unpacked *inode,
-                        u32 snapshot)
-{
-       struct bkey_inode_buf *inode_p =
-               bch2_trans_kmalloc(trans, sizeof(*inode_p));
-
-       if (IS_ERR(inode_p))
-               return PTR_ERR(inode_p);
-
-       bch2_inode_pack(inode_p, inode);
-       inode_p->inode.k.p.snapshot = snapshot;
-
-       return bch2_btree_insert_nonextent(trans, BTREE_ID_inodes,
-                               &inode_p->inode.k_i,
-                               BTREE_UPDATE_INTERNAL_SNAPSHOT_NODE);
-}
-
-static int fsck_write_inode(struct btree_trans *trans,
-                           struct bch_inode_unpacked *inode,
-                           u32 snapshot)
-{
-       int ret = commit_do(trans, NULL, NULL, BCH_TRANS_COMMIT_no_enospc,
-                           __write_inode(trans, inode, snapshot));
-       bch_err_fn(trans->c, ret);
-       return ret;
-}
-
 static int __remove_dirent(struct btree_trans *trans, struct bpos pos)
 {
        struct bch_fs *c = trans->c;
@@ -227,35 +168,44 @@ err:
 }
 
 /* Get lost+found, create if it doesn't exist: */
-static int lookup_lostfound(struct btree_trans *trans, u32 subvol,
+static int lookup_lostfound(struct btree_trans *trans, u32 snapshot,
                            struct bch_inode_unpacked *lostfound)
 {
        struct bch_fs *c = trans->c;
-       struct bch_inode_unpacked root;
-       struct bch_hash_info root_hash_info;
        struct qstr lostfound_str = QSTR("lost+found");
-       subvol_inum root_inum = { .subvol = subvol };
        u64 inum = 0;
        unsigned d_type = 0;
-       u32 snapshot;
        int ret;
 
-       ret = __subvol_lookup(trans, subvol, &snapshot, &root_inum.inum);
+       struct bch_snapshot_tree st;
+       ret = bch2_snapshot_tree_lookup(trans,
+                       bch2_snapshot_tree(c, snapshot), &st);
        if (ret)
                return ret;
 
-       ret = __lookup_inode(trans, root_inum.inum, &root, &snapshot);
+       subvol_inum root_inum = { .subvol = le32_to_cpu(st.master_subvol) };
+       u32 subvol_snapshot;
+
+       ret = subvol_lookup(trans, le32_to_cpu(st.master_subvol),
+                           &subvol_snapshot, &root_inum.inum);
+       bch_err_msg(c, ret, "looking up root subvol");
        if (ret)
                return ret;
 
-       root_hash_info = bch2_hash_info_init(c, &root);
+       struct bch_inode_unpacked root_inode;
+       struct bch_hash_info root_hash_info;
+       u32 root_inode_snapshot = snapshot;
+       ret = lookup_inode(trans, root_inum.inum, &root_inode, &root_inode_snapshot);
+       bch_err_msg(c, ret, "looking up root inode");
+       if (ret)
+               return ret;
 
-       ret = __lookup_dirent(trans, root_hash_info, root_inum,
-                           &lostfound_str, &inum, &d_type);
-       if (bch2_err_matches(ret, ENOENT)) {
-               bch_notice(c, "creating lost+found");
+       root_hash_info = bch2_hash_info_init(c, &root_inode);
+
+       ret = lookup_dirent_in_snapshot(trans, root_hash_info, root_inum,
+                             &lostfound_str, &inum, &d_type, snapshot);
+       if (bch2_err_matches(ret, ENOENT))
                goto create_lostfound;
-       }
 
        bch_err_fn(c, ret);
        if (ret)
@@ -270,20 +220,53 @@ static int lookup_lostfound(struct btree_trans *trans, u32 subvol,
         * The bch2_check_dirents pass has already run, dangling dirents
         * shouldn't exist here:
         */
-       return __lookup_inode(trans, inum, lostfound, &snapshot);
+       ret = lookup_inode(trans, inum, lostfound, &snapshot);
+       bch_err_msg(c, ret, "looking up lost+found %llu:%u in (root inode %llu, snapshot root %u)",
+                   inum, snapshot, root_inum.inum, bch2_snapshot_root(c, snapshot));
+       return ret;
 
 create_lostfound:
+       /*
+        * XXX: we could have a nicer log message here  if we had a nice way to
+        * walk backpointers to print a path
+        */
+       bch_notice(c, "creating lost+found in snapshot %u", le32_to_cpu(st.root_snapshot));
+
+       u64 now = bch2_current_time(c);
+       struct btree_iter lostfound_iter = { NULL };
+       u64 cpu = raw_smp_processor_id();
+
        bch2_inode_init_early(c, lostfound);
+       bch2_inode_init_late(lostfound, now, 0, 0, S_IFDIR|0700, 0, &root_inode);
+       lostfound->bi_dir = root_inode.bi_inum;
+
+       root_inode.bi_nlink++;
+
+       ret = bch2_inode_create(trans, &lostfound_iter, lostfound, snapshot, cpu);
+       if (ret)
+               goto err;
 
-       ret = bch2_create_trans(trans, root_inum, &root,
-                               lostfound, &lostfound_str,
-                               0, 0, S_IFDIR|0700, 0, NULL, NULL,
-                               (subvol_inum) { }, 0);
+       bch2_btree_iter_set_snapshot(&lostfound_iter, snapshot);
+       ret = bch2_btree_iter_traverse(&lostfound_iter);
+       if (ret)
+               goto err;
+
+       ret =   bch2_dirent_create_snapshot(trans,
+                               0, root_inode.bi_inum, snapshot, &root_hash_info,
+                               mode_to_type(lostfound->bi_mode),
+                               &lostfound_str,
+                               lostfound->bi_inum,
+                               &lostfound->bi_dir_offset,
+                               BCH_HASH_SET_MUST_CREATE) ?:
+               bch2_inode_write_flags(trans, &lostfound_iter, lostfound,
+                                      BTREE_UPDATE_INTERNAL_SNAPSHOT_NODE);
+err:
        bch_err_msg(c, ret, "creating lost+found");
+       bch2_trans_iter_exit(trans, &lostfound_iter);
        return ret;
 }
 
-static int __reattach_inode(struct btree_trans *trans,
+static int reattach_inode(struct btree_trans *trans,
                          struct bch_inode_unpacked *inode,
                          u32 inode_snapshot)
 {
@@ -292,56 +275,55 @@ static int __reattach_inode(struct btree_trans *trans,
        char name_buf[20];
        struct qstr name;
        u64 dir_offset = 0;
-       u32 subvol;
+       u32 dirent_snapshot = inode_snapshot;
        int ret;
 
-       ret = __snapshot_lookup_subvol(trans, inode_snapshot, &subvol);
-       if (ret)
-               return ret;
+       if (inode->bi_subvol) {
+               inode->bi_parent_subvol = BCACHEFS_ROOT_SUBVOL;
+
+               u64 root_inum;
+               ret = subvol_lookup(trans, inode->bi_parent_subvol,
+                                   &dirent_snapshot, &root_inum);
+               if (ret)
+                       return ret;
 
-       ret = lookup_lostfound(trans, subvol, &lostfound);
+               snprintf(name_buf, sizeof(name_buf), "subvol-%u", inode->bi_subvol);
+       } else {
+               snprintf(name_buf, sizeof(name_buf), "%llu", inode->bi_inum);
+       }
+
+       ret = lookup_lostfound(trans, dirent_snapshot, &lostfound);
        if (ret)
                return ret;
 
        if (S_ISDIR(inode->bi_mode)) {
                lostfound.bi_nlink++;
 
-               ret = __write_inode(trans, &lostfound, U32_MAX);
+               ret = __bch2_fsck_write_inode(trans, &lostfound, U32_MAX);
                if (ret)
                        return ret;
        }
 
        dir_hash = bch2_hash_info_init(trans->c, &lostfound);
 
-       snprintf(name_buf, sizeof(name_buf), "%llu", inode->bi_inum);
        name = (struct qstr) QSTR(name_buf);
 
-       ret = bch2_dirent_create(trans,
-                                (subvol_inum) {
-                                       .subvol = subvol,
-                                       .inum = lostfound.bi_inum,
-                                },
-                                &dir_hash,
-                                inode_d_type(inode),
-                                &name, inode->bi_inum, &dir_offset,
-                                BCH_HASH_SET_MUST_CREATE);
+       ret = bch2_dirent_create_snapshot(trans,
+                               inode->bi_parent_subvol, lostfound.bi_inum,
+                               dirent_snapshot,
+                               &dir_hash,
+                               inode_d_type(inode),
+                               &name,
+                               inode->bi_subvol ?: inode->bi_inum,
+                               &dir_offset,
+                               BCH_HASH_SET_MUST_CREATE);
        if (ret)
                return ret;
 
        inode->bi_dir           = lostfound.bi_inum;
        inode->bi_dir_offset    = dir_offset;
 
-       return __write_inode(trans, inode, inode_snapshot);
-}
-
-static int reattach_inode(struct btree_trans *trans,
-                         struct bch_inode_unpacked *inode,
-                         u32 inode_snapshot)
-{
-       int ret = commit_do(trans, NULL, NULL, BCH_TRANS_COMMIT_no_enospc,
-                       __reattach_inode(trans, inode, inode_snapshot));
-       bch_err_msg(trans->c, ret, "reattaching inode %llu", inode->bi_inum);
-       return ret;
+       return __bch2_fsck_write_inode(trans, inode, inode_snapshot);
 }
 
 static int remove_backpointer(struct btree_trans *trans,
@@ -599,13 +581,12 @@ static int get_inodes_all_snapshots(struct btree_trans *trans,
 }
 
 static struct inode_walker_entry *
-lookup_inode_for_snapshot(struct bch_fs *c, struct inode_walker *w,
-                         u32 snapshot, bool is_whiteout)
+lookup_inode_for_snapshot(struct bch_fs *c, struct inode_walker *w, struct bkey_s_c k)
 {
-       struct inode_walker_entry *i;
-
-       snapshot = bch2_snapshot_equiv(c, snapshot);
+       bool is_whiteout = k.k->type == KEY_TYPE_whiteout;
+       u32 snapshot = bch2_snapshot_equiv(c, k.k->p.snapshot);
 
+       struct inode_walker_entry *i;
        __darray_for_each(w->inodes, i)
                if (bch2_snapshot_is_ancestor(c, snapshot, i->snapshot))
                        goto found;
@@ -616,20 +597,24 @@ found:
 
        if (snapshot != i->snapshot && !is_whiteout) {
                struct inode_walker_entry new = *i;
-               size_t pos;
-               int ret;
 
                new.snapshot = snapshot;
                new.count = 0;
 
-               bch_info(c, "have key for inode %llu:%u but have inode in ancestor snapshot %u",
-                        w->last_pos.inode, snapshot, i->snapshot);
+               struct printbuf buf = PRINTBUF;
+               bch2_bkey_val_to_text(&buf, c, k);
+
+               bch_info(c, "have key for inode %llu:%u but have inode in ancestor snapshot %u\n"
+                        "unexpected because we should always update the inode when we update a key in that inode\n"
+                        "%s",
+                        w->last_pos.inode, snapshot, i->snapshot, buf.buf);
+               printbuf_exit(&buf);
 
                while (i > w->inodes.data && i[-1].snapshot > snapshot)
                        --i;
 
-               pos = i - w->inodes.data;
-               ret = darray_insert_item(&w->inodes, pos, new);
+               size_t pos = i - w->inodes.data;
+               int ret = darray_insert_item(&w->inodes, pos, new);
                if (ret)
                        return ERR_PTR(ret);
 
@@ -640,21 +625,21 @@ found:
 }
 
 static struct inode_walker_entry *walk_inode(struct btree_trans *trans,
-                                            struct inode_walker *w, struct bpos pos,
-                                            bool is_whiteout)
+                                            struct inode_walker *w,
+                                            struct bkey_s_c k)
 {
-       if (w->last_pos.inode != pos.inode) {
-               int ret = get_inodes_all_snapshots(trans, w, pos.inode);
+       if (w->last_pos.inode != k.k->p.inode) {
+               int ret = get_inodes_all_snapshots(trans, w, k.k->p.inode);
                if (ret)
                        return ERR_PTR(ret);
-       } else if (bkey_cmp(w->last_pos, pos)) {
+       } else if (bkey_cmp(w->last_pos, k.k->p)) {
                darray_for_each(w->inodes, i)
                        i->seen_this_pos = false;
        }
 
-       w->last_pos = pos;
+       w->last_pos = k.k->p;
 
-       return lookup_inode_for_snapshot(trans->c, w, pos.snapshot, is_whiteout);
+       return lookup_inode_for_snapshot(trans->c, w, k);
 }
 
 static int __get_visible_inodes(struct btree_trans *trans,
@@ -729,7 +714,7 @@ static int hash_redo_key(struct btree_trans *trans,
        delete->k.p = k_iter->pos;
        return  bch2_btree_iter_traverse(k_iter) ?:
                bch2_trans_update(trans, k_iter, delete, 0) ?:
-               bch2_hash_set_snapshot(trans, desc, hash_info,
+               bch2_hash_set_in_snapshot(trans, desc, hash_info,
                                       (subvol_inum) { 0, k.k->p.inode },
                                       k.k->p.snapshot, tmp,
                                       BCH_HASH_SET_MUST_CREATE,
@@ -802,6 +787,43 @@ fsck_err:
        goto out;
 }
 
+static struct bkey_s_c_dirent dirent_get_by_pos(struct btree_trans *trans,
+                                               struct btree_iter *iter,
+                                               struct bpos pos)
+{
+       return bch2_bkey_get_iter_typed(trans, iter, BTREE_ID_dirents, pos, 0, dirent);
+}
+
+static struct bkey_s_c_dirent inode_get_dirent(struct btree_trans *trans,
+                                              struct btree_iter *iter,
+                                              struct bch_inode_unpacked *inode,
+                                              u32 *snapshot)
+{
+       if (inode->bi_subvol) {
+               u64 inum;
+               int ret = subvol_lookup(trans, inode->bi_parent_subvol, snapshot, &inum);
+               if (ret)
+                       return ((struct bkey_s_c_dirent) { .k = ERR_PTR(ret) });
+       }
+
+       return dirent_get_by_pos(trans, iter, SPOS(inode->bi_dir, inode->bi_dir_offset, *snapshot));
+}
+
+static bool inode_points_to_dirent(struct bch_inode_unpacked *inode,
+                                  struct bkey_s_c_dirent d)
+{
+       return  inode->bi_dir           == d.k->p.inode &&
+               inode->bi_dir_offset    == d.k->p.offset;
+}
+
+static bool dirent_points_to_inode(struct bkey_s_c_dirent d,
+                                  struct bch_inode_unpacked *inode)
+{
+       return d.v->d_type == DT_SUBVOL
+               ? le32_to_cpu(d.v->d_child_subvol)      == inode->bi_subvol
+               : le64_to_cpu(d.v->d_inum)              == inode->bi_inum;
+}
+
 static int check_inode_deleted_list(struct btree_trans *trans, struct bpos p)
 {
        struct btree_iter iter;
@@ -814,6 +836,49 @@ static int check_inode_deleted_list(struct btree_trans *trans, struct bpos p)
        return k.k->type == KEY_TYPE_set;
 }
 
+static int check_inode_dirent_inode(struct btree_trans *trans, struct bkey_s_c inode_k,
+                                   struct bch_inode_unpacked *inode,
+                                   u32 inode_snapshot, bool *write_inode)
+{
+       struct bch_fs *c = trans->c;
+       struct printbuf buf = PRINTBUF;
+
+       struct btree_iter dirent_iter = {};
+       struct bkey_s_c_dirent d = inode_get_dirent(trans, &dirent_iter, inode, &inode_snapshot);
+       int ret = bkey_err(d);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               return ret;
+
+       if (fsck_err_on(ret,
+                       c, inode_points_to_missing_dirent,
+                       "inode points to missing dirent\n%s",
+                       (bch2_bkey_val_to_text(&buf, c, inode_k), buf.buf)) ||
+           fsck_err_on(!ret && !dirent_points_to_inode(d, inode),
+                       c, inode_points_to_wrong_dirent,
+                       "inode points to dirent that does not point back:\n%s",
+                       (bch2_bkey_val_to_text(&buf, c, inode_k),
+                        prt_newline(&buf),
+                        bch2_bkey_val_to_text(&buf, c, d.s_c), buf.buf))) {
+               /*
+                * We just clear the backpointer fields for now. If we find a
+                * dirent that points to this inode in check_dirents(), we'll
+                * update it then; then when we get to check_path() if the
+                * backpointer is still 0 we'll reattach it.
+                */
+               inode->bi_dir = 0;
+               inode->bi_dir_offset = 0;
+               inode->bi_flags &= ~BCH_INODE_backptr_untrusted;
+               *write_inode = true;
+       }
+
+       ret = 0;
+fsck_err:
+       bch2_trans_iter_exit(trans, &dirent_iter);
+       printbuf_exit(&buf);
+       bch_err_fn(c, ret);
+       return ret;
+}
+
 static int check_inode(struct btree_trans *trans,
                       struct btree_iter *iter,
                       struct bkey_s_c k,
@@ -868,7 +933,8 @@ static int check_inode(struct btree_trans *trans,
 
                u.bi_flags &= ~BCH_INODE_i_size_dirty|BCH_INODE_unlinked;
 
-               ret = __write_inode(trans, &u, iter->pos.snapshot);
+               ret = __bch2_fsck_write_inode(trans, &u, iter->pos.snapshot);
+
                bch_err_msg(c, ret, "in fsck updating inode");
                if (ret)
                        return ret;
@@ -878,8 +944,7 @@ static int check_inode(struct btree_trans *trans,
                return 0;
        }
 
-       if (u.bi_flags & BCH_INODE_unlinked &&
-           c->sb.version >= bcachefs_metadata_version_deleted_inodes) {
+       if (u.bi_flags & BCH_INODE_unlinked) {
                ret = check_inode_deleted_list(trans, k.k->p);
                if (ret < 0)
                        return ret;
@@ -958,8 +1023,49 @@ static int check_inode(struct btree_trans *trans,
                do_update = true;
        }
 
+       if (u.bi_dir || u.bi_dir_offset) {
+               ret = check_inode_dirent_inode(trans, k, &u, k.k->p.snapshot, &do_update);
+               if (ret)
+                       goto err;
+       }
+
+       if (fsck_err_on(u.bi_parent_subvol &&
+                       (u.bi_subvol == 0 ||
+                        u.bi_subvol == BCACHEFS_ROOT_SUBVOL),
+                       c, inode_bi_parent_nonzero,
+                       "inode %llu:%u has subvol %u but nonzero parent subvol %u",
+                       u.bi_inum, k.k->p.snapshot, u.bi_subvol, u.bi_parent_subvol)) {
+               u.bi_parent_subvol = 0;
+               do_update = true;
+       }
+
+       if (u.bi_subvol) {
+               struct bch_subvolume s;
+
+               ret = bch2_subvolume_get(trans, u.bi_subvol, false, 0, &s);
+               if (ret && !bch2_err_matches(ret, ENOENT))
+                       goto err;
+
+               if (fsck_err_on(ret,
+                               c, inode_bi_subvol_missing,
+                               "inode %llu:%u bi_subvol points to missing subvolume %u",
+                               u.bi_inum, k.k->p.snapshot, u.bi_subvol) ||
+                   fsck_err_on(le64_to_cpu(s.inode) != u.bi_inum ||
+                               !bch2_snapshot_is_ancestor(c, le32_to_cpu(s.snapshot),
+                                                          k.k->p.snapshot),
+                               c, inode_bi_subvol_wrong,
+                               "inode %llu:%u points to subvol %u, but subvol points to %llu:%u",
+                               u.bi_inum, k.k->p.snapshot, u.bi_subvol,
+                               le64_to_cpu(s.inode),
+                               le32_to_cpu(s.snapshot))) {
+                       u.bi_subvol = 0;
+                       u.bi_parent_subvol = 0;
+                       do_update = true;
+               }
+       }
+
        if (do_update) {
-               ret = __write_inode(trans, &u, iter->pos.snapshot);
+               ret = __bch2_fsck_write_inode(trans, &u, iter->pos.snapshot);
                bch_err_msg(c, ret, "in fsck updating inode");
                if (ret)
                        return ret;
@@ -990,28 +1096,6 @@ int bch2_check_inodes(struct bch_fs *c)
        return ret;
 }
 
-static struct bkey_s_c_dirent dirent_get_by_pos(struct btree_trans *trans,
-                                               struct btree_iter *iter,
-                                               struct bpos pos)
-{
-       return bch2_bkey_get_iter_typed(trans, iter, BTREE_ID_dirents, pos, 0, dirent);
-}
-
-static bool inode_points_to_dirent(struct bch_inode_unpacked *inode,
-                                  struct bkey_s_c_dirent d)
-{
-       return  inode->bi_dir           == d.k->p.inode &&
-               inode->bi_dir_offset    == d.k->p.offset;
-}
-
-static bool dirent_points_to_inode(struct bkey_s_c_dirent d,
-                                  struct bch_inode_unpacked *inode)
-{
-       return d.v->d_type == DT_SUBVOL
-               ? le32_to_cpu(d.v->d_child_subvol)      == inode->bi_subvol
-               : le64_to_cpu(d.v->d_inum)              == inode->bi_inum;
-}
-
 static int check_i_sectors(struct btree_trans *trans, struct inode_walker *w)
 {
        struct bch_fs *c = trans->c;
@@ -1040,7 +1124,7 @@ static int check_i_sectors(struct btree_trans *trans, struct inode_walker *w)
                                w->last_pos.inode, i->snapshot,
                                i->inode.bi_sectors, i->count)) {
                        i->inode.bi_sectors = i->count;
-                       ret = fsck_write_inode(trans, &i->inode, i->snapshot);
+                       ret = bch2_fsck_write_inode(trans, &i->inode, i->snapshot);
                        if (ret)
                                break;
                }
@@ -1320,7 +1404,7 @@ static int check_extent(struct btree_trans *trans, struct btree_iter *iter,
                        goto err;
        }
 
-       i = walk_inode(trans, inode, equiv, k.k->type == KEY_TYPE_whiteout);
+       i = walk_inode(trans, inode, k);
        ret = PTR_ERR_OR_ZERO(i);
        if (ret)
                goto err;
@@ -1489,7 +1573,7 @@ static int check_subdir_count(struct btree_trans *trans, struct inode_walker *w)
                                "directory %llu:%u with wrong i_nlink: got %u, should be %llu",
                                w->last_pos.inode, i->snapshot, i->inode.bi_nlink, i->count)) {
                        i->inode.bi_nlink = i->count;
-                       ret = fsck_write_inode(trans, &i->inode, i->snapshot);
+                       ret = bch2_fsck_write_inode(trans, &i->inode, i->snapshot);
                        if (ret)
                                break;
                }
@@ -1499,86 +1583,106 @@ fsck_err:
        return ret ?: trans_was_restarted(trans, restart_count);
 }
 
-static int check_dirent_target(struct btree_trans *trans,
-                              struct btree_iter *iter,
-                              struct bkey_s_c_dirent d,
-                              struct bch_inode_unpacked *target,
-                              u32 target_snapshot)
+static int check_dirent_inode_dirent(struct btree_trans *trans,
+                                  struct btree_iter *iter,
+                                  struct bkey_s_c_dirent d,
+                                  struct bch_inode_unpacked *target,
+                                  u32 target_snapshot)
 {
        struct bch_fs *c = trans->c;
-       struct bkey_i_dirent *n;
        struct printbuf buf = PRINTBUF;
-       struct btree_iter bp_iter = { NULL };
        int ret = 0;
 
+       if (inode_points_to_dirent(target, d))
+               return 0;
+
        if (!target->bi_dir &&
            !target->bi_dir_offset) {
                target->bi_dir          = d.k->p.inode;
                target->bi_dir_offset   = d.k->p.offset;
-
-               ret = __write_inode(trans, target, target_snapshot);
-               if (ret)
-                       goto err;
+               return __bch2_fsck_write_inode(trans, target, target_snapshot);
        }
 
-       if (!inode_points_to_dirent(target, d)) {
-               struct bkey_s_c_dirent bp_dirent = dirent_get_by_pos(trans, &bp_iter,
-                                     SPOS(target->bi_dir, target->bi_dir_offset, target_snapshot));
-               ret = bkey_err(bp_dirent);
-               if (ret && !bch2_err_matches(ret, ENOENT))
-                       goto err;
+       struct btree_iter bp_iter = { NULL };
+       struct bkey_s_c_dirent bp_dirent = dirent_get_by_pos(trans, &bp_iter,
+                             SPOS(target->bi_dir, target->bi_dir_offset, target_snapshot));
+       ret = bkey_err(bp_dirent);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               goto err;
 
-               bool backpointer_exists = !ret;
-               ret = 0;
+       bool backpointer_exists = !ret;
+       ret = 0;
+
+       if (fsck_err_on(!backpointer_exists,
+                       c, inode_wrong_backpointer,
+                       "inode %llu:%u has wrong backpointer:\n"
+                       "got       %llu:%llu\n"
+                       "should be %llu:%llu",
+                       target->bi_inum, target_snapshot,
+                       target->bi_dir,
+                       target->bi_dir_offset,
+                       d.k->p.inode,
+                       d.k->p.offset)) {
+               target->bi_dir          = d.k->p.inode;
+               target->bi_dir_offset   = d.k->p.offset;
+               ret = __bch2_fsck_write_inode(trans, target, target_snapshot);
+               goto out;
+       }
 
-               bch2_bkey_val_to_text(&buf, c, d.s_c);
-               prt_newline(&buf);
-               if (backpointer_exists)
-                       bch2_bkey_val_to_text(&buf, c, bp_dirent.s_c);
+       bch2_bkey_val_to_text(&buf, c, d.s_c);
+       prt_newline(&buf);
+       if (backpointer_exists)
+               bch2_bkey_val_to_text(&buf, c, bp_dirent.s_c);
+
+       if (fsck_err_on(backpointer_exists &&
+                       (S_ISDIR(target->bi_mode) ||
+                        target->bi_subvol),
+                       c, inode_dir_multiple_links,
+                       "%s %llu:%u with multiple links\n%s",
+                       S_ISDIR(target->bi_mode) ? "directory" : "subvolume",
+                       target->bi_inum, target_snapshot, buf.buf)) {
+               ret = __remove_dirent(trans, d.k->p);
+               goto out;
+       }
 
-               if (fsck_err_on(S_ISDIR(target->bi_mode) && backpointer_exists,
-                               c, inode_dir_multiple_links,
-                               "directory %llu:%u with multiple links\n%s",
-                               target->bi_inum, target_snapshot, buf.buf)) {
-                       ret = __remove_dirent(trans, d.k->p);
-                       goto out;
-               }
+       /*
+        * hardlinked file with nlink 0:
+        * We're just adjusting nlink here so check_nlinks() will pick
+        * it up, it ignores inodes with nlink 0
+        */
+       if (fsck_err_on(backpointer_exists && !target->bi_nlink,
+                       c, inode_multiple_links_but_nlink_0,
+                       "inode %llu:%u type %s has multiple links but i_nlink 0\n%s",
+                       target->bi_inum, target_snapshot, bch2_d_types[d.v->d_type], buf.buf)) {
+               target->bi_nlink++;
+               target->bi_flags &= ~BCH_INODE_unlinked;
+               ret = __bch2_fsck_write_inode(trans, target, target_snapshot);
+               if (ret)
+                       goto err;
+       }
+out:
+err:
+fsck_err:
+       bch2_trans_iter_exit(trans, &bp_iter);
+       printbuf_exit(&buf);
+       bch_err_fn(c, ret);
+       return ret;
+}
 
-               /*
-                * hardlinked file with nlink 0:
-                * We're just adjusting nlink here so check_nlinks() will pick
-                * it up, it ignores inodes with nlink 0
-                */
-               if (fsck_err_on(backpointer_exists && !target->bi_nlink,
-                               c, inode_multiple_links_but_nlink_0,
-                               "inode %llu:%u type %s has multiple links but i_nlink 0\n%s",
-                               target->bi_inum, target_snapshot, bch2_d_types[d.v->d_type], buf.buf)) {
-                       target->bi_nlink++;
-                       target->bi_flags &= ~BCH_INODE_unlinked;
-
-                       ret = __write_inode(trans, target, target_snapshot);
-                       if (ret)
-                               goto err;
-               }
+static int check_dirent_target(struct btree_trans *trans,
+                              struct btree_iter *iter,
+                              struct bkey_s_c_dirent d,
+                              struct bch_inode_unpacked *target,
+                              u32 target_snapshot)
+{
+       struct bch_fs *c = trans->c;
+       struct bkey_i_dirent *n;
+       struct printbuf buf = PRINTBUF;
+       int ret = 0;
 
-               if (fsck_err_on(!backpointer_exists,
-                               c, inode_wrong_backpointer,
-                               "inode %llu:%u has wrong backpointer:\n"
-                               "got       %llu:%llu\n"
-                               "should be %llu:%llu",
-                               target->bi_inum, target_snapshot,
-                               target->bi_dir,
-                               target->bi_dir_offset,
-                               d.k->p.inode,
-                               d.k->p.offset)) {
-                       target->bi_dir          = d.k->p.inode;
-                       target->bi_dir_offset   = d.k->p.offset;
-
-                       ret = __write_inode(trans, target, target_snapshot);
-                       if (ret)
-                               goto err;
-               }
-       }
+       ret = check_dirent_inode_dirent(trans, iter, d, target, target_snapshot);
+       if (ret)
+               goto err;
 
        if (fsck_err_on(d.v->d_type != inode_d_type(target),
                        c, dirent_d_type_wrong,
@@ -1594,6 +1698,12 @@ static int check_dirent_target(struct btree_trans *trans,
 
                bkey_reassemble(&n->k_i, d.s_c);
                n->v.d_type = inode_d_type(target);
+               if (n->v.d_type == DT_SUBVOL) {
+                       n->v.d_parent_subvol = target->bi_parent_subvol;
+                       n->v.d_child_subvol = target->bi_subvol;
+               } else {
+                       n->v.d_inum = target->bi_inum;
+               }
 
                ret = bch2_trans_update(trans, iter, &n->k_i, 0);
                if (ret)
@@ -1601,34 +1711,134 @@ static int check_dirent_target(struct btree_trans *trans,
 
                d = dirent_i_to_s_c(n);
        }
+err:
+fsck_err:
+       printbuf_exit(&buf);
+       bch_err_fn(c, ret);
+       return ret;
+}
 
-       if (d.v->d_type == DT_SUBVOL &&
-           target->bi_parent_subvol != le32_to_cpu(d.v->d_parent_subvol) &&
-           (c->sb.version < bcachefs_metadata_version_subvol_dirent ||
-            fsck_err(c, dirent_d_parent_subvol_wrong,
-                     "dirent has wrong d_parent_subvol field: got %u, should be %u",
-                     le32_to_cpu(d.v->d_parent_subvol),
-                     target->bi_parent_subvol))) {
-               n = bch2_trans_kmalloc(trans, bkey_bytes(d.k));
-               ret = PTR_ERR_OR_ZERO(n);
+/* find a subvolume that's a descendent of @snapshot: */
+static int find_snapshot_subvol(struct btree_trans *trans, u32 snapshot, u32 *subvolid)
+{
+       struct btree_iter iter;
+       struct bkey_s_c k;
+       int ret;
+
+       for_each_btree_key_norestart(trans, iter, BTREE_ID_subvolumes, POS_MIN, 0, k, ret) {
+               if (k.k->type != KEY_TYPE_subvolume)
+                       continue;
+
+               struct bkey_s_c_subvolume s = bkey_s_c_to_subvolume(k);
+               if (bch2_snapshot_is_ancestor(trans->c, le32_to_cpu(s.v->snapshot), snapshot)) {
+                       bch2_trans_iter_exit(trans, &iter);
+                       *subvolid = k.k->p.offset;
+                       goto found;
+               }
+       }
+       if (!ret)
+               ret = -ENOENT;
+found:
+       bch2_trans_iter_exit(trans, &iter);
+       return ret;
+}
+
+static int check_dirent_to_subvol(struct btree_trans *trans, struct btree_iter *iter,
+                                 struct bkey_s_c_dirent d)
+{
+       struct bch_fs *c = trans->c;
+       struct btree_iter subvol_iter = {};
+       struct bch_inode_unpacked subvol_root;
+       u32 parent_subvol = le32_to_cpu(d.v->d_parent_subvol);
+       u32 target_subvol = le32_to_cpu(d.v->d_child_subvol);
+       u32 parent_snapshot;
+       u64 parent_inum;
+       struct printbuf buf = PRINTBUF;
+       int ret = 0;
+
+       ret = subvol_lookup(trans, parent_subvol, &parent_snapshot, &parent_inum);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               return ret;
+
+       if (fsck_err_on(ret, c, dirent_to_missing_parent_subvol,
+                       "dirent parent_subvol points to missing subvolume\n%s",
+                       (bch2_bkey_val_to_text(&buf, c, d.s_c), buf.buf)) ||
+           fsck_err_on(!ret && !bch2_snapshot_is_ancestor(c, parent_snapshot, d.k->p.snapshot),
+                       c, dirent_not_visible_in_parent_subvol,
+                       "dirent not visible in parent_subvol (not an ancestor of subvol snap %u)\n%s",
+                       parent_snapshot,
+                       (bch2_bkey_val_to_text(&buf, c, d.s_c), buf.buf))) {
+               u32 new_parent_subvol;
+               ret = find_snapshot_subvol(trans, d.k->p.snapshot, &new_parent_subvol);
                if (ret)
                        goto err;
 
-               bkey_reassemble(&n->k_i, d.s_c);
-               n->v.d_parent_subvol = cpu_to_le32(target->bi_parent_subvol);
+               struct bkey_i_dirent *new_dirent = bch2_bkey_make_mut_typed(trans, iter, &d.s_c, 0, dirent);
+               ret = PTR_ERR_OR_ZERO(new_dirent);
+               if (ret)
+                       goto err;
 
-               ret = bch2_trans_update(trans, iter, &n->k_i, 0);
+               new_dirent->v.d_parent_subvol = cpu_to_le32(new_parent_subvol);
+       }
+
+       struct bkey_s_c_subvolume s =
+               bch2_bkey_get_iter_typed(trans, &subvol_iter,
+                                        BTREE_ID_subvolumes, POS(0, target_subvol),
+                                        0, subvolume);
+       ret = bkey_err(s.s_c);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               return ret;
+
+       if (ret) {
+               if (fsck_err(c, dirent_to_missing_subvol,
+                            "dirent points to missing subvolume\n%s",
+                            (bch2_bkey_val_to_text(&buf, c, d.s_c), buf.buf)))
+                       return __remove_dirent(trans, d.k->p);
+               ret = 0;
+               goto out;
+       }
+
+       if (fsck_err_on(le32_to_cpu(s.v->fs_path_parent) != parent_subvol,
+                       c, subvol_fs_path_parent_wrong,
+                       "subvol with wrong fs_path_parent, should be be %u\n%s",
+                       parent_subvol,
+                       (bch2_bkey_val_to_text(&buf, c, s.s_c), buf.buf))) {
+               struct bkey_i_subvolume *n =
+                       bch2_bkey_make_mut_typed(trans, &subvol_iter, &s.s_c, 0, subvolume);
+               ret = PTR_ERR_OR_ZERO(n);
                if (ret)
                        goto err;
 
-               d = dirent_i_to_s_c(n);
+               n->v.fs_path_parent = le32_to_cpu(parent_subvol);
        }
+
+       u64 target_inum = le64_to_cpu(s.v->inode);
+       u32 target_snapshot = le32_to_cpu(s.v->snapshot);
+
+       ret = lookup_inode(trans, target_inum, &subvol_root, &target_snapshot);
+       if (ret && !bch2_err_matches(ret, ENOENT))
+               return ret;
+
+       if (fsck_err_on(parent_subvol != subvol_root.bi_parent_subvol,
+                       c, inode_bi_parent_wrong,
+                       "subvol root %llu has wrong bi_parent_subvol: got %u, should be %u",
+                       target_inum,
+                       subvol_root.bi_parent_subvol, parent_subvol)) {
+               subvol_root.bi_parent_subvol = parent_subvol;
+               ret = __bch2_fsck_write_inode(trans, &subvol_root, target_snapshot);
+               if (ret)
+                       return ret;
+       }
+
+       ret = check_dirent_target(trans, iter, d, &subvol_root,
+                                 target_snapshot);
+       if (ret)
+               return ret;
 out:
 err:
 fsck_err:
-       bch2_trans_iter_exit(trans, &bp_iter);
+       bch2_trans_iter_exit(trans, &subvol_iter);
        printbuf_exit(&buf);
-       bch_err_fn(c, ret);
        return ret;
 }
 
@@ -1670,7 +1880,7 @@ static int check_dirent(struct btree_trans *trans, struct btree_iter *iter,
 
        BUG_ON(!btree_iter_path(trans, iter)->should_be_locked);
 
-       i = walk_inode(trans, dir, equiv, k.k->type == KEY_TYPE_whiteout);
+       i = walk_inode(trans, dir, k);
        ret = PTR_ERR_OR_ZERO(i);
        if (ret < 0)
                goto err;
@@ -1716,50 +1926,7 @@ static int check_dirent(struct btree_trans *trans, struct btree_iter *iter,
        d = bkey_s_c_to_dirent(k);
 
        if (d.v->d_type == DT_SUBVOL) {
-               struct bch_inode_unpacked subvol_root;
-               u32 target_subvol = le32_to_cpu(d.v->d_child_subvol);
-               u32 target_snapshot;
-               u64 target_inum;
-
-               ret = __subvol_lookup(trans, target_subvol,
-                                     &target_snapshot, &target_inum);
-               if (ret && !bch2_err_matches(ret, ENOENT))
-                       goto err;
-
-               if (fsck_err_on(ret, c, dirent_to_missing_subvol,
-                               "dirent points to missing subvolume %u",
-                               le32_to_cpu(d.v->d_child_subvol))) {
-                       ret = __remove_dirent(trans, d.k->p);
-                       goto err;
-               }
-
-               ret = __lookup_inode(trans, target_inum,
-                                  &subvol_root, &target_snapshot);
-               if (ret && !bch2_err_matches(ret, ENOENT))
-                       goto err;
-
-               if (fsck_err_on(ret, c, subvol_to_missing_root,
-                               "subvolume %u points to missing subvolume root %llu",
-                               target_subvol,
-                               target_inum)) {
-                       bch_err(c, "repair not implemented yet");
-                       ret = -EINVAL;
-                       goto err;
-               }
-
-               if (fsck_err_on(subvol_root.bi_subvol != target_subvol,
-                               c, subvol_root_wrong_bi_subvol,
-                               "subvol root %llu has wrong bi_subvol field: got %u, should be %u",
-                               target_inum,
-                               subvol_root.bi_subvol, target_subvol)) {
-                       subvol_root.bi_subvol = target_subvol;
-                       ret = __write_inode(trans, &subvol_root, target_snapshot);
-                       if (ret)
-                               goto err;
-               }
-
-               ret = check_dirent_target(trans, iter, d, &subvol_root,
-                                         target_snapshot);
+               ret = check_dirent_to_subvol(trans, iter, d);
                if (ret)
                        goto err;
        } else {
@@ -1785,12 +1952,11 @@ static int check_dirent(struct btree_trans *trans, struct btree_iter *iter,
                        if (ret)
                                goto err;
                }
-       }
-
-       if (d.v->d_type == DT_DIR)
-               for_each_visible_inode(c, s, dir, equiv.snapshot, i)
-                       i->count++;
 
+               if (d.v->d_type == DT_DIR)
+                       for_each_visible_inode(c, s, dir, equiv.snapshot, i)
+                               i->count++;
+       }
 out:
 err:
 fsck_err:
@@ -1841,7 +2007,7 @@ static int check_xattr(struct btree_trans *trans, struct btree_iter *iter,
        if (ret)
                return ret;
 
-       i = walk_inode(trans, inode, k.k->p, k.k->type == KEY_TYPE_whiteout);
+       i = walk_inode(trans, inode, k);
        ret = PTR_ERR_OR_ZERO(i);
        if (ret)
                return ret;
@@ -1893,7 +2059,7 @@ static int check_root_trans(struct btree_trans *trans)
        u64 inum;
        int ret;
 
-       ret = __subvol_lookup(trans, BCACHEFS_ROOT_SUBVOL, &snapshot, &inum);
+       ret = subvol_lookup(trans, BCACHEFS_ROOT_SUBVOL, &snapshot, &inum);
        if (ret && !bch2_err_matches(ret, ENOENT))
                return ret;
 
@@ -1915,7 +2081,7 @@ static int check_root_trans(struct btree_trans *trans)
                        goto err;
        }
 
-       ret = __lookup_inode(trans, BCACHEFS_ROOT_INO, &root_inode, &snapshot);
+       ret = lookup_inode(trans, BCACHEFS_ROOT_INO, &root_inode, &snapshot);
        if (ret && !bch2_err_matches(ret, ENOENT))
                return ret;
 
@@ -1928,7 +2094,7 @@ static int check_root_trans(struct btree_trans *trans)
                                0, NULL);
                root_inode.bi_inum = inum;
 
-               ret = __write_inode(trans, &root_inode, snapshot);
+               ret = __bch2_fsck_write_inode(trans, &root_inode, snapshot);
                bch_err_msg(c, ret, "writing root inode");
        }
 err:
@@ -1980,62 +2146,52 @@ static int path_down(struct bch_fs *c, pathbuf *p,
  *
  * XXX: we should also be verifying that inodes are in the right subvolumes
  */
-static int check_path(struct btree_trans *trans,
-                     pathbuf *p,
-                     struct bch_inode_unpacked *inode,
-                     u32 snapshot)
+static int check_path(struct btree_trans *trans, pathbuf *p, struct bkey_s_c inode_k)
 {
        struct bch_fs *c = trans->c;
+       struct btree_iter inode_iter = {};
+       struct bch_inode_unpacked inode;
+       struct printbuf buf = PRINTBUF;
+       u32 snapshot = bch2_snapshot_equiv(c, inode_k.k->p.snapshot);
        int ret = 0;
 
-       snapshot = bch2_snapshot_equiv(c, snapshot);
        p->nr = 0;
 
-       while (!(inode->bi_inum == BCACHEFS_ROOT_INO &&
-                inode->bi_subvol == BCACHEFS_ROOT_SUBVOL)) {
+       BUG_ON(bch2_inode_unpack(inode_k, &inode));
+
+       while (!(inode.bi_inum == BCACHEFS_ROOT_INO &&
+                inode.bi_subvol == BCACHEFS_ROOT_SUBVOL)) {
                struct btree_iter dirent_iter;
                struct bkey_s_c_dirent d;
                u32 parent_snapshot = snapshot;
 
-               if (inode->bi_subvol) {
-                       u64 inum;
-
-                       ret = subvol_lookup(trans, inode->bi_parent_subvol,
-                                           &parent_snapshot, &inum);
-                       if (ret)
-                               break;
-               }
-
-               ret = lockrestart_do(trans,
-                       PTR_ERR_OR_ZERO((d = dirent_get_by_pos(trans, &dirent_iter,
-                                         SPOS(inode->bi_dir, inode->bi_dir_offset,
-                                              parent_snapshot))).k));
+               d = inode_get_dirent(trans, &dirent_iter, &inode, &parent_snapshot);
+               ret = bkey_err(d.s_c);
                if (ret && !bch2_err_matches(ret, ENOENT))
                        break;
 
-               if (!ret && !dirent_points_to_inode(d, inode)) {
+               if (!ret && !dirent_points_to_inode(d, &inode)) {
                        bch2_trans_iter_exit(trans, &dirent_iter);
                        ret = -BCH_ERR_ENOENT_dirent_doesnt_match_inode;
                }
 
                if (bch2_err_matches(ret, ENOENT)) {
-                       if (fsck_err(c,  inode_unreachable,
-                                    "unreachable inode %llu:%u, type %s nlink %u backptr %llu:%llu",
-                                    inode->bi_inum, snapshot,
-                                    bch2_d_type_str(inode_d_type(inode)),
-                                    inode->bi_nlink,
-                                    inode->bi_dir,
-                                    inode->bi_dir_offset))
-                               ret = reattach_inode(trans, inode, snapshot);
-                       break;
+                       ret = 0;
+                       if (fsck_err(c, inode_unreachable,
+                                    "unreachable inode\n%s",
+                                    (printbuf_reset(&buf),
+                                     bch2_bkey_val_to_text(&buf, c, inode_k),
+                                     buf.buf)))
+                               ret = reattach_inode(trans, &inode, snapshot);
+                       goto out;
                }
 
                bch2_trans_iter_exit(trans, &dirent_iter);
 
-               if (!S_ISDIR(inode->bi_mode))
+               if (!S_ISDIR(inode.bi_mode))
                        break;
 
-               ret = path_down(c, p, inode->bi_inum, snapshot);
+               ret = path_down(c, p, inode.bi_inum, snapshot);
                if (ret) {
                        bch_err(c, "memory allocation failure");
                        return ret;
@@ -2043,37 +2199,48 @@ static int check_path(struct btree_trans *trans,
 
                snapshot = parent_snapshot;
 
-               ret = lookup_inode(trans, inode->bi_dir, inode, &snapshot);
+               bch2_trans_iter_exit(trans, &inode_iter);
+               inode_k = bch2_bkey_get_iter(trans, &inode_iter, BTREE_ID_inodes,
+                                            SPOS(0, inode.bi_dir, snapshot), 0);
+               ret = bkey_err(inode_k) ?:
+                       !bkey_is_inode(inode_k.k) ? -BCH_ERR_ENOENT_inode
+                       : bch2_inode_unpack(inode_k, &inode);
                if (ret) {
                        /* Should have been caught in dirents pass */
-                       bch_err(c, "error looking up parent directory: %i", ret);
+                       if (!bch2_err_matches(ret, BCH_ERR_transaction_restart))
+                               bch_err(c, "error looking up parent directory: %i", ret);
                        break;
                }
 
-               if (path_is_dup(p, inode->bi_inum, snapshot)) {
+               snapshot = inode_k.k->p.snapshot;
+
+               if (path_is_dup(p, inode.bi_inum, snapshot)) {
                        /* XXX print path */
                        bch_err(c, "directory structure loop");
 
                        darray_for_each(*p, i)
                                pr_err("%llu:%u", i->inum, i->snapshot);
-                       pr_err("%llu:%u", inode->bi_inum, snapshot);
+                       pr_err("%llu:%u", inode.bi_inum, snapshot);
 
-                       if (!fsck_err(c, dir_loop,
-                                     "directory structure loop"))
+                       if (!fsck_err(c, dir_loop, "directory structure loop"))
                                return 0;
 
-                       ret = commit_do(trans, NULL, NULL,
-                                             BCH_TRANS_COMMIT_no_enospc,
-                                       remove_backpointer(trans, inode));
-                       if (ret) {
-                               bch_err(c, "error removing dirent: %i", ret);
+                       ret = remove_backpointer(trans, &inode);
+                       if (ret && !bch2_err_matches(ret, BCH_ERR_transaction_restart))
+                               bch_err_msg(c, ret, "removing dirent");
+                       if (ret)
                                break;
-                       }
 
-                       ret = reattach_inode(trans, inode, snapshot);
+                       ret = reattach_inode(trans, &inode, snapshot);
+                       if (ret && !bch2_err_matches(ret, BCH_ERR_transaction_restart))
+                               bch_err_msg(c, ret, "reattaching inode %llu", inode.bi_inum);
+                       break;
                }
        }
+out:
 fsck_err:
+       bch2_trans_iter_exit(trans, &inode_iter);
+       printbuf_exit(&buf);
        bch_err_fn(c, ret);
        return ret;
 }
@@ -2085,31 +2252,23 @@ fsck_err:
  */
 int bch2_check_directory_structure(struct bch_fs *c)
 {
-       struct btree_trans *trans = bch2_trans_get(c);
-       struct btree_iter iter;
-       struct bkey_s_c k;
-       struct bch_inode_unpacked u;
        pathbuf path = { 0, };
        int ret;
 
-       for_each_btree_key_old(trans, iter, BTREE_ID_inodes, POS_MIN,
-                              BTREE_ITER_INTENT|
-                              BTREE_ITER_PREFETCH|
-                              BTREE_ITER_ALL_SNAPSHOTS, k, ret) {
-               if (!bkey_is_inode(k.k))
-                       continue;
-
-               BUG_ON(bch2_inode_unpack(k, &u));
+       ret = bch2_trans_run(c,
+               for_each_btree_key_commit(trans, iter, BTREE_ID_inodes, POS_MIN,
+                                         BTREE_ITER_INTENT|
+                                         BTREE_ITER_PREFETCH|
+                                         BTREE_ITER_ALL_SNAPSHOTS, k,
+                                         NULL, NULL, BCH_TRANS_COMMIT_no_enospc, ({
+                       if (!bkey_is_inode(k.k))
+                               continue;
 
-               if (u.bi_flags & BCH_INODE_unlinked)
-                       continue;
+                       if (bch2_inode_flags(k) & BCH_INODE_unlinked)
+                               continue;
 
-               ret = check_path(trans, &path, &u, iter.pos.snapshot);
-               if (ret)
-                       break;
-       }
-       bch2_trans_iter_exit(trans, &iter);
-       bch2_trans_put(trans);
+                       check_path(trans, &path, k);
+               })));
        darray_exit(&path);
 
        bch_err_fn(c, ret);
@@ -2304,7 +2463,7 @@ static int check_nlinks_update_inode(struct btree_trans *trans, struct btree_ite
                        u.bi_inum, bch2_d_types[mode_to_type(u.bi_mode)],
                        bch2_inode_nlink_get(&u), link->count)) {
                bch2_inode_nlink_set(&u, link->count);
-               ret = __write_inode(trans, &u, k.k->p.snapshot);
+               ret = __bch2_fsck_write_inode(trans, &u, k.k->p.snapshot);
        }
 fsck_err:
        return ret;