bcachefs: Refactor bch2_check_dirent_target()

Prep work for calling bch2_check_dirent_target() from bch2_lookup().

- Add an inline wrapper, if the target and backpointer match we can skip
  the function call.

- We don't (yet?) want to remove the dirent we did the lookup from (when
  we find a directory or subvol with multiple valid dirents pointing to
  it), we can defer on that until later. For now, add an "are we in
  fsck?" parameter.

Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>

diff --git a/fs/bcachefs/fsck.c b/fs/bcachefs/fsck.c
index f3853b741937a45c498ddb9dca6a4c460e903c31..091057023fc5b3aeb5d8ba8f84543d0d086d0b4b 100644 (file)
--- a/fs/bcachefs/fsck.c
+++ b/fs/bcachefs/fsck.c
@@ -2072,7 +2072,7 @@ static int check_dirent_to_subvol(struct btree_trans *trans, struct btree_iter *
                        goto err;
        }
 
-       ret = bch2_check_dirent_target(trans, iter, d, &subvol_root);
+       ret = bch2_check_dirent_target(trans, iter, d, &subvol_root, true);
        if (ret)
                goto err;
 out:
@@ -2165,7 +2165,7 @@ static int check_dirent(struct btree_trans *trans, struct btree_iter *iter,
                }
 
                darray_for_each(target->inodes, i) {
-                       ret = bch2_check_dirent_target(trans, iter, d, &i->inode);
+                       ret = bch2_check_dirent_target(trans, iter, d, &i->inode, true);
                        if (ret)
                                goto err;
                }

diff --git a/fs/bcachefs/inode.h b/fs/bcachefs/inode.h
index 428b9be6af34b85e605aa10462286d3ebd5976a6..f82cfbf460d094c0557e16589410ab62bac5fd4c 100644 (file)
--- a/fs/bcachefs/inode.h
+++ b/fs/bcachefs/inode.h
@@ -277,6 +277,7 @@ static inline bool bch2_inode_should_have_single_bp(struct bch_inode_unpacked *i
        bool inode_has_bp = inode->bi_dir || inode->bi_dir_offset;
 
        return S_ISDIR(inode->bi_mode) ||
+               inode->bi_subvol ||
                (!inode->bi_nlink && inode_has_bp);
 }
 

diff --git a/fs/bcachefs/namei.c b/fs/bcachefs/namei.c
index 4d0ee85e501631c5859ced9c3bf59e8640dc92a9..93246ad315417e0c237c11fe8ceef88740c9aba8 100644 (file)
--- a/fs/bcachefs/namei.c
+++ b/fs/bcachefs/namei.c
@@ -659,17 +659,10 @@ disconnected:
 
 /* fsck */
 
-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 int bch2_check_dirent_inode_dirent(struct btree_trans *trans,
-                                  struct btree_iter *iter,
-                                  struct bkey_s_c_dirent d,
-                                  struct bch_inode_unpacked *target)
+                                         struct bkey_s_c_dirent d,
+                                         struct bch_inode_unpacked *target,
+                                         bool in_fsck)
 {
        struct bch_fs *c = trans->c;
        struct printbuf buf = PRINTBUF;
@@ -725,52 +718,65 @@ static int bch2_check_dirent_inode_dirent(struct btree_trans *trans,
        bool backpointer_exists = !ret;
        ret = 0;
 
-       if (fsck_err_on(!backpointer_exists,
-                       trans, inode_wrong_backpointer,
-                       "inode %llu:%u has wrong backpointer:\n"
-                       "got       %llu:%llu\n"
-                       "should be %llu:%llu",
-                       target->bi_inum, target->bi_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);
-               goto out;
-       }
-
-       bch2_bkey_val_to_text(&buf, c, d.s_c);
-       prt_newline(&buf);
-       if (backpointer_exists)
+       if (!backpointer_exists) {
+               if (fsck_err(trans, inode_wrong_backpointer,
+                            "inode %llu:%u has wrong backpointer:\n"
+                            "got       %llu:%llu\n"
+                            "should be %llu:%llu",
+                            target->bi_inum, target->bi_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);
+               }
+       } else {
+               bch2_bkey_val_to_text(&buf, c, d.s_c);
+               prt_newline(&buf);
                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),
-                       trans, inode_dir_multiple_links,
-                       "%s %llu:%u with multiple links\n%s",
-                       S_ISDIR(target->bi_mode) ? "directory" : "subvolume",
-                       target->bi_inum, target->bi_snapshot, buf.buf)) {
-               ret = bch2_fsck_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,
-                       trans, inode_multiple_links_but_nlink_0,
-                       "inode %llu:%u type %s has multiple links but i_nlink 0\n%s",
-                       target->bi_inum, target->bi_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);
-               if (ret)
-                       goto err;
+               if (S_ISDIR(target->bi_mode) || target->bi_subvol) {
+                       /*
+                        * XXX: verify connectivity of the other dirent
+                        * up to the root before removing this one
+                        *
+                        * Additionally, bch2_lookup would need to cope with the
+                        * dirent it found being removed - or should we remove
+                        * the other one, even though the inode points to it?
+                        */
+                       if (in_fsck) {
+                               if (fsck_err(trans, inode_dir_multiple_links,
+                                            "%s %llu:%u with multiple links\n%s",
+                                            S_ISDIR(target->bi_mode) ? "directory" : "subvolume",
+                                            target->bi_inum, target->bi_snapshot, buf.buf))
+                                       ret = bch2_fsck_remove_dirent(trans, d.k->p);
+                       } else {
+                               bch2_fs_inconsistent(c,
+                                               "%s %llu:%u with multiple links\n%s",
+                                               S_ISDIR(target->bi_mode) ? "directory" : "subvolume",
+                                               target->bi_inum, target->bi_snapshot, buf.buf);
+                       }
+
+                       goto out;
+               } else {
+                       /*
+                        * 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(!target->bi_nlink,
+                                       trans, inode_multiple_links_but_nlink_0,
+                                       "inode %llu:%u type %s has multiple links but i_nlink 0\n%s",
+                                       target->bi_inum, target->bi_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);
+                               if (ret)
+                                       goto err;
+                       }
+               }
        }
 out:
 err:
@@ -781,16 +787,17 @@ fsck_err:
        return ret;
 }
 
-int bch2_check_dirent_target(struct btree_trans *trans,
-                            struct btree_iter *iter,
-                            struct bkey_s_c_dirent d,
-                            struct bch_inode_unpacked *target)
+int __bch2_check_dirent_target(struct btree_trans *trans,
+                              struct btree_iter *dirent_iter,
+                              struct bkey_s_c_dirent d,
+                              struct bch_inode_unpacked *target,
+                              bool in_fsck)
 {
        struct bch_fs *c = trans->c;
        struct printbuf buf = PRINTBUF;
        int ret = 0;
 
-       ret = bch2_check_dirent_inode_dirent(trans, iter, d, target);
+       ret = bch2_check_dirent_inode_dirent(trans, d, target, in_fsck);
        if (ret)
                goto err;
 
@@ -815,11 +822,9 @@ int bch2_check_dirent_target(struct btree_trans *trans,
                        n->v.d_inum = cpu_to_le64(target->bi_inum);
                }
 
-               ret = bch2_trans_update(trans, iter, &n->k_i, 0);
+               ret = bch2_trans_update(trans, dirent_iter, &n->k_i, 0);
                if (ret)
                        goto err;
-
-               d = dirent_i_to_s_c(n);
        }
 err:
 fsck_err:

diff --git a/fs/bcachefs/namei.h b/fs/bcachefs/namei.h
index 48a2c8cb5fa9b3222337471c50a5dd0d5316f310..2e6f6364767ff1636a475e7670db07468f33db35 100644 (file)
--- a/fs/bcachefs/namei.h
+++ b/fs/bcachefs/namei.h
@@ -44,9 +44,29 @@ bool bch2_reinherit_attrs(struct bch_inode_unpacked *,
 
 int bch2_inum_to_path(struct btree_trans *, subvol_inum, struct printbuf *);
 
-int bch2_check_dirent_target(struct btree_trans *,
-                            struct btree_iter *,
-                            struct bkey_s_c_dirent,
-                            struct bch_inode_unpacked *);
+int __bch2_check_dirent_target(struct btree_trans *,
+                              struct btree_iter *,
+                              struct bkey_s_c_dirent,
+                              struct bch_inode_unpacked *, bool);
+
+static inline 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 inline int bch2_check_dirent_target(struct btree_trans *trans,
+                                          struct btree_iter *dirent_iter,
+                                          struct bkey_s_c_dirent d,
+                                          struct bch_inode_unpacked *target,
+                                          bool in_fsck)
+{
+       if (likely(inode_points_to_dirent(target, d) &&
+                  d.v->d_type == inode_d_type(target)))
+               return 0;
+
+       return __bch2_check_dirent_target(trans, dirent_iter, d, target, in_fsck);
+}
 
 #endif /* _BCACHEFS_NAMEI_H */