From: Andrew Bartlett <abartlet-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
To: linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: [PATCH] Always update the dentry cache with fresh readdir() results
Date: Thu, 05 Jul 2012 18:38:27 +1000 [thread overview]
Message-ID: <1341477507.22307.21.camel@obed> (raw)
[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]
When we do a readdir() in CIFS, we are potentially efficiently
collecting a great deal of current, catchable stat information.
It is important that we always keep the dentry cache current for two
reasons:
- the information may have changed (within the actime timeout).
- if we still have a dentry cache value after that timeout, it is quite
expensive (1xRTT per entry) to find out if it was still correct.
This hits folks who are using CIFS over a WAN very badly. For example
on an emulated 50ms delay I would have ls --color complete in .1
seconds, and a second run take 4.5 seconds, as each stat() (for the
colouring) would create a trans2 query_path_info query for each file,
right after getting the same information in the trans2 find_first2.
This patch implements the simplest approach, I would welcome a
correction on if there is a better approach than d_drop() and dput().
Tested on 3.4.4-3.cifsrevalidate.fc17.i686 with a 50ms WANem emulated
WAN against Samba 4.0 beta3.
Thanks,
Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
[-- Attachment #2: 0001-fs-cifs-Always-recreate-the-cifs-dentry-cache-when-w.patch --]
[-- Type: text/x-patch, Size: 2575 bytes --]
>From 4478a902d3606205313eb37a225da22712841bff Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
Date: Thu, 5 Jul 2012 15:48:08 +1000
Subject: [PATCH] fs/cifs: Always recreate the cifs dentry cache when we have
fresh information
When we do a readdir() in CIFS, we are potentially efficiently
collecting a great deal of current, cache-able stat information.
It is important that we always keep the dentry cache current for two reasons:
- the information may have changed (within the actime timeout).
- if we still have a dentry cache value after that timeout, it is quite
expensive (1xRTT per entry) to find out if it was still correct.
This hits folks who are using CIFS over a WAN very badly. For example
on an emulated 50ms delay I would have ls --color complete in .1
seconds, and a second run take 4.5 seconds, as each stat() (for the
colouring) would create a trans2 query_path_info query for each file,
right after getting the same information in the trans2 find_first2.
This patch implements the simplest approach, I would welcome a
correction on if there is a better approach than d_drop() and dput().
Tested on 3.4.4-3.cifsrevalidate.fc17.i686 against Samba 4.0 beta3.
Andrew Bartlett
Signed-off-by: Andrew Bartlett <abartlet-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
fs/cifs/readdir.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 0a8224d..f964b40 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -66,9 +66,13 @@ static inline void dump_cifs_file_struct(struct file *file, char *label)
#endif /* DEBUG2 */
/*
- * Find the dentry that matches "name". If there isn't one, create one. If it's
- * a negative dentry or the uniqueid changed, then drop it and recreate it.
+ * Find the dentry that matches "name". If there isn't one, create
+ * one. Otherwise recreate it fresh so we start the timeout for
+ * revalidation again. Local locks are much faster than the network
+ * ops to revalidate it, particularly as revalidation will be
+ * per-inode, but here we have a whole directory at once!
*/
+
static struct dentry *
cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
struct cifs_fattr *fattr)
@@ -86,9 +90,6 @@ cifs_readdir_lookup(struct dentry *parent, struct qstr *name,
dentry = d_lookup(parent, name);
if (dentry) {
- /* FIXME: check for inode number changes? */
- if (dentry->d_inode != NULL)
- return dentry;
d_drop(dentry);
dput(dentry);
}
--
1.7.7.6
next reply other threads:[~2012-07-05 8:38 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-05 8:38 Andrew Bartlett [this message]
2012-07-05 10:02 ` [PATCH] Always update the dentry cache with fresh readdir() results Andrew Bartlett
2012-07-05 11:24 ` Jeff Layton
[not found] ` <20120705072401.7eb1a7ee-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2012-07-05 23:31 ` Andrew Bartlett
2012-07-06 1:46 ` Jeff Layton
[not found] ` <20120705214608.2a3a681b-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org>
2012-07-06 6:20 ` Andrew Bartlett
2012-07-06 11:03 ` Jeff Layton
2012-07-06 6:30 ` Andrew Bartlett
2012-07-06 11:11 ` Jeff Layton
[not found] ` <20120706071123.2563c615-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2012-07-06 22:42 ` Andrew Bartlett
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1341477507.22307.21.camel@obed \
--to=abartlet-eunubhrolfbytjvyw6ydsg@public.gmane.org \
--cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox