On Thu, Jul 02, 2020 at 04:46:49PM +0300, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> 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.
