On Fri, 2024-06-14 at 15:59 -0400, Trond Myklebust wrote:
> 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.
Sigh... Never mind. I discovered a discrepancy between what I had in
the topic branch for these features vs. what I had in our master
branch. At some point in the last few years, I must have updated the
latter without remembering to update the former.
>
> Version 2 will be forthcoming.
>
...and it will not affect the behaviour of the regular delegations.
> >
> > I hope this helps!
> > Anna
Yes. I've checked both the patchsets that I sent out the other day, and
I believe this is the only patch which deviates from our master branch.
Again thanks!
> >
> > >
> > > 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
