On 2/6/20 11:26 AM, David Sterba wrote:
On Fri, Jan 24, 2020 at 09:32:44AM -0500, Josef Bacik wrote:@@ -4593,6 +4593,10 @@ int btrfs_recover_relocation(struct btrfs_root *root) if (btrfs_root_refs(&reloc_root->root_item) > 0) { fs_root = read_fs_root(fs_info, reloc_root->root_key.offset); + if (!btrfs_grab_fs_root(fs_root)) { + err = -ENOENT; + goto out; + } if (IS_ERR(fs_root)) { ret = PTR_ERR(fs_root); if (ret != -ENOENT) { @@ -4604,6 +4608,8 @@ int btrfs_recover_relocation(struct btrfs_root *root) err = ret; goto out; } + } else { + btrfs_put_fs_root(fs_root); } }The order of IS_ERR and btrfs_grab_fs_root is reversed but then it looks strange: 4637 fs_root = read_fs_root(fs_info, _ 4638 reloc_root->root_key.offset); 4639 if (IS_ERR(fs_root)) { 4640 ret = PTR_ERR(fs_root); 4641 if (ret != -ENOENT) { 4642 err = ret; 4643 goto out; 4644 } 4645 ret = mark_garbage_root(reloc_root); 4646 if (ret < 0) { 4647 err = ret; 4648 goto out; 4649 } 4650 } else { + 4651 if (!btrfs_grab_fs_root(fs_root)) { + 4652 err = -ENOENT; + 4653 goto out; + 4654 } 4655 btrfs_put_fs_root(fs_root); 4656 } 4657 } Seems that the refcounting is not necessary here at all, it just tries to read the fs root and handle errors if it does not exist, no operation that would want to keep the fs_root.
Yeah we aren't actually using the root here, so strictly speaking we don't need the refcount. But in the future read_fs_root() will return a root with a refcount so we will still have to clean it up. Thanks,
Josef
