Re: [PATCH v3 1/2] nfsd: update mtime/ctime on CLONE in presense of delegated attributes

["Chuck Lever" <cel@kernel.org>]

On Thu Apr 9, 2026 at 12:43 PM EDT, Olga Kornievskaia wrote:
> When delegated attributes are given on open, the file is opened with
> NOCMTIME and modifying operations do not update mtime/ctime as to not get
> out-of-sync with the client's delegated view. However, for CLONE operation,
> the server should update its view of mtime/ctime and reflect that in any
> GETATTR queries.
>
> Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>  fs/nfsd/nfs4proc.c  |  6 ++++++
>  fs/nfsd/nfs4state.c | 14 +-------------
>  fs/nfsd/state.h     | 16 ++++++++++++++++
>  3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 2797da8cc950..5091527a6dc7 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1403,6 +1403,9 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct nfsd4_clone *clone = &u->clone;
>  	struct nfsd_file *src, *dst;
>  	__be32 status;
> +	struct iattr attr = {
> +		.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
> +	};
>  
>  	status = nfsd4_verify_copy(rqstp, cstate, &clone->cl_src_stateid, &src,
>  				   &clone->cl_dst_stateid, &dst);
> @@ -1413,6 +1416,9 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  			dst, clone->cl_dst_pos, clone->cl_count,
>  			EX_ISSYNC(cstate->current_fh.fh_export));
>  
> +	if ((READ_ONCE(dst->nf_file->f_mode) & FMODE_NOCMTIME) != 0 && !status)

Let's write this:

  if (!status && ((READ_ONCE(dst->nf_file->f_mode) & FMODE_NOCMTIME) != 0))


> +		nfsd_update_cmtime_attr(dst->nf_file, &attr);
> +
>  	nfsd_file_put(dst);
>  	nfsd_file_put(src);
>  out:
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c75d3940188c..553102da6f7d 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1225,8 +1225,6 @@ static void put_deleg_file(struct nfs4_file *fp)
>  static void nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct file *f)
>  {
>  	struct iattr ia = { .ia_valid = ATTR_ATIME | ATTR_CTIME | ATTR_MTIME | ATTR_DELEG };
> -	struct inode *inode = file_inode(f);
> -	int ret;
>  
>  	/* don't do anything if FMODE_NOCMTIME isn't set */
>  	if ((READ_ONCE(f->f_mode) & FMODE_NOCMTIME) == 0)
> @@ -1245,17 +1243,7 @@ static void nfsd4_finalize_deleg_timestamps(struct nfs4_delegation *dp, struct f
>  		return;
>  
>  	/* Stamp everything to "now" */
> -	inode_lock(inode);
> -	ret = notify_change(&nop_mnt_idmap, f->f_path.dentry, &ia, NULL);
> -	inode_unlock(inode);
> -	if (ret) {
> -		struct inode *inode = file_inode(f);
> -
> -		pr_notice_ratelimited("nfsd: Unable to update timestamps on inode %02x:%02x:%lu: %d\n",
> -					MAJOR(inode->i_sb->s_dev),
> -					MINOR(inode->i_sb->s_dev),
> -					inode->i_ino, ret);
> -	}
> +	nfsd_update_cmtime_attr(f, &ia);
>  }
>  
>  static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp)
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 811c148f36fc..683c305a8808 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -844,6 +844,22 @@ extern void nfsd4_shutdown_copy(struct nfs4_client *clp);
>  void nfsd4_put_client(struct nfs4_client *clp);
>  void nfsd4_async_copy_reaper(struct nfsd_net *nn);
>  bool nfsd4_has_active_async_copies(struct nfs4_client *clp);
> +static inline void nfsd_update_cmtime_attr(struct file *f,
> +					   struct iattr *attr)
> +{
> +	int ret;
> +	struct inode *inode = file_inode(f);
> +
> +	inode_lock(inode);
> +	ret = notify_change(&nop_mnt_idmap, f->f_path.dentry, attr, NULL);
> +	inode_unlock(inode);
> +	if (ret)
> +		pr_notice_ratelimited("nfsd: Unable to update timestamps on "
> +				      "inode %02x:%02x:%lu: %d\n",
> +				      MAJOR(inode->i_sb->s_dev),
> +				      MINOR(inode->i_sb->s_dev),
> +				      inode->i_ino, ret);
> +}

Move this new function into fs/nfsd/nfs4state.c and add a proper kdoc
comment. This is a little much for a static inline function.


>  extern struct nfs4_client_reclaim *nfs4_client_to_reclaim(struct xdr_netobj name,
>  				struct xdr_netobj princhash, struct nfsd_net *nn);
>  extern bool nfs4_has_reclaimed_state(struct xdr_netobj name, struct nfsd_net *nn);


-- 
Chuck Lever