Re: [PATCH 09/10] btrfs: Remove fail2 label from btrfs_submit_compressed_read

[David Sterba <dsterba@suse.cz>]

On Thu, Jul 02, 2020 at 04:46:49PM +0300, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/compression.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index 2033ce17e5c6..c28ee9fcd15d 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -661,8 +661,7 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  	u64 em_len;
>  	u64 em_start;
>  	struct extent_map *em;
> -	blk_status_t ret = BLK_STS_RESOURCE;
> -	int faili = 0;
> +	blk_status_t ret;
>  	const u16 csum_size = btrfs_super_csum_size(fs_info->super_copy);
>  	u8 *sums;
>  
> @@ -712,12 +711,16 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  		cb->compressed_pages[pg_index] = alloc_page(GFP_NOFS |
>  							      __GFP_HIGHMEM);
>  		if (!cb->compressed_pages[pg_index]) {
> -			faili = pg_index - 1;
> -			ret = BLK_STS_RESOURCE;
> -			goto fail2;
> +			int i;
> +
> +			for (i = 0; i < pg_index; i++)
> +				__free_page(cb->compressed_pages[i]);
> +
> +			kfree(cb->compressed_pages);
> +			kfree(cb);
> +			return BLK_STS_RESOURCE;

No, that's pushing the error cleanup code to the main code path.

>  		}
>  	}
> -	faili = nr_pages - 1;
>  	cb->nr_pages = nr_pages;
>  
>  	add_ra_bio_pages(inode, em_start + em_len, cb);
> @@ -801,14 +804,6 @@ blk_status_t btrfs_submit_compressed_read(struct inode *inode, struct bio *bio,
>  
>  	return 0;
>  
> -fail2:
> -	while (faili >= 0) {
> -		__free_page(cb->compressed_pages[faili]);
> -		faili--;
> -	}
> -
> -	kfree(cb->compressed_pages);
> -	kfree(cb);
>  out:
>  	free_extent_map(em);
>  	return ret;

If there's anything to fix in this function, it's the labels and
variable naming. 'faili' should be eg. last_index, fail1 -> out_free_cb,
fail2 -> out_free_pages, and out -> out_free_map.