RCU caching regression in kernel v4.1+

Commit Message

Leandro Awa Oct. 8, 2015, 5:28 p.m. UTC

Hi Trond,

Sure.   I'm running the test now.  It should be done within the next 4 hours.

Best Regards,
Leandro Awa


-----Original Message-----
From: Trond Myklebust [mailto:[e-mail]] 
Sent: Thursday, October 08, 2015 5:55 AM
To: Alexander Viro
Cc: Linux NFS Mailing List; Leandro Awa; Linux FS-devel Mailing List
Subject: Re: RCU caching regression in kernel v4.1+

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 <[e-mail]>
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 <[e-mail]>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=104911
Fixes: 766c4cbfacd8 ("namei: d_is_negative() should be checked...")
Cc: [e-mail] # v4.1+
Signed-off-by: Trond Myklebust <[e-mail]>
---
 fs/namei.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

 				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 [e-mail]




-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

Patch

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,