Re: [PATCH 2/2] btrfs: reserve extra space for the free space tree

[Nikolay Borisov <nborisov@suse.com>]

On 1.12.21 г. 18:45, Josef Bacik wrote:
> Filipe reported a problem where sometimes he'd get an ENOSPC abort when
> running delayed refs with generic/619 and the free space tree enabled.
> This is partly because we do not reserve space for modifying the free
> space tree, nor do we have a block rsv associated with that tree.  Fix
> this by making sure any free space tree defaults to using the
> delayed_refs_rsv, and make sure we reserve the space for those
> allocations.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  fs/btrfs/block-rsv.c   |  1 +
>  fs/btrfs/delayed-ref.c | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
> index b3086f252ad0..b3ee49b0b1e8 100644
> --- a/fs/btrfs/block-rsv.c
> +++ b/fs/btrfs/block-rsv.c
> @@ -426,6 +426,7 @@ void btrfs_init_root_block_rsv(struct btrfs_root *root)
>  	switch (root->root_key.objectid) {
>  	case BTRFS_CSUM_TREE_OBJECTID:
>  	case BTRFS_EXTENT_TREE_OBJECTID:
> +	case BTRFS_FREE_SPACE_TREE_OBJECTID:
>  		root->block_rsv = &fs_info->delayed_refs_rsv;
>  		break;
>  	case BTRFS_ROOT_TREE_OBJECTID:
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index da9d20813147..533521be8fdf 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -84,6 +84,17 @@ void btrfs_delayed_refs_rsv_release(struct btrfs_fs_info *fs_info, int nr)
>  	u64 num_bytes = btrfs_calc_insert_metadata_size(fs_info, nr);
>  	u64 released = 0;
>  
> +	/*
> +	 * We have to check the mount option here because we could be enabling
> +	 * the free space tree for the first time and don't have the compat_ro
> +	 * option set yet.
> +	 *
> +	 * We need extra reservations if we have the free space tree because
> +	 * we'll have to modify that tree as well.
> +	 */
> +	if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
> +		num_bytes <<= 1;
> +
>  	released = btrfs_block_rsv_release(fs_info, block_rsv, num_bytes, NULL);
>  	if (released)
>  		trace_btrfs_space_reservation(fs_info, "delayed_refs_rsv",
> @@ -108,6 +119,17 @@ void btrfs_update_delayed_refs_rsv(struct btrfs_trans_handle *trans)
>  
>  	num_bytes = btrfs_calc_insert_metadata_size(fs_info,
>  						    trans->delayed_ref_updates);
> +	/*
> +	 * We have to check the mount option here because we could be enabling
> +	 * the free space tree for the first time and don't have the compat_ro
> +	 * option set yet.
> +	 *
> +	 * We need extra reservations if we have the free space tree because
> +	 * we'll have to modify that tree as well.
> +	 */
> +	if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
> +		num_bytes <<= 1;

That num_bytes * 2 is heuristically derived, right? If so I'd like this
to be mentioned explicitly in the changelog.

> +
>  	spin_lock(&delayed_rsv->lock);
>  	delayed_rsv->size += num_bytes;
>  	delayed_rsv->full = 0;
>