linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption
@ 2015-08-13 21:23 Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 1/8] fold d_kill() and d_free() Nicholas A. Bellinger
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel,
	Nicholas Bellinger

From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi Greg-KH, Al, & Co,

So I've recently been repeatedly hitting the same dcache shrink list
corruption bug on v3.14.47 that has been addressed in >= v3.15 code,
but it appears only one of the fixes:

   commit 22213318af7ae265bc6cd8aef2febbc2d69a2440
   Author: Al Viro <viro@zeniv.linux.org.uk>
   Date:   Sat Apr 19 12:30:58 2014 -0400

       fix races between __d_instantiate() and checks of dentry flags

actually made it into v3.14.y code.

The final set of patches from Al, along with the original thread is here:

  https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg637332.html

The series cherry-picked clean into v3.14.y, with the minor exception
of needing to use the older dentry->d_child in __dentry_kill(), instead
of the newer dentry->d_u.d_child present in >= v3.15.y.

If there are no objections and/or concerns from Al, please consider
queueing this series to address the bug for other folks still using
v3.14.y code.

Thank you,

--nab

Al Viro (7):
  fold d_kill() and d_free()
  fold try_prune_one_dentry()
  new helper: dentry_free()
  expand the call of dentry_lru_del() in dentry_kill()
  dentry_kill(): don't try to remove from shrink list
  don't remove from shrink list in select_collect()
  more graceful recovery in umount_collect()

Miklos Szeredi (1):
  dcache: don't need rcu in shrink_dentry_list()

 fs/dcache.c            | 316 ++++++++++++++++---------------------------------
 include/linux/dcache.h |   2 +
 2 files changed, 103 insertions(+), 215 deletions(-)

-- 
1.8.5.3


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 1/8] fold d_kill() and d_free()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 2/8] fold try_prune_one_dentry() Nicholas A. Bellinger
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit 03b3b889e79cdb6b806fc0ba9be0d71c186bbfaa upstream.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 77 +++++++++++++++++++------------------------------------------
 1 file changed, 24 insertions(+), 53 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3d2f27b..8b5adab 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -244,24 +244,6 @@ static void __d_free(struct rcu_head *head)
 	kmem_cache_free(dentry_cache, dentry); 
 }
 
-/*
- * no locks, please.
- */
-static void d_free(struct dentry *dentry)
-{
-	WARN_ON(!hlist_unhashed(&dentry->d_u.d_alias));
-	BUG_ON((int)dentry->d_lockref.count > 0);
-	this_cpu_dec(nr_dentry);
-	if (dentry->d_op && dentry->d_op->d_release)
-		dentry->d_op->d_release(dentry);
-
-	/* if dentry was never visible to RCU, immediate free is OK */
-	if (!(dentry->d_flags & DCACHE_RCUACCESS))
-		__d_free(&dentry->d_u.d_rcu);
-	else
-		call_rcu(&dentry->d_u.d_rcu, __d_free);
-}
-
 /**
  * dentry_rcuwalk_barrier - invalidate in-progress rcu-walk lookups
  * @dentry: the target dentry
@@ -419,40 +401,6 @@ static void dentry_lru_del(struct dentry *dentry)
 }
 
 /**
- * d_kill - kill dentry and return parent
- * @dentry: dentry to kill
- * @parent: parent dentry
- *
- * The dentry must already be unhashed and removed from the LRU.
- *
- * If this is the root of the dentry tree, return NULL.
- *
- * dentry->d_lock and parent->d_lock must be held by caller, and are dropped by
- * d_kill.
- */
-static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
-	__releases(dentry->d_lock)
-	__releases(parent->d_lock)
-	__releases(dentry->d_inode->i_lock)
-{
-	__list_del_entry(&dentry->d_child);
-	/*
-	 * Inform d_walk() that we are no longer attached to the
-	 * dentry tree
-	 */
-	dentry->d_flags |= DCACHE_DENTRY_KILLED;
-	if (parent)
-		spin_unlock(&parent->d_lock);
-	dentry_iput(dentry);
-	/*
-	 * dentry_iput drops the locks, at which point nobody (except
-	 * transient RCU lookups) can reach this dentry.
-	 */
-	d_free(dentry);
-	return parent;
-}
-
-/**
  * d_drop - drop a dentry
  * @dentry: dentry to drop
  *
@@ -545,7 +493,30 @@ relock:
 	dentry_lru_del(dentry);
 	/* if it was on the hash then remove it */
 	__d_drop(dentry);
-	return d_kill(dentry, parent);
+	__list_del_entry(&dentry->d_child);
+	/*
+	 * Inform d_walk() that we are no longer attached to the
+	 * dentry tree
+	 */
+	dentry->d_flags |= DCACHE_DENTRY_KILLED;
+	if (parent)
+		spin_unlock(&parent->d_lock);
+	dentry_iput(dentry);
+	/*
+	 * dentry_iput drops the locks, at which point nobody (except
+	 * transient RCU lookups) can reach this dentry.
+	 */
+	BUG_ON((int)dentry->d_lockref.count > 0);
+	this_cpu_dec(nr_dentry);
+	if (dentry->d_op && dentry->d_op->d_release)
+		dentry->d_op->d_release(dentry);
+
+	/* if dentry was never visible to RCU, immediate free is OK */
+	if (!(dentry->d_flags & DCACHE_RCUACCESS))
+		__d_free(&dentry->d_u.d_rcu);
+	else
+		call_rcu(&dentry->d_u.d_rcu, __d_free);
+	return parent;
 }
 
 /* 
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 2/8] fold try_prune_one_dentry()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 1/8] fold d_kill() and d_free() Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 3/8] new helper: dentry_free() Nicholas A. Bellinger
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit 5c47e6d0ad608987b91affbcf7d1fc12dfbe8fb4 upstream.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 75 +++++++++++++++++++++----------------------------------------
 1 file changed, 25 insertions(+), 50 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 8b5adab..1dc738a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -788,47 +788,9 @@ restart:
 }
 EXPORT_SYMBOL(d_prune_aliases);
 
-/*
- * Try to throw away a dentry - free the inode, dput the parent.
- * Requires dentry->d_lock is held, and dentry->d_count == 0.
- * Releases dentry->d_lock.
- *
- * This may fail if locks cannot be acquired no problem, just try again.
- */
-static struct dentry * try_prune_one_dentry(struct dentry *dentry)
-	__releases(dentry->d_lock)
-{
-	struct dentry *parent;
-
-	parent = dentry_kill(dentry, 0);
-	/*
-	 * If dentry_kill returns NULL, we have nothing more to do.
-	 * if it returns the same dentry, trylocks failed. In either
-	 * case, just loop again.
-	 *
-	 * Otherwise, we need to prune ancestors too. This is necessary
-	 * to prevent quadratic behavior of shrink_dcache_parent(), but
-	 * is also expected to be beneficial in reducing dentry cache
-	 * fragmentation.
-	 */
-	if (!parent)
-		return NULL;
-	if (parent == dentry)
-		return dentry;
-
-	/* Prune ancestors. */
-	dentry = parent;
-	while (dentry) {
-		if (lockref_put_or_lock(&dentry->d_lockref))
-			return NULL;
-		dentry = dentry_kill(dentry, 1);
-	}
-	return NULL;
-}
-
 static void shrink_dentry_list(struct list_head *list)
 {
-	struct dentry *dentry;
+	struct dentry *dentry, *parent;
 
 	rcu_read_lock();
 	for (;;) {
@@ -864,22 +826,35 @@ static void shrink_dentry_list(struct list_head *list)
 		}
 		rcu_read_unlock();
 
+		parent = dentry_kill(dentry, 0);
 		/*
-		 * If 'try_to_prune()' returns a dentry, it will
-		 * be the same one we passed in, and d_lock will
-		 * have been held the whole time, so it will not
-		 * have been added to any other lists. We failed
-		 * to get the inode lock.
-		 *
-		 * We just add it back to the shrink list.
+		 * If dentry_kill returns NULL, we have nothing more to do.
 		 */
-		dentry = try_prune_one_dentry(dentry);
-
-		rcu_read_lock();
-		if (dentry) {
+		if (!parent) {
+			rcu_read_lock();
+			continue;
+		}
+		if (unlikely(parent == dentry)) {
+			/*
+			 * trylocks have failed and d_lock has been held the
+			 * whole time, so it could not have been added to any
+			 * other lists. Just add it back to the shrink list.
+			 */
+			rcu_read_lock();
 			d_shrink_add(dentry, list);
 			spin_unlock(&dentry->d_lock);
+			continue;
 		}
+		/*
+		 * We need to prune ancestors too. This is necessary to prevent
+		 * quadratic behavior of shrink_dcache_parent(), but is also
+		 * expected to be beneficial in reducing dentry cache
+		 * fragmentation.
+		 */
+		dentry = parent;
+		while (dentry && !lockref_put_or_lock(&dentry->d_lockref))
+			dentry = dentry_kill(dentry, 1);
+		rcu_read_lock();
 	}
 	rcu_read_unlock();
 }
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 3/8] new helper: dentry_free()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 1/8] fold d_kill() and d_free() Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 2/8] fold try_prune_one_dentry() Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 4/8] expand the call of dentry_lru_del() in dentry_kill() Nicholas A. Bellinger
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit b4f0354e968f5fabd39bc85b99fedae4a97589fe upstream.

The part of old d_free() that dealt with actual freeing of dentry.
Taken out of dentry_kill() into a separate function.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1dc738a..3a0f252 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -244,6 +244,15 @@ static void __d_free(struct rcu_head *head)
 	kmem_cache_free(dentry_cache, dentry); 
 }
 
+static void dentry_free(struct dentry *dentry)
+{
+	/* if dentry was never visible to RCU, immediate free is OK */
+	if (!(dentry->d_flags & DCACHE_RCUACCESS))
+		__d_free(&dentry->d_u.d_rcu);
+	else
+		call_rcu(&dentry->d_u.d_rcu, __d_free);
+}
+
 /**
  * dentry_rcuwalk_barrier - invalidate in-progress rcu-walk lookups
  * @dentry: the target dentry
@@ -511,11 +520,7 @@ relock:
 	if (dentry->d_op && dentry->d_op->d_release)
 		dentry->d_op->d_release(dentry);
 
-	/* if dentry was never visible to RCU, immediate free is OK */
-	if (!(dentry->d_flags & DCACHE_RCUACCESS))
-		__d_free(&dentry->d_u.d_rcu);
-	else
-		call_rcu(&dentry->d_u.d_rcu, __d_free);
+	dentry_free(dentry);
 	return parent;
 }
 
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 4/8] expand the call of dentry_lru_del() in dentry_kill()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
                   ` (2 preceding siblings ...)
  2015-08-13 21:23 ` [PATCH-v3.14.y 3/8] new helper: dentry_free() Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 5/8] dentry_kill(): don't try to remove from shrink list Nicholas A. Bellinger
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit 01b6035190b024240a43ac1d8e9c6f964f5f1c63 upstream.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3a0f252..16f9066 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -499,7 +499,12 @@ relock:
 	if ((dentry->d_flags & DCACHE_OP_PRUNE) && !d_unhashed(dentry))
 		dentry->d_op->d_prune(dentry);
 
-	dentry_lru_del(dentry);
+	if (dentry->d_flags & DCACHE_LRU_LIST) {
+		if (!(dentry->d_flags & DCACHE_SHRINK_LIST))
+			d_lru_del(dentry);
+		else
+			d_shrink_del(dentry);
+	}
 	/* if it was on the hash then remove it */
 	__d_drop(dentry);
 	__list_del_entry(&dentry->d_child);
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 5/8] dentry_kill(): don't try to remove from shrink list
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
                   ` (3 preceding siblings ...)
  2015-08-13 21:23 ` [PATCH-v3.14.y 4/8] expand the call of dentry_lru_del() in dentry_kill() Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 6/8] don't remove from shrink list in select_collect() Nicholas A. Bellinger
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit 41edf278fc2f042f4e22a12ed87d19c5201210e1 upstream.

If the victim in on the shrink list, don't remove it from there.
If shrink_dentry_list() manages to remove it from the list before
we are done - fine, we'll just free it as usual.  If not - mark
it with new flag (DCACHE_MAY_FREE) and leave it there.

Eventually, shrink_dentry_list() will get to it, remove the sucker
from shrink list and call dentry_kill(dentry, 0).  Which is where
we'll deal with freeing.

Since now dentry_kill(dentry, 0) may happen after or during
dentry_kill(dentry, 1), we need to recognize that (by seeing
DCACHE_DENTRY_KILLED already set), unlock everything
and either free the sucker (in case DCACHE_MAY_FREE has been
set) or leave it for ongoing dentry_kill(dentry, 1) to deal with.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c            | 27 +++++++++++++++++++--------
 include/linux/dcache.h |  2 ++
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 16f9066..099308a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -466,7 +466,14 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	__releases(dentry->d_lock)
 {
 	struct inode *inode;
-	struct dentry *parent;
+	struct dentry *parent = NULL;
+	bool can_free = true;
+
+	if (unlikely(dentry->d_flags & DCACHE_DENTRY_KILLED)) {
+		can_free = dentry->d_flags & DCACHE_MAY_FREE;
+		spin_unlock(&dentry->d_lock);
+		goto out;
+	}
 
 	inode = dentry->d_inode;
 	if (inode && !spin_trylock(&inode->i_lock)) {
@@ -477,9 +484,7 @@ relock:
 		}
 		return dentry; /* try again with same dentry */
 	}
-	if (IS_ROOT(dentry))
-		parent = NULL;
-	else
+	if (!IS_ROOT(dentry))
 		parent = dentry->d_parent;
 	if (parent && !spin_trylock(&parent->d_lock)) {
 		if (inode)
@@ -502,8 +507,6 @@ relock:
 	if (dentry->d_flags & DCACHE_LRU_LIST) {
 		if (!(dentry->d_flags & DCACHE_SHRINK_LIST))
 			d_lru_del(dentry);
-		else
-			d_shrink_del(dentry);
 	}
 	/* if it was on the hash then remove it */
 	__d_drop(dentry);
@@ -525,7 +528,15 @@ relock:
 	if (dentry->d_op && dentry->d_op->d_release)
 		dentry->d_op->d_release(dentry);
 
-	dentry_free(dentry);
+	spin_lock(&dentry->d_lock);
+	if (dentry->d_flags & DCACHE_SHRINK_LIST) {
+		dentry->d_flags |= DCACHE_MAY_FREE;
+		can_free = false;
+	}
+	spin_unlock(&dentry->d_lock);
+out:
+	if (likely(can_free))
+		dentry_free(dentry);
 	return parent;
 }
 
@@ -830,7 +841,7 @@ static void shrink_dentry_list(struct list_head *list)
 		 * We found an inuse dentry which was not removed from
 		 * the LRU because of laziness during lookup. Do not free it.
 		 */
-		if (dentry->d_lockref.count) {
+		if ((int)dentry->d_lockref.count > 0) {
 			spin_unlock(&dentry->d_lock);
 			continue;
 		}
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 0f0eb1c..2a23ecb 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -221,6 +221,8 @@ struct dentry_operations {
 #define DCACHE_SYMLINK_TYPE		0x00300000 /* Symlink */
 #define DCACHE_FILE_TYPE		0x00400000 /* Other file type */
 
+#define DCACHE_MAY_FREE			0x00800000
+
 extern seqlock_t rename_lock;
 
 static inline int dname_external(const struct dentry *dentry)
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 6/8] don't remove from shrink list in select_collect()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
                   ` (4 preceding siblings ...)
  2015-08-13 21:23 ` [PATCH-v3.14.y 5/8] dentry_kill(): don't try to remove from shrink list Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 7/8] more graceful recovery in umount_collect() Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 8/8] dcache: don't need rcu in shrink_dentry_list() Nicholas A. Bellinger
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit fe91522a7ba82ca1a51b07e19954b3825e4aaa22 upstream.

	If we find something already on a shrink list, just increment
data->found and do nothing else.  Loops in shrink_dcache_parent() and
check_submounts_and_drop() will do the right thing - everything we
did put into our list will be evicted and if there had been nothing,
but data->found got non-zero, well, we have somebody else shrinking
those guys; just try again.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 099308a..03e8107 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1231,34 +1231,23 @@ static enum d_walk_ret select_collect(void *_data, struct dentry *dentry)
 	if (data->start == dentry)
 		goto out;
 
-	/*
-	 * move only zero ref count dentries to the dispose list.
-	 *
-	 * Those which are presently on the shrink list, being processed
-	 * by shrink_dentry_list(), shouldn't be moved.  Otherwise the
-	 * loop in shrink_dcache_parent() might not make any progress
-	 * and loop forever.
-	 */
-	if (dentry->d_lockref.count) {
-		dentry_lru_del(dentry);
-	} else if (!(dentry->d_flags & DCACHE_SHRINK_LIST)) {
-		/*
-		 * We can't use d_lru_shrink_move() because we
-		 * need to get the global LRU lock and do the
-		 * LRU accounting.
-		 */
-		d_lru_del(dentry);
-		d_shrink_add(dentry, &data->dispose);
+	if (dentry->d_flags & DCACHE_SHRINK_LIST) {
 		data->found++;
-		ret = D_WALK_NORETRY;
+	} else {
+		if (dentry->d_flags & DCACHE_LRU_LIST)
+			d_lru_del(dentry);
+		if (!dentry->d_lockref.count) {
+			d_shrink_add(dentry, &data->dispose);
+			data->found++;
+		}
 	}
 	/*
 	 * We can return to the caller if we have found some (this
 	 * ensures forward progress). We'll be coming back to find
 	 * the rest.
 	 */
-	if (data->found && need_resched())
-		ret = D_WALK_QUIT;
+	if (!list_empty(&data->dispose))
+		ret = need_resched() ? D_WALK_QUIT : D_WALK_NORETRY;
 out:
 	return ret;
 }
-- 
1.8.5.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 7/8] more graceful recovery in umount_collect()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
                   ` (5 preceding siblings ...)
  2015-08-13 21:23 ` [PATCH-v3.14.y 6/8] don't remove from shrink list in select_collect() Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  2015-08-13 21:23 ` [PATCH-v3.14.y 8/8] dcache: don't need rcu in shrink_dentry_list() Nicholas A. Bellinger
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

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

commit 9c8c10e262e0f62cb2530f1b076de979123183dd upstream.

Start with shrink_dcache_parent(), then scan what remains.

First of all, BUG() is very much an overkill here; we are holding
->s_umount, and hitting BUG() means that a lot of interesting stuff
will be hanging after that point (sync(2), for example).  Moreover,
in cases when there had been more than one leak, we'll be better
off reporting all of them.  And more than just the last component
of pathname - %pd is there for just such uses...

That was the last user of dentry_lru_del(), so kill it off...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 101 +++++++++++++++---------------------------------------------
 1 file changed, 25 insertions(+), 76 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 03e8107..0a92385 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -393,22 +393,6 @@ static void dentry_lru_add(struct dentry *dentry)
 		d_lru_add(dentry);
 }
 
-/*
- * Remove a dentry with references from the LRU.
- *
- * If we are on the shrink list, then we can get to try_prune_one_dentry() and
- * lose our last reference through the parent walk. In this case, we need to
- * remove ourselves from the shrink list, not the LRU.
- */
-static void dentry_lru_del(struct dentry *dentry)
-{
-	if (dentry->d_flags & DCACHE_LRU_LIST) {
-		if (dentry->d_flags & DCACHE_SHRINK_LIST)
-			return d_shrink_del(dentry);
-		d_lru_del(dentry);
-	}
-}
-
 /**
  * d_drop - drop a dentry
  * @dentry: dentry to drop
@@ -1277,45 +1261,35 @@ void shrink_dcache_parent(struct dentry *parent)
 }
 EXPORT_SYMBOL(shrink_dcache_parent);
 
-static enum d_walk_ret umount_collect(void *_data, struct dentry *dentry)
+static enum d_walk_ret umount_check(void *_data, struct dentry *dentry)
 {
-	struct select_data *data = _data;
-	enum d_walk_ret ret = D_WALK_CONTINUE;
+	/* it has busy descendents; complain about those instead */
+	if (!list_empty(&dentry->d_subdirs))
+		return D_WALK_CONTINUE;
 
-	if (dentry->d_lockref.count) {
-		dentry_lru_del(dentry);
-		if (likely(!list_empty(&dentry->d_subdirs)))
-			goto out;
-		if (dentry == data->start && dentry->d_lockref.count == 1)
-			goto out;
-		printk(KERN_ERR
-		       "BUG: Dentry %p{i=%lx,n=%s}"
-		       " still in use (%d)"
-		       " [unmount of %s %s]\n",
+	/* root with refcount 1 is fine */
+	if (dentry == _data && dentry->d_lockref.count == 1)
+		return D_WALK_CONTINUE;
+
+	printk(KERN_ERR "BUG: Dentry %p{i=%lx,n=%pd} "
+			" still in use (%d) [unmount of %s %s]\n",
 		       dentry,
 		       dentry->d_inode ?
 		       dentry->d_inode->i_ino : 0UL,
-		       dentry->d_name.name,
+		       dentry,
 		       dentry->d_lockref.count,
 		       dentry->d_sb->s_type->name,
 		       dentry->d_sb->s_id);
-		BUG();
-	} else if (!(dentry->d_flags & DCACHE_SHRINK_LIST)) {
-		/*
-		 * We can't use d_lru_shrink_move() because we
-		 * need to get the global LRU lock and do the
-		 * LRU accounting.
-		 */
-		if (dentry->d_flags & DCACHE_LRU_LIST)
-			d_lru_del(dentry);
-		d_shrink_add(dentry, &data->dispose);
-		data->found++;
-		ret = D_WALK_NORETRY;
-	}
-out:
-	if (data->found && need_resched())
-		ret = D_WALK_QUIT;
-	return ret;
+	WARN_ON(1);
+	return D_WALK_CONTINUE;
+}
+
+static void do_one_tree(struct dentry *dentry)
+{
+	shrink_dcache_parent(dentry);
+	d_walk(dentry, dentry, umount_check, NULL);
+	d_drop(dentry);
+	dput(dentry);
 }
 
 /*
@@ -1325,40 +1299,15 @@ void shrink_dcache_for_umount(struct super_block *sb)
 {
 	struct dentry *dentry;
 
-	if (down_read_trylock(&sb->s_umount))
-		BUG();
+	WARN(down_read_trylock(&sb->s_umount), "s_umount should've been locked");
 
 	dentry = sb->s_root;
 	sb->s_root = NULL;
-	for (;;) {
-		struct select_data data;
-
-		INIT_LIST_HEAD(&data.dispose);
-		data.start = dentry;
-		data.found = 0;
-
-		d_walk(dentry, &data, umount_collect, NULL);
-		if (!data.found)
-			break;
-
-		shrink_dentry_list(&data.dispose);
-		cond_resched();
-	}
-	d_drop(dentry);
-	dput(dentry);
+	do_one_tree(dentry);
 
 	while (!hlist_bl_empty(&sb->s_anon)) {
-		struct select_data data;
-		dentry = hlist_bl_entry(hlist_bl_first(&sb->s_anon), struct dentry, d_hash);
-
-		INIT_LIST_HEAD(&data.dispose);
-		data.start = NULL;
-		data.found = 0;
-
-		d_walk(dentry, &data, umount_collect, NULL);
-		if (data.found)
-			shrink_dentry_list(&data.dispose);
-		cond_resched();
+		dentry = dget(hlist_bl_entry(hlist_bl_first(&sb->s_anon), struct dentry, d_hash));
+		do_one_tree(dentry);
 	}
 }
 
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH-v3.14.y 8/8] dcache: don't need rcu in shrink_dentry_list()
  2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
                   ` (6 preceding siblings ...)
  2015-08-13 21:23 ` [PATCH-v3.14.y 7/8] more graceful recovery in umount_collect() Nicholas A. Bellinger
@ 2015-08-13 21:23 ` Nicholas A. Bellinger
  7 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2015-08-13 21:23 UTC (permalink / raw)
  To: stable; +Cc: Greg-KH, Al Viro, Miklos Szeredi, linux-fsdevel

From: Miklos Szeredi <mszeredi@suse.cz>

commit 60942f2f235ce7b817166cdf355eed729094834d upstream.

Since now the shrink list is private and nobody can free the dentry while
it is on the shrink list, we can remove RCU protection from this.

Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/dcache.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 0a92385..df323f8 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -797,23 +797,9 @@ static void shrink_dentry_list(struct list_head *list)
 {
 	struct dentry *dentry, *parent;
 
-	rcu_read_lock();
-	for (;;) {
-		dentry = list_entry_rcu(list->prev, struct dentry, d_lru);
-		if (&dentry->d_lru == list)
-			break; /* empty */
-
-		/*
-		 * Get the dentry lock, and re-verify that the dentry is
-		 * this on the shrinking list. If it is, we know that
-		 * DCACHE_SHRINK_LIST and DCACHE_LRU_LIST are set.
-		 */
+	while (!list_empty(list)) {
+		dentry = list_entry(list->prev, struct dentry, d_lru);
 		spin_lock(&dentry->d_lock);
-		if (dentry != list_entry(list->prev, struct dentry, d_lru)) {
-			spin_unlock(&dentry->d_lock);
-			continue;
-		}
-
 		/*
 		 * The dispose list is isolated and dentries are not accounted
 		 * to the LRU here, so we can simply remove it from the list
@@ -829,23 +815,20 @@ static void shrink_dentry_list(struct list_head *list)
 			spin_unlock(&dentry->d_lock);
 			continue;
 		}
-		rcu_read_unlock();
 
 		parent = dentry_kill(dentry, 0);
 		/*
 		 * If dentry_kill returns NULL, we have nothing more to do.
 		 */
-		if (!parent) {
-			rcu_read_lock();
+		if (!parent)
 			continue;
-		}
+
 		if (unlikely(parent == dentry)) {
 			/*
 			 * trylocks have failed and d_lock has been held the
 			 * whole time, so it could not have been added to any
 			 * other lists. Just add it back to the shrink list.
 			 */
-			rcu_read_lock();
 			d_shrink_add(dentry, list);
 			spin_unlock(&dentry->d_lock);
 			continue;
@@ -859,9 +842,7 @@ static void shrink_dentry_list(struct list_head *list)
 		dentry = parent;
 		while (dentry && !lockref_put_or_lock(&dentry->d_lockref))
 			dentry = dentry_kill(dentry, 1);
-		rcu_read_lock();
 	}
-	rcu_read_unlock();
 }
 
 static enum lru_status
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-08-13 21:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-08-13 21:23 [PATCH-v3.14.y 0/8] Stable backport for dcache shrink list corruption Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 1/8] fold d_kill() and d_free() Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 2/8] fold try_prune_one_dentry() Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 3/8] new helper: dentry_free() Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 4/8] expand the call of dentry_lru_del() in dentry_kill() Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 5/8] dentry_kill(): don't try to remove from shrink list Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 6/8] don't remove from shrink list in select_collect() Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 7/8] more graceful recovery in umount_collect() Nicholas A. Bellinger
2015-08-13 21:23 ` [PATCH-v3.14.y 8/8] dcache: don't need rcu in shrink_dentry_list() Nicholas A. Bellinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).