Re: RCU caching regression in kernel v4.1+

[Trond Myklebust <trond.myklebust@primarydata.com>]

On Wed, 2015-10-07 at 14:57 -0400, Trond Myklebust wrote:
> Hi Al,
> 
> Please could you take a look at the bugzilla entry in
> https://bugzilla.kernel.org/show_bug.cgi?id=104911 ?
> 
> It describes a NFS caching regression that appears to be caused by
> commit 766c4cbfacd8634d7580bac6a1b8456e63de3e84 ("namei:
> d_is_negative() should be checked before ->d_seq validation").
> 
> Shouldn't that test for 'if (negative) return -ENOENT;' happen after
> the call to d_revalidate() in lookup_fast()? If not, we can end up
> caching negative dentries forever, AFAICS...
> 
> Cheers
>   Trond

Leandro, can you please test if the following patch helps in any way?

Cheers
  Trond

8<-----------------------------------------------------------------
>From eb61ece5739bb2f3b6d03dd8ca8e335bf0d12687 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Thu, 8 Oct 2015 08:44:00 -0400
Subject: [PATCH] namei: results of d_is_negative() should be checked after
 dentry revalidation

Leandro Awa writes:
After switching to version 4.1.6, our parallelized and distributed workflows now  fail consistently with errors of the form:

T34: ./regex.c:39:22: error: config.h: No such file or directory

>From our 'git bisect' testing, the following commit appears to be
the possible cause of the behavior we've been seeing: commit 766c4cbfacd8

The issue is that revalidation may cause the dentry to be dropped in NFS
if, say, the client notes that the directory timestamps have changed.

Reported-by: Leandro Awa <lawa@nvidia.com>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=104911
Fixes: 766c4cbfacd8 ("namei: d_is_negative() should be checked...")
Cc: stable@vger.kernel.org # v4.1+
Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/namei.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 726d211db484..33e9495a3129 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1558,8 +1558,6 @@ static int lookup_fast(struct nameidata *nd,
 		negative = d_is_negative(dentry);
 		if (read_seqcount_retry(&dentry->d_seq, seq))
 			return -ECHILD;
-		if (negative)
-			return -ENOENT;
 
 		/*
 		 * This sequence count validates that the parent had no
@@ -1580,6 +1578,12 @@ static int lookup_fast(struct nameidata *nd,
 				goto unlazy;
 			}
 		}
+		/*
+		 * Note: do negative dentry check after revalidation in
+		 * case that drops it.
+		 */
+		if (negative)
+			return -ENOENT;
 		path->mnt = mnt;
 		path->dentry = dentry;
 		if (likely(__follow_mount_rcu(nd, path, inode, seqp)))
-- 
2.4.3

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com