VFS: Remove dentry->d_lock locking from shrink_dcache_for_umount*()

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 <[email protected]>
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.

---

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

index 37f72ee..def734f 100644
@@ -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)) {