Re: [PATCH 1/1] nfs: don't skip revalidate on directory delegation when attrs flagged stale

[Thomas Haynes <loghyr@gmail.com>]

On Thu, Apr 23, 2026 at 09:37:21AM -0800, Anna Schumaker wrote:
> Hi Tom
> 
> On Sat, Apr 18, 2026, at 3:03 PM, Tom Haynes wrote:
> > On a local directory mutation (rename/create/unlink) the client marks
> > CHANGE / MTIME / CTIME as invalid in NFS_I(dir)->cache_validity.  When
> > a subsequent stat(2) enters __nfs_revalidate_inode() and finds a
> > directory delegation held, the function currently early-exits and
> > returns the cached (now stale) mtime to userspace without sending a
> > GETATTR RPC.
> >
> > Keep the early-exit for the fast path, but take the RPC when CHANGE,
> > MTIME, or CTIME are already marked invalid.  The delegation alone is
> > not a guarantee of cached-attr freshness once the code itself has
> > flagged the cache as stale.
> 
> Is this a problem only for the attributes you've flagged, or do you think
> it would be a problem for size, nlink, or mode attributes as well? I'm asking
> because we have NFS_INO_INVALID_ATTR which includes all of these attributes
> which might make this a little more generic rather than carving out an
> exception an attribute at a time.

Hey Anna,

Agreed on at least size (and probably atime), which means a little
more generic would be a good thing.

Tom

> 
> Thoughts?
> Anna
> 
> >
> > Assisted-by: Claude:claude-opus-4-7 [bpftrace] [tshark]
> > Signed-off-by: Tom Haynes <loghyr@gmail.com>
> > ---
> >  fs/nfs/inode.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
> > index 98a8f0de1199..936bc329f462 100644
> > --- a/fs/nfs/inode.c
> > +++ b/fs/nfs/inode.c
> > @@ -1390,7 +1390,11 @@ __nfs_revalidate_inode(struct nfs_server 
> > *server, struct inode *inode)
> >  		status = pnfs_sync_inode(inode, false);
> >  		if (status)
> >  			goto out;
> > -	} else if (nfs_have_directory_delegation(inode)) {
> > +	} else if (nfs_have_directory_delegation(inode) &&
> > +		   !(NFS_I(inode)->cache_validity &
> > +		     (NFS_INO_INVALID_CHANGE |
> > +		      NFS_INO_INVALID_MTIME  |
> > +		      NFS_INO_INVALID_CTIME))) {
> >  		status = 0;
> >  		goto out;
> >  	}
> > -- 
> > 2.53.0

-- 
---
Tom Haynes <loghyr@gmail.com>