| Message ID | 20200304161830.2360-4-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | relocation error handling fixes | expand |
On 2020/3/5 上午12:18, Josef Bacik wrote: > If we fail to load an fs root, or fail to start a transaction we can > bail without unsetting the reloc control, which leads to problems later > when we free the reloc control but still have it attached to the file > system. > > In the normal path we'll end up calling unset_reloc_control() twice, but > all it does is set fs_info->reloc_control = NULL, and we can only have > one balance at a time so it's not racey. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/relocation.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index 2141519a9dd0..c496f8ed8c7e 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -4657,9 +4657,8 @@ int btrfs_recover_relocation(struct btrfs_root *root) > > trans = btrfs_join_transaction(rc->extent_root); > if (IS_ERR(trans)) { > - unset_reloc_control(rc); > err = PTR_ERR(trans); > - goto out_free; > + goto out_unset; > } > > rc->merge_reloc_tree = 1; > @@ -4679,7 +4678,7 @@ int btrfs_recover_relocation(struct btrfs_root *root) > if (IS_ERR(fs_root)) { > err = PTR_ERR(fs_root); > list_add_tail(&reloc_root->root_list, &reloc_roots); > - goto out_free; > + goto out_unset; > } > > err = __add_reloc_root(reloc_root); > @@ -4690,7 +4689,7 @@ int btrfs_recover_relocation(struct btrfs_root *root) > > err = btrfs_commit_transaction(trans); > if (err) > - goto out_free; > + goto out_unset; > > merge_reloc_roots(rc); > > @@ -4706,6 +4705,8 @@ int btrfs_recover_relocation(struct btrfs_root *root) > ret = clean_dirty_subvols(rc); > if (ret < 0 && !err) > err = ret; > +out_unset: > + unset_reloc_control(rc); > out_free: > kfree(rc); > out: >
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 2141519a9dd0..c496f8ed8c7e 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -4657,9 +4657,8 @@ int btrfs_recover_relocation(struct btrfs_root *root) trans = btrfs_join_transaction(rc->extent_root); if (IS_ERR(trans)) { - unset_reloc_control(rc); err = PTR_ERR(trans); - goto out_free; + goto out_unset; } rc->merge_reloc_tree = 1; @@ -4679,7 +4678,7 @@ int btrfs_recover_relocation(struct btrfs_root *root) if (IS_ERR(fs_root)) { err = PTR_ERR(fs_root); list_add_tail(&reloc_root->root_list, &reloc_roots); - goto out_free; + goto out_unset; } err = __add_reloc_root(reloc_root); @@ -4690,7 +4689,7 @@ int btrfs_recover_relocation(struct btrfs_root *root) err = btrfs_commit_transaction(trans); if (err) - goto out_free; + goto out_unset; merge_reloc_roots(rc); @@ -4706,6 +4705,8 @@ int btrfs_recover_relocation(struct btrfs_root *root) ret = clean_dirty_subvols(rc); if (ret < 0 && !err) err = ret; +out_unset: + unset_reloc_control(rc); out_free: kfree(rc); out:
If we fail to load an fs root, or fail to start a transaction we can bail without unsetting the reloc control, which leads to problems later when we free the reloc control but still have it attached to the file system. In the normal path we'll end up calling unset_reloc_control() twice, but all it does is set fs_info->reloc_control = NULL, and we can only have one balance at a time so it's not racey. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)