Re: [f2fs-dev] [PATCH] f2fs: fix race of pending_pages in decompression

From: Daeho Jeong
Date: Fri Dec 04 2020 - 22:42:23 EST


Yep, we need to come back to v1 and enable verity in a unit of cluster.
Plus, as I told you, I'll prevent newly verity enalbed pages from
being merged with verity disabled bio.

Thanks,

2020년 12월 5일 (토) 오전 3:29, Jaegeuk Kim <jaegeuk@xxxxxxxxxx>님이 작성:
>
> On 12/04, Daeho Jeong wrote:
> > Thanks for the explanation about verity.
> > I got your point. Thanks~
>
> Possible fix can be like this?
>
> ---
> fs/f2fs/compress.c | 2 --
> fs/f2fs/data.c | 19 +++++++++++++------
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 89f73a7c8667..c5fee4d7ea72 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1491,8 +1491,6 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc)
> dic->magic = F2FS_COMPRESSED_PAGE_MAGIC;
> dic->inode = cc->inode;
> atomic_set(&dic->pending_pages, cc->nr_cpages);
> - if (fsverity_active(cc->inode))
> - atomic_set(&dic->verity_pages, cc->nr_cpages);
> dic->cluster_idx = cc->cluster_idx;
> dic->cluster_size = cc->cluster_size;
> dic->log_cluster_size = cc->log_cluster_size;
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index e3168f32f943..657fb562d7d4 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1035,7 +1035,8 @@ static inline bool f2fs_need_verity(const struct inode *inode, pgoff_t idx)
>
> static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
> unsigned nr_pages, unsigned op_flag,
> - pgoff_t first_idx, bool for_write)
> + pgoff_t first_idx, bool for_write,
> + bool for_verity)
> {
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> struct bio *bio;
> @@ -1057,7 +1058,7 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
> post_read_steps |= 1 << STEP_DECRYPT;
> if (f2fs_compressed_file(inode))
> post_read_steps |= 1 << STEP_DECOMPRESS_NOWQ;
> - if (f2fs_need_verity(inode, first_idx))
> + if (for_verity && f2fs_need_verity(inode, first_idx))
> post_read_steps |= 1 << STEP_VERITY;
>
> if (post_read_steps) {
> @@ -1087,7 +1088,7 @@ static int f2fs_submit_page_read(struct inode *inode, struct page *page,
> struct bio *bio;
>
> bio = f2fs_grab_read_bio(inode, blkaddr, 1, op_flags,
> - page->index, for_write);
> + page->index, for_write, true);
> if (IS_ERR(bio))
> return PTR_ERR(bio);
>
> @@ -2141,7 +2142,7 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page,
> if (bio == NULL) {
> bio = f2fs_grab_read_bio(inode, block_nr, nr_pages,
> is_readahead ? REQ_RAHEAD : 0, page->index,
> - false);
> + false, true);
> if (IS_ERR(bio)) {
> ret = PTR_ERR(bio);
> bio = NULL;
> @@ -2188,6 +2189,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> const unsigned blkbits = inode->i_blkbits;
> const unsigned blocksize = 1 << blkbits;
> struct decompress_io_ctx *dic = NULL;
> + bool for_verity = false;
> int i;
> int ret = 0;
>
> @@ -2253,6 +2255,11 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> goto out_put_dnode;
> }
>
> + if (fsverity_active(cc->inode)) {
> + atomic_set(&dic->verity_pages, cc->nr_cpages);
> + for_verity = true;
> + }
> +
> for (i = 0; i < dic->nr_cpages; i++) {
> struct page *page = dic->cpages[i];
> block_t blkaddr;
> @@ -2272,7 +2279,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> if (!bio) {
> bio = f2fs_grab_read_bio(inode, blkaddr, nr_pages,
> is_readahead ? REQ_RAHEAD : 0,
> - page->index, for_write);
> + page->index, for_write, for_verity);
> if (IS_ERR(bio)) {
> unsigned int remained = dic->nr_cpages - i;
> bool release = false;
> @@ -2280,7 +2287,7 @@ int f2fs_read_multi_pages(struct compress_ctx *cc, struct bio **bio_ret,
> ret = PTR_ERR(bio);
> dic->failed = true;
>
> - if (fsverity_active(inode)) {
> + if (for_verity) {
> if (!atomic_sub_return(remained,
> &dic->verity_pages))
> release = true;
> --
> 2.29.2.576.ga3fc446d84-goog
>