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

[Daeho Jeong]

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
>