Re: [PATCH RFC] fs: don't force i_version increment when timestamps change

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 27, 2024 at 4:30 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> inode_maybe_inc_iversion will increment the i_version if it has been
> queried. We can also set the "force" parameter to force an increment.
> When we originally did this, the idea was to set it to force when we
> were going to be otherwise updating the inode timestamps anyway --
> purely a "might as well" measure.
>
> When we used coarse-grained timestamps exclusively, this would give us
> an extra cmpxchg operation roughly every jiffy when a file is under
> heavy writes. With the advent of multigrain timestamps however, this can
> fire more frequently.
>
> There is no requirement to force an increment to the i_version just
> because a timestamp changed, so stop doing it.
>
> Cc: Mateusz Guzik <mjguzik@xxxxxxxxx>
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> I've not tested this other than for compilation, but it should be fine.
> Mateusz, does this help your workload at all? There may be other places
> where we can just set this to false (maybe even convert some of the
> inode_inc_iversion() calls to this.

This does not make a difference for me since any call to
inode_maybe_inc_iversion is guaranteed to provide a full barrier, this
is at best moving it from the cmpxchg to spelled out smp_mb

As to whether the patch makes sense otherwise I have no idea.

> ---
>  fs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index 10c4619faeef..2abd6317839b 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1962,7 +1962,7 @@ int inode_update_timestamps(struct inode *inode, int flags)
>                         inode_set_mtime_to_ts(inode, now);
>                         updated |= S_MTIME;
>                 }
> -               if (IS_I_VERSION(inode) && inode_maybe_inc_iversion(inode, updated))
> +               if (IS_I_VERSION(inode) && inode_maybe_inc_iversion(inode, false))
>                         updated |= S_VERSION;
>         } else {
>                 now = current_time(inode);
>
> ---
> base-commit: 3e9bff3bbe1355805de919f688bef4baefbfd436
> change-id: 20240827-iversion-afa53f0a070b
>
> Best regards,
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>


-- 
Mateusz Guzik <mjguzik gmail.com>





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux