Re: [f2fs-dev] [PATCH] f2fs: fix to avoid potential section-unaligned pinfile

[Zhiguo Niu]

Chao Yu <chao@xxxxxxxxxx> 于2026年6月30日周二 19:02写道：
>
> On 6/30/26 16:50, Zhiguo Niu wrote:
> > Chao Yu via Linux-f2fs-devel <linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx>
> > 于2026年6月29日周一 19:52写道：
> >>
> >> Blocks of pinfile may not aligned to section size due to wrong use
> >> on pinfile, result in heavy overhead of GC, let avoid this by
> >> adding additional check condition in f2fs_setattr().
> >>
> >> - truncate -s 8mb pinfile
> >> : random checkpoint may persist filesize w/ inode
> >> - fallocate -o 0 -l 8mb pinfile
> >>  - f2fs_fallocate
> >>   - f2fs_expand_inode_data
> >>    - f2fs_allocate_pinning_section
> >>    - f2fs_map_blocks
> >>     - f2fs_map_lock
> >>     - __allocate_data_block
> >>     - file_need_truncate
> >>     : w/ FADVISE_TRUNC_BIT, we can expect unaligned mapping can be
> >>       truncated while open() if f2fs is not umount abnormally
> >>     - f2fs_map_unlock
> >>     : following f2fs checkpoint and sudden power-cut
> >>
> >> - mount
> >> - open pinfile
> >>  - f2fs_file_open
> >>   - finish_preallocate_blocks
> >>    - truncate_setsize
> >>    : filesize is 8mb
> >>    - f2fs_truncate
> >>    : can only truncate block outside filesize, rather than truncating
> >>      unaligned blocks inside filesize
> > Hi Chao,
> > 8MB is section alinged when fallocated by f2fs_allocate_pinning_section??
> > so How could there are unaligned blocks inside filesize?
>
> Zhiguo,
>
> As partial blkaddrs were persisted and recovered, see comments around
> f2fs_map_unlock().
>
> >>
> >> Fixes: f5a53edcf01e ("f2fs: support aligned pinned file")
> >> Cc: stable@xxxxxxxxxx
> >> Cc: Daeho Jeong <daehojeong@xxxxxxxxxx>
> >> Signed-off-by: Chao Yu <chao@xxxxxxxxxx>
> >> ---
> >>  fs/f2fs/file.c | 28 +++++++++++++++++-----------
> >>  1 file changed, 17 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> >> index f4facd409d9b..11cc8d79c235 100644
> >> --- a/fs/f2fs/file.c
> >> +++ b/fs/f2fs/file.c
> >> @@ -1107,17 +1107,23 @@ int f2fs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
> >>                         !IS_ALIGNED(attr->ia_size,
> >>                         F2FS_BLK_TO_BYTES(fi->i_cluster_size)))
> >>                         return -EINVAL;
> >> -               /*
> >> -                * To prevent scattered pin block generation, we don't allow
> >> -                * smaller/equal size unaligned truncation for pinned file.
> >> -                * We only support overwrite IO to pinned file, so don't
> >> -                * care about larger size truncation.
> >> -                */
> >> -               if (f2fs_is_pinned_file(inode) &&
> >> -                       attr->ia_size <= i_size_read(inode) &&
> >> -                       !IS_ALIGNED(attr->ia_size,
> >> -                       F2FS_BLK_TO_BYTES(CAP_BLKS_PER_SEC(sbi))))
> >> -                       return -EINVAL;
> >> +
> >> +               if (f2fs_is_pinned_file(inode)) {
> >> +                       /*
> >> +                        * It may break section-aligned fallocate recovery
> >> +                        * mechanism, so do not allow larger size truncation.
> >> +                        */
> >> +                       if (attr->ia_size > i_size_read(inode))
> >> +                               return -EINVAL;
> > Is it ok if we allow larger size truncation and also  limit it to
> > aligned "F2FS_BLK_TO_BYTES(CAP_BLKS_PER_SEC(sbi)"?
>
> No, I think above example will still suffer section-unaligned issue.
>
> Thanks,
Hi Chao,
got it and thanks for your explaination, so
Reviewed-by: Zhiguo Niu <zhiguo.niu@xxxxxxxxxx>
thanks!

>
> > thanks!
> >> +                       /*
> >> +                        * To prevent scattered pin block generation, we don't
> >> +                        * allow smaller/equal size unaligned truncation for
> >> +                        * pinned file.
> >> +                        */
> >> +                       else if (!IS_ALIGNED(attr->ia_size,
> >> +                               F2FS_BLK_TO_BYTES(CAP_BLKS_PER_SEC(sbi))))
> >> +                               return -EINVAL;
> >> +               }
> >>         }
> >>
> >>         if (is_quota_modification(idmap, inode, attr)) {
> >> --
> >> 2.49.0
> >>
> >>
> >>
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>