Re: [PATCH 2/3] Btrfs: fix wrong ctime and mtime of a directory after log replay

[Nikolay Borisov <nborisov@suse.com>]

On 15.05.19 г. 18:02 ч., fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When replaying a log that contains a new file or directory name that needs
> to be added to its parent directory, we end up updating the mtime and the
> ctime of the parent directory to the current time after we have set their
> values to the correct ones (set at fsync time), efectivelly losing them.
> 
> Sample reproducer:
> 
>   $ mkfs.btrfs -f /dev/sdb
>   $ mount /dev/sdb /mnt
> 
>   $ mkdir /mnt/dir
>   $ touch /mnt/dir/file
> 
>   # fsync of the directory is optional, not needed
>   $ xfs_io -c fsync /mnt/dir
>   $ xfs_io -c fsync /mnt/dir/file
> 
>   $ stat -c %Y /mnt/dir
>   1557856079
> 
>   <power failure>
> 
>   $ sleep 3
>   $ mount /dev/sdb /mnt
>   $ stat -c %Y /mnt/dir
>   1557856082
> 
>     --> should have been 1557856079, the mtime is updated to the current
>         time when replaying the log
> 
> Fix this by not updating the mtime and ctime to the current time at
> btrfs_add_link() when we are replaying a log tree.
> 
> This could be triggered by my recent fsync fuzz tester for fstests, for
> which an fstests patch exists titled "fstests: generic, fsync fuzz tester
> with fsstress".
> 
> Fixes: e02119d5a7b43 ("Btrfs: Add a write ahead tree log to optimize synchronous operations")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/inode.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index cacde7040329..8e6d7b0bd988 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6381,8 +6381,18 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
>  	btrfs_i_size_write(parent_inode, parent_inode->vfs_inode.i_size +
>  			   name_len * 2);
>  	inode_inc_iversion(&parent_inode->vfs_inode);
> -	parent_inode->vfs_inode.i_mtime = parent_inode->vfs_inode.i_ctime =
> -		current_time(&parent_inode->vfs_inode);
> +	/*
> +	 * If we are replaying a log tree, we do not want to update the mtime
> +	 * and ctime of the parent directory with the current time, since the
> +	 * log replay procedure is responsible for setting them to their correct
> +	 * values (the ones it had when the fsync was done).
> +	 */
> +	if (!test_bit(BTRFS_FS_LOG_RECOVERING, &root->fs_info->flags)) {
> +		struct timespec64 now = current_time(&parent_inode->vfs_inode);
> +
> +		parent_inode->vfs_inode.i_mtime = now;
> +		parent_inode->vfs_inode.i_ctime = now;
> +	}
>  	ret = btrfs_update_inode(trans, root, &parent_inode->vfs_inode);
>  	if (ret)
>  		btrfs_abort_transaction(trans, ret);
>