From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <chuck.lever@oracle.com>, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <Dai.Ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] nfsd: don't set the ctime on delegated atime updates
Date: Tue, 15 Jul 2025 13:33:13 -0400 [thread overview]
Message-ID: <c3f4b22f65aa76c6d77becdf16174c8f55845fec.camel@kernel.org> (raw)
In-Reply-To: <479201d1-f153-4264-812a-2f0e084af268@oracle.com>
On Tue, 2025-07-15 at 12:46 -0400, Chuck Lever wrote:
> On 7/15/25 7:34 AM, Jeff Layton wrote:
> > Clients will typically precede a DELEGRETURN for a delegation with
> > delegated timestamp with a SETATTR to set the timestamps on the server
> > to match what the client has.
> >
> > knfsd implements this by using the nfsd_setattr() infrastructure, which
> > will set ATTR_CTIME on any update that goes to notify_change(). This is
> > problematic as it means that the client will get a spurious ctime
> > updates when updating the atime.
> >
> > Fix this by pushing the handling of ATTR_CTIME down into the decoder
> > functions so that they are set earlier and more deliberately, and stop
> > nfsd_setattr() from implicitly adding it to every notify_change() call.
> >
> > Fixes: 7e13f4f8d27d ("nfsd: handle delegated timestamps in SETATTR")
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> I was concerned about the description making a claim without reference
> that modifying atime does not modify ctime. That claim has been somewhat
> contentious in the past.
>
> Search engines suggest that IEEE 1003.1 does indeed specify that an
> atime modification does not require a ctime change. POSIX does not make
> these standards free, however. Maybe a Linux-specific document has
> something on point.
>
Ok, good. We probably ought to spell this out in some manpage...
> Even so, I'm comfortable taking this for now and putting it through the
> normal set of regression testing.
>
> Thank you for pursuing this one, Jeff!
>
Thanks. I think the principle is correct. Unfortunately, this patch
misses a place where we need to set ATTR_CTIME. You may want to drop it
for now.
I'm working on a different approach that makes it skip setting
ATTR_CTIME if ATTR_DELEG is set. ATTR_DELEG gets set on any delegated
timestamp update, so that should make it do the right thing (and the
patch will be less invasive).
Thanks for the review so far!
--
Jeff Layton <jlayton@kernel.org>
prev parent reply other threads:[~2025-07-15 17:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-15 11:34 [PATCH] nfsd: don't set the ctime on delegated atime updates Jeff Layton
2025-07-15 15:42 ` Chuck Lever
2025-07-15 16:46 ` Chuck Lever
2025-07-15 17:33 ` Jeff Layton [this message]
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=c3f4b22f65aa76c6d77becdf16174c8f55845fec.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=Dai.Ngo@oracle.com \
--cc=chuck.lever@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.