| Message ID | 970a8c4f8a0593de2a8498ceaddc2b00ee2d352f.1733118459.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] btrfs: don't BUG_ON() in btrfs_drop_extents() | expand |
在 2024/12/2 16:18, Johannes Thumshirn 写道: > 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. > > Reviewed-by: Filipe Manana <fdmanana@suse.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> Although I really prefer one extra line explaining the reason, but it should be good enough for now, until we hit one in the real world. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > Changes to v2: > - Dump leaf when hitting the WARN (Qu) > > Link to v2: > - https://lore.kernel.org/linux-btrfs/be18a9fcfa768add6a23e3dee5dfcce55b0814f5.1732812639.git.jth@kernel.org > > 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 | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c > index fbb753300071..b3bfd7425efc 100644 > --- a/fs/btrfs/file.c > +++ b/fs/btrfs/file.c > @@ -36,6 +36,7 @@ > #include "ioctl.h" > #include "file.h" > #include "super.h" > +#include "print-tree.h" > > /* > * Helper to fault in page and copy. This should go away and be replaced with > @@ -245,7 +246,11 @@ 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)) { > + btrfs_print_leaf(leaf); > + ret = -EINVAL; > + break; > + } > ret = btrfs_next_leaf(root, path); > if (ret < 0) > break; > @@ -321,7 +326,11 @@ 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)) { > + btrfs_print_leaf(leaf); > + ret = -EINVAL; > + break; > + } > if (extent_type == BTRFS_FILE_EXTENT_INLINE) { > ret = -EOPNOTSUPP; > break; > @@ -409,7 +418,11 @@ 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)) { > + btrfs_print_leaf(leaf); > + ret = -EINVAL; > + break; > + } > if (extent_type == BTRFS_FILE_EXTENT_INLINE) { > ret = -EOPNOTSUPP; > break; > @@ -437,7 +450,11 @@ 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])) { > + btrfs_print_leaf(leaf); > + ret = -EINVAL; > + break; > + } > del_nr++; > } >
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index fbb753300071..b3bfd7425efc 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -36,6 +36,7 @@ #include "ioctl.h" #include "file.h" #include "super.h" +#include "print-tree.h" /* * Helper to fault in page and copy. This should go away and be replaced with @@ -245,7 +246,11 @@ 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)) { + btrfs_print_leaf(leaf); + ret = -EINVAL; + break; + } ret = btrfs_next_leaf(root, path); if (ret < 0) break; @@ -321,7 +326,11 @@ 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)) { + btrfs_print_leaf(leaf); + ret = -EINVAL; + break; + } if (extent_type == BTRFS_FILE_EXTENT_INLINE) { ret = -EOPNOTSUPP; break; @@ -409,7 +418,11 @@ 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)) { + btrfs_print_leaf(leaf); + ret = -EINVAL; + break; + } if (extent_type == BTRFS_FILE_EXTENT_INLINE) { ret = -EOPNOTSUPP; break; @@ -437,7 +450,11 @@ 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])) { + btrfs_print_leaf(leaf); + ret = -EINVAL; + break; + } del_nr++; }