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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.