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

[Jeff Layton <jlayton@kernel.org>]

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>