Skip to content

Commit 50f83ff

Browse files
adam900710gregkh
authored andcommitted
btrfs: scrub: Don't check free space before marking a block group RO
commit b12de52 upstream. [BUG] When running btrfs/072 with only one online CPU, it has a pretty high chance to fail: # btrfs/072 12s ... _check_dmesg: something found in dmesg (see xfstests-dev/results//btrfs/072.dmesg) # - output mismatch (see xfstests-dev/results//btrfs/072.out.bad) # --- tests/btrfs/072.out 2019-10-22 15:18:14.008965340 +0800 # +++ /xfstests-dev/results//btrfs/072.out.bad 2019-11-14 15:56:45.877152240 +0800 # @@ -1,2 +1,3 @@ # QA output created by 072 # Silence is golden # +Scrub find errors in "-m dup -d single" test # ... And with the following call trace: BTRFS info (device dm-5): scrub: started on devid 1 ------------[ cut here ]------------ BTRFS: Transaction aborted (error -27) WARNING: CPU: 0 PID: 55087 at fs/btrfs/block-group.c:1890 btrfs_create_pending_block_groups+0x3e6/0x470 [btrfs] CPU: 0 PID: 55087 Comm: btrfs Tainted: G W O 5.4.0-rc1-custom+ #13 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 RIP: 0010:btrfs_create_pending_block_groups+0x3e6/0x470 [btrfs] Call Trace: __btrfs_end_transaction+0xdb/0x310 [btrfs] btrfs_end_transaction+0x10/0x20 [btrfs] btrfs_inc_block_group_ro+0x1c9/0x210 [btrfs] scrub_enumerate_chunks+0x264/0x940 [btrfs] btrfs_scrub_dev+0x45c/0x8f0 [btrfs] btrfs_ioctl+0x31a1/0x3fb0 [btrfs] do_vfs_ioctl+0x636/0xaa0 ksys_ioctl+0x67/0x90 __x64_sys_ioctl+0x43/0x50 do_syscall_64+0x79/0xe0 entry_SYSCALL_64_after_hwframe+0x49/0xbe ---[ end trace 166c865cec7688e7 ]--- [CAUSE] The error number -27 is -EFBIG, returned from the following call chain: btrfs_end_transaction() |- __btrfs_end_transaction() |- btrfs_create_pending_block_groups() |- btrfs_finish_chunk_alloc() |- btrfs_add_system_chunk() This happens because we have used up all space of btrfs_super_block::sys_chunk_array. The root cause is, we have the following bad loop of creating tons of system chunks: 1. The only SYSTEM chunk is being scrubbed It's very common to have only one SYSTEM chunk. 2. New SYSTEM bg will be allocated As btrfs_inc_block_group_ro() will check if we have enough space after marking current bg RO. If not, then allocate a new chunk. 3. New SYSTEM bg is still empty, will be reclaimed During the reclaim, we will mark it RO again. 4. That newly allocated empty SYSTEM bg get scrubbed We go back to step 2, as the bg is already mark RO but still not cleaned up yet. If the cleaner kthread doesn't get executed fast enough (e.g. only one CPU), then we will get more and more empty SYSTEM chunks, using up all the space of btrfs_super_block::sys_chunk_array. [FIX] Since scrub/dev-replace doesn't always need to allocate new extent, especially chunk tree extent, so we don't really need to do chunk pre-allocation. To break above spiral, here we introduce a new parameter to btrfs_inc_block_group(), @do_chunk_alloc, which indicates whether we need extra chunk pre-allocation. For relocation, we pass @do_chunk_alloc=true, while for scrub, we pass @do_chunk_alloc=false. This should keep unnecessary empty chunks from popping up for scrub. Also, since there are two parameters for btrfs_inc_block_group_ro(), add more comment for it. Reviewed-by: Filipe Manana <[email protected]> Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 591ea83 commit 50f83ff

File tree

4 files changed

+54
-20
lines changed

4 files changed

+54
-20
lines changed

fs/btrfs/block-group.c

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,8 +2048,17 @@ static u64 update_block_group_flags(struct btrfs_fs_info *fs_info, u64 flags)
20482048
return flags;
20492049
}
20502050

2051-
int btrfs_inc_block_group_ro(struct btrfs_block_group_cache *cache)
2052-
2051+
/*
2052+
* Mark one block group RO, can be called several times for the same block
2053+
* group.
2054+
*
2055+
* @cache: the destination block group
2056+
* @do_chunk_alloc: whether need to do chunk pre-allocation, this is to
2057+
* ensure we still have some free space after marking this
2058+
* block group RO.
2059+
*/
2060+
int btrfs_inc_block_group_ro(struct btrfs_block_group_cache *cache,
2061+
bool do_chunk_alloc)
20532062
{
20542063
struct btrfs_fs_info *fs_info = cache->fs_info;
20552064
struct btrfs_trans_handle *trans;
@@ -2079,25 +2088,29 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group_cache *cache)
20792088
goto again;
20802089
}
20812090

2082-
/*
2083-
* if we are changing raid levels, try to allocate a corresponding
2084-
* block group with the new raid level.
2085-
*/
2086-
alloc_flags = update_block_group_flags(fs_info, cache->flags);
2087-
if (alloc_flags != cache->flags) {
2088-
ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE);
2091+
if (do_chunk_alloc) {
20892092
/*
2090-
* ENOSPC is allowed here, we may have enough space
2091-
* already allocated at the new raid level to
2092-
* carry on
2093+
* If we are changing raid levels, try to allocate a
2094+
* corresponding block group with the new raid level.
20932095
*/
2094-
if (ret == -ENOSPC)
2095-
ret = 0;
2096-
if (ret < 0)
2097-
goto out;
2096+
alloc_flags = update_block_group_flags(fs_info, cache->flags);
2097+
if (alloc_flags != cache->flags) {
2098+
ret = btrfs_chunk_alloc(trans, alloc_flags,
2099+
CHUNK_ALLOC_FORCE);
2100+
/*
2101+
* ENOSPC is allowed here, we may have enough space
2102+
* already allocated at the new raid level to carry on
2103+
*/
2104+
if (ret == -ENOSPC)
2105+
ret = 0;
2106+
if (ret < 0)
2107+
goto out;
2108+
}
20982109
}
20992110

2100-
ret = inc_block_group_ro(cache, 0);
2111+
ret = inc_block_group_ro(cache, !do_chunk_alloc);
2112+
if (!do_chunk_alloc)
2113+
goto unlock_out;
21012114
if (!ret)
21022115
goto out;
21032116
alloc_flags = btrfs_get_alloc_profile(fs_info, cache->space_info->flags);
@@ -2112,6 +2125,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group_cache *cache)
21122125
check_system_chunk(trans, alloc_flags);
21132126
mutex_unlock(&fs_info->chunk_mutex);
21142127
}
2128+
unlock_out:
21152129
mutex_unlock(&fs_info->ro_block_group_mutex);
21162130

21172131
btrfs_end_transaction(trans);

fs/btrfs/block-group.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,8 @@ int btrfs_read_block_groups(struct btrfs_fs_info *info);
205205
int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used,
206206
u64 type, u64 chunk_offset, u64 size);
207207
void btrfs_create_pending_block_groups(struct btrfs_trans_handle *trans);
208-
int btrfs_inc_block_group_ro(struct btrfs_block_group_cache *cache);
208+
int btrfs_inc_block_group_ro(struct btrfs_block_group_cache *cache,
209+
bool do_chunk_alloc);
209210
void btrfs_dec_block_group_ro(struct btrfs_block_group_cache *cache);
210211
int btrfs_start_dirty_block_groups(struct btrfs_trans_handle *trans);
211212
int btrfs_write_dirty_block_groups(struct btrfs_trans_handle *trans);

fs/btrfs/relocation.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4428,7 +4428,7 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
44284428
rc->extent_root = extent_root;
44294429
rc->block_group = bg;
44304430

4431-
ret = btrfs_inc_block_group_ro(rc->block_group);
4431+
ret = btrfs_inc_block_group_ro(rc->block_group, true);
44324432
if (ret) {
44334433
err = ret;
44344434
goto out;

fs/btrfs/scrub.c

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3560,7 +3560,26 @@ int scrub_enumerate_chunks(struct scrub_ctx *sctx,
35603560
* -> btrfs_scrub_pause()
35613561
*/
35623562
scrub_pause_on(fs_info);
3563-
ret = btrfs_inc_block_group_ro(cache);
3563+
3564+
/*
3565+
* Don't do chunk preallocation for scrub.
3566+
*
3567+
* This is especially important for SYSTEM bgs, or we can hit
3568+
* -EFBIG from btrfs_finish_chunk_alloc() like:
3569+
* 1. The only SYSTEM bg is marked RO.
3570+
* Since SYSTEM bg is small, that's pretty common.
3571+
* 2. New SYSTEM bg will be allocated
3572+
* Due to regular version will allocate new chunk.
3573+
* 3. New SYSTEM bg is empty and will get cleaned up
3574+
* Before cleanup really happens, it's marked RO again.
3575+
* 4. Empty SYSTEM bg get scrubbed
3576+
* We go back to 2.
3577+
*
3578+
* This can easily boost the amount of SYSTEM chunks if cleaner
3579+
* thread can't be triggered fast enough, and use up all space
3580+
* of btrfs_super_block::sys_chunk_array
3581+
*/
3582+
ret = btrfs_inc_block_group_ro(cache, false);
35643583
if (!ret && sctx->is_dev_replace) {
35653584
/*
35663585
* If we are doing a device replace wait for any tasks

0 commit comments

Comments
 (0)