On 2019/4/18 下午3:24, Nikolay Borisov wrote:
>
>
> On 18.04.19 г. 10:21 ч., Qu Wenruo wrote:
>> There is a BUG_ON() in __clear_extent_bit() for memory allocation
>> failure.
>>
>> While comment of __clear_extent_bit() says it can return error, but we
>> always return 0.
>>
>> Some __clear_extent_bit() callers just ignore the return value, while
>> some still expect error.
>>
>> Let's return proper error for this memory allocation anyway, to remove
>> that BUG_ON() as a first step, so at least we can continue test.
>
> I remember Josef did some changes into this code and said that prealloc
> shouldn't fail because this will cause mayhem down the road i.e. proper
> error handling is missing. If anything I think it should be added first
> and then remove the BUG_ONs.
That's true, we could have some strange lockup due to
lock_extent_bits(), as if some clear_extent_bits() failed due to ENOMEM
and caller just ignore the error, we could have a lockup.
I'll try to pre-allocate certain amount of extent_state as the last
chance of redemption.
Anyway, such BUG_ON() right after kmalloc() is really a blockage for
error injection test.
Thanks,
Qu
>
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>> ---
>> fs/btrfs/extent_io.c | 12 ++++++++++--
>> 1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index 828708f6510c..f1bfc632ef7b 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -743,7 +743,10 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>>
>> if (state->start < start) {
>> 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);
>> @@ -766,7 +769,10 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>> */
>> if (state->start <= end && state->end > 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);
>> @@ -801,6 +807,8 @@ int __clear_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
>> if (prealloc)
>> free_extent_state(prealloc);
>>
>> + if (err)
>> + return err;
>> return 0;
>>
>> }
>>
