On Fri, 2024-06-14 at 12:32 -0400, Anna Schumaker wrote:
> Hi Trond,
>
> On Thu, Jun 13, 2024 at 12:18 AM <trondmy@xxxxxxxxx> wrote:
> >
> > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> >
> > nfs_setattr calls nfs_update_inode() directly, so we have to reset
> > the
> > m/ctime there.
> >
> > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > Signed-off-by: Lance Shelton <lance.shelton@xxxxxxxxxxxxxxx>
> > Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> > ---
> > fs/nfs/inode.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
>
> After applying this patch I see a handful of new failures on
> xfstests:
> generic/075, generic/086, generic/112, generic/332, generic/346,
> generic/647, and generic/729. I see the first five on NFS v4.2, but
> 647 and 729 both fail on v4.1 in addition to v4.2.
Thanks Anna!
Yes, I think that is a consequence of the changes that were made in
commit cc7f2dae63bc ("NFS: Don't store NFS_INO_REVAL_FORCED"). I can
just remove that "if (!(cache_validity & NFS_INO_REVAL_FORCED))"
altogether with that change applied.
Version 2 will be forthcoming.
>
> I hope this helps!
> Anna
>
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 91c0aeaf6c1e..e03c512c8535 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -605,6 +605,46 @@ nfs_fhget(struct super_block *sb, struct
> > nfs_fh *fh, struct nfs_fattr *fattr)
> > }
> > EXPORT_SYMBOL_GPL(nfs_fhget);
> >
> > +static void
> > +nfs_fattr_fixup_delegated(struct inode *inode, struct nfs_fattr
> > *fattr)
> > +{
> > + unsigned long cache_validity = NFS_I(inode)-
> > >cache_validity;
> > +
> > + if (!nfs_have_read_or_write_delegation(inode))
> > + return;
> > +
> > + if (!(cache_validity & NFS_INO_REVAL_FORCED))
> > + cache_validity &= ~(NFS_INO_INVALID_ATIME
> > + | NFS_INO_INVALID_CHANGE
> > + | NFS_INO_INVALID_CTIME
> > + | NFS_INO_INVALID_MTIME
> > + | NFS_INO_INVALID_SIZE);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_SIZE))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PRESIZE
> > + | NFS_ATTR_FATTR_SIZE);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_CHANGE))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PRECHANGE
> > + | NFS_ATTR_FATTR_CHANGE);
> > +
> > + if (nfs_have_delegated_mtime(inode)) {
> > + if (!(cache_validity & NFS_INO_INVALID_CTIME))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PRECTIME
> > + | NFS_ATTR_FATTR_CTIME);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_MTIME))
> > + fattr->valid &= ~(NFS_ATTR_FATTR_PREMTIME
> > + | NFS_ATTR_FATTR_MTIME);
> > +
> > + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> > + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> > + } else if (nfs_have_delegated_atime(inode)) {
> > + if (!(cache_validity & NFS_INO_INVALID_ATIME))
> > + fattr->valid &= ~NFS_ATTR_FATTR_ATIME;
> > + }
> > +}
> > +
> > void nfs_update_delegated_atime(struct inode *inode)
> > {
> > spin_lock(&inode->i_lock);
> > @@ -2163,6 +2203,9 @@ static int nfs_update_inode(struct inode
> > *inode, struct nfs_fattr *fattr)
> > */
> > nfsi->read_cache_jiffies = fattr->time_start;
> >
> > + /* Fix up any delegated attributes in the struct nfs_fattr
> > */
> > + nfs_fattr_fixup_delegated(inode, fattr);
> > +
> > save_cache_validity = nfsi->cache_validity;
> > nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
> > | NFS_INO_INVALID_ATIME
> > --
> > 2.45.2
> >
> >
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx
