Re: readdir vs. getattr

From: NeilBrown <neilb@suse.de>
To: Jeff Layton <jlayton@poochiereds.net>
Cc: "Mkrtchyan, Tigran" <tigran.mkrtchyan@desy.de>,
	Trond Myklebust <trond.myklebust@primarydata.com>,
	Jim Rees <rees@umich.edu>, linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: readdir vs. getattr
Date: Thu, 6 Feb 2014 13:51:01 +1100	[thread overview]
Message-ID: <20140206135101.1cc83442@notabene.brown> (raw)
In-Reply-To: <20140206134539.53d09434@notabene.brown>

[-- Attachment #1: Type: text/plain, Size: 2915 bytes --]


On Thu, 6 Feb 2014 13:45:39 +1100 NeilBrown <neilb@suse.de> wrote:


> The change to nfs_update_inode fixes an issue that had me stumped for a
> while.  It was still sending lots of GETATTR requests even after it had
> switched to READDIRPLUS instead of using cached info.  So that might be a
> genuine bug that should be fixed independently of this patch.

I managed to post the wrong version of the patch, which didn't have this
change.  Sorry.

Here is the real one.

NeilBrown

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index be38b573495a..b237fd7f2e0e 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -845,9 +845,12 @@ static int nfs_readdir(struct file *file, struct dir_context *ctx)
 	desc->dir_cookie = &dir_ctx->dir_cookie;
 	desc->decode = NFS_PROTO(inode)->decode_dirent;
 	desc->plus = nfs_use_readdirplus(inode, ctx) ? 1 : 0;
+	if (desc->plus && ctx->pos == 0)
+		clear_bit(NFS_INO_DID_FLUSH, &NFS_I(inode)->flags);
 
 	nfs_block_sillyrename(dentry);
-	if (ctx->pos == 0 || nfs_attribute_cache_expired(inode))
+	if (ctx->pos == 0 || nfs_attribute_cache_expired(inode) ||
+	    (NFS_I(inode)->cache_validity & NFS_INO_INVALID_DATA))
 		res = nfs_revalidate_mapping(inode, file->f_mapping);
 	if (res < 0)
 		goto out;
@@ -1080,6 +1083,16 @@ static int nfs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 
 	/* Force a full look up iff the parent directory has changed */
 	if (!nfs_is_exclusive_create(dir, flags) && nfs_check_verifier(dir, dentry)) {
+		if (nfs_server_capable(dir, NFS_CAP_READDIRPLUS)
+		    && ((NFS_I(inode)->cache_validity &
+			 (NFS_INO_INVALID_ATTR|NFS_INO_INVALID_LABEL))
+			|| nfs_attribute_cache_expired(inode))
+		    && !test_and_set_bit(NFS_INO_DID_FLUSH, &NFS_I(dir)->flags)
+			) {
+			nfs_advise_use_readdirplus(dir);
+			goto out_zap_parent;
+		}
+
 		if (nfs_lookup_verify_inode(inode, flags))
 			goto out_zap_parent;
 		goto out_valid;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 28a0a3cbd3b7..3996e6c7a728 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -1526,6 +1526,7 @@ static int nfs_update_inode(struct inode *inode, struct nfs_fattr *fattr)
 
 	save_cache_validity = nfsi->cache_validity;
 	nfsi->cache_validity &= ~(NFS_INO_INVALID_ATTR
+			| NFS_INO_INVALID_LABEL
 			| NFS_INO_INVALID_ATIME
 			| NFS_INO_REVAL_FORCED
 			| NFS_INO_REVAL_PAGECACHE);
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 0ae5807480f4..c282ce3e5349 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -218,6 +218,7 @@ struct nfs_inode {
 #define NFS_INO_COMMIT		(7)		/* inode is committing unstable writes */
 #define NFS_INO_LAYOUTCOMMIT	(9)		/* layoutcommit required */
 #define NFS_INO_LAYOUTCOMMITTING (10)		/* layoutcommit inflight */
+#define NFS_INO_DID_FLUSH	(11)
 
 static inline struct nfs_inode *NFS_I(const struct inode *inode)
 {

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
