Skip to content

Commit d8ccbd2

Browse files
fdmananakdave
fdmanana
authored and
kdave
committed
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>
1 parent 8dbfc14 commit d8ccbd2

File tree

3 files changed

+53
-26
lines changed

3 files changed

+53
-26
lines changed

‎fs/btrfs/block-group.c

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -499,12 +499,16 @@ static void fragment_free_space(struct btrfs_block_group *block_group)
499499
* used yet since their free space will be released as soon as the transaction
500500
* commits.
501501
*/
502-
u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end)
502+
int add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end,
503+
u64 *total_added_ret)
503504
{
504505
struct btrfs_fs_info *info = block_group->fs_info;
505-
u64 extent_start, extent_end, size, total_added = 0;
506+
u64 extent_start, extent_end, size;
506507
int ret;
507508

509+
if (total_added_ret)
510+
*total_added_ret = 0;
511+
508512
while (start < end) {
509513
ret = find_first_extent_bit(&info->excluded_extents, start,
510514
&extent_start, &extent_end,
@@ -517,10 +521,12 @@ u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end
517521
start = extent_end + 1;
518522
} else if (extent_start > start && extent_start < end) {
519523
size = extent_start - start;
520-
total_added += size;
521524
ret = btrfs_add_free_space_async_trimmed(block_group,
522525
start, size);
523-
BUG_ON(ret); /* -ENOMEM or logic error */
526+
if (ret)
527+
return ret;
528+
if (total_added_ret)
529+
*total_added_ret += size;
524530
start = extent_end + 1;
525531
} else {
526532
break;
@@ -529,13 +535,15 @@ u64 add_new_free_space(struct btrfs_block_group *block_group, u64 start, u64 end
529535

530536
if (start < end) {
531537
size = end - start;
532-
total_added += size;
533538
ret = btrfs_add_free_space_async_trimmed(block_group, start,
534539
size);
535-
BUG_ON(ret); /* -ENOMEM or logic error */
540+
if (ret)
541+
return ret;
542+
if (total_added_ret)
543+
*total_added_ret += size;
536544
}
537545

538-
return total_added;
546+
return 0;
539547
}
540548

541549
/*
@@ -779,8 +787,13 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
779787

780788
if (key.type == BTRFS_EXTENT_ITEM_KEY ||
781789
key.type == BTRFS_METADATA_ITEM_KEY) {
782-
total_found += add_new_free_space(block_group, last,
783-
key.objectid);
790+
u64 space_added;
791+
792+
ret = add_new_free_space(block_group, last, key.objectid,
793+
&space_added);
794+
if (ret)
795+
goto out;
796+
total_found += space_added;
784797
if (key.type == BTRFS_METADATA_ITEM_KEY)
785798
last = key.objectid +
786799
fs_info->nodesize;
@@ -795,11 +808,10 @@ static int load_extent_tree_free(struct btrfs_caching_control *caching_ctl)
795808
}
796809
path->slots[0]++;
797810
}
798-
ret = 0;
799-
800-
total_found += add_new_free_space(block_group, last,
801-
block_group->start + block_group->length);
802811

812+
ret = add_new_free_space(block_group, last,
813+
block_group->start + block_group->length,
814+
NULL);
803815
out:
804816
btrfs_free_path(path);
805817
return ret;
@@ -2294,9 +2306,11 @@ static int read_one_block_group(struct btrfs_fs_info *info,
22942306
btrfs_free_excluded_extents(cache);
22952307
} else if (cache->used == 0) {
22962308
cache->cached = BTRFS_CACHE_FINISHED;
2297-
add_new_free_space(cache, cache->start,
2298-
cache->start + cache->length);
2309+
ret = add_new_free_space(cache, cache->start,
2310+
cache->start + cache->length, NULL);
22992311
btrfs_free_excluded_extents(cache);
2312+
if (ret)
2313+
goto error;
23002314
}
23012315

23022316
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
27402754
return ERR_PTR(ret);
27412755
}
27422756

2743-
add_new_free_space(cache, chunk_offset, chunk_offset + size);
2744-
2757+
ret = add_new_free_space(cache, chunk_offset, chunk_offset + size, NULL);
27452758
btrfs_free_excluded_extents(cache);
2759+
if (ret) {
2760+
btrfs_put_block_group(cache);
2761+
return ERR_PTR(ret);
2762+
}
27462763

27472764
/*
27482765
* Ensure the corresponding space_info object is created and

‎fs/btrfs/block-group.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,8 @@ int btrfs_cache_block_group(struct btrfs_block_group *cache, bool wait);
289289
void btrfs_put_caching_control(struct btrfs_caching_control *ctl);
290290
struct btrfs_caching_control *btrfs_get_caching_control(
291291
struct btrfs_block_group *cache);
292-
u64 add_new_free_space(struct btrfs_block_group *block_group,
293-
u64 start, u64 end);
292+
int add_new_free_space(struct btrfs_block_group *block_group,
293+
u64 start, u64 end, u64 *total_added_ret);
294294
struct btrfs_trans_handle *btrfs_start_trans_remove_block_group(
295295
struct btrfs_fs_info *fs_info,
296296
const u64 chunk_offset);

‎fs/btrfs/free-space-tree.c

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,9 +1515,13 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
15151515
if (prev_bit == 0 && bit == 1) {
15161516
extent_start = offset;
15171517
} else if (prev_bit == 1 && bit == 0) {
1518-
total_found += add_new_free_space(block_group,
1519-
extent_start,
1520-
offset);
1518+
u64 space_added;
1519+
1520+
ret = add_new_free_space(block_group, extent_start,
1521+
offset, &space_added);
1522+
if (ret)
1523+
goto out;
1524+
total_found += space_added;
15211525
if (total_found > CACHING_CTL_WAKE_UP) {
15221526
total_found = 0;
15231527
wake_up(&caching_ctl->wait);
@@ -1529,8 +1533,9 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
15291533
}
15301534
}
15311535
if (prev_bit == 1) {
1532-
total_found += add_new_free_space(block_group, extent_start,
1533-
end);
1536+
ret = add_new_free_space(block_group, extent_start, end, NULL);
1537+
if (ret)
1538+
goto out;
15341539
extent_count++;
15351540
}
15361541

@@ -1569,6 +1574,8 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
15691574
end = block_group->start + block_group->length;
15701575

15711576
while (1) {
1577+
u64 space_added;
1578+
15721579
ret = btrfs_next_item(root, path);
15731580
if (ret < 0)
15741581
goto out;
@@ -1583,8 +1590,11 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
15831590
ASSERT(key.type == BTRFS_FREE_SPACE_EXTENT_KEY);
15841591
ASSERT(key.objectid < end && key.objectid + key.offset <= end);
15851592

1586-
total_found += add_new_free_space(block_group, key.objectid,
1587-
key.objectid + key.offset);
1593+
ret = add_new_free_space(block_group, key.objectid,
1594+
key.objectid + key.offset, &space_added);
1595+
if (ret)
1596+
goto out;
1597+
total_found += space_added;
15881598
if (total_found > CACHING_CTL_WAKE_UP) {
15891599
total_found = 0;
15901600
wake_up(&caching_ctl->wait);

0 commit comments

Comments
 (0)