On 2020/1/7 上午12:50, Josef Bacik wrote:
> btrfs/061 has been failing consistently for me recently with a
> transaction abort. We run out of space in the system chunk array, which
> means we've allocated way too many system chunks than we need.
Isn't that caused by scrubbing creating unnecessary system chunks?
IIRC I had a patch to address that problem by just simply not allocating
system chunks for scrub.
("btrfs: scrub: Don't check free space before marking a block group RO")
Although that doesn't address the whole problem, but it should at least
reduce the possibility.
Furthermore, with the newer over-commit behavior for inc_block_group_ro
("btrfs: use btrfs_can_overcommit in inc_block_group_ro"), we won't
really allocate new system chunks anymore if we can over-commit.
With those two patches, I guess we should have solved the problem.
Or did I miss something?
Thanks,
Qu
>
> Chris added this a long time ago for balance as a poor mans restriping.
> If you had a single disk and then added another disk and then did a
> balance, update_block_group_flags would then figure out which RAID level
> you needed.
>
> Fast forward to today and we have restriping behavior, so we can
> explicitly tell the fs that we're trying to change the raid level. This
> is accomplished through the normal get_alloc_profile path.
>
> Furthermore this code actually causes btrfs/061 to fail, because we do
> things like mkfs -m dup -d single with multiple devices. This trips
> this check
>
> alloc_flags = update_block_group_flags(fs_info, cache->flags);
> if (alloc_flags != cache->flags) {
> ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE);
>
> in btrfs_inc_block_group_ro. Because we're balancing and scrubbing, but
> not actually restriping, we keep forcing chunk allocation of RAID1
> chunks. This eventually causes us to run out of system space and the
> file system aborts and flips read only.
>
> We don't need this poor mans restriping any more, simply use the normal
> get_alloc_profile helper, which will get the correct alloc_flags and
> thus make the right decision for chunk allocation. This keeps us from
> allocating a billion system chunks and falling over.
>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
> fs/btrfs/block-group.c | 52 ++----------------------------------------
> 1 file changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index c79eccf188c5..0257e6f1efb1 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -1975,54 +1975,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used,
> return 0;
> }
>
> -static u64 update_block_group_flags(struct btrfs_fs_info *fs_info, u64 flags)
> -{
> - u64 num_devices;
> - u64 stripped;
> -
> - /*
> - * if restripe for this chunk_type is on pick target profile and
> - * return, otherwise do the usual balance
> - */
> - stripped = get_restripe_target(fs_info, flags);
> - if (stripped)
> - return extended_to_chunk(stripped);
> -
> - num_devices = fs_info->fs_devices->rw_devices;
> -
> - stripped = BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID56_MASK |
> - BTRFS_BLOCK_GROUP_RAID1_MASK | BTRFS_BLOCK_GROUP_RAID10;
> -
> - if (num_devices == 1) {
> - stripped |= BTRFS_BLOCK_GROUP_DUP;
> - stripped = flags & ~stripped;
> -
> - /* turn raid0 into single device chunks */
> - if (flags & BTRFS_BLOCK_GROUP_RAID0)
> - return stripped;
> -
> - /* turn mirroring into duplication */
> - if (flags & (BTRFS_BLOCK_GROUP_RAID1_MASK |
> - BTRFS_BLOCK_GROUP_RAID10))
> - return stripped | BTRFS_BLOCK_GROUP_DUP;
> - } else {
> - /* they already had raid on here, just return */
> - if (flags & stripped)
> - return flags;
> -
> - stripped |= BTRFS_BLOCK_GROUP_DUP;
> - stripped = flags & ~stripped;
> -
> - /* switch duplicated blocks with raid1 */
> - if (flags & BTRFS_BLOCK_GROUP_DUP)
> - return stripped | BTRFS_BLOCK_GROUP_RAID1;
> -
> - /* this is drive concat, leave it alone */
> - }
> -
> - return flags;
> -}
> -
> int btrfs_inc_block_group_ro(struct btrfs_block_group *cache)
>
> {
> @@ -2058,7 +2010,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache)
> * if we are changing raid levels, try to allocate a corresponding
> * block group with the new raid level.
> */
> - alloc_flags = update_block_group_flags(fs_info, cache->flags);
> + alloc_flags = get_alloc_profile(fs_info, cache->flags);
> if (alloc_flags != cache->flags) {
> ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE);
> /*
> @@ -2082,7 +2034,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache)
> ret = inc_block_group_ro(cache, 0);
> out:
> if (cache->flags & BTRFS_BLOCK_GROUP_SYSTEM) {
> - alloc_flags = update_block_group_flags(fs_info, cache->flags);
> + alloc_flags = get_alloc_profile(fs_info, cache->flags);
> mutex_lock(&fs_info->chunk_mutex);
> check_system_chunk(trans, alloc_flags);
> mutex_unlock(&fs_info->chunk_mutex);
>
Attachment:
signature.asc
Description: OpenPGP digital signature
