2019-12-15 17:13:53

by Aditya Pakki

Subject: [PATCH] btrfs: remove BUG_ON used as assertions

alloc_extent_state_atomic() allocates extents via GFP_ATOMIC flag
and cannot fail. There are multiple invocations of BUG_ON on the
return value to check for failure. The patch replaces certain
invocations of BUG_ON by returning the error upstream.

Signed-off-by: Aditya Pakki <[email protected]>
---
fs/btrfs/extent_io.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index eb8bd0258360..e72e5a333e71 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -989,7 +989,10 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
node = tree_search_for_insert(tree, start, &p, &parent);
if (!node) {
prealloc = alloc_extent_state_atomic(prealloc);
- BUG_ON(!prealloc);
+ if (!prealloc) {
+ err = -ENOMEM;
+ goto out;
+ }
err = insert_state(tree, prealloc, start, end,
&p, &parent, &bits, changeset);
if (err)
@@ -1054,7 +1057,10 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
}

prealloc = alloc_extent_state_atomic(prealloc);
- BUG_ON(!prealloc);
+ if (!prealloc) {
+ err = -ENOMEM;
+ goto out;
+ }
err = split_state(tree, state, prealloc, start);
if (err)
extent_io_tree_panic(tree, err);
@@ -1091,7 +1097,10 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
this_end = last_start - 1;

prealloc = alloc_extent_state_atomic(prealloc);
- BUG_ON(!prealloc);
+ if (!prealloc) {
+ err = -ENOMEM;
+ goto out;
+ }

/*
* Avoid to free 'prealloc' if it can be merged with
@@ -1121,7 +1130,10 @@ __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
}

prealloc = alloc_extent_state_atomic(prealloc);
- BUG_ON(!prealloc);
+ if (!prealloc) {
+ err = -ENOMEM;
+ goto out;
+ }
err = split_state(tree, state, prealloc, end + 1);
if (err)
extent_io_tree_panic(tree, err);
--
2.20.1

2019-12-15 17:44:06

by Nikolay Borisov

Subject: Re: [PATCH] btrfs: remove BUG_ON used as assertions

On 15.12.19 г. 19:12 ч., Aditya Pakki wrote:
> alloc_extent_state_atomic() allocates extents via GFP_ATOMIC flag
> and cannot fail. There are multiple invocations of BUG_ON on the
> return value to check for failure. The patch replaces certain
> invocations of BUG_ON by returning the error upstream.
>
> Signed-off-by: Aditya Pakki <[email protected]>

Have you actually audited all callers of __set_extent_bit whether they
correctly handle failures?