Re: [f2fs-dev] [PATCH v4 4/6] f2fs: compress: fix to avoid inconsistence bewteen i_blocks and dnode

[Daeho Jeong]

Reviewed-by: Daeho Jeong <daehojeong@xxxxxxxxxx>

On Wed, Jan 10, 2024 at 10:43 PM Chao Yu <chao@xxxxxxxxxx> wrote:
>
> In reserve_compress_blocks(), we update blkaddrs of dnode in prior to
> inc_valid_block_count(), it may cause inconsistent status bewteen
> i_blocks and blkaddrs once inc_valid_block_count() fails.
>
> To fix this issue, it needs to reverse their invoking order.
>
> Fixes: c75488fb4d82 ("f2fs: introduce F2FS_IOC_RESERVE_COMPRESS_BLOCKS")
> Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
> ---
>  fs/f2fs/data.c    |  5 +++--
>  fs/f2fs/f2fs.h    |  7 ++++++-
>  fs/f2fs/file.c    | 26 ++++++++++++++------------
>  fs/f2fs/segment.c |  2 +-
>  4 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index b171a9980f6a..8d2ace723310 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1219,7 +1219,8 @@ int f2fs_reserve_new_blocks(struct dnode_of_data *dn, blkcnt_t count)
>
>         if (unlikely(is_inode_flag_set(dn->inode, FI_NO_ALLOC)))
>                 return -EPERM;
> -       if (unlikely((err = inc_valid_block_count(sbi, dn->inode, &count))))
> +       err = inc_valid_block_count(sbi, dn->inode, &count, true);
> +       if (unlikely(err))
>                 return err;
>
>         trace_f2fs_reserve_new_blocks(dn->inode, dn->nid,
> @@ -1476,7 +1477,7 @@ static int __allocate_data_block(struct dnode_of_data *dn, int seg_type)
>
>         dn->data_blkaddr = f2fs_data_blkaddr(dn);
>         if (dn->data_blkaddr == NULL_ADDR) {
> -               err = inc_valid_block_count(sbi, dn->inode, &count);
> +               err = inc_valid_block_count(sbi, dn->inode, &count, true);
>                 if (unlikely(err))
>                         return err;
>         }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 50f3d546ded8..69e71460a950 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2252,7 +2252,7 @@ static inline bool __allow_reserved_blocks(struct f2fs_sb_info *sbi,
>
>  static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool);
>  static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
> -                                struct inode *inode, blkcnt_t *count)
> +                                struct inode *inode, blkcnt_t *count, bool partial)
>  {
>         blkcnt_t diff = 0, release = 0;
>         block_t avail_user_block_count;
> @@ -2292,6 +2292,11 @@ static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
>                         avail_user_block_count = 0;
>         }
>         if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
> +               if (!partial) {
> +                       spin_unlock(&sbi->stat_lock);
> +                       goto enospc;
> +               }
> +
>                 diff = sbi->total_valid_block_count - avail_user_block_count;
>                 if (diff > *count)
>                         diff = *count;
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 80d9c4c096f0..53c495651789 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3614,14 +3614,16 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
>                 blkcnt_t reserved;
>                 int ret;
>
> -               for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
> -                       blkaddr = f2fs_data_blkaddr(dn);
> +               for (i = 0; i < cluster_size; i++) {
> +                       blkaddr = data_blkaddr(dn->inode, dn->node_page,
> +                                               dn->ofs_in_node + i);
>
>                         if (i == 0) {
> -                               if (blkaddr == COMPRESS_ADDR)
> -                                       continue;
> -                               dn->ofs_in_node += cluster_size;
> -                               goto next;
> +                               if (blkaddr != COMPRESS_ADDR) {
> +                                       dn->ofs_in_node += cluster_size;
> +                                       goto next;
> +                               }
> +                               continue;
>                         }
>
>                         /*
> @@ -3634,20 +3636,20 @@ static int reserve_compress_blocks(struct dnode_of_data *dn, pgoff_t count)
>                                 compr_blocks++;
>                                 continue;
>                         }
> -
> -                       f2fs_set_data_blkaddr(dn, NEW_ADDR);
>                 }
>
>                 reserved = cluster_size - compr_blocks;
>                 if (!reserved)
>                         goto next;
>
> -               ret = inc_valid_block_count(sbi, dn->inode, &reserved);
> -               if (ret)
> +               ret = inc_valid_block_count(sbi, dn->inode, &reserved, false);
> +               if (unlikely(ret))
>                         return ret;
>
> -               if (reserved != cluster_size - compr_blocks)
> -                       return -ENOSPC;
> +               for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
> +                       if (f2fs_data_blkaddr(dn) == NULL_ADDR)
> +                               f2fs_set_data_blkaddr(dn, NEW_ADDR);
> +               }
>
>                 f2fs_i_compr_blocks_update(dn->inode, compr_blocks, true);
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 4c8836ded90f..ef5b3848426b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -248,7 +248,7 @@ static int __replace_atomic_write_block(struct inode *inode, pgoff_t index,
>         } else {
>                 blkcnt_t count = 1;
>
> -               err = inc_valid_block_count(sbi, inode, &count);
> +               err = inc_valid_block_count(sbi, inode, &count, true);
>                 if (err) {
>                         f2fs_put_dnode(&dn);
>                         return err;
> --
> 2.40.1
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel