Re: [PATCH v7 13/13] btrfs: convert to multigrain timestamps

Jan Kara
2023-08-08T17:08:03Z

On Mon 07-08-23 15:38:44, Jeff Layton wrote:
> Enable multigrain timestamps, which should ensure that there is an
> apparent change to the timestamp whenever it has been written after
> being actively observed via getattr.
>
> Beyond enabling the FS_MGTIME flag, this patch eliminates
> update_time_for_write, which goes to great pains to avoid in-memory
> stores. Just have it overwrite the timestamps unconditionally.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> Acked-by: David Sterba <dsterba@suse.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

Honza

> ---
> fs/btrfs/file.c | 24 ++++--------------------
> fs/btrfs/super.c | 5 +++--
> 2 files changed, 7 insertions(+), 22 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index d7a9ece7a40b..b9e75c9f95ac 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1106,25 +1106,6 @@ void btrfs_check_nocow_unlock(struct btrfs_inode *inode)
> btrfs_drew_write_unlock(&inode->root->snapshot_lock);
> }
>
> -static void update_time_for_write(struct inode *inode)
> -{
> - struct timespec64 now, ctime;
> -
> - if (IS_NOCMTIME(inode))
> - return;
> -
> - now = current_time(inode);
> - if (!timespec64_equal(&inode->i_mtime, &now))
> - inode->i_mtime = now;
> -
> - ctime = inode_get_ctime(inode);
> - if (!timespec64_equal(&ctime, &now))
> - inode_set_ctime_to_ts(inode, now);
> -
> - if (IS_I_VERSION(inode))
> - inode_inc_iversion(inode);
> -}
> -
> static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
> size_t count)
> {
> @@ -1156,7 +1137,10 @@ static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
> * need to start yet another transaction to update the inode as we will
> * update the inode when we finish writing whatever data we write.
> */
> - update_time_for_write(inode);
> + if (!IS_NOCMTIME(inode)) {
> + inode->i_mtime = inode_set_ctime_current(inode);
> + inode_inc_iversion(inode);
> + }
>
> start_pos = round_down(pos, fs_info->sectorsize);
> oldsize = i_size_read(inode);
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index f1dd172d8d5b..8eda51b095c9 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -2144,7 +2144,7 @@ static struct file_system_type btrfs_fs_type = {
> .name = "btrfs",
> .mount = btrfs_mount,
> .kill_sb = btrfs_kill_super,
> - .fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA,
> + .fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_MGTIME,
> };
>
> static struct file_system_type btrfs_root_fs_type = {
> @@ -2152,7 +2152,8 @@ static struct file_system_type btrfs_root_fs_type = {
> .name = "btrfs",
> .mount = btrfs_mount_root,
> .kill_sb = btrfs_kill_super,
> - .fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA | FS_ALLOW_IDMAP,
> + .fs_flags = FS_REQUIRES_DEV | FS_BINARY_MOUNTDATA |
> + FS_ALLOW_IDMAP | FS_MGTIME,
> };
>
> MODULE_ALIAS_FS("btrfs");
>
> --
> 2.41.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR