[Patch 3.14 stable 00/16] vfs: fix dentry shrink list corruption

[Patch 3.14 stable 00/16] vfs: fix dentry shrink list corruption

[Cong Wang]

On at least 3 different machines, we have seen dentry shrink list corruption
with 3.14 stable kernel (see the end of this email), which I think is
fixed by a set of patches from Al.

This bug is probably introduced by the following commit in 3.12:

	commit dd1f6b2e43a53ee58eb87d5e623cf44e277d005d
	Author: Dave Chinner <dchinner@redhat.com>
	Date:   Wed Aug 28 10:17:55 2013 +1000

	    dcache: remove dentries from LRU before putting on dispose list

since previously we held the per sb lock. It is hard to reproduce this bug,
because it is a race condition which happens when we update two contiguous
dentries on a shrink list. Matt ran some ltp stress tests and doesn't see
any problem with the patched kernel. And, 3.13 needs them too, but we have
no time or resource to backport or test them on 3.13.

The first 7 patches are the original patchset from Al, the rest of patches
are either needed for context or are folllow-up fixes, especially the
last 2 patches. Of course all of them are merged in upstream, see each patch
for details. Note, I don't get any conflict when backporting them to 3.14.

Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Tested-by: Matthew Mullins <mmullins@twopensource.com>

------------->

[45204.957357] ------------[ cut here ]------------
[45204.957370] WARNING: CPU: 6 PID: 26375 at lib/list_debug.c:33
__list_add+0x63/0xa7()
[45204.957373] list_add corruption. prev->next should be next
(ffff880c0205d280), but was ffff880cfa3d9d00. (prev=ffff8807c5ad7c08).
[45204.957375] Modules linked in: cls_basic act_mirred cls_u32 veth
sch_ingress netconsole configfs ipv6 dm_multipath scsi_dh video sbs
sbshc acpi_p
ad acpi_ipmi parport_pc lp parport tcp_diag inet_diag ipmi_devintf sg
iTCO_wdt iTCO_vendor_support dcdbas igb i2c_algo_bit ptp pps_core wmi
ipmi_si sb_edac hed edac_core ipmi_msghandler i2c_i801
 ioatdma i2c_core microcode lpc_ich mfd_core dca shpchp ahci libahci
libata sd_mod scsi_mod
[45204.957421] CPU: 6 PID: 26375 Comm: python2.6 Not tainted 3.14.14 #1
[45204.957423] Hardware name: Dell Inc. PowerEdge C6220/03C9JJ, BIOS
1.2.1 05/27/2013
[45204.957426]  0000000000000000 ffff8807c5ad7ad0 ffffffff814a9b67
ffff8807c5ad7b18
[45204.957453]  ffff8807c5ad7b08 ffffffff8105b901 ffffffff812787ff
ffff8807c5ad7c08
[45204.957459]  ffff880c0205d280 ffff880be8bbc8c0 ffff880850fd3680
ffff8807c5ad7b68
[45204.957465] Call Trace:
[45204.957475]  [<ffffffff814a9b67>] dump_stack+0x45/0x56
[45204.957482]  [<ffffffff8105b901>] warn_slowpath_common+0x7f/0x98
[45204.957486]  [<ffffffff812787ff>] ? __list_add+0x63/0xa7
[45204.957490]  [<ffffffff8105b966>] warn_slowpath_fmt+0x4c/0x4e
[45204.957494]  [<ffffffff812787ff>] __list_add+0x63/0xa7
[45204.957500]  [<ffffffff811630fa>] list_add+0xc/0xe
[45204.957504]  [<ffffffff81163146>] d_shrink_add+0x4a/0x5e
[45204.957509]  [<ffffffff81163b0b>] shrink_dentry_list+0xaa/0xbb
[45204.957513]  [<ffffffff81164485>] shrink_dcache_parent+0x50/0x60
[45204.957520]  [<ffffffff811a5070>] proc_flush_task+0x14c/0x18b
[45204.957526]  [<ffffffff8105c49f>] release_task+0x30/0x36d
[45204.957530]  [<ffffffff8105dd43>] do_exit+0x809/0x8ee
[45204.957537]  [<ffffffff810afa48>] ? __unqueue_futex+0x53/0x56
[45204.957541]  [<ffffffff8105dea2>] do_group_exit+0x44/0x9a
[45204.957548]  [<ffffffff81069cc7>] get_signal_to_deliver+0x44b/0x4bc
[45204.957555]  [<ffffffff8100224f>] do_signal+0x49/0x5d5
[45204.957561]  [<ffffffff811442de>] ? kmem_cache_free+0x100/0x138
[45204.957565]  [<ffffffff8105915f>] ? __mmdrop+0x82/0x89
[45204.957570]  [<ffffffff81002804>] do_notify_resume+0x29/0x65
[45204.957576]  [<ffffffff814b5662>] int_signal+0x12/0x17
[45204.957579] ---[ end trace 0decfbe793859fa2 ]---
[45204.959359] ------------[ cut here ]------------

[Patch 3.14 stable 01/16] fold d_kill() and d_free()

[Cong Wang]

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

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit 03b3b889e79cdb6b806fc0ba9be0d71c186bbfaa)
---
 fs/dcache.c | 76 +++++++++++++++++++------------------------------------------
 1 file changed, 24 insertions(+), 52 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 58d57da..d289bb9 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -245,23 +245,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)
-{
-	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 +402,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(&dentry->d_u.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 +494,30 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	dentry_lru_del(dentry);
 	/* if it was on the hash then remove it */
 	__d_drop(dentry);
-	return d_kill(dentry, parent);
+	list_del(&dentry->d_u.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.3.1

[Patch 3.14 stable 02/16] fold try_prune_one_dentry()

[Cong Wang]

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

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

diff --git a/fs/dcache.c b/fs/dcache.c
index d289bb9..bea71ca 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -786,47 +786,9 @@ void d_prune_aliases(struct inode *inode)
 }
 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 (;;) {
@@ -862,22 +824,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.3.1

[Patch 3.14 stable 03/16] new helper: dentry_free()

[Cong Wang]

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

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>
(cherry picked from commit b4f0354e968f5fabd39bc85b99fedae4a97589fe)
---
 fs/dcache.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index bea71ca..d96d271 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -245,6 +245,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
@@ -512,11 +521,7 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	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.3.1

[Patch 3.14 stable 04/16] expand the call of dentry_lru_del() in dentry_kill()

[Cong Wang]

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

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

diff --git a/fs/dcache.c b/fs/dcache.c
index d96d271..3f07e5a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -500,7 +500,12 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	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(&dentry->d_u.d_child);
-- 
1.8.3.1

[Patch 3.14 stable 05/16] dentry_kill(): don't try to remove from shrink list

[Cong Wang]

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

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>
(cherry picked from commit 41edf278fc2f042f4e22a12ed87d19c5201210e1)
---
 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 3f07e5a..8fcf964 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -467,7 +467,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)) {
@@ -478,9 +485,7 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 		}
 		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)
@@ -503,8 +508,6 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	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);
@@ -526,7 +529,15 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	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;
 }
 
@@ -828,7 +839,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 3b50cac..5e9b083 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.3.1

[Patch 3.14 stable 06/16] don't remove from shrink list in select_collect()

[Cong Wang]

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

	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>
(cherry picked from commit fe91522a7ba82ca1a51b07e19954b3825e4aaa22)
---
 fs/dcache.c | 31 ++++++++++---------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 8fcf964..1c11c5e 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1228,34 +1228,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.3.1

[Patch 3.14 stable 07/16] more graceful recovery in umount_collect()

[Cong Wang]

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

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>
(cherry picked from commit 9c8c10e262e0f62cb2530f1b076de979123183dd)
---
 fs/dcache.c | 101 +++++++++++++++---------------------------------------------
 1 file changed, 25 insertions(+), 76 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1c11c5e..752db63 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -394,22 +394,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
@@ -1274,45 +1258,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);
 }
 
 /*
@@ -1322,40 +1296,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.3.1

[Patch 3.14 stable 08/16] dcache: don't need rcu in shrink_dentry_list()

[Cong Wang]

From: Miklos Szeredi <mszeredi@suse.cz>

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>
(cherry picked from commit 60942f2f235ce7b817166cdf355eed729094834d)
---
 fs/dcache.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 752db63..d20bae3 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -795,23 +795,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
@@ -827,23 +813,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;
@@ -857,9 +840,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.3.1

[Patch 3.14 stable 09/16] lift the "already marked killed" case into shrink_dentry_list()

[Cong Wang]

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

It can happen only when dentry_kill() is called with unlock_on_failure
equal to 0 - other callers had dentry pinned until the moment they've
got ->d_lock and DCACHE_DENTRY_KILLED is set only after lockref_mark_dead().

IOW, only one of three call sites of dentry_kill() might end up reaching
that code.  Just move it there.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit 64fd72e0a44bdd62c5ca277cb24d0d02b2d8e9dc)
---
 fs/dcache.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index d20bae3..b2ad032 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -454,12 +454,6 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	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)) {
 relock:
@@ -814,6 +808,15 @@ static void shrink_dentry_list(struct list_head *list)
 			continue;
 		}
 
+
+		if (unlikely(dentry->d_flags & DCACHE_DENTRY_KILLED)) {
+			bool can_free = dentry->d_flags & DCACHE_MAY_FREE;
+			spin_unlock(&dentry->d_lock);
+			if (can_free)
+				dentry_free(dentry);
+			continue;
+		}
+
 		parent = dentry_kill(dentry, 0);
 		/*
 		 * If dentry_kill returns NULL, we have nothing more to do.
-- 
1.8.3.1

[Patch 3.14 stable 10/16] split dentry_kill()

[Cong Wang]

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

... into trylocks and everything else.  The latter (actual killing)
is __dentry_kill().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit e55fd011549eae01a230e3cace6f4d031b6a3453)
---
 fs/dcache.c | 62 +++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 36 insertions(+), 26 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index b2ad032..9a0ea05 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -440,36 +440,12 @@ void d_drop(struct dentry *dentry)
 }
 EXPORT_SYMBOL(d_drop);
 
-/*
- * Finish off a dentry we've decided to kill.
- * dentry->d_lock must be held, returns with it unlocked.
- * If ref is non-zero, then decrement the refcount too.
- * Returns dentry requiring refcount drop, or NULL if we're done.
- */
-static struct dentry *
-dentry_kill(struct dentry *dentry, int unlock_on_failure)
-	__releases(dentry->d_lock)
+static void __dentry_kill(struct dentry *dentry)
 {
-	struct inode *inode;
 	struct dentry *parent = NULL;
 	bool can_free = true;
-
-	inode = dentry->d_inode;
-	if (inode && !spin_trylock(&inode->i_lock)) {
-relock:
-		if (unlock_on_failure) {
-			spin_unlock(&dentry->d_lock);
-			cpu_relax();
-		}
-		return dentry; /* try again with same dentry */
-	}
 	if (!IS_ROOT(dentry))
 		parent = dentry->d_parent;
-	if (parent && !spin_trylock(&parent->d_lock)) {
-		if (inode)
-			spin_unlock(&inode->i_lock);
-		goto relock;
-	}
 
 	/*
 	 * The dentry is now unrecoverably dead to the world.
@@ -513,10 +489,44 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 		can_free = false;
 	}
 	spin_unlock(&dentry->d_lock);
-out:
 	if (likely(can_free))
 		dentry_free(dentry);
+}
+
+/*
+ * Finish off a dentry we've decided to kill.
+ * dentry->d_lock must be held, returns with it unlocked.
+ * If ref is non-zero, then decrement the refcount too.
+ * Returns dentry requiring refcount drop, or NULL if we're done.
+ */
+static struct dentry *
+dentry_kill(struct dentry *dentry, int unlock_on_failure)
+	__releases(dentry->d_lock)
+{
+	struct inode *inode = dentry->d_inode;
+	struct dentry *parent = NULL;
+
+	if (inode && unlikely(!spin_trylock(&inode->i_lock)))
+		goto failed;
+
+	if (!IS_ROOT(dentry)) {
+		parent = dentry->d_parent;
+		if (unlikely(!spin_trylock(&parent->d_lock))) {
+			if (inode)
+				spin_unlock(&inode->i_lock);
+			goto failed;
+		}
+	}
+
+	__dentry_kill(dentry);
 	return parent;
+
+failed:
+	if (unlock_on_failure) {
+		spin_unlock(&dentry->d_lock);
+		cpu_relax();
+	}
+	return dentry; /* try again with same dentry */
 }
 
 /* 
-- 
1.8.3.1

[Patch 3.14 stable 11/16] expand dentry_kill(dentry, 0) in shrink_dentry_list()

[Cong Wang]

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

Result will be massaged to saner shape in the next commits.  It is
ugly, no questions - the point of that one is to be a provably
equivalent transformation (and it might be worth splitting a bit
more).

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit ff2fde9929feb2aef45377ce56b8b12df85dda69)
---
 fs/dcache.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 9a0ea05..5cdd171 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -800,6 +800,7 @@ static void shrink_dentry_list(struct list_head *list)
 	struct dentry *dentry, *parent;
 
 	while (!list_empty(list)) {
+		struct inode *inode;
 		dentry = list_entry(list->prev, struct dentry, d_lru);
 		spin_lock(&dentry->d_lock);
 		/*
@@ -827,23 +828,26 @@ static void shrink_dentry_list(struct list_head *list)
 			continue;
 		}
 
-		parent = dentry_kill(dentry, 0);
-		/*
-		 * If dentry_kill returns NULL, we have nothing more to do.
-		 */
-		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.
-			 */
+		inode = dentry->d_inode;
+		if (inode && unlikely(!spin_trylock(&inode->i_lock))) {
 			d_shrink_add(dentry, list);
 			spin_unlock(&dentry->d_lock);
 			continue;
 		}
+
+		parent = NULL;
+		if (!IS_ROOT(dentry)) {
+			parent = dentry->d_parent;
+			if (unlikely(!spin_trylock(&parent->d_lock))) {
+				if (inode)
+					spin_unlock(&inode->i_lock);
+				d_shrink_add(dentry, list);
+				spin_unlock(&dentry->d_lock);
+				continue;
+			}
+		}
+
+		__dentry_kill(dentry);
 		/*
 		 * We need to prune ancestors too. This is necessary to prevent
 		 * quadratic behavior of shrink_dcache_parent(), but is also
-- 
1.8.3.1

[Patch 3.14 stable 12/16] shrink_dentry_list(): take parent's ->d_lock earlier

[Cong Wang]

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

The cause of livelocks there is that we are taking ->d_lock on
dentry and its parent in the wrong order, forcing us to use
trylock on the parent's one.  d_walk() takes them in the right
order, and unfortunately it's not hard to create a situation
when shrink_dentry_list() can't make progress since trylock
keeps failing, and shrink_dcache_parent() or check_submounts_and_drop()
keeps calling d_walk() disrupting the very shrink_dentry_list() it's
waiting for.

Solution is straightforward - if that trylock fails, let's unlock
the dentry itself and take locks in the right order.  We need to
stabilize ->d_parent without holding ->d_lock, but that's doable
using RCU.  And we'd better do that in the very beginning of the
loop in shrink_dentry_list(), since the checks on refcount, etc.
would need to be redone anyway.

That deals with a half of the problem - killing dentries on the
shrink list itself.  Another one (dropping their parents) is
in the next commit.

locking parent is interesting - it would be easy to do rcu_read_lock(),
lock whatever we think is a parent, lock dentry itself and check
if the parent is still the right one.  Except that we need to check
that *before* locking the dentry, or we are risking taking ->d_lock
out of order.  Fortunately, once the D1 is locked, we can check if
D2->d_parent is equal to D1 without the need to lock D2; D2->d_parent
can start or stop pointing to D1 only under D1->d_lock, so taking
D1->d_lock is enough.  In other words, the right solution is
rcu_read_lock/lock what looks like parent right now/check if it's
still our parent/rcu_read_unlock/lock the child.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit 046b961b45f93a92e4c70525a12f3d378bced130)
---
 fs/dcache.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 5cdd171..1c1b14c 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -529,6 +529,38 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	return dentry; /* try again with same dentry */
 }
 
+static inline struct dentry *lock_parent(struct dentry *dentry)
+{
+	struct dentry *parent = dentry->d_parent;
+	if (IS_ROOT(dentry))
+		return NULL;
+	if (likely(spin_trylock(&parent->d_lock)))
+		return parent;
+	spin_unlock(&dentry->d_lock);
+	rcu_read_lock();
+again:
+	parent = ACCESS_ONCE(dentry->d_parent);
+	spin_lock(&parent->d_lock);
+	/*
+	 * We can't blindly lock dentry until we are sure
+	 * that we won't violate the locking order.
+	 * Any changes of dentry->d_parent must have
+	 * been done with parent->d_lock held, so
+	 * spin_lock() above is enough of a barrier
+	 * for checking if it's still our child.
+	 */
+	if (unlikely(parent != dentry->d_parent)) {
+		spin_unlock(&parent->d_lock);
+		goto again;
+	}
+	rcu_read_unlock();
+	if (parent != dentry)
+		spin_lock(&dentry->d_lock);
+	else
+		parent = NULL;
+	return parent;
+}
+
 /* 
  * This is dput
  *
@@ -803,6 +835,8 @@ static void shrink_dentry_list(struct list_head *list)
 		struct inode *inode;
 		dentry = list_entry(list->prev, struct dentry, d_lru);
 		spin_lock(&dentry->d_lock);
+		parent = lock_parent(dentry);
+
 		/*
 		 * The dispose list is isolated and dentries are not accounted
 		 * to the LRU here, so we can simply remove it from the list
@@ -816,6 +850,8 @@ static void shrink_dentry_list(struct list_head *list)
 		 */
 		if ((int)dentry->d_lockref.count > 0) {
 			spin_unlock(&dentry->d_lock);
+			if (parent)
+				spin_unlock(&parent->d_lock);
 			continue;
 		}
 
@@ -823,6 +859,8 @@ static void shrink_dentry_list(struct list_head *list)
 		if (unlikely(dentry->d_flags & DCACHE_DENTRY_KILLED)) {
 			bool can_free = dentry->d_flags & DCACHE_MAY_FREE;
 			spin_unlock(&dentry->d_lock);
+			if (parent)
+				spin_unlock(&parent->d_lock);
 			if (can_free)
 				dentry_free(dentry);
 			continue;
@@ -832,22 +870,13 @@ static void shrink_dentry_list(struct list_head *list)
 		if (inode && unlikely(!spin_trylock(&inode->i_lock))) {
 			d_shrink_add(dentry, list);
 			spin_unlock(&dentry->d_lock);
+			if (parent)
+				spin_unlock(&parent->d_lock);
 			continue;
 		}
 
-		parent = NULL;
-		if (!IS_ROOT(dentry)) {
-			parent = dentry->d_parent;
-			if (unlikely(!spin_trylock(&parent->d_lock))) {
-				if (inode)
-					spin_unlock(&inode->i_lock);
-				d_shrink_add(dentry, list);
-				spin_unlock(&dentry->d_lock);
-				continue;
-			}
-		}
-
 		__dentry_kill(dentry);
+
 		/*
 		 * We need to prune ancestors too. This is necessary to prevent
 		 * quadratic behavior of shrink_dcache_parent(), but is also
-- 
1.8.3.1

[Patch 3.14 stable 13/16] dealing with the rest of shrink_dentry_list() livelock

[Cong Wang]

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

We have the same problem with ->d_lock order in the inner loop, where
we are dropping references to ancestors.  Same solution, basically -
instead of using dentry_kill() we use lock_parent() (introduced in the
previous commit) to get that lock in a safe way, recheck ->d_count
(in case if lock_parent() has ended up dropping and retaking ->d_lock
and somebody managed to grab a reference during that window), trylock
the inode->i_lock and use __dentry_kill() to do the rest.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit b2b80195d8829921506880f6dccd21cabd163d0d)
---
 fs/dcache.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1c1b14c..1b9f89f 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -884,8 +884,26 @@ static void shrink_dentry_list(struct list_head *list)
 		 * fragmentation.
 		 */
 		dentry = parent;
-		while (dentry && !lockref_put_or_lock(&dentry->d_lockref))
-			dentry = dentry_kill(dentry, 1);
+		while (dentry && !lockref_put_or_lock(&dentry->d_lockref)) {
+			parent = lock_parent(dentry);
+			if (dentry->d_lockref.count != 1) {
+				dentry->d_lockref.count--;
+				spin_unlock(&dentry->d_lock);
+				if (parent)
+					spin_unlock(&parent->d_lock);
+				break;
+			}
+			inode = dentry->d_inode;	/* can't be NULL */
+			if (unlikely(!spin_trylock(&inode->i_lock))) {
+				spin_unlock(&dentry->d_lock);
+				if (parent)
+					spin_unlock(&parent->d_lock);
+				cpu_relax();
+				continue;
+			}
+			__dentry_kill(dentry);
+			dentry = parent;
+		}
 	}
 }
 
-- 
1.8.3.1

[Patch 3.14 stable 14/16] dentry_kill() doesn't need the second argument now

[Cong Wang]

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

it's 1 in the only remaining caller.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit 8cbf74da435d1bd13dbb790f94c7ff67b2fb6af4)
---
 fs/dcache.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1b9f89f..bb72e82 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -499,8 +499,7 @@ static void __dentry_kill(struct dentry *dentry)
  * If ref is non-zero, then decrement the refcount too.
  * Returns dentry requiring refcount drop, or NULL if we're done.
  */
-static struct dentry *
-dentry_kill(struct dentry *dentry, int unlock_on_failure)
+static struct dentry *dentry_kill(struct dentry *dentry)
 	__releases(dentry->d_lock)
 {
 	struct inode *inode = dentry->d_inode;
@@ -522,10 +521,8 @@ dentry_kill(struct dentry *dentry, int unlock_on_failure)
 	return parent;
 
 failed:
-	if (unlock_on_failure) {
-		spin_unlock(&dentry->d_lock);
-		cpu_relax();
-	}
+	spin_unlock(&dentry->d_lock);
+	cpu_relax();
 	return dentry; /* try again with same dentry */
 }
 
@@ -614,7 +611,7 @@ void dput(struct dentry *dentry)
 	return;
 
 kill_it:
-	dentry = dentry_kill(dentry, 1);
+	dentry = dentry_kill(dentry);
 	if (dentry)
 		goto repeat;
 }
-- 
1.8.3.1

[Patch 3.14 stable 15/16] dcache: add missing lockdep annotation

[Cong Wang]

From: Linus Torvalds <torvalds@linux-foundation.org>

lock_parent() very much on purpose does nested locking of dentries, and
is careful to maintain the right order (lock parent first).  But because
it didn't annotate the nested locking order, lockdep thought it might be
a deadlock on d_lock, and complained.

Add the proper annotation for the inner locking of the child dentry to
make lockdep happy.

Introduced by commit 046b961b45f9 ("shrink_dentry_list(): take parent's
->d_lock earlier").

Reported-and-tested-by: Josh Boyer <jwboyer@fedoraproject.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(cherry picked from commit 9f12600fe425bc28f0ccba034a77783c09c15af4)
---
 fs/dcache.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index bb72e82..c0f89ea 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -552,7 +552,7 @@ static inline struct dentry *lock_parent(struct dentry *dentry)
 	}
 	rcu_read_unlock();
 	if (parent != dentry)
-		spin_lock(&dentry->d_lock);
+		spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
 	else
 		parent = NULL;
 	return parent;
-- 
1.8.3.1

[Patch 3.14 stable 16/16] lock_parent: don't step on stale ->d_parent of all-but-freed one

[Cong Wang]

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

Dentry that had been through (or into) __dentry_kill() might be seen
by shrink_dentry_list(); that's normal, it'll be taken off the shrink
list and freed if __dentry_kill() has already finished.  The problem
is, its ->d_parent might be pointing to already freed dentry, so
lock_parent() needs to be careful.

We need to check that dentry hasn't already gone into __dentry_kill()
*and* grab rcu_read_lock() before dropping ->d_lock - the latter makes
sure that whatever we see in ->d_parent after dropping ->d_lock it
won't be freed until we drop rcu_read_lock().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
(cherry picked from commit c2338f2dc7c1e9f6202f370c64ffd7f44f3d4b51)
---
 fs/dcache.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index c0f89ea..b4251db 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -531,10 +531,12 @@ static inline struct dentry *lock_parent(struct dentry *dentry)
 	struct dentry *parent = dentry->d_parent;
 	if (IS_ROOT(dentry))
 		return NULL;
+	if (unlikely((int)dentry->d_lockref.count < 0))
+		return NULL;
 	if (likely(spin_trylock(&parent->d_lock)))
 		return parent;
-	spin_unlock(&dentry->d_lock);
 	rcu_read_lock();
+	spin_unlock(&dentry->d_lock);
 again:
 	parent = ACCESS_ONCE(dentry->d_parent);
 	spin_lock(&parent->d_lock);
-- 
1.8.3.1

Re: [Patch 3.14 stable 00/16] vfs: fix dentry shrink list corruption

[Greg KH]

On Thu, Nov 06, 2014 at 11:37:04AM -0800, Cong Wang wrote:
> On at least 3 different machines, we have seen dentry shrink list corruption
> with 3.14 stable kernel (see the end of this email), which I think is
> fixed by a set of patches from Al.
> 
> This bug is probably introduced by the following commit in 3.12:
> 
> 	commit dd1f6b2e43a53ee58eb87d5e623cf44e277d005d
> 	Author: Dave Chinner <dchinner@redhat.com>
> 	Date:   Wed Aug 28 10:17:55 2013 +1000
> 
> 	    dcache: remove dentries from LRU before putting on dispose list
> 
> since previously we held the per sb lock. It is hard to reproduce this bug,
> because it is a race condition which happens when we update two contiguous
> dentries on a shrink list. Matt ran some ltp stress tests and doesn't see
> any problem with the patched kernel. And, 3.13 needs them too, but we have
> no time or resource to backport or test them on 3.13.
> 
> The first 7 patches are the original patchset from Al, the rest of patches
> are either needed for context or are folllow-up fixes, especially the
> last 2 patches. Of course all of them are merged in upstream, see each patch
> for details. Note, I don't get any conflict when backporting them to 3.14.
> 
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> Tested-by: Matthew Mullins <mmullins@twopensource.com>

I'm not ignoring these, just waiting for the other major 3.14-stable
patches that I've merged over the past few releases to settle down and
ensure they work properly.  It will probably be a few more weeks before
I can get to these, don't worry, they aren't lost.

thanks,

greg k-h

Re: [Patch 3.14 stable 00/16] vfs: fix dentry shrink list corruption

[Cong Wang]

On Tue, Feb 3, 2015 at 3:11 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> I'm not ignoring these, just waiting for the other major 3.14-stable
> patches that I've merged over the past few releases to settle down and
> ensure they work properly.  It will probably be a few more weeks before
> I can get to these, don't worry, they aren't lost.
>

Awesome! Thanks for taking care of them!

Re: [Patch 3.14 stable 01/16] fold d_kill() and d_free()

[Greg KH]

On Thu, Nov 06, 2014 at 11:37:05AM -0800, Cong Wang wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> (cherry picked from commit 03b3b889e79cdb6b806fc0ba9be0d71c186bbfaa)
> ---
>  fs/dcache.c | 76 +++++++++++++++++++------------------------------------------
>  1 file changed, 24 insertions(+), 52 deletions(-)

This patch doesn't apply to the 3.14-stable tree anymore.

I know it's been a while, but could you respin this series and resend
them if you think it's still needed for 3.14-stable?

thanks,

greg k-h

[PATCH 3.14] fold d_kill() and d_free()

[Vinson Lee]

From: Vinson Lee <vlee@twitter.com>

This is a backport of 03b3b889e79cdb6b806fc0ba9be0d71c186bbfaa "fold 
d_kill() and d_free()" to stable kernel 3.14.

The other 15 patches (patches 2-16) from the "vfs: fix dentry shrink 
list corruption" stable request still apply on top of 3.14.43.

Al Viro (1):
  fold d_kill() and d_free()

 fs/dcache.c | 77 +++++++++++++++++++------------------------------------------
 1 file changed, 24 insertions(+), 53 deletions(-)

-- 
2.1.0

[PATCH] fold d_kill() and d_free()

[Vinson Lee]

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

commit 03b3b889e79cdb6b806fc0ba9be0d71c186bbfaa upstream.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
[ vlee: Backported to 3.14. Adjusted context. ]
Signed-off-by: Vinson Lee <vlee@twitter.com>
---
 fs/dcache.c | 77 +++++++++++++++++++------------------------------------------
 1 file changed, 24 insertions(+), 53 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index a9231c8..8486607 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(&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;
 }
 
 /* 
-- 
2.1.0

Re: [PATCH 3.14] fold d_kill() and d_free()

[Greg KH]

On Wed, May 20, 2015 at 05:31:58PM -0700, Vinson Lee wrote:
> From: Vinson Lee <vlee@twitter.com>
> 
> This is a backport of 03b3b889e79cdb6b806fc0ba9be0d71c186bbfaa "fold 
> d_kill() and d_free()" to stable kernel 3.14.
> 
> The other 15 patches (patches 2-16) from the "vfs: fix dentry shrink 
> list corruption" stable request still apply on top of 3.14.43.

What stable request?

I don't see thos in my queue, what is this all supposed to be fixing?

Context please, I have none.

greg k-h