deal with deadlock in d_walk()
authorAl Viro <viro@zeniv.linux.org.uk>
Sun, 26 Oct 2014 23:31:10 +0000 (19:31 -0400)
committerBen Hutchings <ben@decadent.org.uk>
Thu, 1 Jan 2015 01:27:51 +0000 (01:27 +0000)
commit ca5358ef75fc69fee5322a38a340f5739d997c10 upstream.

... by not hitting rename_retry for reasons other than rename having
happened.  In other words, do _not_ restart when finding that
between unlocking the child and locking the parent the former got
into __dentry_kill().  Skip the killed siblings instead...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[bwh: Backported to 3.2:
 - As we only have try_to_ascend() and not d_walk(), apply this
   change to all callers of try_to_ascend()
 - Adjust context to make __dentry_kill() apply to d_kill()]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
fs/dcache.c

index 226a93a..3f65742 100644 (file)
@@ -306,9 +306,9 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
        __releases(parent->d_lock)
        __releases(dentry->d_inode->i_lock)
 {
-       list_del(&dentry->d_child);
+       __list_del_entry(&dentry->d_child);
        /*
-        * Inform try_to_ascend() that we are no longer attached to the
+        * Inform ascending readers that we are no longer attached to the
         * dentry tree
         */
        dentry->d_flags |= DCACHE_DENTRY_KILLED;
@@ -949,34 +949,6 @@ void shrink_dcache_for_umount(struct super_block *sb)
        }
 }
 
-/*
- * This tries to ascend one level of parenthood, but
- * we can race with renaming, so we need to re-check
- * the parenthood after dropping the lock and check
- * that the sequence number still matches.
- */
-static struct dentry *try_to_ascend(struct dentry *old, int locked, unsigned seq)
-{
-       struct dentry *new = old->d_parent;
-
-       rcu_read_lock();
-       spin_unlock(&old->d_lock);
-       spin_lock(&new->d_lock);
-
-       /*
-        * might go back up the wrong parent if we have had a rename
-        * or deletion
-        */
-       if (new != old->d_parent ||
-                (old->d_flags & DCACHE_DENTRY_KILLED) ||
-                (!locked && read_seqretry(&rename_lock, seq))) {
-               spin_unlock(&new->d_lock);
-               new = NULL;
-       }
-       rcu_read_unlock();
-       return new;
-}
-
 
 /*
  * Search for at least 1 mount point in the dentry's subdirs.
@@ -1032,17 +1004,32 @@ resume:
        /*
         * All done at this level ... ascend and resume the search.
         */
+       rcu_read_lock();
+ascend:
        if (this_parent != parent) {
                struct dentry *child = this_parent;
-               this_parent = try_to_ascend(this_parent, locked, seq);
-               if (!this_parent)
+               this_parent = child->d_parent;
+
+               spin_unlock(&child->d_lock);
+               spin_lock(&this_parent->d_lock);
+
+               /* might go back up the wrong parent if we have had a rename */
+               if (!locked && read_seqretry(&rename_lock, seq))
                        goto rename_retry;
                next = child->d_child.next;
+               while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+                       if (next == &this_parent->d_subdirs)
+                               goto ascend;
+                       child = list_entry(next, struct dentry, d_child);
+                       next = next->next;
+               }
+               rcu_read_unlock();
                goto resume;
        }
-       spin_unlock(&this_parent->d_lock);
        if (!locked && read_seqretry(&rename_lock, seq))
                goto rename_retry;
+       spin_unlock(&this_parent->d_lock);
+       rcu_read_unlock();
        if (locked)
                write_sequnlock(&rename_lock);
        return 0; /* No mount points found in tree */
@@ -1054,6 +1041,8 @@ positive:
        return 1;
 
 rename_retry:
+       spin_unlock(&this_parent->d_lock);
+       rcu_read_unlock();
        if (locked)
                goto again;
        locked = 1;
@@ -1139,23 +1128,40 @@ resume:
        /*
         * All done at this level ... ascend and resume the search.
         */
+       rcu_read_lock();
+ascend:
        if (this_parent != parent) {
                struct dentry *child = this_parent;
-               this_parent = try_to_ascend(this_parent, locked, seq);
-               if (!this_parent)
+               this_parent = child->d_parent;
+
+               spin_unlock(&child->d_lock);
+               spin_lock(&this_parent->d_lock);
+
+               /* might go back up the wrong parent if we have had a rename */
+               if (!locked && read_seqretry(&rename_lock, seq))
                        goto rename_retry;
                next = child->d_child.next;
+               while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+                       if (next == &this_parent->d_subdirs)
+                               goto ascend;
+                       child = list_entry(next, struct dentry, d_child);
+                       next = next->next;
+               }
+               rcu_read_unlock();
                goto resume;
        }
 out:
-       spin_unlock(&this_parent->d_lock);
        if (!locked && read_seqretry(&rename_lock, seq))
                goto rename_retry;
+       spin_unlock(&this_parent->d_lock);
+       rcu_read_unlock();
        if (locked)
                write_sequnlock(&rename_lock);
        return found;
 
 rename_retry:
+       spin_unlock(&this_parent->d_lock);
+       rcu_read_unlock();
        if (found)
                return found;
        if (locked)
@@ -2914,26 +2920,43 @@ resume:
                }
                spin_unlock(&dentry->d_lock);
        }
+       rcu_read_lock();
+ascend:
        if (this_parent != root) {
                struct dentry *child = this_parent;
                if (!(this_parent->d_flags & DCACHE_GENOCIDE)) {
                        this_parent->d_flags |= DCACHE_GENOCIDE;
                        this_parent->d_count--;
                }
-               this_parent = try_to_ascend(this_parent, locked, seq);
-               if (!this_parent)
+               this_parent = child->d_parent;
+
+               spin_unlock(&child->d_lock);
+               spin_lock(&this_parent->d_lock);
+
+               /* might go back up the wrong parent if we have had a rename */
+               if (!locked && read_seqretry(&rename_lock, seq))
                        goto rename_retry;
                next = child->d_child.next;
+               while (unlikely(child->d_flags & DCACHE_DENTRY_KILLED)) {
+                       if (next == &this_parent->d_subdirs)
+                               goto ascend;
+                       child = list_entry(next, struct dentry, d_child);
+                       next = next->next;
+               }
+               rcu_read_unlock();
                goto resume;
        }
-       spin_unlock(&this_parent->d_lock);
        if (!locked && read_seqretry(&rename_lock, seq))
                goto rename_retry;
+       spin_unlock(&this_parent->d_lock);
+       rcu_read_unlock();
        if (locked)
                write_sequnlock(&rename_lock);
        return;
 
 rename_retry:
+       spin_unlock(&this_parent->d_lock);
+       rcu_read_unlock();
        if (locked)
                goto again;
        locked = 1;