| Message ID | 20220714104810.2733591-1-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] btrfs: simplify error handling in btrfs_lookup_dentry | expand |
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Thu, Jul 14, 2022 at 01:48:10PM +0300, Nikolay Borisov wrote: > In btrfs_lookup_dentry releasing the reference of the sub_root and the > running orphan cleanup should only happen if the dentry found actually > represents a subvolume. This can only be true in the 'else' branch as > otherwise either fixup_tree_root_location returned an ENOENT error, in > which case sub_root wouldn't have been changed or if we got a different > errno this means btrfs_get_fs_root couldn't have executed successfully > again meaning sub_root will equal to root. So simplify all the branches > by moving the code into the 'else'. > > Signed-off-by: Nikolay Borisov <nborisov@suse.com> Thanks, I find this version more comprehensible than v1, added to misc-next.
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 0b17335555e0..ee874ca90041 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -5818,14 +5818,14 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry) if (ret != -ENOENT) inode = ERR_PTR(ret); else - inode = new_simple_dir(dir->i_sb, &location, sub_root); + inode = new_simple_dir(dir->i_sb, &location, root); } else { inode = btrfs_iget(dir->i_sb, location.objectid, sub_root); - } - if (root != sub_root) btrfs_put_root(sub_root); - if (!IS_ERR(inode) && root != sub_root) { + if (IS_ERR(inode)) + return inode; + down_read(&fs_info->cleanup_work_sem); if (!sb_rdonly(inode->i_sb)) ret = btrfs_orphan_cleanup(sub_root);
In btrfs_lookup_dentry releasing the reference of the sub_root and the running orphan cleanup should only happen if the dentry found actually represents a subvolume. This can only be true in the 'else' branch as otherwise either fixup_tree_root_location returned an ENOENT error, in which case sub_root wouldn't have been changed or if we got a different errno this means btrfs_get_fs_root couldn't have executed successfully again meaning sub_root will equal to root. So simplify all the branches by moving the code into the 'else'. Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- V2: - Replace sub_root with root parameter when calling new_dimple_dir since in this particular branch sub_root is always guaranteed to be equal to root - Properly handle error from btrfs_iget. fs/btrfs/inode.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) -- 2.25.1