[PATCH v3 0/2] nfsd update mtime/ctime on CLONE/COPY with delegated attritutes

[PATCH v3 0/2] nfsd update mtime/ctime on CLONE/COPY with delegated attritutes

[Olga Kornievskaia]

generic/407 is failing in presence of delegated attributes and this patch series
explores the solution where the CLONE compound returns updated mtime/ctime
in the GETATTR op in the compound.

Both CLONE and COPY need to update destination file mtime/ctime
(in presence of delegated timestamp) when file was modified.

v3 changes:
--- Consolidated synchronous and asynchronous COPY patches into single
patch.
patch1: pulled out notify_change() code from nfsd4_finalize_deleg_timestamps as
it's shared with nfsd_update_cmtime_attr and both paths log an error if
notify_change() fails
patch2: dentry can be pulled from copy->nf_dst->nf_file.fh_path.dentry and
no longer requires additional tracking.

Olga Kornievskaia (2):
  nfsd: update mtime/ctime on CLONE in presense of delegated attributes
  nfsd: update mtime/ctime on COPY in presence of delegated attributes

 fs/nfsd/nfs4proc.c  | 30 +++++++++++++++++++++++++++++-
 fs/nfsd/nfs4state.c | 14 +-------------
 fs/nfsd/state.h     | 16 ++++++++++++++++
 3 files changed, 46 insertions(+), 14 deletions(-)

-- 
2.52.0

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

[Olga Kornievskaia]

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)
+		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);
+}
 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);
-- 
2.52.0

[PATCH v3 2/2] nfsd: update mtime/ctime on COPY in presence of delegated attributes

[Olga Kornievskaia]

COPY should update destination file's mtime/ctime upon completion.

Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 fs/nfsd/nfs4proc.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 5091527a6dc7..4418e71c8458 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -2124,8 +2124,22 @@ static int nfsd4_do_async_copy(void *data)
 
 	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
 	trace_nfsd_copy_async_done(copy);
-	nfsd4_send_cb_offload(copy);
 	atomic_dec(&copy->cp_nn->pending_async_copies);
+	/*
+	 * choosing to check for existence of set dentry pointer to indicate
+	 * that we need to update the attributes and do a dput because the
+	 * file flag could be cleared by a DELEGRETURN and then we'd lose
+	 * that copy was started with file opened with NOCMTIME and we got
+	 * a reference on the dentry.
+	 */
+	if (copy->cp_res.wr_bytes_written > 0) {
+		struct iattr ia = {
+			.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
+		};
+
+		nfsd_update_cmtime_attr(copy->nf_dst->nf_file, &ia);
+	}
+	nfsd4_send_cb_offload(copy);
 	return 0;
 }
 
@@ -2201,8 +2215,16 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		wake_up_process(async_copy->copy_task);
 		status = nfs_ok;
 	} else {
+		struct iattr ia = {
+			.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
+		};
+
 		status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
 				       copy->nf_dst->nf_file, true);
+		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
+			       FMODE_NOCMTIME) != 0 &&
+				copy->cp_res.wr_bytes_written > 0)
+			nfsd_update_cmtime_attr(copy->nf_dst->nf_file, &ia);
 	}
 out:
 	trace_nfsd_copy_done(copy, status);
-- 
2.52.0

Re: [PATCH v3 0/2] nfsd update mtime/ctime on CLONE/COPY with delegated attritutes

[Jeff Layton]

On Thu, 2026-04-09 at 12:43 -0400, Olga Kornievskaia wrote:
> generic/407 is failing in presence of delegated attributes and this patch series
> explores the solution where the CLONE compound returns updated mtime/ctime
> in the GETATTR op in the compound.
> 
> Both CLONE and COPY need to update destination file mtime/ctime
> (in presence of delegated timestamp) when file was modified.
> 
> v3 changes:
> --- Consolidated synchronous and asynchronous COPY patches into single
> patch.
> patch1: pulled out notify_change() code from nfsd4_finalize_deleg_timestamps as
> it's shared with nfsd_update_cmtime_attr and both paths log an error if
> notify_change() fails
> patch2: dentry can be pulled from copy->nf_dst->nf_file.fh_path.dentry and
> no longer requires additional tracking.
> 
> Olga Kornievskaia (2):
>   nfsd: update mtime/ctime on CLONE in presense of delegated attributes
>   nfsd: update mtime/ctime on COPY in presence of delegated attributes
> 
>  fs/nfsd/nfs4proc.c  | 30 +++++++++++++++++++++++++++++-
>  fs/nfsd/nfs4state.c | 14 +-------------
>  fs/nfsd/state.h     | 16 ++++++++++++++++
>  3 files changed, 46 insertions(+), 14 deletions(-)

Nice work.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3 2/2] nfsd: update mtime/ctime on COPY in presence of delegated attributes

[Chuck Lever]

On Thu Apr 9, 2026 at 12:43 PM EDT, Olga Kornievskaia wrote:
> COPY should update destination file's mtime/ctime upon completion.

Let's provide similar rationale here as was done in v3 1/2.


> Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>  fs/nfsd/nfs4proc.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 5091527a6dc7..4418e71c8458 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -2124,8 +2124,22 @@ static int nfsd4_do_async_copy(void *data)
>  
>  	set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
>  	trace_nfsd_copy_async_done(copy);
> -	nfsd4_send_cb_offload(copy);
>  	atomic_dec(&copy->cp_nn->pending_async_copies);
> +	/*
> +	 * choosing to check for existence of set dentry pointer to indicate
> +	 * that we need to update the attributes and do a dput because the
> +	 * file flag could be cleared by a DELEGRETURN and then we'd lose
> +	 * that copy was started with file opened with NOCMTIME and we got
> +	 * a reference on the dentry.
> +	 */

Now that you're not dealing with dget/dput, this new comment is
perhaps stale.


> +	if (copy->cp_res.wr_bytes_written > 0) {

Don't you need the FMODE_NOCTIME guard here too?


> +		struct iattr ia = {
> +			.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
> +		};
> +
> +		nfsd_update_cmtime_attr(copy->nf_dst->nf_file, &ia);
> +	}
> +	nfsd4_send_cb_offload(copy);
>  	return 0;
>  }
>  
> @@ -2201,8 +2215,16 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		wake_up_process(async_copy->copy_task);
>  		status = nfs_ok;
>  	} else {
> +		struct iattr ia = {
> +			.ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
> +		};

You could move "struct iattr ia" into the helper, and just pass a "0"
at these call sites and "ATTR_ATIME" at the
nfsd4_finalize_deleg_timestamps() call site.


> +
>  		status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
>  				       copy->nf_dst->nf_file, true);
> +		if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
> +			       FMODE_NOCMTIME) != 0 &&
> +				copy->cp_res.wr_bytes_written > 0)
> +			nfsd_update_cmtime_attr(copy->nf_dst->nf_file, &ia);
>  	}
>  out:
>  	trace_nfsd_copy_done(copy, status);


-- 
Chuck Lever

Re: [PATCH v3 2/2] nfsd: update mtime/ctime on COPY in presence of delegated attributes

[Olga Kornievskaia]

On Thu, Apr 9, 2026 at 2:38 PM Chuck Lever <cel@kernel.org> wrote:
>
> On Thu Apr 9, 2026 at 12:43 PM EDT, Olga Kornievskaia wrote:
> > COPY should update destination file's mtime/ctime upon completion.
>
> Let's provide similar rationale here as was done in v3 1/2.
>
>
> > Fixes: e5e9b24ab8fa ("nfsd: freeze c/mtime updates with outstanding WRITE_ATTRS delegation")
> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> >  fs/nfsd/nfs4proc.c | 24 +++++++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > index 5091527a6dc7..4418e71c8458 100644
> > --- a/fs/nfsd/nfs4proc.c
> > +++ b/fs/nfsd/nfs4proc.c
> > @@ -2124,8 +2124,22 @@ static int nfsd4_do_async_copy(void *data)
> >
> >       set_bit(NFSD4_COPY_F_COMPLETED, &copy->cp_flags);
> >       trace_nfsd_copy_async_done(copy);
> > -     nfsd4_send_cb_offload(copy);
> >       atomic_dec(&copy->cp_nn->pending_async_copies);
> > +     /*
> > +      * choosing to check for existence of set dentry pointer to indicate
> > +      * that we need to update the attributes and do a dput because the
> > +      * file flag could be cleared by a DELEGRETURN and then we'd lose
> > +      * that copy was started with file opened with NOCMTIME and we got
> > +      * a reference on the dentry.
> > +      */
>
> Now that you're not dealing with dget/dput, this new comment is
> perhaps stale.

Missed that. Will fix.

> > +     if (copy->cp_res.wr_bytes_written > 0) {
>
> Don't you need the FMODE_NOCTIME guard here too?

Yeah I realized that now checking for the flag might mean that if the
flag gets cleared we won't do the update. To handle that case, we
again need to add something to nfsd4_copy to say do-the-update by
checking the flag in nfsd4_copy(). Alternatively, I thought it would
not be harmful to always update the c/mtime upon completion regardless
of the delegation?

Do you want me to add the do-the-update flag instead?

> > +             struct iattr ia = {
> > +                     .ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
> > +             };
> > +
> > +             nfsd_update_cmtime_attr(copy->nf_dst->nf_file, &ia);
> > +     }
> > +     nfsd4_send_cb_offload(copy);
> >       return 0;
> >  }
> >
> > @@ -2201,8 +2215,16 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> >               wake_up_process(async_copy->copy_task);
> >               status = nfs_ok;
> >       } else {
> > +             struct iattr ia = {
> > +                     .ia_valid = ATTR_CTIME | ATTR_MTIME | ATTR_DELEG,
> > +             };
>
> You could move "struct iattr ia" into the helper, and just pass a "0"
> at these call sites and "ATTR_ATIME" at the
> nfsd4_finalize_deleg_timestamps() call site.

Will do. Thanks.

>
>
> > +
> >               status = nfsd4_do_copy(copy, copy->nf_src->nf_file,
> >                                      copy->nf_dst->nf_file, true);
> > +             if ((READ_ONCE(copy->nf_dst->nf_file->f_mode) &
> > +                            FMODE_NOCMTIME) != 0 &&
> > +                             copy->cp_res.wr_bytes_written > 0)
> > +                     nfsd_update_cmtime_attr(copy->nf_dst->nf_file, &ia);
> >       }
> >  out:
> >       trace_nfsd_copy_done(copy, status);
>
>
> --
> Chuck Lever
>
>

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

[Chuck Lever]

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