Re: [External] : Re: NFS dentry caching regression? was Re: [GIT PULL] Please pull NFS client updates for Linux 6.19

[Anna Schumaker <anna.schumaker@oracle.com>]

Hi Christoph,

On 12/17/25 9:23 AM, Anna Schumaker wrote:
> Hi Christoph,
> 
> On 12/17/25 4:53 AM, Christoph Hellwig wrote:
>> On Wed, Dec 17, 2025 at 01:32:30AM -0800, Christoph Hellwig wrote:
>>> Hi all,
>>>
>>> the merge of this branch causes NFS lookup operation to shoot up a lot
>>> for me.  And with merge I mean merge - both parent of the merge on their
>>> own are fine.
>>>
>>> With the script below that simulates running python scripts with lots
>>> of imports that was created to benchmark delegation performance, the
>>> number of lookups in the measurement period shoots up from 4 to about
>>> 410000, which is a bit suboptimal.  I have no idea how this could
>>> happen, but it must be related to some sort of pathname lookup changes
>>> I guess.  Other operations looks roughly the same.
>>
>> To further pin this down, I rebased the patches from the NFS pull request
>> on top of the baseline (d358e5254674b70f34c847715ca509e46eb81e6f) and
>> bisected that.  This ends up in:
> 
> Thanks for reporting this, and for the reproducer script. I'll dig into this
> today and try to fix what I broke!
> 
> Anna

Does the following help? If so, then I'll probably split it up into a couple of
patches before sending to the list.

Thanks,
Anna

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 23a78a742b61..c80f4d2289cd 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1516,14 +1516,6 @@ static int nfs_check_verifier(struct inode *dir, struct dentry *dentry,
 	if (!nfs_dentry_verify_change(dir, dentry))
 		return 0;
 
-	/*
-	 * If we have a directory delegation then we don't need to revalidate
-	 * the directory. The delegation will either get recalled or we will
-	 * receive a notification when it changes.
-	 */
-	if (nfs_have_directory_delegation(dir))
-		return 0;
-
 	/* Revalidate nfsi->cache_change_attribute before we declare a match */
 	if (nfs_mapping_need_revalidate_inode(dir)) {
 		if (rcu_walk)
@@ -2216,13 +2208,6 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 }
 EXPORT_SYMBOL_GPL(nfs_atomic_open);
 
-static int
-nfs_lookup_revalidate_delegated_parent(struct inode *dir, struct dentry *dentry,
-				       struct inode *inode)
-{
-	return nfs_lookup_revalidate_done(dir, dentry, inode, 1);
-}
-
 static int
 nfs4_lookup_revalidate(struct inode *dir, const struct qstr *name,
 		       struct dentry *dentry, unsigned int flags)
@@ -2247,12 +2232,10 @@ nfs4_lookup_revalidate(struct inode *dir, const struct qstr *name,
 	if (inode == NULL)
 		goto full_reval;
 
-	if (nfs_verifier_is_delegated(dentry))
+	if (nfs_verifier_is_delegated(dentry) ||
+	    nfs_have_directory_delegation(dir))
 		return nfs_lookup_revalidate_delegated(dir, dentry, inode);
 
-	if (nfs_have_directory_delegation(dir))
-		return nfs_lookup_revalidate_delegated_parent(dir, dentry, inode);
-
 	/* NFS only supports OPEN on regular files */
 	if (!S_ISREG(inode->i_mode))
 		goto full_reval;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ec1ce593dea2..d74cd8659999 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4830,7 +4830,6 @@ static int _nfs4_proc_access(struct inode *inode, struct nfs_access_entry *entry
 	int status = 0;
 
 	if (!nfs4_have_delegation(inode, FMODE_READ, 0)) {
-		nfs_request_directory_delegation(inode);
 		res.fattr = nfs_alloc_fattr();
 		if (res.fattr == NULL)
 			return -ENOMEM;
@@ -5422,6 +5421,10 @@ static int nfs4_proc_readdir(struct nfs_readdir_arg *arg,
 		.interruptible = true,
 	};
 	int err;
+
+	if (arg->plus)
+		nfs_request_directory_delegation(d_inode(arg->dentry));
+
 	do {
 		err = _nfs4_proc_readdir(arg, res);
 		trace_nfs4_readdir(d_inode(arg->dentry), err);


> 
>>
>> NFS: Shortcut lookup revalidations if we have a directory delegation
>>
> 
>