| Message ID | 6570c4cd1433a02a9d84e6fcd93f9a971d7f42fd.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: > In corruption cases we could have paths from a block up to no root at > all, and thus we'll BUG_ON(!root) in select_one_root. Handle this by > adding an ASSERT() for developers, and returning an error for normal > users. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/relocation.c | 19 ++++++++++++++++--- > 1 file changed, 16 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index d4656a8f507d..91479979d2a7 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -2200,7 +2200,16 @@ struct btrfs_root *select_one_root(struct btrfs_backref_node *node) > cond_resched(); > next = walk_up_backref(next, edges, &index); > root = next->root; > - BUG_ON(!root); > + > + /* > + * This can occur if we have incomplete extent refs leading all > + * the way up a particular path, in this case return -EUCLEAN. > + * However leave as an ASSERT() for developers, because it could > + * indicate a bug in the backref code. > + */ > + ASSERT(root); > + if (!root) > + return ERR_PTR(-EUCLEAN); Just the same comment on using ASSERT(0) in the error branch. Despite that looks OK to me. Thanks, Qu > > /* No other choice for non-shareable tree */ > if (!test_bit(BTRFS_ROOT_SHAREABLE, &root->state)) > @@ -2598,8 +2607,12 @@ static int relocate_tree_block(struct btrfs_trans_handle *trans, > > BUG_ON(node->processed); > root = select_one_root(node); > - if (root == ERR_PTR(-ENOENT)) { > - update_processed_blocks(rc, node); > + if (IS_ERR(root)) { > + ret = PTR_ERR(root); > + if (ret == -ENOENT) { > + ret = 0; > + update_processed_blocks(rc, node); > + } > goto out; > } > >
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index d4656a8f507d..91479979d2a7 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2200,7 +2200,16 @@ struct btrfs_root *select_one_root(struct btrfs_backref_node *node) cond_resched(); next = walk_up_backref(next, edges, &index); root = next->root; - BUG_ON(!root); + + /* + * This can occur if we have incomplete extent refs leading all + * the way up a particular path, in this case return -EUCLEAN. + * However leave as an ASSERT() for developers, because it could + * indicate a bug in the backref code. + */ + ASSERT(root); + if (!root) + return ERR_PTR(-EUCLEAN); /* No other choice for non-shareable tree */ if (!test_bit(BTRFS_ROOT_SHAREABLE, &root->state)) @@ -2598,8 +2607,12 @@ static int relocate_tree_block(struct btrfs_trans_handle *trans, BUG_ON(node->processed); root = select_one_root(node); - if (root == ERR_PTR(-ENOENT)) { - update_processed_blocks(rc, node); + if (IS_ERR(root)) { + ret = PTR_ERR(root); + if (ret == -ENOENT) { + ret = 0; + update_processed_blocks(rc, node); + } goto out; }
In corruption cases we could have paths from a block up to no root at all, and thus we'll BUG_ON(!root) in select_one_root. Handle this by adding an ASSERT() for developers, and returning an error for normal users. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)