[PATCH 1/1] NFSv4.2: fix COPY attrs in presence of delegated timestamps

[PATCH 1/1] NFSv4.2: fix COPY attrs in presence of delegated timestamps

[Olga Kornievskaia]

A similar to generic/407 test can be done with a COPY operation
instead of CLONE (reflink). And it leads to same problem that ctime
and mtime saved before doing a "cp" operation are the same as after.

Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
---
 fs/nfs/nfs42proc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
index f77372a78be7..ea420dc94875 100644
--- a/fs/nfs/nfs42proc.c
+++ b/fs/nfs/nfs42proc.c
@@ -502,6 +502,12 @@ static ssize_t _nfs42_proc_copy(struct file *src,
 
 	nfs42_copy_dest_done(dst, pos_dst, res->write_res.count, oldsize_dst);
 	nfs_invalidate_atime(src_inode);
+	if (nfs_have_delegated_attributes(dst_inode)) {
+		nfs_update_delegated_mtime(dst_inode);
+		spin_lock(&dst_inode->i_lock);
+		nfs_set_cache_invalid(dst_inode, NFS_INO_INVALID_BLOCKS);
+		spin_unlock(&dst_inode->i_lock);
+	}
 	status = res->write_res.count;
 out:
 	if (args->sync)
-- 
2.52.0

Re: [PATCH 1/1] NFSv4.2: fix COPY attrs in presence of delegated timestamps

[Olga Kornievskaia]

On Fri, Mar 27, 2026 at 12:58 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>
> A similar to generic/407 test can be done with a COPY operation
> instead of CLONE (reflink). And it leads to same problem that ctime
> and mtime saved before doing a "cp" operation are the same as after.

Some extra comments here.

Currently the COPY compound does not add a GETATTR after the COPY.
Jeff's solution to REMOVEXATTR not updating ctime was to add GETATTR
to the compound and then call update attributes. This could be the
alternative solution here instead. I'm not sure such an approach would
be preferred. But then there is a question of whether or not the
server does update its attributes on COPY.

> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>  fs/nfs/nfs42proc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index f77372a78be7..ea420dc94875 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -502,6 +502,12 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>
>         nfs42_copy_dest_done(dst, pos_dst, res->write_res.count, oldsize_dst);
>         nfs_invalidate_atime(src_inode);
> +       if (nfs_have_delegated_attributes(dst_inode)) {
> +               nfs_update_delegated_mtime(dst_inode);
> +               spin_lock(&dst_inode->i_lock);
> +               nfs_set_cache_invalid(dst_inode, NFS_INO_INVALID_BLOCKS);
> +               spin_unlock(&dst_inode->i_lock);
> +       }
>         status = res->write_res.count;
>  out:
>         if (args->sync)
> --
> 2.52.0
>
>

Re: [PATCH 1/1] NFSv4.2: fix COPY attrs in presence of delegated timestamps

[Jeff Layton]

On Fri, 2026-03-27 at 13:08 -0400, Olga Kornievskaia wrote:
> On Fri, Mar 27, 2026 at 12:58 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
> > 
> > A similar to generic/407 test can be done with a COPY operation
> > instead of CLONE (reflink). And it leads to same problem that ctime
> > and mtime saved before doing a "cp" operation are the same as after.
> 
> Some extra comments here.
> 
> Currently the COPY compound does not add a GETATTR after the COPY.
> Jeff's solution to REMOVEXATTR not updating ctime was to add GETATTR
> to the compound and then call update attributes. This could be the
> alternative solution here instead. I'm not sure such an approach would
> be preferred. But then there is a question of whether or not the
> server does update its attributes on COPY.
> 

I think that'd ultimately be more efficient, and I think the server
should be updating the c/mtime on COPY, even if there is an outstanding
timestamp delegation. The server is doing the COPY. The client isn't
really involved so why should it be trying to stamp the file?

A GETATTR is cheap and in this case, the inode will even likely be in
cache on the server. We might as well update the attributes without
forcing another round trip in this case, since we pretty much know that
they will have changed.

If the server isn't updating its attrs on COPY in this case, then it's
probably because we're using the open file description in the
destination stateid and it has FMODE_NOCMTIME set. We can probably fix
this by just turning that flag off while the copy is running, and then
reenabling it before returning.


> > Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> > ---
> >  fs/nfs/nfs42proc.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> > index f77372a78be7..ea420dc94875 100644
> > --- a/fs/nfs/nfs42proc.c
> > +++ b/fs/nfs/nfs42proc.c
> > @@ -502,6 +502,12 @@ static ssize_t _nfs42_proc_copy(struct file *src,
> > 
> >         nfs42_copy_dest_done(dst, pos_dst, res->write_res.count, oldsize_dst);
> >         nfs_invalidate_atime(src_inode);
> > +       if (nfs_have_delegated_attributes(dst_inode)) {
> > +               nfs_update_delegated_mtime(dst_inode);
> > +               spin_lock(&dst_inode->i_lock);
> > +               nfs_set_cache_invalid(dst_inode, NFS_INO_INVALID_BLOCKS);
> > +               spin_unlock(&dst_inode->i_lock);
> > +       }
> >         status = res->write_res.count;
> >  out:
> >         if (args->sync)
> > --
> > 2.52.0
> > 
> > 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 1/1] NFSv4.2: fix COPY attrs in presence of delegated timestamps

[Olga Kornievskaia]

On Fri, Mar 27, 2026 at 12:58 PM Olga Kornievskaia <okorniev@redhat.com> wrote:
>
> A similar to generic/407 test can be done with a COPY operation
> instead of CLONE (reflink). And it leads to same problem that ctime
> and mtime saved before doing a "cp" operation are the same as after.
>
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>

This patch isn't needed if the server updates mtime/ctime after doing a COPY.

In the process of all the work, I have also created a patch that adds
a GETATTR to the COPY compound. I'm not sure if it's needed or not but
I can post it.

> ---
>  fs/nfs/nfs42proc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index f77372a78be7..ea420dc94875 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -502,6 +502,12 @@ static ssize_t _nfs42_proc_copy(struct file *src,
>
>         nfs42_copy_dest_done(dst, pos_dst, res->write_res.count, oldsize_dst);
>         nfs_invalidate_atime(src_inode);
> +       if (nfs_have_delegated_attributes(dst_inode)) {
> +               nfs_update_delegated_mtime(dst_inode);
> +               spin_lock(&dst_inode->i_lock);
> +               nfs_set_cache_invalid(dst_inode, NFS_INO_INVALID_BLOCKS);
> +               spin_unlock(&dst_inode->i_lock);
> +       }
>         status = res->write_res.count;
>  out:
>         if (args->sync)
> --
> 2.52.0
>
>

Re: [PATCH 1/1] NFSv4.2: fix COPY attrs in presence of delegated timestamps

[Trond Myklebust]

On Fri, 2026-03-27 at 12:57 -0400, Olga Kornievskaia wrote:
> A similar to generic/407 test can be done with a COPY operation
> instead of CLONE (reflink). And it leads to same problem that ctime
> and mtime saved before doing a "cp" operation are the same as after.
> 
> Signed-off-by: Olga Kornievskaia <okorniev@redhat.com>
> ---
>  fs/nfs/nfs42proc.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/nfs/nfs42proc.c b/fs/nfs/nfs42proc.c
> index f77372a78be7..ea420dc94875 100644
> --- a/fs/nfs/nfs42proc.c
> +++ b/fs/nfs/nfs42proc.c
> @@ -502,6 +502,12 @@ static ssize_t _nfs42_proc_copy(struct file
> *src,
>  
>  	nfs42_copy_dest_done(dst, pos_dst, res->write_res.count,
> oldsize_dst);
>  	nfs_invalidate_atime(src_inode);
> +	if (nfs_have_delegated_attributes(dst_inode)) {
> +		nfs_update_delegated_mtime(dst_inode);
> +		spin_lock(&dst_inode->i_lock);
> +		nfs_set_cache_invalid(dst_inode,
> NFS_INO_INVALID_BLOCKS);
> +		spin_unlock(&dst_inode->i_lock);

Isn't NFS_INO_INVALID_BLOCKS already set it nfs42_copy_dest_done()?

> +	}
>  	status = res->write_res.count;
>  out:
>  	if (args->sync)

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com