Re: readdir vs. getattr

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

On Thu, 2014-02-06 at 13:51 +1100, NeilBrown wrote:
> 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

Hi Neil,

Is there any reason why this patch couldn't just be replaced with the
following?
(Please note the separate fix for the labeled NFS regression that you
pointed out.)

Cheers
  Trond

8<-------------------------------------------------------------------
>From fda6d0fb7988fd7d541ef6d1023bca92b0e10333 Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Thu, 6 Feb 2014 16:45:21 -0500
Subject: [PATCH] NFS: Use readdirplus in order to efficiently revalidate whole
 directories

When we call nfs_force_lookup_revalidate() in order to force a full
lookup of all child dentries of a directory, it makes sense to use
readdirplus to perform that revalidation as efficiently as possible.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/dir.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index be38b573495a..2abcae330ad0 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -437,6 +437,24 @@ void nfs_advise_use_readdirplus(struct inode *dir)
 	set_bit(NFS_INO_ADVISE_RDPLUS, &NFS_I(dir)->flags);
 }
 
+/*
+ * Force use of readdirplus to ensure efficient revalidation of
+ * the directory child dentries when we know that we will need
+ * to revalidate the whole directory.
+ *
+ * Caller must hold dir->i_lock.
+ */
+static
+void nfs_force_use_readdirplus(struct inode *dir)
+{
+	if (nfs_server_capable(dir, NFS_CAP_READDIRPLUS)) {
+		struct nfs_inode *nfsi = NFS_I(dir);
+
+		set_bit(NFS_INO_ADVISE_RDPLUS, &nfsi->flags);
+		nfsi->cache_validity |= NFS_INO_INVALID_DATA;
+	}
+}
+
 static
 void nfs_prime_dcache(struct dentry *parent, struct nfs_entry *entry)
 {
@@ -942,12 +960,14 @@ static int nfs_fsync_dir(struct file *filp, loff_t start, loff_t end,
  * This forces the revalidation code in nfs_lookup_revalidate() to do a
  * full lookup on all child dentries of 'dir' whenever a change occurs
  * on the server that might have invalidated our dcache.
+ * Call nfs_force_use_readdirplus for efficiency.
  *
  * The caller should be holding dir->i_lock
  */
 void nfs_force_lookup_revalidate(struct inode *dir)
 {
 	NFS_I(dir)->cache_change_attribute++;
+	nfs_force_use_readdirplus(dir);
 }
 EXPORT_SYMBOL_GPL(nfs_force_lookup_revalidate);
 
-- 
1.8.5.3


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