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

Nikolay Borisov
2019-05-16T07:09:04Z

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);
>