| Message ID | bba6b87c894fc35055d99f51b16b66e662f1b127.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:50, Josef Bacik wrote: > We have a couple of BUG_ON()'s in relocate_tree_block() that can be > tripped if we have file system corruption. Convert these to ASSERT()'s > so developers still get yelled at when they break the backref code, but > error out nicely for users so the whole box doesn't go down. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/relocation.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c > index d0ce771a2a8d..4333ee329290 100644 > --- a/fs/btrfs/relocation.c > +++ b/fs/btrfs/relocation.c > @@ -2456,8 +2456,28 @@ static int relocate_tree_block(struct btrfs_trans_handle *trans, > > if (root) { > if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state)) { > - BUG_ON(node->new_bytenr); > - BUG_ON(!list_empty(&node->list)); > + /* > + * This block was the root block of a root, and this is > + * the first time we're processing the block and thus it > + * should not have had the ->new_bytenr modified and > + * should have not been included on the changed list. > + * > + * However in the case of corruption we could have > + * multiple refs pointing to the same block improperly, > + * and thus we would trip over these checks. ASSERT() > + * for the developer case, because it could indicate a > + * bug in the backref code, however error out for a > + * normal user in the case of corruption. > + */ > + ASSERT(node->new_bytenr == 0); > + ASSERT(list_empty(&node->list)); > + if (node->new_bytenr || !list_empty(&node->list)) { > + btrfs_err(root->fs_info, > + "bytenr %llu has improper references to it", > + node->bytenr); > + ret = -EUCLEAN; > + goto out; > + } > btrfs_record_root_in_trans(trans, root); > root = root->reloc_root; > node->new_bytenr = root->node->start; >
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index d0ce771a2a8d..4333ee329290 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2456,8 +2456,28 @@ static int relocate_tree_block(struct btrfs_trans_handle *trans, if (root) { if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state)) { - BUG_ON(node->new_bytenr); - BUG_ON(!list_empty(&node->list)); + /* + * This block was the root block of a root, and this is + * the first time we're processing the block and thus it + * should not have had the ->new_bytenr modified and + * should have not been included on the changed list. + * + * However in the case of corruption we could have + * multiple refs pointing to the same block improperly, + * and thus we would trip over these checks. ASSERT() + * for the developer case, because it could indicate a + * bug in the backref code, however error out for a + * normal user in the case of corruption. + */ + ASSERT(node->new_bytenr == 0); + ASSERT(list_empty(&node->list)); + if (node->new_bytenr || !list_empty(&node->list)) { + btrfs_err(root->fs_info, + "bytenr %llu has improper references to it", + node->bytenr); + ret = -EUCLEAN; + goto out; + } btrfs_record_root_in_trans(trans, root); root = root->reloc_root; node->new_bytenr = root->node->start;
We have a couple of BUG_ON()'s in relocate_tree_block() that can be tripped if we have file system corruption. Convert these to ASSERT()'s so developers still get yelled at when they break the backref code, but error out nicely for users so the whole box doesn't go down. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/relocation.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-)