Re: [PATCH 3/6] btrfs: consolidate the error handling in run_delalloc_nocow

[Boris Burkov <boris@bur.io>]

On Mon, Jul 24, 2023 at 07:22:40AM -0700, Christoph Hellwig wrote:
> Share the calls to extent_clear_unlock_delalloc for btrfs_path allocation
> failure handling and the normal exit path.
> 
> This relies on btrfs_free_path ignoring a NULL pointer, and the
> initialization of cur_offset to start at the beginning of the function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/btrfs/inode.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 212aca4eea442b..2d465b50c228ed 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -1973,21 +1973,14 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>  	struct btrfs_path *path;
>  	u64 cow_start = (u64)-1;
>  	u64 cur_offset = start;
> -	int ret;
> +	int ret = -ENOMEM;
>  	bool check_prev = true;
>  	u64 ino = btrfs_ino(inode);
>  	struct can_nocow_file_extent_args nocow_args = { 0 };
>  
>  	path = btrfs_alloc_path();
> -	if (!path) {
> -		extent_clear_unlock_delalloc(inode, start, end, locked_page,
> -					     EXTENT_LOCKED | EXTENT_DELALLOC |
> -					     EXTENT_DO_ACCOUNTING |
> -					     EXTENT_DEFRAG, PAGE_UNLOCK |
> -					     PAGE_START_WRITEBACK |
> -					     PAGE_END_WRITEBACK);
> -		return -ENOMEM;
> -	}
> +	if (!path)
> +		goto error;

nit: I think it's nicer to do ret = -ENOMEM here rather than relying on
initializion. It makes it less likely for a different change to
accidentally disrupt the implicit assumption that ret == -ENOMEM.

>  
>  	nocow_args.end = end;
>  	nocow_args.writeback_path = true;
> -- 
> 2.39.2
>