Re: [PATCH] Btrfs: fix easily get into ENOSPC in mixed case

[Sergei Trofimovich <slyich@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3848 bytes --]

On Fri, 08 Apr 2011 16:44:37 +0800
liubo <liubo2009@cn.fujitsu.com> wrote:

> 
> When a btrfs disk is created by mixed data & metadata option, it will have no
> pure data or pure metadata space info.
> 
> In btrfs's for-linus branch, commit 78b1ea13838039cd88afdd62519b40b344d6c920
> (Btrfs: fix OOPS of empty filesystem after balance) initializes space infos at
> the very beginning.  The problem is this initialization does not take the mixed
> case into account, which will cause btrfs will easily get into ENOSPC in mixed
> case.
> 
> Signed-off-by: Liu Bo <liubo2009@cn.fujitsu.com>

Hi Chris!

I confirm it fixes OOpses for me.
Some clarification of what happened down the thread with me in order to convince
this patch actually makes things better.

1. The initial OOps reported by me was a result of using bad mkfs (not interesting,
    but generated a lot of noise here)

2. Then I've patched mkfs properly (used tmp branch), reformatted my partitions and
    caught -ENOSPC on 'rm -rf /var/tmp/' (vanilla kernel), added debugging info and have come to the
    conclusion: this patch helps in this situation, as free space was completely miscomputed
    (it's a regression since 2.6.38). I've applied it and things _mostly_ (see 3.) solved (--mixed
    ~4G lzo filesystem) for me

3. Then I've caught more -ENOSPC oopses, which occured to be a bug in mkfs for which Arne posted
    a fix today, see his email:
        [PATCH] btrfs progs: fix extra metadata chunk allocation in --mixed case

    Arne's mkfs patch (or btrfs fi balance workaround) fixed rest of OOpses for me.

So, my thoughts:
    - this patch is quite critical for --mixed filesystems
    - it (or it's equivalent) should be considered for pulling-in to .39 kernel (ideally), as it's a regression since 2.6.38

Thanks!

> ---
>  fs/btrfs/extent-tree.c |   37 ++++++++++++++++++++++++++-----------
>  1 files changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index f619c3c..1b47ae4 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -8781,23 +8781,38 @@ out:
>  int btrfs_init_space_info(struct btrfs_fs_info *fs_info)
>  {
>  	struct btrfs_space_info *space_info;
> +	struct btrfs_super_block *disk_super;
> +	u64 features;
> +	u64 flags;
> +	int mixed = 0;
>  	int ret;
>  
> -	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_SYSTEM, 0, 0,
> -								 &space_info);
> -	if (ret)
> -		return ret;
> +	disk_super = &fs_info->super_copy;
> +	if (!btrfs_super_root(disk_super))
> +		return 1;
>  
> -	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_METADATA, 0, 0,
> -								 &space_info);
> -	if (ret)
> -		return ret;
> +	features = btrfs_super_incompat_flags(disk_super);
> +	if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
> +		mixed = 1;
>  
> -	ret = update_space_info(fs_info, BTRFS_BLOCK_GROUP_DATA, 0, 0,
> -								 &space_info);
> +	flags = BTRFS_BLOCK_GROUP_SYSTEM;
> +	ret = update_space_info(fs_info, flags, 0, 0, &space_info);
>  	if (ret)
> -		return ret;
> +		goto out;
>  
> +	if (mixed) {
> +		flags = BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_DATA;
> +		ret = update_space_info(fs_info, flags, 0, 0, &space_info);
> +	} else {
> +		flags = BTRFS_BLOCK_GROUP_METADATA;
> +		ret = update_space_info(fs_info, flags, 0, 0, &space_info);
> +		if (ret)
> +			goto out;
> +
> +		flags = BTRFS_BLOCK_GROUP_DATA;
> +		ret = update_space_info(fs_info, flags, 0, 0, &space_info);
> +	}
> +out:
>  	return ret;
>  }
>  
> -- 
> 1.6.5.2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 

  Sergei

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]