diff options
| author | David Howells <dhowells@redhat.com> | 2011-06-07 09:09:20 -0400 |
|---|---|---|
| committer | Al Viro <viro@zeniv.linux.org.uk> | 2011-08-01 02:27:57 -0400 |
| commit | c6627c60c07c43b51ef88e352627fa786d1e1592 (patch) | |
| tree | 2d0924acf0c6fa37d0fc7277a9f17940496b6461 | |
| parent | 35f40ef00204c456f5c181c0e7f54e25bb93cd49 (diff) | |
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:
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.
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: David Howells <dhowells@redhat.com>
Cc: Nick Piggin <npiggin@kernel.dk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
| -rw-r--r-- | fs/dcache.c | 48 |
1 files changed, 24 insertions, 24 deletions
diff --git a/fs/dcache.c b/fs/dcache.c index 75590572ff7a..9df8b861e18e 100644 --- a/fs/dcache.c +++ b/fs/dcache.c | |||
| @@ -301,6 +301,27 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent) | |||
| 301 | return parent; | 301 | return parent; |
| 302 | } | 302 | } |
| 303 | 303 | ||
| 304 | /* | ||
| 305 | * Unhash a dentry without inserting an RCU walk barrier or checking that | ||
| 306 | * dentry->d_lock is locked. The caller must take care of that, if | ||
| 307 | * appropriate. | ||
| 308 | */ | ||
| 309 | static void __d_shrink(struct dentry *dentry) | ||
| 310 | { | ||
| 311 | if (!d_unhashed(dentry)) { | ||
| 312 | struct hlist_bl_head *b; | ||
| 313 | if (unlikely(dentry->d_flags & DCACHE_DISCONNECTED)) | ||
| 314 | b = &dentry->d_sb->s_anon; | ||
| 315 | else | ||
| 316 | b = d_hash(dentry->d_parent, dentry->d_name.hash); | ||
| 317 | |||
| 318 | hlist_bl_lock(b); | ||
| 319 | __hlist_bl_del(&dentry->d_hash); | ||
| 320 | dentry->d_hash.pprev = NULL; | ||
| 321 | hlist_bl_unlock(b); | ||
| 322 | } | ||
| 323 | } | ||
| 324 | |||
| 304 | /** | 325 | /** |
| 305 | * d_drop - drop a dentry | 326 | * d_drop - drop a dentry |
| 306 | * @dentry: dentry to drop | 327 | * @dentry: dentry to drop |
| @@ -319,17 +340,7 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent) | |||
| 319 | void __d_drop(struct dentry *dentry) | 340 | void __d_drop(struct dentry *dentry) |
| 320 | { | 341 | { |
| 321 | if (!d_unhashed(dentry)) { | 342 | if (!d_unhashed(dentry)) { |
| 322 | struct hlist_bl_head *b; | 343 | __d_shrink(dentry); |
| 323 | if (unlikely(dentry->d_flags & DCACHE_DISCONNECTED)) | ||
| 324 | b = &dentry->d_sb->s_anon; | ||
| 325 | else | ||
| 326 | b = d_hash(dentry->d_parent, dentry->d_name.hash); | ||
| 327 | |||
| 328 | hlist_bl_lock(b); | ||
| 329 | __hlist_bl_del(&dentry->d_hash); | ||
| 330 | dentry->d_hash.pprev = NULL; | ||
| 331 | hlist_bl_unlock(b); | ||
| 332 | |||
| 333 | dentry_rcuwalk_barrier(dentry); | 344 | dentry_rcuwalk_barrier(dentry); |
| 334 | } | 345 | } |
| 335 | } | 346 | } |
| @@ -832,10 +843,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) | |||
| 832 | BUG_ON(!IS_ROOT(dentry)); | 843 | BUG_ON(!IS_ROOT(dentry)); |
| 833 | 844 | ||
| 834 | /* detach this root from the system */ | 845 | /* detach this root from the system */ |
| 835 | spin_lock(&dentry->d_lock); | ||
| 836 | dentry_lru_del(dentry); | 846 | dentry_lru_del(dentry); |
| 837 | __d_drop(dentry); | 847 | __d_shrink(dentry); |
| 838 | spin_unlock(&dentry->d_lock); | ||
| 839 | 848 | ||
| 840 | for (;;) { | 849 | for (;;) { |
| 841 | /* descend to the first leaf in the current subtree */ | 850 | /* descend to the first leaf in the current subtree */ |
| @@ -844,16 +853,11 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) | |||
| 844 | 853 | ||
| 845 | /* this is a branch with children - detach all of them | 854 | /* this is a branch with children - detach all of them |
| 846 | * from the system in one go */ | 855 | * from the system in one go */ |
| 847 | spin_lock(&dentry->d_lock); | ||
| 848 | list_for_each_entry(loop, &dentry->d_subdirs, | 856 | list_for_each_entry(loop, &dentry->d_subdirs, |
| 849 | d_u.d_child) { | 857 | d_u.d_child) { |
| 850 | spin_lock_nested(&loop->d_lock, | ||
| 851 | DENTRY_D_LOCK_NESTED); | ||
| 852 | dentry_lru_del(loop); | 858 | dentry_lru_del(loop); |
| 853 | __d_drop(loop); | 859 | __d_shrink(loop); |
| 854 | spin_unlock(&loop->d_lock); | ||
| 855 | } | 860 | } |
| 856 | spin_unlock(&dentry->d_lock); | ||
| 857 | 861 | ||
| 858 | /* move to the first child */ | 862 | /* move to the first child */ |
| 859 | dentry = list_entry(dentry->d_subdirs.next, | 863 | dentry = list_entry(dentry->d_subdirs.next, |
| @@ -885,10 +889,8 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) | |||
| 885 | list_del(&dentry->d_u.d_child); | 889 | list_del(&dentry->d_u.d_child); |
| 886 | } else { | 890 | } else { |
| 887 | parent = dentry->d_parent; | 891 | parent = dentry->d_parent; |
| 888 | spin_lock(&parent->d_lock); | ||
| 889 | parent->d_count--; | 892 | parent->d_count--; |
| 890 | list_del(&dentry->d_u.d_child); | 893 | list_del(&dentry->d_u.d_child); |
| 891 | spin_unlock(&parent->d_lock); | ||
| 892 | } | 894 | } |
| 893 | 895 | ||
| 894 | inode = dentry->d_inode; | 896 | inode = dentry->d_inode; |
| @@ -935,9 +937,7 @@ void shrink_dcache_for_umount(struct super_block *sb) | |||
| 935 | 937 | ||
| 936 | dentry = sb->s_root; | 938 | dentry = sb->s_root; |
| 937 | sb->s_root = NULL; | 939 | sb->s_root = NULL; |
| 938 | spin_lock(&dentry->d_lock); | ||
| 939 | dentry->d_count--; | 940 | dentry->d_count--; |
| 940 | spin_unlock(&dentry->d_lock); | ||
| 941 | shrink_dcache_for_umount_subtree(dentry); | 941 | shrink_dcache_for_umount_subtree(dentry); |
| 942 | 942 | ||
| 943 | while (!hlist_bl_empty(&sb->s_anon)) { | 943 | while (!hlist_bl_empty(&sb->s_anon)) { |
