[v6,2/7] fs: add infrastructure for multigrain timestamps

The VFS always uses coarse-grained timestamps when updating the ctime
and mtime after a change. This has the benefit of allowing filesystems
to optimize away a lot metadata updates, down to around 1 per jiffy,
even when a file is under heavy writes.

Unfortunately, this has always been an issue when we're exporting via
NFSv3, which relies on timestamps to validate caches. A lot of changes
can happen in a jiffy, so timestamps aren't sufficient to help the
client decide to invalidate the cache. Even with NFSv4, a lot of
exported filesystems don't properly support a change attribute and are
subject to the same problems with timestamp granularity. Other
applications have similar issues with timestamps (e.g backup
applications).

If we were to always use fine-grained timestamps, that would improve the
situation, but that becomes rather expensive, as the underlying
filesystem would have to log a lot more metadata updates.

What we need is a way to only use fine-grained timestamps when they are
being actively queried.

POSIX generally mandates that when the the mtime changes, the ctime must
also change. The kernel always stores normalized ctime values, so only
the first 30 bits of the tv_nsec field are ever used.

Use the 31st bit of the ctime tv_nsec field to indicate that something
has queried the inode for the mtime or ctime. When this flag is set,
on the next mtime or ctime update, the kernel will fetch a fine-grained
timestamp instead of the usual coarse-grained one.

Filesytems can opt into this behavior by setting the FS_MGTIME flag in
the fstype. Filesystems that don't set this flag will continue to use
coarse-grained timestamps.

Later patches will convert individual filesystems to use the new
infrastructure.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/inode.c         | 98 ++++++++++++++++++++++++++++++++++++++----------------
 fs/stat.c          | 41 +++++++++++++++++++++--
 include/linux/fs.h | 45 +++++++++++++++++++++++--
 3 files changed, 151 insertions(+), 33 deletions(-)

On Tue 25-07-23 10:58:15, Jeff Layton wrote:
> The VFS always uses coarse-grained timestamps when updating the ctime
> and mtime after a change. This has the benefit of allowing filesystems
> to optimize away a lot metadata updates, down to around 1 per jiffy,
> even when a file is under heavy writes.
> 
> Unfortunately, this has always been an issue when we're exporting via
> NFSv3, which relies on timestamps to validate caches. A lot of changes
> can happen in a jiffy, so timestamps aren't sufficient to help the
> client decide to invalidate the cache. Even with NFSv4, a lot of
> exported filesystems don't properly support a change attribute and are
> subject to the same problems with timestamp granularity. Other
> applications have similar issues with timestamps (e.g backup
> applications).
> 
> If we were to always use fine-grained timestamps, that would improve the
> situation, but that becomes rather expensive, as the underlying
> filesystem would have to log a lot more metadata updates.
> 
> What we need is a way to only use fine-grained timestamps when they are
> being actively queried.
> 
> POSIX generally mandates that when the the mtime changes, the ctime must
> also change. The kernel always stores normalized ctime values, so only
> the first 30 bits of the tv_nsec field are ever used.
> 
> Use the 31st bit of the ctime tv_nsec field to indicate that something
> has queried the inode for the mtime or ctime. When this flag is set,
> on the next mtime or ctime update, the kernel will fetch a fine-grained
> timestamp instead of the usual coarse-grained one.
> 
> Filesytems can opt into this behavior by setting the FS_MGTIME flag in
> the fstype. Filesystems that don't set this flag will continue to use
> coarse-grained timestamps.
> 
> Later patches will convert individual filesystems to use the new
> infrastructure.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/inode.c         | 98 ++++++++++++++++++++++++++++++++++++++----------------
>  fs/stat.c          | 41 +++++++++++++++++++++--
>  include/linux/fs.h | 45 +++++++++++++++++++++++--
>  3 files changed, 151 insertions(+), 33 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index d4ab92233062..369621e7faf5 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1919,6 +1919,21 @@ int inode_update_time(struct inode *inode, struct timespec64 *time, int flags)
>  }
>  EXPORT_SYMBOL(inode_update_time);
>  
> +/**
> + * current_coarse_time - Return FS time
> + * @inode: inode.
> + *
> + * Return the current coarse-grained time truncated to the time
> + * granularity supported by the fs.
> + */
> +static struct timespec64 current_coarse_time(struct inode *inode)
> +{
> +	struct timespec64 now;
> +
> +	ktime_get_coarse_real_ts64(&now);
> +	return timestamp_truncate(now, inode);
> +}
> +
>  /**
>   *	atime_needs_update	-	update the access time
>   *	@path: the &struct path to update
> @@ -1952,7 +1967,7 @@ bool atime_needs_update(const struct path *path, struct inode *inode)
>  	if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
>  		return false;
>  
> -	now = current_time(inode);
> +	now = current_coarse_time(inode);
>  
>  	if (!relatime_need_update(mnt, inode, now))
>  		return false;
> @@ -1986,7 +2001,7 @@ void touch_atime(const struct path *path)
>  	 * We may also fail on filesystems that have the ability to make parts
>  	 * of the fs read only, e.g. subvolumes in Btrfs.
>  	 */
> -	now = current_time(inode);
> +	now = current_coarse_time(inode);
>  	inode_update_time(inode, &now, S_ATIME);
>  	__mnt_drop_write(mnt);
>  skip_update:

There are also calls in fs/smb/client/file.c:cifs_readpage_worker() and in
fs/ocfs2/file.c:ocfs2_update_inode_atime() that should probably use
current_coarse_time() to avoid needless querying of fine grained
timestamps. But see below...

> @@ -2072,6 +2087,56 @@ int file_remove_privs(struct file *file)
>  }
>  EXPORT_SYMBOL(file_remove_privs);
>  
> +/**
> + * current_mgtime - Return FS time (possibly fine-grained)
> + * @inode: inode.
> + *
> + * Return the current time truncated to the time granularity supported by
> + * the fs, as suitable for a ctime/mtime change. If the ctime is flagged
> + * as having been QUERIED, get a fine-grained timestamp.
> + */
> +static struct timespec64 current_mgtime(struct inode *inode)
> +{
> +	struct timespec64 now;
> +	atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
> +	long nsec = atomic_long_read(pnsec);
> +
> +	if (nsec & I_CTIME_QUERIED) {
> +		ktime_get_real_ts64(&now);
> +	} else {
> +		struct timespec64 ctime;
> +
> +		ktime_get_coarse_real_ts64(&now);
> +
> +		/*
> +		 * If we've recently fetched a fine-grained timestamp
> +		 * then the coarse-grained one may still be earlier than the
> +		 * existing one. Just keep the existing ctime if so.
> +		 */
> +		ctime = inode_get_ctime(inode);
> +		if (timespec64_compare(&ctime, &now) > 0)
> +			now = ctime;
> +	}
> +
> +	return timestamp_truncate(now, inode);
> +}
> +
> +/**
> + * current_time - Return timestamp suitable for ctime update
> + * @inode: inode to eventually be updated
> + *
> + * Return the current time, which is usually coarse-grained but may be fine
> + * grained if the filesystem uses multigrain timestamps and the existing
> + * ctime was queried since the last update.
> + */
> +struct timespec64 current_time(struct inode *inode)
> +{
> +	if (is_mgtime(inode))
> +		return current_mgtime(inode);
> +	return current_coarse_time(inode);
> +}
> +EXPORT_SYMBOL(current_time);
> +

So if you modify current_time() to handle multigrain timestamps the code
will be still racy. In particular fill_mg_cmtime() can race with
inode_set_ctime_current() like:

fill_mg_cmtime()				inode_set_ctime_current()
  stat->mtime = inode->i_mtime;
  stat->ctime.tv_sec = inode->__i_ctime.tv_sec;
						  now = current_time();
							/* fetches coarse
							 * grained timestamp */
  stat->ctime.tv_nsec = atomic_long_fetch_or(I_CTIME_QUERIED, pnsec) &
				~I_CTIME_QUERIED;
						  inode_set_ctime(inode, now.tv_sec, now.tv_nsec);

and the information about a need for finegrained timestamp update gets
lost. So what I'd propose is to leave current_time() alone (just always
reporting coarse grained timestamps) and put all the magic into
inode_set_ctime_current() only. There we need something like:

struct timespec64 inode_set_ctime_current(struct inode *inode)
{
	... variables ...

	nsec = READ_ONCE(inode->__i_ctime.tv_nsec);
 	if (!(nsec & I_CTIME_QUERIED)) {
		now = current_time(inode);

		if (!is_gmtime(inode)) {
			inode_set_ctime_to_ts(inode, now);
		} else {
			/*
			 * If we've recently fetched a fine-grained
			 * timestamp then the coarse-grained one may still
			 * be earlier than the existing one. Just keep the
			 * existing ctime if so.
			 */
			ctime = inode_get_ctime(inode);
			if (timespec64_compare(&ctime, &now) > 0)
				now = ctime;

			/*
			 * Ctime updates are generally protected by inode
			 * lock but we could have raced with setting of
			 * I_CTIME_QUERIED flag.
			 */
			if (cmpxchg(&inode->__i_ctime.tv_nsec, nsec,
				    now.tv_nsec) != nsec)
				goto fine_grained;
			inode->__i_ctime.tv_sec = now.tv_sec;
		}
		return now;
	}
fine_grained:
	ktime_get_real_ts64(&now);
	inode_set_ctime_to_ts(inode, now);

	return now;
}

								Honza

>  static int inode_needs_update_time(struct inode *inode, struct timespec64 *now)
>  {
>  	int sync_it = 0;
> @@ -2480,37 +2545,12 @@ struct timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode)
>  }
>  EXPORT_SYMBOL(timestamp_truncate);
>  
> -/**
> - * current_time - Return FS time
> - * @inode: inode.
> - *
> - * Return the current time truncated to the time granularity supported by
> - * the fs.
> - *
> - * Note that inode and inode->sb cannot be NULL.
> - * Otherwise, the function warns and returns time without truncation.
> - */
> -struct timespec64 current_time(struct inode *inode)
> -{
> -	struct timespec64 now;
> -
> -	ktime_get_coarse_real_ts64(&now);
> -
> -	if (unlikely(!inode->i_sb)) {
> -		WARN(1, "current_time() called with uninitialized super_block in the inode");
> -		return now;
> -	}
> -
> -	return timestamp_truncate(now, inode);
> -}
> -EXPORT_SYMBOL(current_time);
> -
>  /**
>   * inode_set_ctime_current - set the ctime to current_time
>   * @inode: inode
>   *
> - * Set the inode->i_ctime to the current value for the inode. Returns
> - * the current value that was assigned to i_ctime.
> + * Set the inode->__i_ctime to the current value for the inode. Returns
> + * the current value that was assigned to __i_ctime.
>   */
>  struct timespec64 inode_set_ctime_current(struct inode *inode)
>  {
> diff --git a/fs/stat.c b/fs/stat.c
> index 062f311b5386..51effd1c2bc2 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -26,6 +26,37 @@
>  #include "internal.h"
>  #include "mount.h"
>  
> +/**
> + * fill_mg_cmtime - Fill in the mtime and ctime and flag ctime as QUERIED
> + * @request_mask: STATX_* values requested
> + * @inode: inode from which to grab the c/mtime
> + * @stat: where to store the resulting values
> + *
> + * Given @inode, grab the ctime and mtime out if it and store the result
> + * in @stat. When fetching the value, flag it as queried so the next write
> + * will use a fine-grained timestamp.
> + */
> +void fill_mg_cmtime(u32 request_mask, struct inode *inode, struct kstat *stat)
> +{
> +	atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
> +
> +	/* If neither time was requested, then don't report them */
> +	if (!(request_mask & (STATX_CTIME|STATX_MTIME))) {
> +		stat->result_mask &= ~(STATX_CTIME|STATX_MTIME);
> +		return;
> +	}
> +
> +	stat->mtime = inode->i_mtime;
> +	stat->ctime.tv_sec = inode->__i_ctime.tv_sec;
> +	/*
> +	 * Atomically set the QUERIED flag and fetch the new value with
> +	 * the flag masked off.
> +	 */
> +	stat->ctime.tv_nsec = atomic_long_fetch_or(I_CTIME_QUERIED, pnsec) &
> +					~I_CTIME_QUERIED;
> +}
> +EXPORT_SYMBOL(fill_mg_cmtime);
> +
>  /**
>   * generic_fillattr - Fill in the basic attributes from the inode struct
>   * @idmap:	idmap of the mount the inode was found from
> @@ -58,8 +89,14 @@ void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
>  	stat->rdev = inode->i_rdev;
>  	stat->size = i_size_read(inode);
>  	stat->atime = inode->i_atime;
> -	stat->mtime = inode->i_mtime;
> -	stat->ctime = inode_get_ctime(inode);
> +
> +	if (is_mgtime(inode)) {
> +		fill_mg_cmtime(request_mask, inode, stat);
> +	} else {
> +		stat->mtime = inode->i_mtime;
> +		stat->ctime = inode_get_ctime(inode);
> +	}
> +
>  	stat->blksize = i_blocksize(inode);
>  	stat->blocks = inode->i_blocks;
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 42d1434cc427..a0bdbefbf293 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1477,15 +1477,43 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
>  struct timespec64 current_time(struct inode *inode);
>  struct timespec64 inode_set_ctime_current(struct inode *inode);
>  
> +/*
> + * Multigrain timestamps
> + *
> + * Conditionally use fine-grained ctime and mtime timestamps when there
> + * are users actively observing them via getattr. The primary use-case
> + * for this is NFS clients that use the ctime to distinguish between
> + * different states of the file, and that are often fooled by multiple
> + * operations that occur in the same coarse-grained timer tick.
> + *
> + * The kernel always keeps normalized struct timespec64 values in the ctime,
> + * which means that only the first 30 bits of the value are used. Use the
> + * 31st bit of the ctime's tv_nsec field as a flag to indicate that the value
> + * has been queried since it was last updated.
> + */
> +#define I_CTIME_QUERIED		(1L<<30)
> +
>  /**
>   * inode_get_ctime - fetch the current ctime from the inode
>   * @inode: inode from which to fetch ctime
>   *
> - * Grab the current ctime from the inode and return it.
> + * Grab the current ctime tv_nsec field from the inode, mask off the
> + * I_CTIME_QUERIED flag and return it. This is mostly intended for use by
> + * internal consumers of the ctime that aren't concerned with ensuring a
> + * fine-grained update on the next change (e.g. when preparing to store
> + * the value in the backing store for later retrieval).
> + *
> + * This is safe to call regardless of whether the underlying filesystem
> + * is using multigrain timestamps.
>   */
>  static inline struct timespec64 inode_get_ctime(const struct inode *inode)
>  {
> -	return inode->__i_ctime;
> +	struct timespec64 ctime;
> +
> +	ctime.tv_sec = inode->__i_ctime.tv_sec;
> +	ctime.tv_nsec = inode->__i_ctime.tv_nsec & ~I_CTIME_QUERIED;
> +
> +	return ctime;
>  }
>  
>  /**
> @@ -2261,6 +2289,7 @@ struct file_system_type {
>  #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
>  #define FS_DISALLOW_NOTIFY_PERM	16	/* Disable fanotify permission events */
>  #define FS_ALLOW_IDMAP         32      /* FS has been updated to handle vfs idmappings. */
> +#define FS_MGTIME		64	/* FS uses multigrain timestamps */
>  #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
>  	int (*init_fs_context)(struct fs_context *);
>  	const struct fs_parameter_spec *parameters;
> @@ -2284,6 +2313,17 @@ struct file_system_type {
>  
>  #define MODULE_ALIAS_FS(NAME) MODULE_ALIAS("fs-" NAME)
>  
> +/**
> + * is_mgtime: is this inode using multigrain timestamps
> + * @inode: inode to test for multigrain timestamps
> + *
> + * Return true if the inode uses multigrain timestamps, false otherwise.
> + */
> +static inline bool is_mgtime(const struct inode *inode)
> +{
> +	return inode->i_sb->s_type->fs_flags & FS_MGTIME;
> +}
> +
>  extern struct dentry *mount_bdev(struct file_system_type *fs_type,
>  	int flags, const char *dev_name, void *data,
>  	int (*fill_super)(struct super_block *, void *, int));
> @@ -2919,6 +2959,7 @@ extern void page_put_link(void *);
>  extern int page_symlink(struct inode *inode, const char *symname, int len);
>  extern const struct inode_operations page_symlink_inode_operations;
>  extern void kfree_link(void *);
> +void fill_mg_cmtime(u32 request_mask, struct inode *inode, struct kstat *stat);
>  void generic_fillattr(struct mnt_idmap *, u32, struct inode *, struct kstat *);
>  void generic_fill_statx_attr(struct inode *inode, struct kstat *stat);
>  extern int vfs_getattr_nosec(const struct path *, struct kstat *, u32, unsigned int);
> 
> -- 
> 2.41.0
>

On Wed, 2023-08-02 at 21:35 +0200, Jan Kara wrote:
> On Tue 25-07-23 10:58:15, Jeff Layton wrote:
> > The VFS always uses coarse-grained timestamps when updating the ctime
> > and mtime after a change. This has the benefit of allowing filesystems
> > to optimize away a lot metadata updates, down to around 1 per jiffy,
> > even when a file is under heavy writes.
> > 
> > Unfortunately, this has always been an issue when we're exporting via
> > NFSv3, which relies on timestamps to validate caches. A lot of changes
> > can happen in a jiffy, so timestamps aren't sufficient to help the
> > client decide to invalidate the cache. Even with NFSv4, a lot of
> > exported filesystems don't properly support a change attribute and are
> > subject to the same problems with timestamp granularity. Other
> > applications have similar issues with timestamps (e.g backup
> > applications).
> > 
> > If we were to always use fine-grained timestamps, that would improve the
> > situation, but that becomes rather expensive, as the underlying
> > filesystem would have to log a lot more metadata updates.
> > 
> > What we need is a way to only use fine-grained timestamps when they are
> > being actively queried.
> > 
> > POSIX generally mandates that when the the mtime changes, the ctime must
> > also change. The kernel always stores normalized ctime values, so only
> > the first 30 bits of the tv_nsec field are ever used.
> > 
> > Use the 31st bit of the ctime tv_nsec field to indicate that something
> > has queried the inode for the mtime or ctime. When this flag is set,
> > on the next mtime or ctime update, the kernel will fetch a fine-grained
> > timestamp instead of the usual coarse-grained one.
> > 
> > Filesytems can opt into this behavior by setting the FS_MGTIME flag in
> > the fstype. Filesystems that don't set this flag will continue to use
> > coarse-grained timestamps.
> > 
> > Later patches will convert individual filesystems to use the new
> > infrastructure.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/inode.c         | 98 ++++++++++++++++++++++++++++++++++++++----------------
> >  fs/stat.c          | 41 +++++++++++++++++++++--
> >  include/linux/fs.h | 45 +++++++++++++++++++++++--
> >  3 files changed, 151 insertions(+), 33 deletions(-)
> > 
> > diff --git a/fs/inode.c b/fs/inode.c
> > index d4ab92233062..369621e7faf5 100644
> > --- a/fs/inode.c
> > +++ b/fs/inode.c
> > @@ -1919,6 +1919,21 @@ int inode_update_time(struct inode *inode, struct timespec64 *time, int flags)
> >  }
> >  EXPORT_SYMBOL(inode_update_time);
> >  
> > +/**
> > + * current_coarse_time - Return FS time
> > + * @inode: inode.
> > + *
> > + * Return the current coarse-grained time truncated to the time
> > + * granularity supported by the fs.
> > + */
> > +static struct timespec64 current_coarse_time(struct inode *inode)
> > +{
> > +	struct timespec64 now;
> > +
> > +	ktime_get_coarse_real_ts64(&now);
> > +	return timestamp_truncate(now, inode);
> > +}
> > +
> >  /**
> >   *	atime_needs_update	-	update the access time
> >   *	@path: the &struct path to update
> > @@ -1952,7 +1967,7 @@ bool atime_needs_update(const struct path *path, struct inode *inode)
> >  	if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
> >  		return false;
> >  
> > -	now = current_time(inode);
> > +	now = current_coarse_time(inode);
> >  
> >  	if (!relatime_need_update(mnt, inode, now))
> >  		return false;
> > @@ -1986,7 +2001,7 @@ void touch_atime(const struct path *path)
> >  	 * We may also fail on filesystems that have the ability to make parts
> >  	 * of the fs read only, e.g. subvolumes in Btrfs.
> >  	 */
> > -	now = current_time(inode);
> > +	now = current_coarse_time(inode);
> >  	inode_update_time(inode, &now, S_ATIME);
> >  	__mnt_drop_write(mnt);
> >  skip_update:
> 
> There are also calls in fs/smb/client/file.c:cifs_readpage_worker() and in
> fs/ocfs2/file.c:ocfs2_update_inode_atime() that should probably use
> current_coarse_time() to avoid needless querying of fine grained
> timestamps. But see below...
> 

Technically, they already devolve to current_coarse_time anyway, but
changing them would allow them to skip the fstype flag check, but I like
your idea below better anyway.

> > @@ -2072,6 +2087,56 @@ int file_remove_privs(struct file *file)
> >  }
> >  EXPORT_SYMBOL(file_remove_privs);
> >  
> > +/**
> > + * current_mgtime - Return FS time (possibly fine-grained)
> > + * @inode: inode.
> > + *
> > + * Return the current time truncated to the time granularity supported by
> > + * the fs, as suitable for a ctime/mtime change. If the ctime is flagged
> > + * as having been QUERIED, get a fine-grained timestamp.
> > + */
> > +static struct timespec64 current_mgtime(struct inode *inode)
> > +{
> > +	struct timespec64 now;
> > +	atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
> > +	long nsec = atomic_long_read(pnsec);
> > +
> > +	if (nsec & I_CTIME_QUERIED) {
> > +		ktime_get_real_ts64(&now);
> > +	} else {
> > +		struct timespec64 ctime;
> > +
> > +		ktime_get_coarse_real_ts64(&now);
> > +
> > +		/*
> > +		 * If we've recently fetched a fine-grained timestamp
> > +		 * then the coarse-grained one may still be earlier than the
> > +		 * existing one. Just keep the existing ctime if so.
> > +		 */
> > +		ctime = inode_get_ctime(inode);
> > +		if (timespec64_compare(&ctime, &now) > 0)
> > +			now = ctime;
> > +	}
> > +
> > +	return timestamp_truncate(now, inode);
> > +}
> > +
> > +/**
> > + * current_time - Return timestamp suitable for ctime update
> > + * @inode: inode to eventually be updated
> > + *
> > + * Return the current time, which is usually coarse-grained but may be fine
> > + * grained if the filesystem uses multigrain timestamps and the existing
> > + * ctime was queried since the last update.
> > + */
> > +struct timespec64 current_time(struct inode *inode)
> > +{
> > +	if (is_mgtime(inode))
> > +		return current_mgtime(inode);
> > +	return current_coarse_time(inode);
> > +}
> > +EXPORT_SYMBOL(current_time);
> > +
> 
> So if you modify current_time() to handle multigrain timestamps the code
> will be still racy. In particular fill_mg_cmtime() can race with
> inode_set_ctime_current() like:
> 
> fill_mg_cmtime()				inode_set_ctime_current()
>   stat->mtime = inode->i_mtime;
>   stat->ctime.tv_sec = inode->__i_ctime.tv_sec;
> 						  now = current_time();
> 							/* fetches coarse
> 							 * grained timestamp */
>   stat->ctime.tv_nsec = atomic_long_fetch_or(I_CTIME_QUERIED, pnsec) &
> 				~I_CTIME_QUERIED;
> 						  inode_set_ctime(inode, now.tv_sec, now.tv_nsec);
> 
> and the information about a need for finegrained timestamp update gets
> lost. So what I'd propose is to leave current_time() alone (just always
> reporting coarse grained timestamps) and put all the magic into
> inode_set_ctime_current() only. There we need something like:
> 
> struct timespec64 inode_set_ctime_current(struct inode *inode)
> {
> 	... variables ...
> 
> 	nsec = READ_ONCE(inode->__i_ctime.tv_nsec);
>  	if (!(nsec & I_CTIME_QUERIED)) {
> 		now = current_time(inode);
> 
> 		if (!is_gmtime(inode)) {
> 			inode_set_ctime_to_ts(inode, now);
> 		} else {
> 			/*
> 			 * If we've recently fetched a fine-grained
> 			 * timestamp then the coarse-grained one may still
> 			 * be earlier than the existing one. Just keep the
> 			 * existing ctime if so.
> 			 */
> 			ctime = inode_get_ctime(inode);
> 			if (timespec64_compare(&ctime, &now) > 0)
> 				now = ctime;
> 
> 			/*
> 			 * Ctime updates are generally protected by inode
> 			 * lock but we could have raced with setting of
> 			 * I_CTIME_QUERIED flag.
> 			 */
> 			if (cmpxchg(&inode->__i_ctime.tv_nsec, nsec,
> 				    now.tv_nsec) != nsec)
> 				goto fine_grained;
> 			inode->__i_ctime.tv_sec = now.tv_sec;
> 		}
> 		return now;
> 	}
> fine_grained:
> 	ktime_get_real_ts64(&now);
> 	inode_set_ctime_to_ts(inode, now);
> 
> 	return now;
> }
> 
> 								Honza
> 

This is a great idea. I'll rework the series along the lines you
suggest. That also answers my earlier question to Christian:

I'll just resend the whole series (it's not very big anyway), and I'll
include the fill_mg_cmtime prototype change.

Cheers,

> >  static int inode_needs_update_time(struct inode *inode, struct timespec64 *now)
> >  {
> >  	int sync_it = 0;
> > @@ -2480,37 +2545,12 @@ struct timespec64 timestamp_truncate(struct timespec64 t, struct inode *inode)
> >  }
> >  EXPORT_SYMBOL(timestamp_truncate);
> >  
> > -/**
> > - * current_time - Return FS time
> > - * @inode: inode.
> > - *
> > - * Return the current time truncated to the time granularity supported by
> > - * the fs.
> > - *
> > - * Note that inode and inode->sb cannot be NULL.
> > - * Otherwise, the function warns and returns time without truncation.
> > - */
> > -struct timespec64 current_time(struct inode *inode)
> > -{
> > -	struct timespec64 now;
> > -
> > -	ktime_get_coarse_real_ts64(&now);
> > -
> > -	if (unlikely(!inode->i_sb)) {
> > -		WARN(1, "current_time() called with uninitialized super_block in the inode");
> > -		return now;
> > -	}
> > -
> > -	return timestamp_truncate(now, inode);
> > -}
> > -EXPORT_SYMBOL(current_time);
> > -
> >  /**
> >   * inode_set_ctime_current - set the ctime to current_time
> >   * @inode: inode
> >   *
> > - * Set the inode->i_ctime to the current value for the inode. Returns
> > - * the current value that was assigned to i_ctime.
> > + * Set the inode->__i_ctime to the current value for the inode. Returns
> > + * the current value that was assigned to __i_ctime.
> >   */
> >  struct timespec64 inode_set_ctime_current(struct inode *inode)
> >  {
> > diff --git a/fs/stat.c b/fs/stat.c
> > index 062f311b5386..51effd1c2bc2 100644
> > --- a/fs/stat.c
> > +++ b/fs/stat.c
> > @@ -26,6 +26,37 @@
> >  #include "internal.h"
> >  #include "mount.h"
> >  
> > +/**
> > + * fill_mg_cmtime - Fill in the mtime and ctime and flag ctime as QUERIED
> > + * @request_mask: STATX_* values requested
> > + * @inode: inode from which to grab the c/mtime
> > + * @stat: where to store the resulting values
> > + *
> > + * Given @inode, grab the ctime and mtime out if it and store the result
> > + * in @stat. When fetching the value, flag it as queried so the next write
> > + * will use a fine-grained timestamp.
> > + */
> > +void fill_mg_cmtime(u32 request_mask, struct inode *inode, struct kstat *stat)
> > +{
> > +	atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
> > +
> > +	/* If neither time was requested, then don't report them */
> > +	if (!(request_mask & (STATX_CTIME|STATX_MTIME))) {
> > +		stat->result_mask &= ~(STATX_CTIME|STATX_MTIME);
> > +		return;
> > +	}
> > +
> > +	stat->mtime = inode->i_mtime;
> > +	stat->ctime.tv_sec = inode->__i_ctime.tv_sec;
> > +	/*
> > +	 * Atomically set the QUERIED flag and fetch the new value with
> > +	 * the flag masked off.
> > +	 */
> > +	stat->ctime.tv_nsec = atomic_long_fetch_or(I_CTIME_QUERIED, pnsec) &
> > +					~I_CTIME_QUERIED;
> > +}
> > +EXPORT_SYMBOL(fill_mg_cmtime);
> > +
> >  /**
> >   * generic_fillattr - Fill in the basic attributes from the inode struct
> >   * @idmap:	idmap of the mount the inode was found from
> > @@ -58,8 +89,14 @@ void generic_fillattr(struct mnt_idmap *idmap, u32 request_mask,
> >  	stat->rdev = inode->i_rdev;
> >  	stat->size = i_size_read(inode);
> >  	stat->atime = inode->i_atime;
> > -	stat->mtime = inode->i_mtime;
> > -	stat->ctime = inode_get_ctime(inode);
> > +
> > +	if (is_mgtime(inode)) {
> > +		fill_mg_cmtime(request_mask, inode, stat);
> > +	} else {
> > +		stat->mtime = inode->i_mtime;
> > +		stat->ctime = inode_get_ctime(inode);
> > +	}
> > +
> >  	stat->blksize = i_blocksize(inode);
> >  	stat->blocks = inode->i_blocks;
> >  
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index 42d1434cc427..a0bdbefbf293 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -1477,15 +1477,43 @@ static inline bool fsuidgid_has_mapping(struct super_block *sb,
> >  struct timespec64 current_time(struct inode *inode);
> >  struct timespec64 inode_set_ctime_current(struct inode *inode);
> >  
> > +/*
> > + * Multigrain timestamps
> > + *
> > + * Conditionally use fine-grained ctime and mtime timestamps when there
> > + * are users actively observing them via getattr. The primary use-case
> > + * for this is NFS clients that use the ctime to distinguish between
> > + * different states of the file, and that are often fooled by multiple
> > + * operations that occur in the same coarse-grained timer tick.
> > + *
> > + * The kernel always keeps normalized struct timespec64 values in the ctime,
> > + * which means that only the first 30 bits of the value are used. Use the
> > + * 31st bit of the ctime's tv_nsec field as a flag to indicate that the value
> > + * has been queried since it was last updated.
> > + */
> > +#define I_CTIME_QUERIED		(1L<<30)
> > +
> >  /**
> >   * inode_get_ctime - fetch the current ctime from the inode
> >   * @inode: inode from which to fetch ctime
> >   *
> > - * Grab the current ctime from the inode and return it.
> > + * Grab the current ctime tv_nsec field from the inode, mask off the
> > + * I_CTIME_QUERIED flag and return it. This is mostly intended for use by
> > + * internal consumers of the ctime that aren't concerned with ensuring a
> > + * fine-grained update on the next change (e.g. when preparing to store
> > + * the value in the backing store for later retrieval).
> > + *
> > + * This is safe to call regardless of whether the underlying filesystem
> > + * is using multigrain timestamps.
> >   */
> >  static inline struct timespec64 inode_get_ctime(const struct inode *inode)
> >  {
> > -	return inode->__i_ctime;
> > +	struct timespec64 ctime;
> > +
> > +	ctime.tv_sec = inode->__i_ctime.tv_sec;
> > +	ctime.tv_nsec = inode->__i_ctime.tv_nsec & ~I_CTIME_QUERIED;
> > +
> > +	return ctime;
> >  }
> >  
> >  /**
> > @@ -2261,6 +2289,7 @@ struct file_system_type {
> >  #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
> >  #define FS_DISALLOW_NOTIFY_PERM	16	/* Disable fanotify permission events */
> >  #define FS_ALLOW_IDMAP         32      /* FS has been updated to handle vfs idmappings. */
> > +#define FS_MGTIME		64	/* FS uses multigrain timestamps */
> >  #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
> >  	int (*init_fs_context)(struct fs_context *);
> >  	const struct fs_parameter_spec *parameters;
> > @@ -2284,6 +2313,17 @@ struct file_system_type {
> >  
> >  #define MODULE_ALIAS_FS(NAME) MODULE_ALIAS("fs-" NAME)
> >  
> > +/**
> > + * is_mgtime: is this inode using multigrain timestamps
> > + * @inode: inode to test for multigrain timestamps
> > + *
> > + * Return true if the inode uses multigrain timestamps, false otherwise.
> > + */
> > +static inline bool is_mgtime(const struct inode *inode)
> > +{
> > +	return inode->i_sb->s_type->fs_flags & FS_MGTIME;
> > +}
> > +
> >  extern struct dentry *mount_bdev(struct file_system_type *fs_type,
> >  	int flags, const char *dev_name, void *data,
> >  	int (*fill_super)(struct super_block *, void *, int));
> > @@ -2919,6 +2959,7 @@ extern void page_put_link(void *);
> >  extern int page_symlink(struct inode *inode, const char *symname, int len);
> >  extern const struct inode_operations page_symlink_inode_operations;
> >  extern void kfree_link(void *);
> > +void fill_mg_cmtime(u32 request_mask, struct inode *inode, struct kstat *stat);
> >  void generic_fillattr(struct mnt_idmap *, u32, struct inode *, struct kstat *);
> >  void generic_fill_statx_attr(struct inode *inode, struct kstat *stat);
> >  extern int vfs_getattr_nosec(const struct path *, struct kstat *, u32, unsigned int);
> > 
> > -- 
> > 2.41.0
> >

On Wed, Aug 02, 2023 at 04:54:09PM -0400, Jeff Layton wrote:
> On Wed, 2023-08-02 at 21:35 +0200, Jan Kara wrote:
> > On Tue 25-07-23 10:58:15, Jeff Layton wrote:
> > > The VFS always uses coarse-grained timestamps when updating the ctime
> > > and mtime after a change. This has the benefit of allowing filesystems
> > > to optimize away a lot metadata updates, down to around 1 per jiffy,
> > > even when a file is under heavy writes.
> > > 
> > > Unfortunately, this has always been an issue when we're exporting via
> > > NFSv3, which relies on timestamps to validate caches. A lot of changes
> > > can happen in a jiffy, so timestamps aren't sufficient to help the
> > > client decide to invalidate the cache. Even with NFSv4, a lot of
> > > exported filesystems don't properly support a change attribute and are
> > > subject to the same problems with timestamp granularity. Other
> > > applications have similar issues with timestamps (e.g backup
> > > applications).
> > > 
> > > If we were to always use fine-grained timestamps, that would improve the
> > > situation, but that becomes rather expensive, as the underlying
> > > filesystem would have to log a lot more metadata updates.
> > > 
> > > What we need is a way to only use fine-grained timestamps when they are
> > > being actively queried.
> > > 
> > > POSIX generally mandates that when the the mtime changes, the ctime must
> > > also change. The kernel always stores normalized ctime values, so only
> > > the first 30 bits of the tv_nsec field are ever used.
> > > 
> > > Use the 31st bit of the ctime tv_nsec field to indicate that something
> > > has queried the inode for the mtime or ctime. When this flag is set,
> > > on the next mtime or ctime update, the kernel will fetch a fine-grained
> > > timestamp instead of the usual coarse-grained one.
> > > 
> > > Filesytems can opt into this behavior by setting the FS_MGTIME flag in
> > > the fstype. Filesystems that don't set this flag will continue to use
> > > coarse-grained timestamps.
> > > 
> > > Later patches will convert individual filesystems to use the new
> > > infrastructure.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/inode.c         | 98 ++++++++++++++++++++++++++++++++++++++----------------
> > >  fs/stat.c          | 41 +++++++++++++++++++++--
> > >  include/linux/fs.h | 45 +++++++++++++++++++++++--
> > >  3 files changed, 151 insertions(+), 33 deletions(-)
> > > 
> > > diff --git a/fs/inode.c b/fs/inode.c
> > > index d4ab92233062..369621e7faf5 100644
> > > --- a/fs/inode.c
> > > +++ b/fs/inode.c
> > > @@ -1919,6 +1919,21 @@ int inode_update_time(struct inode *inode, struct timespec64 *time, int flags)
> > >  }
> > >  EXPORT_SYMBOL(inode_update_time);
> > >  
> > > +/**
> > > + * current_coarse_time - Return FS time
> > > + * @inode: inode.
> > > + *
> > > + * Return the current coarse-grained time truncated to the time
> > > + * granularity supported by the fs.
> > > + */
> > > +static struct timespec64 current_coarse_time(struct inode *inode)
> > > +{
> > > +	struct timespec64 now;
> > > +
> > > +	ktime_get_coarse_real_ts64(&now);
> > > +	return timestamp_truncate(now, inode);
> > > +}
> > > +
> > >  /**
> > >   *	atime_needs_update	-	update the access time
> > >   *	@path: the &struct path to update
> > > @@ -1952,7 +1967,7 @@ bool atime_needs_update(const struct path *path, struct inode *inode)
> > >  	if ((mnt->mnt_flags & MNT_NODIRATIME) && S_ISDIR(inode->i_mode))
> > >  		return false;
> > >  
> > > -	now = current_time(inode);
> > > +	now = current_coarse_time(inode);
> > >  
> > >  	if (!relatime_need_update(mnt, inode, now))
> > >  		return false;
> > > @@ -1986,7 +2001,7 @@ void touch_atime(const struct path *path)
> > >  	 * We may also fail on filesystems that have the ability to make parts
> > >  	 * of the fs read only, e.g. subvolumes in Btrfs.
> > >  	 */
> > > -	now = current_time(inode);
> > > +	now = current_coarse_time(inode);
> > >  	inode_update_time(inode, &now, S_ATIME);
> > >  	__mnt_drop_write(mnt);
> > >  skip_update:
> > 
> > There are also calls in fs/smb/client/file.c:cifs_readpage_worker() and in
> > fs/ocfs2/file.c:ocfs2_update_inode_atime() that should probably use
> > current_coarse_time() to avoid needless querying of fine grained
> > timestamps. But see below...
> > 
> 
> Technically, they already devolve to current_coarse_time anyway, but
> changing them would allow them to skip the fstype flag check, but I like
> your idea below better anyway.
> 
> > > @@ -2072,6 +2087,56 @@ int file_remove_privs(struct file *file)
> > >  }
> > >  EXPORT_SYMBOL(file_remove_privs);
> > >  
> > > +/**
> > > + * current_mgtime - Return FS time (possibly fine-grained)
> > > + * @inode: inode.
> > > + *
> > > + * Return the current time truncated to the time granularity supported by
> > > + * the fs, as suitable for a ctime/mtime change. If the ctime is flagged
> > > + * as having been QUERIED, get a fine-grained timestamp.
> > > + */
> > > +static struct timespec64 current_mgtime(struct inode *inode)
> > > +{
> > > +	struct timespec64 now;
> > > +	atomic_long_t *pnsec = (atomic_long_t *)&inode->__i_ctime.tv_nsec;
> > > +	long nsec = atomic_long_read(pnsec);
> > > +
> > > +	if (nsec & I_CTIME_QUERIED) {
> > > +		ktime_get_real_ts64(&now);
> > > +	} else {
> > > +		struct timespec64 ctime;
> > > +
> > > +		ktime_get_coarse_real_ts64(&now);
> > > +
> > > +		/*
> > > +		 * If we've recently fetched a fine-grained timestamp
> > > +		 * then the coarse-grained one may still be earlier than the
> > > +		 * existing one. Just keep the existing ctime if so.
> > > +		 */
> > > +		ctime = inode_get_ctime(inode);
> > > +		if (timespec64_compare(&ctime, &now) > 0)
> > > +			now = ctime;
> > > +	}
> > > +
> > > +	return timestamp_truncate(now, inode);
> > > +}
> > > +
> > > +/**
> > > + * current_time - Return timestamp suitable for ctime update
> > > + * @inode: inode to eventually be updated
> > > + *
> > > + * Return the current time, which is usually coarse-grained but may be fine
> > > + * grained if the filesystem uses multigrain timestamps and the existing
> > > + * ctime was queried since the last update.
> > > + */
> > > +struct timespec64 current_time(struct inode *inode)
> > > +{
> > > +	if (is_mgtime(inode))
> > > +		return current_mgtime(inode);
> > > +	return current_coarse_time(inode);
> > > +}
> > > +EXPORT_SYMBOL(current_time);
> > > +
> > 
> > So if you modify current_time() to handle multigrain timestamps the code
> > will be still racy. In particular fill_mg_cmtime() can race with
> > inode_set_ctime_current() like:
> > 
> > fill_mg_cmtime()				inode_set_ctime_current()
> >   stat->mtime = inode->i_mtime;
> >   stat->ctime.tv_sec = inode->__i_ctime.tv_sec;
> > 						  now = current_time();
> > 							/* fetches coarse
> > 							 * grained timestamp */
> >   stat->ctime.tv_nsec = atomic_long_fetch_or(I_CTIME_QUERIED, pnsec) &
> > 				~I_CTIME_QUERIED;
> > 						  inode_set_ctime(inode, now.tv_sec, now.tv_nsec);
> > 
> > and the information about a need for finegrained timestamp update gets
> > lost. So what I'd propose is to leave current_time() alone (just always
> > reporting coarse grained timestamps) and put all the magic into
> > inode_set_ctime_current() only. There we need something like:
> > 
> > struct timespec64 inode_set_ctime_current(struct inode *inode)
> > {
> > 	... variables ...
> > 
> > 	nsec = READ_ONCE(inode->__i_ctime.tv_nsec);
> >  	if (!(nsec & I_CTIME_QUERIED)) {
> > 		now = current_time(inode);
> > 
> > 		if (!is_gmtime(inode)) {
> > 			inode_set_ctime_to_ts(inode, now);
> > 		} else {
> > 			/*
> > 			 * If we've recently fetched a fine-grained
> > 			 * timestamp then the coarse-grained one may still
> > 			 * be earlier than the existing one. Just keep the
> > 			 * existing ctime if so.
> > 			 */
> > 			ctime = inode_get_ctime(inode);
> > 			if (timespec64_compare(&ctime, &now) > 0)
> > 				now = ctime;
> > 
> > 			/*
> > 			 * Ctime updates are generally protected by inode
> > 			 * lock but we could have raced with setting of
> > 			 * I_CTIME_QUERIED flag.
> > 			 */
> > 			if (cmpxchg(&inode->__i_ctime.tv_nsec, nsec,
> > 				    now.tv_nsec) != nsec)
> > 				goto fine_grained;
> > 			inode->__i_ctime.tv_sec = now.tv_sec;
> > 		}
> > 		return now;
> > 	}
> > fine_grained:
> > 	ktime_get_real_ts64(&now);
> > 	inode_set_ctime_to_ts(inode, now);
> > 
> > 	return now;
> > }
> > 
> > 								Honza
> > 
> 
> This is a great idea. I'll rework the series along the lines you
> suggest. That also answers my earlier question to Christian:
> 
> I'll just resend the whole series (it's not very big anyway), and I'll
> include the fill_mg_cmtime prototype change.

Ok, sound good!