| Message ID | 77e4be6162916c2d23987cec9542acbc60ec2bde.1686219923.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: some fixes and updates around handling errors for tree mod log operations | expand |
On 2023/6/8 18:27, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > At balance_level() we are calling btrfs_handle_fs_error() when the middle > child only has 1 item and the left child is missing, however we can simply > use btrfs_abort_transaction(), which achieves the same purposes: to turn > the fs to error state, abort the current transaction and turn the fs to > RO mode. Besides that, btrfs_abort_transaction() also prints a stack trace > which makes it more useful. > > Also, as this is a highly unexpected case and it's about a b+tree > inconsistency, change the error code from -EROFS to -EUCLEAN, tag the if > branch as 'unlikely' and log an explicit error message. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/ctree.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 4dcdcf25c3fe..00eea2925d1d 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1164,9 +1164,13 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, > * otherwise we would have pulled some pointers from the > * right > */ > - if (!left) { > - ret = -EROFS; > - btrfs_handle_fs_error(fs_info, ret, NULL); > + if (unlikely(!left)) { > + btrfs_crit(fs_info, > +"missing left child when middle child only has 1 item, parent bytenr %llu level %d mid bytenr %llu root %llu", > + parent->start, btrfs_header_level(parent), > + mid->start, btrfs_root_id(root)); > + ret = -EUCLEAN; > + btrfs_abort_transaction(trans, ret); > goto out; > } > wret = balance_node_right(trans, mid, left);
On 2023/6/8 18:27, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > At balance_level() we are calling btrfs_handle_fs_error() when the middle > child only has 1 item and the left child is missing, however we can simply > use btrfs_abort_transaction(), which achieves the same purposes: to turn > the fs to error state, abort the current transaction and turn the fs to > RO mode. Besides that, btrfs_abort_transaction() also prints a stack trace > which makes it more useful. > > Also, as this is a highly unexpected case and it's about a b+tree > inconsistency, change the error code from -EROFS to -EUCLEAN, tag the if > branch as 'unlikely' and log an explicit error message. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > fs/btrfs/ctree.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c > index 4dcdcf25c3fe..00eea2925d1d 100644 > --- a/fs/btrfs/ctree.c > +++ b/fs/btrfs/ctree.c > @@ -1164,9 +1164,13 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, > * otherwise we would have pulled some pointers from the > * right > */ > - if (!left) { > - ret = -EROFS; > - btrfs_handle_fs_error(fs_info, ret, NULL); > + if (unlikely(!left)) { > + btrfs_crit(fs_info, > +"missing left child when middle child only has 1 item, parent bytenr %llu level %d mid bytenr %llu root %llu", > + parent->start, btrfs_header_level(parent), > + mid->start, btrfs_root_id(root)); > + ret = -EUCLEAN; > + btrfs_abort_transaction(trans, ret); > goto out; > } > wret = balance_node_right(trans, mid, left);
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c index 4dcdcf25c3fe..00eea2925d1d 100644 --- a/fs/btrfs/ctree.c +++ b/fs/btrfs/ctree.c @@ -1164,9 +1164,13 @@ static noinline int balance_level(struct btrfs_trans_handle *trans, * otherwise we would have pulled some pointers from the * right */ - if (!left) { - ret = -EROFS; - btrfs_handle_fs_error(fs_info, ret, NULL); + if (unlikely(!left)) { + btrfs_crit(fs_info, +"missing left child when middle child only has 1 item, parent bytenr %llu level %d mid bytenr %llu root %llu", + parent->start, btrfs_header_level(parent), + mid->start, btrfs_root_id(root)); + ret = -EUCLEAN; + btrfs_abort_transaction(trans, ret); goto out; } wret = balance_node_right(trans, mid, left);