| Message ID | 02ccc686cbc2210d1ce995e2066085bfdee68fde.1607019557.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup error handling in relocation | expand |
On 2020/12/4 上午2:22, Josef Bacik wrote: > Generally speaking this shouldn't ever fail, the corresponding fs root > for the reloc root will already be in memory, so we won't get -ENOMEM > here. > > However if there is no corresponding root for the reloc root then we > could get -ENOMEM when we try to allocate it or we could get -ENOENT > when we look it up and see that it doesn't exist. > > Convert these BUG_ON()'s into ASSERT()'s + proper error handling for the > case of corruption. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Qu Wenruo <wqu@suse.com> THanks, Qu > --- > fs/btrfs/relocation.c | 23 +++++++++++++++++++++-- > 1 file changed, 21 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index a3ad44605695..90a0a8500a83 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -1973,8 +1973,27 @@ static int record_reloc_root_in_trans(struct btrfs_trans_handle *trans, > return 0; > > root = btrfs_get_fs_root(fs_info, reloc_root->root_key.offset, false); > - BUG_ON(IS_ERR(root)); > - BUG_ON(root->reloc_root != reloc_root); > + > + /* > + * This should succeed, since we can't have a reloc root without having > + * already looked up the actual root and created the reloc root for this > + * root. > + * > + * However if there's some sort of corruption where we have a ref to a > + * reloc root without a corresponding root this could return -ENOENT. > + */ > + if (IS_ERR(root)) { > + ASSERT(0); > + return PTR_ERR(root); > + } > + if (root->reloc_root != reloc_root) { > + ASSERT(0); > + btrfs_err(fs_info, > + "root %llu has two reloc roots associated with it", > + reloc_root->root_key.offset); > + btrfs_put_root(root); > + return -EUCLEAN; > + } > ret = btrfs_record_root_in_trans(trans, root); > btrfs_put_root(root); > >
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index a3ad44605695..90a0a8500a83 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -1973,8 +1973,27 @@ static int record_reloc_root_in_trans(struct btrfs_trans_handle *trans, return 0; root = btrfs_get_fs_root(fs_info, reloc_root->root_key.offset, false); - BUG_ON(IS_ERR(root)); - BUG_ON(root->reloc_root != reloc_root); + + /* + * This should succeed, since we can't have a reloc root without having + * already looked up the actual root and created the reloc root for this + * root. + * + * However if there's some sort of corruption where we have a ref to a + * reloc root without a corresponding root this could return -ENOENT. + */ + if (IS_ERR(root)) { + ASSERT(0); + return PTR_ERR(root); + } + if (root->reloc_root != reloc_root) { + ASSERT(0); + btrfs_err(fs_info, + "root %llu has two reloc roots associated with it", + reloc_root->root_key.offset); + btrfs_put_root(root); + return -EUCLEAN; + } ret = btrfs_record_root_in_trans(trans, root); btrfs_put_root(root);
Generally speaking this shouldn't ever fail, the corresponding fs root for the reloc root will already be in memory, so we won't get -ENOMEM here. However if there is no corresponding root for the reloc root then we could get -ENOMEM when we try to allocate it or we could get -ENOENT when we look it up and see that it doesn't exist. Convert these BUG_ON()'s into ASSERT()'s + proper error handling for the case of corruption. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-)