Re: [PATCH 2/3] btrfs: only reserve metadata_size for inodes

[Nikolay Borisov <nborisov@suse.com>]

On 16.08.19 г. 18:05 ч., Josef Bacik wrote:
> Historically we reserved worst case for every btree operation, and
> generally speaking we want to do that in cases where it could be the
> worst case.  However for updating inodes we know the inode items are
> already in the tree, so it will only be an update operation and never an
> insert operation.  This allows us to always reserve only the
> metadata_size amount for inode updates rather than the
> insert_metadata_size amount.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

This alleviates some of the reservation pressure so :

Reviewed-by: Nikolay Borisov <nborisov@suse.com>, however one small nit
below.

> ---
>  fs/btrfs/delalloc-space.c | 15 ++++++++++++---
>  fs/btrfs/delayed-inode.c  |  2 +-
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
> index 2412be4a3de2..b8111ebdc92a 100644
> --- a/fs/btrfs/delalloc-space.c
> +++ b/fs/btrfs/delalloc-space.c
> @@ -251,9 +251,16 @@ static void btrfs_calculate_inode_block_rsv_size(struct btrfs_fs_info *fs_info,
>  
>  	lockdep_assert_held(&inode->lock);
>  	outstanding_extents = inode->outstanding_extents;
> -	if (outstanding_extents)
> +
> +	/*
> +	 * Insert size for the number of outstanding extents, 1 normal size for
> +	 * updating the inode.
> +	 */
> +	if (outstanding_extents) {
>  		reserve_size = btrfs_calc_insert_metadata_size(fs_info,
> -						outstanding_extents + 1);
> +						outstanding_extents);
> +		reserve_size += btrfs_calc_metadata_size(fs_info, 1);
> +	}
>  	csum_leaves = btrfs_csum_bytes_to_leaves(fs_info,
>  						 inode->csum_bytes);
>  	reserve_size += btrfs_calc_insert_metadata_size(fs_info,
> @@ -278,10 +285,12 @@ static void calc_inode_reservations(struct btrfs_fs_info *fs_info,
>  {
>  	u64 nr_extents = count_max_extents(num_bytes);
>  	u64 csum_leaves = btrfs_csum_bytes_to_leaves(fs_info, num_bytes);
> +	u64 inode_update = btrfs_calc_metadata_size(fs_info, 1);
>  
>  	/* We add one for the inode update at finish ordered time */

This comment becomes somewhat outdated and should be removed/reworded.
Perhaps put above *meta_reserve += inode_update.

>  	*meta_reserve = btrfs_calc_insert_metadata_size(fs_info,
> -						nr_extents + csum_leaves + 1);
> +						nr_extents + csum_leaves);
> +	*meta_reserve += inode_update;
>  	*qgroup_reserve = nr_extents * fs_info->nodesize;
>  }
>  
> diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> index de87ea7ce84d..9318cf761a07 100644
> --- a/fs/btrfs/delayed-inode.c
> +++ b/fs/btrfs/delayed-inode.c
> @@ -612,7 +612,7 @@ static int btrfs_delayed_inode_reserve_metadata(
>  	src_rsv = trans->block_rsv;
>  	dst_rsv = &fs_info->delayed_block_rsv;
>  
> -	num_bytes = btrfs_calc_insert_metadata_size(fs_info, 1);
> +	num_bytes = btrfs_calc_metadata_size(fs_info, 1);
>  
>  	/*
>  	 * btrfs_dirty_inode will update the inode under btrfs_join_transaction
>