From: Jeff Layton <jlayton@kernel.org>
To: Olga Kornievskaia <aglo@umich.edu>
Cc: Olga Kornievskaia <okorniev@redhat.com>,
chuck.lever@oracle.com, linux-nfs@vger.kernel.org,
neilb@brown.name, Dai.Ngo@oracle.com, tom@talpey.com
Subject: Re: [PATCH 1/1] nfsd: update mtime/ctime on CLONE and COPY in presence of delegated attributes
Date: Mon, 06 Apr 2026 12:53:44 -0400 [thread overview]
Message-ID: <0c134e9a9670a30d858b7f28da5b92420fd0ac8f.camel@kernel.org> (raw)
In-Reply-To: <CAN-5tyE7yW=27HObj+cfHW-6HJsiGx=m6JbAcfEw+fTA2HXgsA@mail.gmail.com>
On Mon, 2026-04-06 at 09:59 -0400, Olga Kornievskaia wrote:
> On Fri, Apr 3, 2026 at 1:02 PM Jeff Layton <jlayton@kernel.org> wrote:
> >
> > On Fri, 2026-04-03 at 12:53 -0400, 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 operations CLONE/COPY 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 | 27 ++++++++++++++++++++++++++-
> > > 1 file changed, 26 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> > > index 99b44b6ec056..66bde3732b03 100644
> > > --- a/fs/nfsd/nfs4proc.c
> > > +++ b/fs/nfsd/nfs4proc.c
> > > @@ -1396,6 +1396,24 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > goto out;
> > > }
> > >
> > > +static bool nfsd4_clear_nocmtime(struct file *f)
> > > +{
> > > + if ((READ_ONCE(f->f_mode) & FMODE_NOCMTIME) != 0) {
> > > + spin_lock(&f->f_lock);
> > > + f->f_mode &= ~FMODE_NOCMTIME;
> > > + spin_unlock(&f->f_lock);
> > > + return true;
> > > + }
> > > + return false;
> > > +}
> > > +
> > > +static void nfsd4_restore_nocmtime(struct file *f)
> > > +{
> > > + spin_lock(&f->f_lock);
> > > + f->f_mode |= FMODE_NOCMTIME;
> > > + spin_unlock(&f->f_lock);
> > > +}
> > > +
> > > static __be32
> > > nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > union nfsd4_op_u *u)
> > > @@ -1403,16 +1421,19 @@ nfsd4_clone(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > struct nfsd4_clone *clone = &u->clone;
> > > struct nfsd_file *src, *dst;
> > > __be32 status;
> > > + bool restore_nocmtime = false;
> > >
> > > status = nfsd4_verify_copy(rqstp, cstate, &clone->cl_src_stateid, &src,
> > > &clone->cl_dst_stateid, &dst);
> > > if (status)
> > > goto out;
> > >
> > > + restore_nocmtime = nfsd4_clear_nocmtime(dst->nf_file);
> > > status = nfsd4_clone_file_range(rqstp, src, clone->cl_src_pos,
> > > dst, clone->cl_dst_pos, clone->cl_count,
> > > EX_ISSYNC(cstate->current_fh.fh_export));
> > > -
> > > + if (restore_nocmtime)
> > > + nfsd4_restore_nocmtime(dst->nf_file);
> > > nfsd_file_put(dst);
> > > nfsd_file_put(src);
> > > out:
> > > @@ -2132,6 +2153,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > struct nfsd4_copy *copy = &u->copy;
> > > struct nfsd42_write_res *result;
> > > __be32 status;
> > > + bool restore_nocmtime = false;
> > >
> > > result = ©->cp_res;
> > > nfsd_copy_write_verifier((__be32 *)&result->wr_verifier.data, nn);
> > > @@ -2157,6 +2179,7 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > }
> > > }
> > >
> > > + restore_nocmtime = nfsd4_clear_nocmtime(copy->nf_dst->nf_file);
> > > memcpy(©->fh, &cstate->current_fh.fh_handle,
> > > sizeof(struct knfsd_fh));
> > > if (nfsd4_copy_is_async(copy)) {
> > > @@ -2199,6 +2222,8 @@ nfsd4_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > > copy->nf_dst->nf_file, true);
> > > }
> > > out:
> > > + if (restore_nocmtime)
> > > + nfsd4_restore_nocmtime(copy->nf_dst->nf_file);
> > > trace_nfsd_copy_done(copy, status);
> > > release_copy_files(copy);
> > > return status;
> >
> > This seems simple enough, but there is some raciness involved if other
> > calls are running concurrently with the COPY/CLONE. You might end up
> > reenabling FMODE_NOCMTIME before a second COPY/CLONE finishes.
> >
> > A simpler idea might be to just do a notify_change() for ATTR_MTIME
> > after each COPY or CLONE operation done on a file with FMODE_NOCMTIME
> > set.
> >
> > That should set the timestamp to something pretty close to whatever the
> > last write() would have set it to, but without having to monkey with
> > the fmode.
>
> Thanks Jeff and Chuck. I've changed the code to use the
> notify_change() but before I send out I have a question regarding
> where you think this notify_change () should take place in case of a
> copy (specifically thinking about async copy vs sync copy). The
> easiest option is to do it (regardless of sync or async) upon
> completion of the COPY compound (ie not at the end of when async copy
> completes). Or for the async copy should it happen upon (successful)
> copy completion?
>
>
I think you want to do it when the copy completes. If that's sync copy
then you'd stamp it before you send the COPY reply. For async copy, I
think you'd want to do it before sending a CB_OFFLOAD.
--
Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2026-04-06 16:53 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-03 16:53 [PATCH 1/1] nfsd: update mtime/ctime on CLONE and COPY in presence of delegated attributes Olga Kornievskaia
2026-04-03 17:00 ` Jeff Layton
2026-04-06 13:59 ` Olga Kornievskaia
2026-04-06 16:53 ` Jeff Layton [this message]
2026-04-04 15:01 ` Chuck Lever
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0c134e9a9670a30d858b7f28da5b92420fd0ac8f.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=aglo@umich.edu \
--cc=chuck.lever@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@brown.name \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).