Re: [PATCH 2/5] btrfs: always reserve our entire size for the global reserve

[Nikolay Borisov <nborisov@suse.com>]

On 16.08.19 г. 18:20 ч., Josef Bacik wrote:
> While messing with the overcommit logic I noticed that sometimes we'd
> ENOSPC out when really we should have run out of space much earlier.  It
> turns out it's because we'll only reserve up to the free amount left in
> the space info for the global reserve, but that doesn't make sense with
> overcommit because we could be well above our actual size.  This results
> in the global reserve not carving out it's entire reservation, and thus
> not putting enough pressure on the rest of the infrastructure to do the
> right thing and ENOSPC out at a convenient time.  Fix this by always
> taking our full reservation amount for the global reserve.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>


In effect you could possibly overcommit by increasing bytes_may_use you
potentially cause callers of reserve_metadata_bytes to get ENOSPC, am I
right? In any case the code itself looks ok:

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

> ---
>  fs/btrfs/block-rsv.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
> index 657675eef443..18a0af20ee5a 100644
> --- a/fs/btrfs/block-rsv.c
> +++ b/fs/btrfs/block-rsv.c
> @@ -295,15 +295,10 @@ void btrfs_update_global_block_rsv(struct btrfs_fs_info *fs_info)
>  	block_rsv->size = min_t(u64, num_bytes, SZ_512M);
>  
>  	if (block_rsv->reserved < block_rsv->size) {
> -		num_bytes = btrfs_space_info_used(sinfo, true);
> -		if (sinfo->total_bytes > num_bytes) {
> -			num_bytes = sinfo->total_bytes - num_bytes;
> -			num_bytes = min(num_bytes,
> -					block_rsv->size - block_rsv->reserved);
> -			block_rsv->reserved += num_bytes;
> -			btrfs_space_info_update_bytes_may_use(fs_info, sinfo,
> -							      num_bytes);
> -		}
> +		num_bytes = block_rsv->size - block_rsv->reserved;
> +		block_rsv->reserved += num_bytes;
> +		btrfs_space_info_update_bytes_may_use(fs_info, sinfo,
> +						      num_bytes);
>  	} else if (block_rsv->reserved > block_rsv->size) {
>  		num_bytes = block_rsv->reserved - block_rsv->size;
>  		btrfs_space_info_update_bytes_may_use(fs_info, sinfo,
>