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@xxxxxxxxxx>
> 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@xxxxxxxxxx>
