btrfs: remove BUG_ON()'s in add_new_free_space()

At add_new_free_space() we have these BUG_ON()'s that are there to deal
with any failure to add free space to the in memory free space cache.
Such failures are mostly -ENOMEM that should be very rare. However there's
no need to have these BUG_ON()'s, we can just return any error to the
caller and all callers and their upper call chain are already dealing with
errors.

So just make add_new_free_space() return any errors, while removing the
BUG_ON()'s, and returning the total amount of added free space to an
optional u64 pointer argument.

Reported-by: syzbot+3ba856e07b7127889d8c@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/000000000000e9cb8305ff4e8327@google.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index b7da148486866fc7382c9ee8ff27fe8fe26ab7fd..63c3b7172ba54f6ba9773b8a511e6a3bac7d5790 100644 (file)
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -499,12 +499,16 @@ static void fragment_free_space(struct btrfs_block_group *block_group)
  * used yet since their free space will be released as soon as the transaction
  * commits.
  */
-u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end)
+int add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end,
+                      u64 *total_added_ret)
 {
        struct btrfs_fs_info *info = block_group->fs_info;
-       u64 extent_start, extent_end, size, total_added = 0;
+       u64 extent_start, extent_end, size;
        int ret;
 
+       if (total_added_ret)
+               *total_added_ret = 0;
+
        while (start < end) {
                ret = find_first_extent_bit(&info->excluded_extents, start,
                                            &extent_start, &extent_end,
@@ -517,10 +521,12 @@ u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end
                        start = extent_end + 1;
                } else if (extent_start > start && extent_start < end) {
                        size = extent_start - start;
-                       total_added += size;
                        ret = btrfs_add_free_space_async_trimmed(block_group,
                                                                 start, size);
-                       BUG_ON(ret); /* -ENOMEM or logic error */
+                       if (ret)
+                               return ret;
+                       if (total_added_ret)
+                               *total_added_ret += size;
                        start = extent_end + 1;
                } else {
                        break;
@@ -529,13 +535,15 @@ u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end
 
        if (start < end) {
                size = end - start;
-               total_added += size;
                ret = btrfs_add_free_space_async_trimmed(block_group, start,
                                                         size);
-               BUG_ON(ret); /* -ENOMEM or logic error */
+               if (ret)
+                       return ret;
+               if (total_added_ret)
+                       *total_added_ret += size;
        }
 
-       return total_added;
+       return 0;
 }
 
 /*
@@ -779,8 +787,13 @@ next:
 
                if (key.type == BTRFS_EXTENT_ITEM_KEY ||
                    key.type == BTRFS_METADATA_ITEM_KEY) {
-                       total_found += add_new_free_space(block_group, last,
-                                                         key.objectid);
+                       u64 space_added;
+
+                       ret = add_new_free_space(block_group, last, key.objectid,
+                                                &space_added);
+                       if (ret)
+                               goto out;
+                       total_found += space_added;
                        if (key.type == BTRFS_METADATA_ITEM_KEY)
                                last = key.objectid +
                                        fs_info->nodesize;
@@ -795,11 +808,10 @@ next:
                }
                path->slots[0]++;
        }
-       ret = 0;
-
-       total_found += add_new_free_space(block_group, last,
-                               block_group->start + block_group->length);
 
+       ret = add_new_free_space(block_group, last,
+                                block_group->start + block_group->length,
+                                NULL);
 out:
        btrfs_free_path(path);
        return ret;
@@ -2294,9 +2306,11 @@ static int read_one_block_group(struct btrfs_fs_info *info,
                btrfs_free_excluded_extents(cache);
        } else if (cache->used == 0) {
                cache->cached = BTRFS_CACHE_FINISHED;
-               add_new_free_space(cache, cache->start,
-                                  cache->start + cache->length);
+               ret = add_new_free_space(cache, cache->start,
+                                        cache->start + cache->length, NULL);
                btrfs_free_excluded_extents(cache);
+               if (ret)
+                       goto error;
        }
 
        ret = btrfs_add_block_group_cache(info, cache);
@@ -2740,9 +2754,12 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran
                return ERR_PTR(ret);
        }
 
-       add_new_free_space(cache, chunk_offset, chunk_offset + size);
-
+       ret = add_new_free_space(cache, chunk_offset, chunk_offset + size, NULL);
        btrfs_free_excluded_extents(cache);
+       if (ret) {
+               btrfs_put_block_group(cache);
+               return ERR_PTR(ret);
+       }
 
        /*
         * Ensure the corresponding space_info object is created and

diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index 381c54a56417f523fcaa74a9811bdfb2cb63f3d7..aba5dff66c19f1d2187be22423b193ff19d49c43 100644 (file)
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -289,8 +289,8 @@ int btrfs_cache_block_group(struct btrfs_block_group *cache, bool wait);
 void btrfs_put_caching_control(struct btrfs_caching_control *ctl);
 struct btrfs_caching_control *btrfs_get_caching_control(
                struct btrfs_block_group *cache);
-u64 add_new_free_space(struct btrfs_block_group *block_group,
-                      u64 start, u64 end);
+int add_new_free_space(struct btrfs_block_group *block_group,
+                      u64 start, u64 end, u64 *total_added_ret);
 struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
                                struct btrfs_fs_info *fs_info,
                                const u64 chunk_offset);

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 045ddce32eca45310ce08499607cb8aec494d6e2..f169378e2ca6e4b7d194fa52a937be1ddf340887 100644 (file)
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1515,9 +1515,13 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
                        if (prev_bit == 0 && bit == 1) {
                                extent_start = offset;
                        } else if (prev_bit == 1 && bit == 0) {
-                               total_found += add_new_free_space(block_group,
-                                                                 extent_start,
-                                                                 offset);
+                               u64 space_added;
+
+                               ret = add_new_free_space(block_group, extent_start,
+                                                        offset, &space_added);
+                               if (ret)
+                                       goto out;
+                               total_found += space_added;
                                if (total_found > CACHING_CTL_WAKE_UP) {
                                        total_found = 0;
                                        wake_up(&caching_ctl->wait);
@@ -1529,8 +1533,9 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
                }
        }
        if (prev_bit == 1) {
-               total_found += add_new_free_space(block_group, extent_start,
-                                                 end);
+               ret = add_new_free_space(block_group, extent_start, end, NULL);
+               if (ret)
+                       goto out;
                extent_count++;
        }
 
@@ -1569,6 +1574,8 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
        end = block_group->start + block_group->length;
 
        while (1) {
+               u64 space_added;
+
                ret = btrfs_next_item(root, path);
                if (ret < 0)
                        goto out;
@@ -1583,8 +1590,11 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
                ASSERT(key.type == BTRFS_FREE_SPACE_EXTENT_KEY);
                ASSERT(key.objectid < end && key.objectid + key.offset <= end);
 
-               total_found += add_new_free_space(block_group, key.objectid,
-                                                 key.objectid + key.offset);
+               ret = add_new_free_space(block_group, key.objectid,
+                                        key.objectid + key.offset, &space_added);
+               if (ret)
+                       goto out;
+               total_found += space_added;
                if (total_found > CACHING_CTL_WAKE_UP) {
                        total_found = 0;
                        wake_up(&caching_ctl->wait);