All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	Ben Hutchings <ben@decadent.org.uk>,
	hujianyang <hujianyang@huawei.com>
Subject: [PATCH 3.10 27/31] deal with deadlock in d_walk()
Date: Sun, 26 Apr 2015 15:46:47 +0200	[thread overview]
Message-ID: <20150426134210.529222061@linuxfoundation.org> (raw)
In-Reply-To: <20150426134209.255099785@linuxfoundation.org>

3.10-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Al Viro <viro@zeniv.linux.org.uk>

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>
Cc: Ben Hutchings <ben@decadent.org.uk>
[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 <hujianyang@huawei.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 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;



  parent reply	other threads:[~2015-04-26 13:57 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-26 13:46 [PATCH 3.10 00/31] 3.10.76-stable review Greg Kroah-Hartman
2015-04-26 13:48 ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 01/31] conditionally define U32_MAX Greg Kroah-Hartman
2015-04-26 13:48   ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 02/31] remove extra definitions of U32_MAX Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 03/31] tcp: prevent fetching dst twice in early demux code Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 04/31] ipv6: Dont reduce hop limit for an interface Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 05/31] tcp: fix FRTO undo on cumulative ACK of SACKed range Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 06/31] tcp: tcp_make_synack() should clear skb->tstamp Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-27  4:02   ` Willy Tarreau
2015-04-27  4:23     ` Eric Dumazet
2015-04-27  4:45     ` David Miller
2015-04-26 13:46 ` [PATCH 3.10 07/31] 8139cp: Call dev_kfree_skby_any instead of kfree_skb Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 08/31] 8139too: Call dev_kfree_skby_any instead of dev_kfree_skb Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 09/31] r8169: " Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 10/31] bnx2: " Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 11/31] tg3: " Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 12/31] ixgb: " Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 13/31] benet: Call dev_kfree_skby_any instead of kfree_skb Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 14/31] serial: 8250_dw: Fix deadlock in LCR workaround Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 15/31] jfs: fix readdir regression Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 16/31] splice: Apply generic position and size checks to each write Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 17/31] mm: Fix NULL pointer dereference in madvise(MADV_WILLNEED) support Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 18/31] Bluetooth: Enable Atheros 0cf3:311e for firmware upload Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 19/31] Bluetooth: Add firmware update for Atheros 0cf3:311f Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 20/31] Bluetooth: btusb: Add IMC Networks (Broadcom based) Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 21/31] Bluetooth: Add support for Intel bootloader devices Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 22/31] Bluetooth: Ignore isochronous endpoints for Intel USB bootloader Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 23/31] netfilter: conntrack: disable generic tracking for known protocols Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 24/31] KVM: x86: SYSENTER emulation is broken Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 26/31] move d_rcu from overlapping d_child to overlapping d_alias Greg Kroah-Hartman
2015-04-26 13:46 ` Greg Kroah-Hartman [this message]
2015-04-27  1:20   ` [PATCH 3.10 27/31] deal with deadlock in d_walk() Ben Hutchings
2015-04-27  7:53     ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 28/31] vm: add VM_FAULT_SIGSEGV handling support Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 29/31] vm: make stack guard page errors return VM_FAULT_SIGSEGV rather than SIGBUS Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 30/31] x86: mm: move mmap_sem unlock from mm_fault_error() to caller Greg Kroah-Hartman
2015-04-26 13:46 ` [PATCH 3.10 31/31] sb_edac: avoid INTERNAL ERROR message in EDAC with unspecified channel Greg Kroah-Hartman
2015-04-26 13:49   ` Greg Kroah-Hartman
2015-04-26 15:15 ` [PATCH 3.10 00/31] 3.10.76-stable review Guenter Roeck
2015-04-26 17:12   ` Greg Kroah-Hartman
2015-04-26 17:14     ` Guenter Roeck
2015-04-26 20:01 ` Guenter Roeck
2015-04-27 17:19 ` Shuah Khan

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=20150426134210.529222061@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ben@decadent.org.uk \
    --cc=hujianyang@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.