Re: [f2fs-dev] [PATCH] f2fs: write the directory inode in fsync before checkpoint

[Chao Yu]

On 10/8/26 06:36, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@xxxxxxxxxx>
> 
> fsync() on a directory is done by a checkpoint, but the checkpoint
> does not write timestamp-only inode updates. Since commit 7c45729a4d6d
> ("f2fs: keep dirty inodes selectively for checkpoint"), such updates
> are marked with sync=false and left to inode writeback on purpose.
> 
> fsync() on a regular file calls f2fs_write_inode() first when the
> inode is dirty, but fsync() on a directory goes to the checkpoint
> directly and misses it. So if the last change to a directory only
> updated its timestamps, e.g. unlinking a file in it, the new mtime is
> lost after a sudden power off even though fsync() returned. generic/547
> fails sometimes because of this.

Has 9ec09d5f4b31 ("f2fs: dirty directory inodes on mtime/ctime update") fixed
this issue?

Thanks,

> 
> Call f2fs_write_inode() for a directory before the checkpoint, as the
> regular file path does. The inode block becomes dirty and is written by
> the same checkpoint, so no extra checkpoint or flush is added.
> Timestamp-only updates without fsync() are still not written by the
> checkpoint.
> 
> Fixes: 7c45729a4d6d ("f2fs: keep dirty inodes selectively for checkpoint")
> Signed-off-by: Daeho Jeong <daehojeong@xxxxxxxxxx>
> ---
>  fs/f2fs/file.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 56c686b8580..b012cc22307 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -351,8 +351,15 @@ static int f2fs_do_sync_file(struct file *file, loff_t start, loff_t end,
>  
>  	trace_f2fs_sync_file_enter(inode);
>  
> -	if (S_ISDIR(inode->i_mode))
> +	if (S_ISDIR(inode->i_mode)) {
> +		/*
> +		 * Checkpoint writes only the inodes on the dirty meta list.
> +		 * Timestamp updates of a directory, e.g. by unlinking a file
> +		 * in it, do not put it on the list, so write it here.
> +		 */
> +		f2fs_write_inode(inode, NULL);
>  		goto go_write;
> +	}
>  
>  	/* if fdatasync is triggered, let's do in-place-update */
>  	if (datasync || get_dirty_pages(inode) <= SM_I(sbi)->min_fsync_blocks)