bcachefs: Run may_delete_deleted_inode() checks in bch2_inode_rm()

We had a bug where bch2_evict_inode() incorrectly called bch2_inode_rm()
- the journal clearly showed the inode was not unlinked.

We've got checks that we use in recovery when cleaning up deleted
inodes, lift them to bch2_inode_rm() as well.

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

diff --git a/fs/bcachefs/errcode.h b/fs/bcachefs/errcode.h
index 6e0bf7ff2641a005dfa36b63cc917d95e8543ab5..ac3264134a15a168090a651366e4ba4f43c5b2f6 100644 (file)
--- a/fs/bcachefs/errcode.h
+++ b/fs/bcachefs/errcode.h
@@ -214,6 +214,8 @@
        x(EINVAL,                       remove_would_lose_data)                 \
        x(EINVAL,                       no_resize_with_buckets_nouse)           \
        x(EINVAL,                       inode_unpack_error)                     \
+       x(EINVAL,                       inode_not_unlinked)                     \
+       x(EINVAL,                       inode_has_child_snapshot)               \
        x(EINVAL,                       varint_decode_error)                    \
        x(EINVAL,                       erasure_coding_found_btree_node)        \
        x(EINVAL,                       option_negative)                        \

diff --git a/fs/bcachefs/fs.c b/fs/bcachefs/fs.c
index 41008d4a2b99bbaa74de999d16306e27776851cb..85d13f800165cd3863283d6a401d361e17aeae4e 100644 (file)
--- a/fs/bcachefs/fs.c
+++ b/fs/bcachefs/fs.c
@@ -2180,7 +2180,13 @@ static void bch2_evict_inode(struct inode *vinode)
                                KEY_TYPE_QUOTA_WARN);
                bch2_quota_acct(c, inode->ei_qid, Q_INO, -1,
                                KEY_TYPE_QUOTA_WARN);
-               bch2_inode_rm(c, inode_inum(inode));
+               int ret = bch2_inode_rm(c, inode_inum(inode));
+               if (ret && !bch2_err_matches(ret, EROFS)) {
+                       bch_err_msg(c, ret, "VFS incorrectly tried to delete inode %llu:%llu",
+                                   inode->ei_inum.subvol,
+                                   inode->ei_inum.inum);
+                       bch2_sb_error_count(c, BCH_FSCK_ERR_vfs_bad_inode_rm);
+               }
 
                /*
                 * If we are deleting, we need it present in the vfs hash table

diff --git a/fs/bcachefs/inode.c b/fs/bcachefs/inode.c
index e76721c11eef6f647ac3df46f13d87ad231f03a2..53e5dc1f6ac1884e4e045272966669ae9f06feb3 100644 (file)
--- a/fs/bcachefs/inode.c
+++ b/fs/bcachefs/inode.c
@@ -38,6 +38,7 @@ static const char * const bch2_inode_flag_strs[] = {
 #undef  x
 
 static int delete_ancestor_snapshot_inodes(struct btree_trans *, struct bpos);
+static int may_delete_deleted_inum(struct btree_trans *, subvol_inum);
 
 static const u8 byte_table[8] = { 1, 2, 3, 4, 6, 8, 10, 13 };
 
@@ -1130,19 +1131,23 @@ int bch2_inode_rm(struct bch_fs *c, subvol_inum inum)
        u32 snapshot;
        int ret;
 
+       ret = lockrestart_do(trans, may_delete_deleted_inum(trans, inum));
+       if (ret)
+               goto err2;
+
        /*
         * If this was a directory, there shouldn't be any real dirents left -
         * but there could be whiteouts (from hash collisions) that we should
         * delete:
         *
-        * XXX: the dirent could ideally would delete whiteouts when they're no
+        * XXX: the dirent code ideally would delete whiteouts when they're no
         * longer needed
         */
        ret   = bch2_inode_delete_keys(trans, inum, BTREE_ID_extents) ?:
                bch2_inode_delete_keys(trans, inum, BTREE_ID_xattrs) ?:
                bch2_inode_delete_keys(trans, inum, BTREE_ID_dirents);
        if (ret)
-               goto err;
+               goto err2;
 retry:
        bch2_trans_begin(trans);
 
@@ -1392,7 +1397,8 @@ int bch2_inode_rm_snapshot(struct btree_trans *trans, u64 inum, u32 snapshot)
                delete_ancestor_snapshot_inodes(trans, SPOS(0, inum, snapshot));
 }
 
-static int may_delete_deleted_inode(struct btree_trans *trans, struct bpos pos)
+static int may_delete_deleted_inode(struct btree_trans *trans, struct bpos pos,
+                                   bool from_deleted_inodes)
 {
        struct bch_fs *c = trans->c;
        struct btree_iter inode_iter;
@@ -1406,12 +1412,14 @@ static int may_delete_deleted_inode(struct btree_trans *trans, struct bpos pos)
        if (ret)
                return ret;
 
-       ret = bkey_is_inode(k.k) ? 0 : -BCH_ERR_ENOENT_inode;
-       if (fsck_err_on(!bkey_is_inode(k.k),
+       ret = bkey_is_inode(k.k) ? 0 : bch_err_throw(c, ENOENT_inode);
+       if (fsck_err_on(from_deleted_inodes && ret,
                        trans, deleted_inode_missing,
                        "nonexistent inode %llu:%u in deleted_inodes btree",
                        pos.offset, pos.snapshot))
                goto delete;
+       if (ret)
+               goto out;
 
        ret = bch2_inode_unpack(k, &inode);
        if (ret)
@@ -1419,7 +1427,8 @@ static int may_delete_deleted_inode(struct btree_trans *trans, struct bpos pos)
 
        if (S_ISDIR(inode.bi_mode)) {
                ret = bch2_empty_dir_snapshot(trans, pos.offset, 0, pos.snapshot);
-               if (fsck_err_on(bch2_err_matches(ret, ENOTEMPTY),
+               if (fsck_err_on(from_deleted_inodes &&
+                               bch2_err_matches(ret, ENOTEMPTY),
                                trans, deleted_inode_is_dir,
                                "non empty directory %llu:%u in deleted_inodes btree",
                                pos.offset, pos.snapshot))
@@ -1428,17 +1437,25 @@ static int may_delete_deleted_inode(struct btree_trans *trans, struct bpos pos)
                        goto out;
        }
 
-       if (fsck_err_on(!(inode.bi_flags & BCH_INODE_unlinked),
+       ret = inode.bi_flags & BCH_INODE_unlinked ? 0 : bch_err_throw(c, inode_not_unlinked);
+       if (fsck_err_on(from_deleted_inodes && ret,
                        trans, deleted_inode_not_unlinked,
                        "non-deleted inode %llu:%u in deleted_inodes btree",
                        pos.offset, pos.snapshot))
                goto delete;
+       if (ret)
+               goto out;
+
+       ret = !(inode.bi_flags & BCH_INODE_has_child_snapshot)
+               ? 0 : bch_err_throw(c, inode_has_child_snapshot);
 
-       if (fsck_err_on(inode.bi_flags & BCH_INODE_has_child_snapshot,
+       if (fsck_err_on(from_deleted_inodes && ret,
                        trans, deleted_inode_has_child_snapshots,
                        "inode with child snapshots %llu:%u in deleted_inodes btree",
                        pos.offset, pos.snapshot))
                goto delete;
+       if (ret)
+               goto out;
 
        ret = bch2_inode_has_child_snapshots(trans, k.k->p);
        if (ret < 0)
@@ -1455,19 +1472,28 @@ static int may_delete_deleted_inode(struct btree_trans *trans, struct bpos pos)
                        if (ret)
                                goto out;
                }
+
+               if (!from_deleted_inodes) {
+                       ret =   bch2_trans_commit(trans, NULL, NULL, BCH_TRANS_COMMIT_no_enospc) ?:
+                               bch_err_throw(c, inode_has_child_snapshot);
+                       goto out;
+               }
+
                goto delete;
 
        }
 
-       if (test_bit(BCH_FS_clean_recovery, &c->flags) &&
-           !fsck_err(trans, deleted_inode_but_clean,
-                     "filesystem marked as clean but have deleted inode %llu:%u",
-                     pos.offset, pos.snapshot)) {
-               ret = 0;
-               goto out;
-       }
+       if (from_deleted_inodes) {
+               if (test_bit(BCH_FS_clean_recovery, &c->flags) &&
+                   !fsck_err(trans, deleted_inode_but_clean,
+                             "filesystem marked as clean but have deleted inode %llu:%u",
+                             pos.offset, pos.snapshot)) {
+                       ret = 0;
+                       goto out;
+               }
 
-       ret = 1;
+               ret = 1;
+       }
 out:
 fsck_err:
        bch2_trans_iter_exit(trans, &inode_iter);
@@ -1478,6 +1504,14 @@ delete:
        goto out;
 }
 
+static int may_delete_deleted_inum(struct btree_trans *trans, subvol_inum inum)
+{
+       u32 snapshot;
+
+       return bch2_subvolume_get_snapshot(trans, inum.subvol, &snapshot) ?:
+               may_delete_deleted_inode(trans, SPOS(0, inum.inum, snapshot), false);
+}
+
 int bch2_delete_dead_inodes(struct bch_fs *c)
 {
        struct btree_trans *trans = bch2_trans_get(c);
@@ -1501,7 +1535,7 @@ int bch2_delete_dead_inodes(struct bch_fs *c)
        ret = for_each_btree_key_commit(trans, iter, BTREE_ID_deleted_inodes, POS_MIN,
                                        BTREE_ITER_prefetch|BTREE_ITER_all_snapshots, k,
                                        NULL, NULL, BCH_TRANS_COMMIT_no_enospc, ({
-               ret = may_delete_deleted_inode(trans, k.k->p);
+               ret = may_delete_deleted_inode(trans, k.k->p, true);
                if (ret > 0) {
                        bch_verbose_ratelimited(c, "deleting unlinked inode %llu:%u",
                                                k.k->p.offset, k.k->p.snapshot);

diff --git a/fs/bcachefs/sb-errors_format.h b/fs/bcachefs/sb-errors_format.h
index 8999dab1ff9bb4ce3dab9dbacb10597acd5e8f53..6fdbf265e4c0fd6f1df859dea8340d004ff8fe7c 100644 (file)
--- a/fs/bcachefs/sb-errors_format.h
+++ b/fs/bcachefs/sb-errors_format.h
@@ -244,6 +244,7 @@ enum bch_fsck_flags {
        x(inode_parent_has_case_insensitive_not_set,            317,    FSCK_AUTOFIX)   \
        x(vfs_inode_i_blocks_underflow,                         311,    FSCK_AUTOFIX)   \
        x(vfs_inode_i_blocks_not_zero_at_truncate,              313,    FSCK_AUTOFIX)   \
+       x(vfs_bad_inode_rm,                                     320,    0)              \
        x(deleted_inode_but_clean,                              211,    FSCK_AUTOFIX)   \
        x(deleted_inode_missing,                                212,    FSCK_AUTOFIX)   \
        x(deleted_inode_is_dir,                                 213,    FSCK_AUTOFIX)   \
@@ -329,7 +330,7 @@ enum bch_fsck_flags {
        x(dirent_stray_data_after_cf_name,                      305,    0)              \
        x(rebalance_work_incorrectly_set,                       309,    FSCK_AUTOFIX)   \
        x(rebalance_work_incorrectly_unset,                     310,    FSCK_AUTOFIX)   \
-       x(MAX,                                                  320,    0)
+       x(MAX,                                                  321,    0)
 
 enum bch_sb_error_id {
 #define x(t, n, ...) BCH_FSCK_ERR_##t = n,