Re: [PATCH 4/5] btrfs: do not account global reserve in can_overcommit

[Nikolay Borisov <nborisov@suse.com>]

On 16.08.19 г. 18:20 ч., Josef Bacik wrote:
> We ran into a problem in production where a box with plenty of space was
> getting wedged doing ENOSPC flushing.  These boxes only had 20% of the
> disk allocated, but their metadata space + global reserve was right at
> the size of their metadata chunk.
> 
> In this case can_overcommit should be allowing allocations without
> problem, but there's logic in can_overcommit that doesn't allow us to
> overcommit if there's not enough real space to satisfy the global
> reserve.
> 
> This is for historical reasons.  Before there were only certain places
> we could allocate chunks.  We could go to commit the transaction and not
> have enough space for our pending delayed refs and such and be unable to
> allocate a new chunk.  This would result in a abort because of ENOSPC.
> This code was added to solve this problem.
> 
> However since then we've gained the ability to always be able to
> allocate a chunk.  So we can easily overcommit in these cases without
> risking a transaction abort because of ENOSPC.
> 
> Also prior to now the global reserve really would be used because that's
> the space we relied on for delayed refs.  With delayed refs being
> tracked separately we no longer have to worry about running out of
> delayed refs space while committing.  We are much less likely to
> exhaust our global reserve space during transaction commit.
> 
> Fix the can_overcommit code to simply see if our current usage + what we
> want is less than our current free space plus whatever slack space we
> have in the disk is.  This solves the problem we were seeing in
> production and keeps us from flushing as aggressively as we approach our
> actual metadata size usage.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/space-info.c | 19 +------------------
>  1 file changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
> index 9c5f81074cd5..3d3f301bae26 100644
> --- a/fs/btrfs/space-info.c
> +++ b/fs/btrfs/space-info.c
> @@ -165,9 +165,7 @@ static int can_overcommit(struct btrfs_fs_info *fs_info,
>  			  enum btrfs_reserve_flush_enum flush,
>  			  bool system_chunk)
>  {
> -	struct btrfs_block_rsv *global_rsv = &fs_info->global_block_rsv;
>  	u64 profile;
> -	u64 space_size;
>  	u64 avail;
>  	u64 used;
>  	int factor;
> @@ -181,22 +179,7 @@ static int can_overcommit(struct btrfs_fs_info *fs_info,
>  	else
>  		profile = btrfs_metadata_alloc_profile(fs_info);
>  
> -	used = btrfs_space_info_used(space_info, false);
> -
> -	/*
> -	 * We only want to allow over committing if we have lots of actual space
> -	 * free, but if we don't have enough space to handle the global reserve
> -	 * space then we could end up having a real enospc problem when trying
> -	 * to allocate a chunk or some other such important allocation.
> -	 */
> -	spin_lock(&global_rsv->lock);
> -	space_size = calc_global_rsv_need_space(global_rsv);
> -	spin_unlock(&global_rsv->lock);
> -	if (used + space_size >= space_info->total_bytes)
> -		return 0;
> -
> -	used += space_info->bytes_may_use;
> -
> +	used = btrfs_space_info_used(space_info, true);
>  	avail = atomic64_read(&fs_info->free_chunk_space);
>  
>  	/*
>