[RFC][PATCH] VFS: Remove dentry->d_lock locking from shrink_dcache_for_umount*()

[David Howells <dhowells@redhat.com>]

Here's a potential patch to remove all the locks of dentry->d_lock from
shrink_dcache_for_umount*().  Getting these locks ought to be unnecessary as
these functions are only called during superblock destruction and, as such,
should never see d_lock locked - I think.

However, this patch causes the assertion:

		dentry_rcuwalk_barrier(dentry);
		--> assert_spin_locked(&dentry->d_lock);

in __d_drop() to trigger.

In this particular instance (sb destruction) is this assertion actually
necessary, or can I just strip out all the locking from these functions and
rely on RCU freeing to prevent transit through the hash bucket chains from
going wrong?  I think that it shouldn't be a problem, but I presume Nick
must've had his reasons for not getting rid of the locking himself.

David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] VFS: Remove dentry->d_lock locking from shrink_dcache_for_umount*()

Locks of the dcache_lock were replaced by locks of dentry->d_lock in commits
such as:

	2304450783dfde7b0b94ae234edd0dbffa865073
	2fd6b7f50797f2e993eea59e0a0b8c6399c811dc

as part of the RCU-based pathwalk changes, despite the fact that the caller
(shrink_dcache_for_umount()) notes in the banner comment the reasons that
d_lock is not necessary in these functions:

/*
 * destroy the dentries attached to a superblock on unmounting
 * - we don't need to use dentry->d_lock because:
 *   - the superblock is detached from all mountings and open files, so the
 *     dentry trees will not be rearranged by the VFS
 *   - s_umount is write-locked, so the memory pressure shrinker will ignore
 *     any dentries belonging to this superblock that it comes across
 *   - the filesystem itself is no longer permitted to rearrange the dentries
 *     in this superblock
 */

So remove these locks.  If the locks are actually necessary, then this banner
comment should be altered instead.

Note that the functions should be renamed - they operate during superblock
destruction rather than unmounting specifically.

The hash table chains are protected by 1-bit locks in the hash table heads, so
those shouldn't be a problem.

Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Nick Piggin <npiggin@kernel.dk>
---

 fs/dcache.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)


diff --git a/fs/dcache.c b/fs/dcache.c
index 37f72ee..def734f 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -878,10 +878,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
 	BUG_ON(!IS_ROOT(dentry));
 
 	/* detach this root from the system */
-	spin_lock(&dentry->d_lock);
 	dentry_lru_del(dentry);
 	__d_drop(dentry);
-	spin_unlock(&dentry->d_lock);
 
 	for (;;) {
 		/* descend to the first leaf in the current subtree */
@@ -890,16 +888,11 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
 
 			/* this is a branch with children - detach all of them
 			 * from the system in one go */
-			spin_lock(&dentry->d_lock);
 			list_for_each_entry(loop, &dentry->d_subdirs,
 					    d_u.d_child) {
-				spin_lock_nested(&loop->d_lock,
-						DENTRY_D_LOCK_NESTED);
 				dentry_lru_del(loop);
 				__d_drop(loop);
-				spin_unlock(&loop->d_lock);
 			}
-			spin_unlock(&dentry->d_lock);
 
 			/* move to the first child */
 			dentry = list_entry(dentry->d_subdirs.next,
@@ -931,10 +924,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
 				list_del(&dentry->d_u.d_child);
 			} else {
 				parent = dentry->d_parent;
-				spin_lock(&parent->d_lock);
 				parent->d_count--;
 				list_del(&dentry->d_u.d_child);
-				spin_unlock(&parent->d_lock);
 			}
 
 			detached++;
@@ -983,9 +974,7 @@ void shrink_dcache_for_umount(struct super_block *sb)
 
 	dentry = sb->s_root;
 	sb->s_root = NULL;
-	spin_lock(&dentry->d_lock);
 	dentry->d_count--;
-	spin_unlock(&dentry->d_lock);
 	shrink_dcache_for_umount_subtree(dentry);
 
 	while (!hlist_bl_empty(&sb->s_anon)) {