Re: [PATCH] btrfs: check error value from btrfs_update_inode in tree log
From: Qu Wenruo <hidden>
Date: 2021-05-20 01:07:36
On 2021/5/19 下午11:26, Josef Bacik wrote:
Error injection testing uncovered a case where we ended up with invalid link counts on an inode. This happened because we failed to notice an error when updating the inode while replaying the tree log, and committed the transaction with an invalid file system. Fix this by checking the return value of btrfs_update_inode. This resolved the link count errors I was seeing, and we already properly handle passing up the error values in these paths. Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Qu Wenruo <redacted> But one thing unrelated to notice, inlined below.
quoted hunk ↗ jump to hunk
--- fs/btrfs/tree-log.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index 326be57f2828..4dc74949040d 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c@@ -1574,7 +1574,9 @@ static noinline int add_inode_ref(struct btrfs_trans_handle *trans, if (ret) goto out; - btrfs_update_inode(trans, root, BTRFS_I(inode));
I did a quick grep and found that we have other locations where we call
btrfs_uppdate_inode() without catching the return value:
$ grep -IRe "^\s\+btrfs_update_inode(" fs/btrfs/
fs/btrfs/free-space-cache.c: btrfs_update_inode(trans, root,
BTRFS_I(inode));
fs/btrfs/free-space-cache.c: btrfs_update_inode(trans, root,
BTRFS_I(inode));
fs/btrfs/inode.c: btrfs_update_inode(trans, root, inode);
fs/btrfs/inode.c: btrfs_update_inode(trans, root, BTRFS_I(inode));
Maybe it's better to make btrfs_update_inode() to have __must_check prefix?
Thanks,
Qu
quoted hunk ↗ jump to hunk
+ ret = btrfs_update_inode(trans, root, BTRFS_I(inode)); + if (ret) + goto out; } ref_ptr = (unsigned long)(ref_ptr + ref_struct_size) + namelen;@@ -1749,7 +1751,9 @@ static noinline int fixup_inode_link_count(struct btrfs_trans_handle *trans, if (nlink != inode->i_nlink) { set_nlink(inode, nlink); - btrfs_update_inode(trans, root, BTRFS_I(inode)); + ret = btrfs_update_inode(trans, root, BTRFS_I(inode)); + if (ret) + goto out; } BTRFS_I(inode)->index_cnt = (u64)-1;