linux-kernel - Re: [PATCH] erofs: avoid unnecessary z_erofs_decompressqueue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220122121827.0000243c.zbestahu@gmail.com>
Date:   Sat, 22 Jan 2022 12:18:27 +0800
From:   Yue Hu <zbestahu@...il.com>
To:     Gao Xiang <hsiangkao@...ux.alibaba.com>
Cc:     linux-erofs@...ts.ozlabs.org, Chao Yu <chao@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] erofs: avoid unnecessary z_erofs_decompressqueue_work()
 declaration

On Fri, 21 Jan 2022 17:14:12 +0800
Gao Xiang <hsiangkao@...ux.alibaba.com> wrote:

> Just code rearrange. No logic changes.
> 
> Signed-off-by: Gao Xiang <hsiangkao@...ux.alibaba.com>
> ---
>  fs/erofs/zdata.c | 113 +++++++++++++++++++++++------------------------
>  1 file changed, 56 insertions(+), 57 deletions(-)
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index 498b7666efe8..423bc1a61da5 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -810,68 +810,11 @@ static bool z_erofs_get_sync_decompress_policy(struct erofs_sb_info *sbi,
>  	return false;
>  }
>  
> -static void z_erofs_decompressqueue_work(struct work_struct *work);
> -static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
> -				       bool sync, int bios)
> -{
> -	struct erofs_sb_info *const sbi = EROFS_SB(io->sb);
> -
> -	/* wake up the caller thread for sync decompression */
> -	if (sync) {
> -		unsigned long flags;
> -
> -		spin_lock_irqsave(&io->u.wait.lock, flags);
> -		if (!atomic_add_return(bios, &io->pending_bios))
> -			wake_up_locked(&io->u.wait);
> -		spin_unlock_irqrestore(&io->u.wait.lock, flags);
> -		return;
> -	}
> -
> -	if (atomic_add_return(bios, &io->pending_bios))
> -		return;
> -	/* Use workqueue and sync decompression for atomic contexts only */
> -	if (in_atomic() || irqs_disabled()) {
> -		queue_work(z_erofs_workqueue, &io->u.work);
> -		/* enable sync decompression for readahead */
> -		if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
> -			sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_FORCE_ON;
> -		return;
> -	}
> -	z_erofs_decompressqueue_work(&io->u.work);
> -}
> -
>  static bool z_erofs_page_is_invalidated(struct page *page)
>  {
>  	return !page->mapping && !z_erofs_is_shortlived_page(page);
>  }
>  
> -static void z_erofs_decompressqueue_endio(struct bio *bio)
> -{
> -	tagptr1_t t = tagptr_init(tagptr1_t, bio->bi_private);
> -	struct z_erofs_decompressqueue *q = tagptr_unfold_ptr(t);
> -	blk_status_t err = bio->bi_status;
> -	struct bio_vec *bvec;
> -	struct bvec_iter_all iter_all;
> -
> -	bio_for_each_segment_all(bvec, bio, iter_all) {
> -		struct page *page = bvec->bv_page;
> -
> -		DBG_BUGON(PageUptodate(page));
> -		DBG_BUGON(z_erofs_page_is_invalidated(page));
> -
> -		if (err)
> -			SetPageError(page);
> -
> -		if (erofs_page_is_managed(EROFS_SB(q->sb), page)) {
> -			if (!err)
> -				SetPageUptodate(page);
> -			unlock_page(page);
> -		}
> -	}
> -	z_erofs_decompress_kickoff(q, tagptr_unfold_tags(t), -1);
> -	bio_put(bio);
> -}
> -
>  static int z_erofs_decompress_pcluster(struct super_block *sb,
>  				       struct z_erofs_pcluster *pcl,
>  				       struct page **pagepool)
> @@ -1123,6 +1066,35 @@ static void z_erofs_decompressqueue_work(struct work_struct *work)
>  	kvfree(bgq);
>  }
>  
> +static void z_erofs_decompress_kickoff(struct z_erofs_decompressqueue *io,
> +				       bool sync, int bios)
> +{
> +	struct erofs_sb_info *const sbi = EROFS_SB(io->sb);
> +
> +	/* wake up the caller thread for sync decompression */
> +	if (sync) {
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&io->u.wait.lock, flags);
> +		if (!atomic_add_return(bios, &io->pending_bios))
> +			wake_up_locked(&io->u.wait);
> +		spin_unlock_irqrestore(&io->u.wait.lock, flags);
> +		return;
> +	}
> +
> +	if (atomic_add_return(bios, &io->pending_bios))
> +		return;
> +	/* Use workqueue and sync decompression for atomic contexts only */
> +	if (in_atomic() || irqs_disabled()) {
> +		queue_work(z_erofs_workqueue, &io->u.work);
> +		/* enable sync decompression for readahead */
> +		if (sbi->opt.sync_decompress == EROFS_SYNC_DECOMPRESS_AUTO)
> +			sbi->opt.sync_decompress = EROFS_SYNC_DECOMPRESS_FORCE_ON;
> +		return;
> +	}
> +	z_erofs_decompressqueue_work(&io->u.work);
> +}
> +
>  static struct page *pickup_page_for_submission(struct z_erofs_pcluster *pcl,
>  					       unsigned int nr,
>  					       struct page **pagepool,
> @@ -1300,6 +1272,33 @@ static void move_to_bypass_jobqueue(struct z_erofs_pcluster *pcl,
>  	qtail[JQ_BYPASS] = &pcl->next;
>  }
>  
> +static void z_erofs_decompressqueue_endio(struct bio *bio)
> +{
> +	tagptr1_t t = tagptr_init(tagptr1_t, bio->bi_private);
> +	struct z_erofs_decompressqueue *q = tagptr_unfold_ptr(t);
> +	blk_status_t err = bio->bi_status;
> +	struct bio_vec *bvec;
> +	struct bvec_iter_all iter_all;
> +
> +	bio_for_each_segment_all(bvec, bio, iter_all) {
> +		struct page *page = bvec->bv_page;
> +
> +		DBG_BUGON(PageUptodate(page));
> +		DBG_BUGON(z_erofs_page_is_invalidated(page));
> +
> +		if (err)
> +			SetPageError(page);
> +
> +		if (erofs_page_is_managed(EROFS_SB(q->sb), page)) {
> +			if (!err)
> +				SetPageUptodate(page);
> +			unlock_page(page);
> +		}
> +	}
> +	z_erofs_decompress_kickoff(q, tagptr_unfold_tags(t), -1);
> +	bio_put(bio);
> +}
> +

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

>  static void z_erofs_submit_queue(struct super_block *sb,
>  				 struct z_erofs_decompress_frontend *f,
>  				 struct page **pagepool,