Re: [PATCH 02/16] btrfs: qgroup flush responsibility of the caller

[Josef Bacik <josef@toxicpanda.com>]

On Tue, Nov 15, 2022 at 12:00:20PM -0600, Goldwyn Rodrigues wrote:
> Qgroup reservation will be performed under extent locks. If qgroup runs
> low, btrfs starts a flush to squeeze out the last available space from
> over-reserved uncommitted extents. Move the flush outside of the
> reserve function so it can be called by the callee of the
> reserving function.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> ---
>  fs/btrfs/block-group.c    |  2 +-
>  fs/btrfs/delalloc-space.c | 17 +++++++----------
>  fs/btrfs/delalloc-space.h |  5 ++---
>  fs/btrfs/file.c           | 19 ++++++++++++++++---
>  fs/btrfs/inode.c          | 23 +++++++++++++----------
>  fs/btrfs/qgroup.c         | 27 +--------------------------
>  fs/btrfs/qgroup.h         | 16 ++++++----------
>  fs/btrfs/relocation.c     |  3 +--
>  fs/btrfs/root-tree.c      |  3 +--
>  9 files changed, 48 insertions(+), 67 deletions(-)
> 
> diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
> index 708d843daa72..578c6cbdef3b 100644
> --- a/fs/btrfs/block-group.c
> +++ b/fs/btrfs/block-group.c
> @@ -2945,7 +2945,7 @@ static int cache_save_setup(struct btrfs_block_group *block_group,
>  	cache_size *= fs_info->sectorsize;
>  
>  	ret = btrfs_check_data_free_space(BTRFS_I(inode), &data_reserved, 0,
> -					  cache_size, false);
> +					  cache_size);
>  	if (ret)
>  		goto out_put;
>  
> diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
> index 7ddb1d104e8e..b46614bec817 100644
> --- a/fs/btrfs/delalloc-space.c
> +++ b/fs/btrfs/delalloc-space.c
> @@ -130,7 +130,7 @@ int btrfs_alloc_data_chunk_ondemand(struct btrfs_inode *inode, u64 bytes)
>  
>  int btrfs_check_data_free_space(struct btrfs_inode *inode,
>  				struct extent_changeset **reserved, u64 start,
> -				u64 len, bool noflush)
> +				u64 len)
>  {
>  	struct btrfs_fs_info *fs_info = inode->root->fs_info;
>  	enum btrfs_reserve_flush_enum flush = BTRFS_RESERVE_FLUSH_DATA;
> @@ -141,9 +141,7 @@ int btrfs_check_data_free_space(struct btrfs_inode *inode,
>  	      round_down(start, fs_info->sectorsize);
>  	start = round_down(start, fs_info->sectorsize);
>  
> -	if (noflush)
> -		flush = BTRFS_RESERVE_NO_FLUSH;
> -	else if (btrfs_is_free_space_inode(inode))
> +	if (btrfs_is_free_space_inode(inode))
>  		flush = BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE;
> 

You can't do this, because we also use this path for the nowait stuff, so we
need to be able to do this noflush thing.  Additionally this patch changes a
bunch of different things at once, tho tbf it's changing this and that has
cascading effects.  Take this particular change out and see what the patch looks
like and then you can decide if it needs to be split more.  Thanks,

Josef