Re: [f2fs-dev] [PATCH] f2fs: do f2fs_zero_post_eof_page for all the time

[Jaegeuk Kim]

On 10/28, Chao Yu via Linux-f2fs-devel wrote:
> On 10/28/26 13:06, Jaegeuk Kim via Linux-f2fs-devel wrote:
> > This fixes xfstests/generic/794, since the issue is not related to the
> > posix=strict mode only.
> 
> Jaegeuk,
> 
> Actually, I fixed this only in fsync_mode=strict because I don't see such
> scenario (described in original patch) in Android:
> 
>     Steps of generic/794:
>     1. write 4096 bytes to file w/ 0x5a
>     2. use fiemap to get PBA of first block in file
>     3. truncate file to 4080
>     4. umount; write 4096 bytes to file w/ 0x5a directly via PBA; mount
>     5. extend filesize via
>        a) append 4096 from offset 4096, or
>        b) truncate 8192, or
>        c) fallocate 4096 from offset 4096
>     6. verify the gap is zeroed in memory [4080,4096)
>     7. sync range 4096 from offset 4096; shutdown -f (flush meta before shutdown)
>     8. umount; mount; verify [4080,4096) is zeroed or not.

I saw your comment and I think we should not connect posix=strict to who's
using that. It was supposed to handle the myth of the recovery on rename().
This sequence looks general which seems we should guarantee.

> 
> Maybe we should only enable fsync_mode=strict for 794 to let it pass?
> 
> Thanks,
> 
> > 
> > Cc: stable@xxxxxxxxxx
> > Fixes: 5eced87b7d19 ("f2fs: fix to zero post-EOF data when extending file size")
> > Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> > ---
> >  fs/f2fs/file.c | 14 ++------------
> >  1 file changed, 2 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 8b4a7a9d7dbe..823df06ae0f2 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -74,15 +74,10 @@ static int f2fs_zero_post_eof_page(struct inode *inode,
> >  					loff_t new_size, bool lock, bool writeback)
> >  {
> >  	loff_t old_size = i_size_read(inode);
> > -	bool strict =
> > -		F2FS_OPTION(F2FS_I_SB(inode)).fsync_mode == FSYNC_MODE_STRICT;
> >  
> >  	if (old_size >= new_size)
> >  		return 0;
> >  
> > -	if (!strict && mapping_empty(inode->i_mapping))
> > -		return 0;
> > -
> >  	if (lock)
> >  		filemap_invalidate_lock(inode->i_mapping);
> >  	/* zero or drop pages only in range of [old_size, new_size] */
> > @@ -90,14 +85,9 @@ static int f2fs_zero_post_eof_page(struct inode *inode,
> >  	if (lock)
> >  		filemap_invalidate_unlock(inode->i_mapping);
> >  
> > -	if (!writeback || !strict)
> > +	if (!writeback)
> >  		return 0;
> > -	/*
> > -	 * In fsync_mode=strict, when we expand an unaligned EOF size, we
> > -	 * should zero post EOF data and writeback the data immediately,
> > -	 * so that it can avoid exposing stale data after metadata flush
> > -	 * and POR.
> > -	 */
> > +
> >  	return do_zero_post_eof_page(inode, new_size);
> >  }
> >  
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel