| Message ID | bf1e7b86cd6c3e7c2584cd785a7d4e595c1491e2.1606938211.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup error handling in relocation | expand |
On 2020/12/3 上午3:51, Josef Bacik wrote: > We have a BUG_ON() if we get an error back from btrfs_get_fs_root(). > This honestly should never fail, as at this point we have a solid > coordination of fs root to reloc root, and these roots will all be in > memory. But in the name of killing BUG_ON()'s remove this one and > handle the error properly. Change the remaining BUG_ON() to an > ASSERT(). > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/relocation.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index 91479979d2a7..099a64b47020 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -1949,9 +1949,18 @@ void merge_reloc_roots(struct reloc_control *rc) > > root = btrfs_get_fs_root(fs_info, reloc_root->root_key.offset, > false); > + if (IS_ERR(root)) { > + /* > + * This likely won't happen, since we would have failed > + * at a higher level. However for correctness sake > + * handle the error anyway. > + */ Maybe another ASSERT(0)? Despite that looks good. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > + ret = PTR_ERR(root); > + goto out; > + } > + > if (btrfs_root_refs(&reloc_root->root_item) > 0) { > - BUG_ON(IS_ERR(root)); > - BUG_ON(root->reloc_root != reloc_root); > + ASSERT(root->reloc_root == reloc_root); > ret = merge_reloc_root(rc, root); > btrfs_put_root(root); > if (ret) { >
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 91479979d2a7..099a64b47020 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -1949,9 +1949,18 @@ void merge_reloc_roots(struct reloc_control *rc) root = btrfs_get_fs_root(fs_info, reloc_root->root_key.offset, false); + if (IS_ERR(root)) { + /* + * This likely won't happen, since we would have failed + * at a higher level. However for correctness sake + * handle the error anyway. + */ + ret = PTR_ERR(root); + goto out; + } + if (btrfs_root_refs(&reloc_root->root_item) > 0) { - BUG_ON(IS_ERR(root)); - BUG_ON(root->reloc_root != reloc_root); + ASSERT(root->reloc_root == reloc_root); ret = merge_reloc_root(rc, root); btrfs_put_root(root); if (ret) {
We have a BUG_ON() if we get an error back from btrfs_get_fs_root(). This honestly should never fail, as at this point we have a solid coordination of fs root to reloc root, and these roots will all be in memory. But in the name of killing BUG_ON()'s remove this one and handle the error properly. Change the remaining BUG_ON() to an ASSERT(). Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)