[PATCH 0/1] nfs: fix directory mtime staleness under directory delegation after local mutations

[PATCH 0/1] nfs: fix directory mtime staleness under directory delegation after local mutations

[Tom Haynes]

Hi all,

A cross-parent rename(2) against a Linux NFSv4.2 server leaves the
client's cached mtime on both parent directories stale, even though
the server's in-memory inode mtime is updated correctly.  The bug is
entirely on the client side in the directory-delegation + mutation
interaction.

Server instrumentation (bpftrace on vfs_getattr + xfs_trans_ichgtime)
showed the backing XFS and knfsd path both advance mtime correctly
through vfs_rename -> xfs_dir_rename_children -> xfs_trans_ichgtime.
Subsequent vfs_getattr calls on the parent dirs on the server side
return the fresh mtime.

On the client:

 1. The local rename path calls nfs4_update_changeattr_locked() and
    marks NFS_INO_INVALID_NLINK | NFS_INO_INVALID_DATA on each parent.
 2. The next stat(2) enters __nfs_revalidate_inode() which early-exits
    on a held directory delegation, returning the cached (stale) mtime
    without sending a GETATTR RPC.

The delegation early-exit unconditionally trusts cached attrs, but the
rename/create/unlink paths have already flagged some attrs as stale.
This patch keeps the early-exit for the fast path but takes the
RPC when CHANGE/MTIME/CTIME is already marked invalid.

Minimal reproducer (client):

  mount -t nfs -o vers=4.2 SERVER:/export /mnt/x
  cd /mnt/x
  rm -rf src dst && mkdir src dst && touch src/f
  stat -c 'src_before: %y' src
  sleep 3
  mv src/f dst/f
  stat -c 'src_after:  %y' src

Before: src_after == src_before (stale).
After:  src_after advances by the sleep interval.

Verification
------------

 - Loopback (client and server on the same Linux 7.0 host): test
   passes after the patch with default directory_delegations=Y.
 - Separate physical client (Linux 7.0 with the patch) against
   a Linux 7.0 server: test passes with default settings.
 - Disabling directory_delegations on the client (echo N >
   /sys/module/nfsv4/parameters/directory_delegations) is a known
   workaround that also makes the test pass -- which is what led to
   the diagnosis.  The patch removes the need for the workaround.

Reproduces against every NFSv4 server we tested (multiple Linux knfsd
versions, reffs, FreeBSD NFS) which is consistent with a client-only
bug.

Test that surfaced it
---------------------

nfs-conformance op_rename_nlink case_parent_timestamps:
https://github.com/loghyr/nfs-conformance/blob/main/op_rename_nlink.c

Alternative fixes considered
----------------------------

An alternative would be to call nfs_update_delegated_mtime_locked() on
the directory from the rename_done path, treating the client as
authoritative for the delegated mtime (similar to what the write path
does for files via nfs_update_mtime).  That's more invasive and only
addresses the rename path; the __nfs_revalidate_inode fix in this
patch is defense-in-depth for any code path that marks an attr stale
without taking the delegation back.

Happy to rework if you'd prefer the call-update-mtime shape instead,
or if the guard should live in a helper next to
nfs_have_directory_delegation().

Thanks,
Tom

Tom Haynes (1):
  nfs: don't skip revalidate on directory delegation when attrs flagged
    stale

 fs/nfs/inode.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.53.0

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

[Tom Haynes]

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.

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

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

[Anna Schumaker]

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.

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

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

[Thomas Haynes]

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>