On Sun, 2015-04-26 at 15:49 +0200, Greg Kroah-Hartman wrote: > 3.10-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Al Viro <[email protected]> > > 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 <[email protected]> > Cc: Ben Hutchings <[email protected]> > [hujianyang: Backported to 3.10 refer to the work of Ben Hutchings in 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: hujianyang <[email protected]> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
This is broken; you need to fold in commit 20defcec264c from 3.2.y
("dcache: Fix locking bugs in backported "deal with deadlock in
d_walk()"").
Ben.
> ---
> fs/dcache.c | 102
> ++++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 62 insertions(+), 40 deletions(-)
>
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -364,9 +364,9 @@ static struct dentry *d_kill(struct dent
> __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;
> @@ -988,35 +988,6 @@ void shrink_dcache_for_umount(struct sup
> }
>
> /*
> - * 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.
> * We descend to the next level whenever the d_subdirs
> * list is non-empty and continue searching.
> @@ -1070,17 +1041,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 */
> @@ -1092,6 +1078,8 @@ positive:
> return 1;
>
> rename_retry:
> + spin_unlock(&this_parent->d_lock);
> + rcu_read_unlock();
> if (locked)
> goto again;
> locked = 1;
> @@ -1177,23 +1165,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)
> @@ -2954,26 +2959,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;
>
>
--
Ben Hutchings
I'm not a reverse psychological virus. Please don't copy me into your sig.
signature.asc
Description: This is a digitally signed message part
