Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 15 Jul 2022 14:22:53 +0800
From:   Yue Hu <zbestahu@gmail.com>
To:     Gao Xiang <hsiangkao@linux.alibaba.com>
Cc:     linux-erofs@lists.ozlabs.org, Chao Yu <chao@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>, zhangwen@coolpad.com
Subject: Re: [PATCH 03/16] erofs: introduce `z_erofs_parse_out_bvecs()'
Message-ID: <20220715142253.00005239.zbestahu@gmail.com>
In-Reply-To: <20220714132051.46012-4-hsiangkao@linux.alibaba.com>
References: <20220714132051.46012-1-hsiangkao@linux.alibaba.com>
        <20220714132051.46012-4-hsiangkao@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 14 Jul 2022 21:20:38 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> `z_erofs_decompress_pcluster()' is too long therefore it'd be better
> to introduce another helper to parse decompressed pages (or laterly,
> decompressed bvecs.)
> 
> BTW, since `decompressed_bvecs' is too long as a part of the function
> name, `out_bvecs' is used instead.
> 
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
>  fs/erofs/zdata.c | 81 +++++++++++++++++++++++++-----------------------
>  1 file changed, 43 insertions(+), 38 deletions(-)
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index c7be447ac64d..c183cd0bc42b 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -778,18 +778,58 @@ static bool z_erofs_page_is_invalidated(struct page *page)
>  	return !page->mapping && !z_erofs_is_shortlived_page(page);
>  }
>  
> +static int z_erofs_parse_out_bvecs(struct z_erofs_pcluster *pcl,
> +				   struct page **pages, struct page **pagepool)
> +{
> +	struct z_erofs_pagevec_ctor ctor;
> +	enum z_erofs_page_type page_type;
> +	int i, err = 0;
> +
> +	z_erofs_pagevec_ctor_init(&ctor, Z_EROFS_NR_INLINE_PAGEVECS,
> +				  pcl->pagevec, 0);
> +	for (i = 0; i < pcl->vcnt; ++i) {
> +		struct page *page = z_erofs_pagevec_dequeue(&ctor, &page_type);
> +		unsigned int pagenr;
> +
> +		/* all pages in pagevec ought to be valid */
> +		DBG_BUGON(!page);
> +		DBG_BUGON(z_erofs_page_is_invalidated(page));
> +
> +		if (z_erofs_put_shortlivedpage(pagepool, page))
> +			continue;
> +
> +		if (page_type == Z_EROFS_VLE_PAGE_TYPE_HEAD)
> +			pagenr = 0;
> +		else
> +			pagenr = z_erofs_onlinepage_index(page);
> +
> +		DBG_BUGON(pagenr >= pcl->nr_pages);
> +		/*
> +		 * currently EROFS doesn't support multiref(dedup),
> +		 * so here erroring out one multiref page.
> +		 */
> +		if (pages[pagenr]) {
> +			DBG_BUGON(1);
> +			SetPageError(pages[pagenr]);
> +			z_erofs_onlinepage_endio(pages[pagenr]);
> +			err = -EFSCORRUPTED;
> +		}
> +		pages[pagenr] = page;
> +	}
> +	z_erofs_pagevec_ctor_exit(&ctor, true);
> +	return err;
> +}
> +
>  static int z_erofs_decompress_pcluster(struct super_block *sb,
>  				       struct z_erofs_pcluster *pcl,
>  				       struct page **pagepool)
>  {
>  	struct erofs_sb_info *const sbi = EROFS_SB(sb);
>  	unsigned int pclusterpages = z_erofs_pclusterpages(pcl);
> -	struct z_erofs_pagevec_ctor ctor;
>  	unsigned int i, inputsize, outputsize, llen, nr_pages;
>  	struct page *pages_onstack[Z_EROFS_VMAP_ONSTACK_PAGES];
>  	struct page **pages, **compressed_pages, *page;
>  
> -	enum z_erofs_page_type page_type;
>  	bool overlapped, partial;
>  	int err;
>  
> @@ -823,42 +863,7 @@ static int z_erofs_decompress_pcluster(struct super_block *sb,
>  	for (i = 0; i < nr_pages; ++i)
>  		pages[i] = NULL;
>  
> -	err = 0;
> -	z_erofs_pagevec_ctor_init(&ctor, Z_EROFS_NR_INLINE_PAGEVECS,
> -				  pcl->pagevec, 0);
> -
> -	for (i = 0; i < pcl->vcnt; ++i) {
> -		unsigned int pagenr;
> -
> -		page = z_erofs_pagevec_dequeue(&ctor, &page_type);
> -
> -		/* all pages in pagevec ought to be valid */
> -		DBG_BUGON(!page);
> -		DBG_BUGON(z_erofs_page_is_invalidated(page));
> -
> -		if (z_erofs_put_shortlivedpage(pagepool, page))
> -			continue;
> -
> -		if (page_type == Z_EROFS_VLE_PAGE_TYPE_HEAD)
> -			pagenr = 0;
> -		else
> -			pagenr = z_erofs_onlinepage_index(page);
> -
> -		DBG_BUGON(pagenr >= nr_pages);
> -
> -		/*
> -		 * currently EROFS doesn't support multiref(dedup),
> -		 * so here erroring out one multiref page.
> -		 */
> -		if (pages[pagenr]) {
> -			DBG_BUGON(1);
> -			SetPageError(pages[pagenr]);
> -			z_erofs_onlinepage_endio(pages[pagenr]);
> -			err = -EFSCORRUPTED;
> -		}
> -		pages[pagenr] = page;
> -	}
> -	z_erofs_pagevec_ctor_exit(&ctor, true);
> +	err = z_erofs_parse_out_bvecs(pcl, pages, pagepool);
>  
>  	overlapped = false;
>  	compressed_pages = pcl->compressed_pages;

Reviewed-by: Yue Hu <huyue2@coolpad.com>