Commit c6627c60 authored by David Howells's avatar David Howells Committed by Al Viro

VFS: Remove dentry->d_lock locking from shrink_dcache_for_umount_subtree()

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

	23044507
	2fd6b7f5

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.

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

Note that to make this work, __d_drop() has to be split so that the RCUwalk
barrier can be avoided.  This causes problems otherwise as it has an assertion
that dentry->d_lock is locked - but there is no need for that as no one else
can be trying to access this dentry, except to step over it (and that should
be handled by d_free(), I think).
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Cc: Nick Piggin <npiggin@kernel.dk>
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 35f40ef0
...@@ -301,6 +301,27 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent) ...@@ -301,6 +301,27 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
return parent; return parent;
} }
/*
* Unhash a dentry without inserting an RCU walk barrier or checking that
* dentry->d_lock is locked. The caller must take care of that, if
* appropriate.
*/
static void __d_shrink(struct dentry *dentry)
{
if (!d_unhashed(dentry)) {
struct hlist_bl_head *b;
if (unlikely(dentry->d_flags & DCACHE_DISCONNECTED))
b = &dentry->d_sb->s_anon;
else
b = d_hash(dentry->d_parent, dentry->d_name.hash);
hlist_bl_lock(b);
__hlist_bl_del(&dentry->d_hash);
dentry->d_hash.pprev = NULL;
hlist_bl_unlock(b);
}
}
/** /**
* d_drop - drop a dentry * d_drop - drop a dentry
* @dentry: dentry to drop * @dentry: dentry to drop
...@@ -319,17 +340,7 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent) ...@@ -319,17 +340,7 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
void __d_drop(struct dentry *dentry) void __d_drop(struct dentry *dentry)
{ {
if (!d_unhashed(dentry)) { if (!d_unhashed(dentry)) {
struct hlist_bl_head *b; __d_shrink(dentry);
if (unlikely(dentry->d_flags & DCACHE_DISCONNECTED))
b = &dentry->d_sb->s_anon;
else
b = d_hash(dentry->d_parent, dentry->d_name.hash);
hlist_bl_lock(b);
__hlist_bl_del(&dentry->d_hash);
dentry->d_hash.pprev = NULL;
hlist_bl_unlock(b);
dentry_rcuwalk_barrier(dentry); dentry_rcuwalk_barrier(dentry);
} }
} }
...@@ -832,10 +843,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) ...@@ -832,10 +843,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
BUG_ON(!IS_ROOT(dentry)); BUG_ON(!IS_ROOT(dentry));
/* detach this root from the system */ /* detach this root from the system */
spin_lock(&dentry->d_lock);
dentry_lru_del(dentry); dentry_lru_del(dentry);
__d_drop(dentry); __d_shrink(dentry);
spin_unlock(&dentry->d_lock);
for (;;) { for (;;) {
/* descend to the first leaf in the current subtree */ /* descend to the first leaf in the current subtree */
...@@ -844,16 +853,11 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) ...@@ -844,16 +853,11 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
/* this is a branch with children - detach all of them /* this is a branch with children - detach all of them
* from the system in one go */ * from the system in one go */
spin_lock(&dentry->d_lock);
list_for_each_entry(loop, &dentry->d_subdirs, list_for_each_entry(loop, &dentry->d_subdirs,
d_u.d_child) { d_u.d_child) {
spin_lock_nested(&loop->d_lock,
DENTRY_D_LOCK_NESTED);
dentry_lru_del(loop); dentry_lru_del(loop);
__d_drop(loop); __d_shrink(loop);
spin_unlock(&loop->d_lock);
} }
spin_unlock(&dentry->d_lock);
/* move to the first child */ /* move to the first child */
dentry = list_entry(dentry->d_subdirs.next, dentry = list_entry(dentry->d_subdirs.next,
...@@ -885,10 +889,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) ...@@ -885,10 +889,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
list_del(&dentry->d_u.d_child); list_del(&dentry->d_u.d_child);
} else { } else {
parent = dentry->d_parent; parent = dentry->d_parent;
spin_lock(&parent->d_lock);
parent->d_count--; parent->d_count--;
list_del(&dentry->d_u.d_child); list_del(&dentry->d_u.d_child);
spin_unlock(&parent->d_lock);
} }
inode = dentry->d_inode; inode = dentry->d_inode;
...@@ -935,9 +937,7 @@ void shrink_dcache_for_umount(struct super_block *sb) ...@@ -935,9 +937,7 @@ void shrink_dcache_for_umount(struct super_block *sb)
dentry = sb->s_root; dentry = sb->s_root;
sb->s_root = NULL; sb->s_root = NULL;
spin_lock(&dentry->d_lock);
dentry->d_count--; dentry->d_count--;
spin_unlock(&dentry->d_lock);
shrink_dcache_for_umount_subtree(dentry); shrink_dcache_for_umount_subtree(dentry);
while (!hlist_bl_empty(&sb->s_anon)) { while (!hlist_bl_empty(&sb->s_anon)) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment