| Message ID | be18a9fcfa768add6a23e3dee5dfcce55b0814f5.1732812639.git.jth@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] btrfs: don't BUG_ON() in btrfs_drop_extents() | expand |
On Fri, Nov 29, 2024 at 1:11 PM Johannes Thumshirn <jth@kernel.org> wrote: > > From: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > btrfs_drop_extents() calls BUG_ON() in case the counter of to be deleted > extents is greater than 0. But all of these code paths can handle errors, > so there's no need to crash the kernel. Instead WARN() that the condition > has been met and gracefully bail out. > > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Reviewed-by: Filipe Manana <fdmanana@suse.com> Looks good, thanks. > --- > Changes to v1: > - Fix spelling error in commit message > - Change ASSERT() to WARN_ON() > - Take care of the other BUG_ON() cases as well > > Link to v1: > - https://lore.kernel.org/linux-btrfs/20241128093428.21485-1-jth@kernel.org > --- > fs/btrfs/file.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index fbb753300071..f70ce6c65d12 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -245,7 +245,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, > next_slot: > leaf = path->nodes[0]; > if (path->slots[0] >= btrfs_header_nritems(leaf)) { > - BUG_ON(del_nr > 0); > + if (WARN_ON(del_nr > 0)) { > + ret = -EINVAL; > + break; > + } > ret = btrfs_next_leaf(root, path); > if (ret < 0) > break; > @@ -321,7 +324,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, > * | -------- extent -------- | > */ > if (args->start > key.offset && args->end < extent_end) { > - BUG_ON(del_nr > 0); > + if (WARN_ON(del_nr > 0)) { > + ret = -EINVAL; > + break; > + } > if (extent_type == BTRFS_FILE_EXTENT_INLINE) { > ret = -EOPNOTSUPP; > break; > @@ -409,7 +415,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, > * | -------- extent -------- | > */ > if (args->start > key.offset && args->end >= extent_end) { > - BUG_ON(del_nr > 0); > + if (WARN_ON(del_nr > 0)) { > + ret = -EINVAL; > + break; > + } > if (extent_type == BTRFS_FILE_EXTENT_INLINE) { > ret = -EOPNOTSUPP; > break; > @@ -437,7 +446,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, > del_slot = path->slots[0]; > del_nr = 1; > } else { > - BUG_ON(del_slot + del_nr != path->slots[0]); > + if (WARN_ON(del_slot + del_nr != path->slots[0])) { > + ret = -EINVAL; > + break; > + } > del_nr++; > } > > -- > 2.43.0 > >
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index fbb753300071..f70ce6c65d12 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -245,7 +245,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, next_slot: leaf = path->nodes[0]; if (path->slots[0] >= btrfs_header_nritems(leaf)) { - BUG_ON(del_nr > 0); + if (WARN_ON(del_nr > 0)) { + ret = -EINVAL; + break; + } ret = btrfs_next_leaf(root, path); if (ret < 0) break; @@ -321,7 +324,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, * | -------- extent -------- | */ if (args->start > key.offset && args->end < extent_end) { - BUG_ON(del_nr > 0); + if (WARN_ON(del_nr > 0)) { + ret = -EINVAL; + break; + } if (extent_type == BTRFS_FILE_EXTENT_INLINE) { ret = -EOPNOTSUPP; break; @@ -409,7 +415,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, * | -------- extent -------- | */ if (args->start > key.offset && args->end >= extent_end) { - BUG_ON(del_nr > 0); + if (WARN_ON(del_nr > 0)) { + ret = -EINVAL; + break; + } if (extent_type == BTRFS_FILE_EXTENT_INLINE) { ret = -EOPNOTSUPP; break; @@ -437,7 +446,10 @@ int btrfs_drop_extents(struct btrfs_trans_handle *trans, del_slot = path->slots[0]; del_nr = 1; } else { - BUG_ON(del_slot + del_nr != path->slots[0]); + if (WARN_ON(del_slot + del_nr != path->slots[0])) { + ret = -EINVAL; + break; + } del_nr++; }