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.
