[PATCH] Fix dcache lookup

__d_lookup() has leftover stuff from earlier code to protect it against
rename.  The smp_rmb() there was needed for the sequence counter logic.

Original dcache_rcu had :

+               move_count = dentry->d_move_count;
+               smp_rmb();
+
                if (dentry->d_name.hash != hash)
                        continue;
                if (dentry->d_parent != parent)
                        continue;

This was to make sure that comparisons didn't happen before before the
sequence counter was snapshotted.  This logic is now gone and memory
barrier is not needed.  Removing this should also improve performance.

The other change is the leftover smp_read_barrier_depends(), later
converted to rcu_dereference().  Originally, the name comparison was not
protected against d_move() and there could have been a mismatch of
allocation size of the name string and dentry->d_name.len.  This was
avoided by making the qstr update in dentry atomic using a d_qstr pointer.
Now, we do ->d_compare() or memcmp() with the d_lock held and it is safe
against d_move().  So, there is no need to rcu_dereference() anything.  In
fact, the current code is meaningless.

Signed-off-by: Dipankar Sarma <dipankar@in.ibm.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/fs/dcache.c b/fs/dcache.c
index 3c17f9815b7e4b5673fb19866a84c8c79696b749..e9285720356ffb9eb515a39453acc9ed3ce5b38f 100644 (file)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -979,8 +979,6 @@ struct dentry * __d_lookup(struct dentry * parent, struct qstr * name)
 
                dentry = hlist_entry(node, struct dentry, d_hash);
 
-               smp_rmb();
-
                if (dentry->d_name.hash != hash)
                        continue;
                if (dentry->d_parent != parent)
@@ -1003,7 +1001,11 @@ struct dentry * __d_lookup(struct dentry * parent, struct qstr * name)
                if (dentry->d_parent != parent)
                        goto next;
 
-               qstr = rcu_dereference(&dentry->d_name);
+               /*
+                * It is safe to compare names since d_move() cannot
+                * change the qstr (protected by d_lock).
+                */
+               qstr = &dentry->d_name;
                if (parent->d_op && parent->d_op->d_compare) {
                        if (parent->d_op->d_compare(parent, qstr, name))
                                goto next;