From c46c887744b330795eba55fdb96343c36d481765 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 26 Jul 2011 13:33:16 -0400 Subject: vfs: document locking requirements for d_move, __d_move and d_materialise_unique Adding a comment to d_materialise_unique per Al's request... d_move and __d_move have some pretty substantial locking requirements, but they are not clearly documented. Add some comments spelling them out. Also, document the requirement for the i_mutex of the parent in d_materialise_unique. Cc: Al Viro Signed-off-by: Jeff Layton Signed-off-by: Al Viro --- fs/dcache.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index be18598c7fd..b05aac3a8cf 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2138,8 +2138,9 @@ static void dentry_unlock_parents_for_move(struct dentry *dentry, * @target: new dentry * * Update the dcache to reflect the move of a file name. Negative - * dcache entries should not be moved in this way. Caller hold - * rename_lock. + * dcache entries should not be moved in this way. Caller must hold + * rename_lock, the i_mutex of the source and target directories, + * and the sb->s_vfs_rename_mutex if they differ. See lock_rename(). */ static void __d_move(struct dentry * dentry, struct dentry * target) { @@ -2202,7 +2203,8 @@ static void __d_move(struct dentry * dentry, struct dentry * target) * @target: new dentry * * Update the dcache to reflect the move of a file name. Negative - * dcache entries should not be moved in this way. + * dcache entries should not be moved in this way. See the locking + * requirements for __d_move. */ void d_move(struct dentry *dentry, struct dentry *target) { @@ -2320,7 +2322,8 @@ static void __d_materialise_dentry(struct dentry *dentry, struct dentry *anon) * @inode: inode to bind to the dentry, to which aliases may be attached * * Introduces an dentry into the tree, substituting an extant disconnected - * root directory alias in its place if there is one + * root directory alias in its place if there is one. Caller must hold the + * i_mutex of the parent directory. */ struct dentry *d_materialise_unique(struct dentry *dentry, struct inode *inode) { -- cgit v1.2.3-70-g09d2 From 35f40ef00204c456f5c181c0e7f54e25bb93cd49 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 7 Jun 2011 14:09:10 +0100 Subject: VFS: Remove detached-dentry counter from shrink_dcache_for_umount_subtree() Remove the detached-dentry counter from shrink_dcache_for_umount_subtree() as the value it computes is no longer used as of commit 312d3ca856d369bb04d0443846b85b4cdde6fa8a which made the nr_dentry counters summed per-CPU rather than global atomic. Signed-off-by: David Howells Signed-off-by: Al Viro --- fs/dcache.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index b05aac3a8cf..75590572ff7 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -828,7 +828,6 @@ EXPORT_SYMBOL(shrink_dcache_sb); static void shrink_dcache_for_umount_subtree(struct dentry *dentry) { struct dentry *parent; - unsigned detached = 0; BUG_ON(!IS_ROOT(dentry)); @@ -892,8 +891,6 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) spin_unlock(&parent->d_lock); } - detached++; - inode = dentry->d_inode; if (inode) { dentry->d_inode = NULL; -- cgit v1.2.3-70-g09d2 From c6627c60c07c43b51ef88e352627fa786d1e1592 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 7 Jun 2011 14:09:20 +0100 Subject: 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 Cc: Nick Piggin Signed-off-by: Al Viro --- fs/dcache.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index 75590572ff7..9df8b861e18 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -301,6 +301,27 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *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 * @dentry: dentry to drop @@ -319,17 +340,7 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent) void __d_drop(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_shrink(dentry); dentry_rcuwalk_barrier(dentry); } } @@ -832,10 +843,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); + __d_shrink(dentry); for (;;) { /* descend to the first leaf in the current subtree */ @@ -844,16 +853,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); + __d_shrink(loop); } - spin_unlock(&dentry->d_lock); /* move to the first child */ dentry = list_entry(dentry->d_subdirs.next, @@ -885,10 +889,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); } inode = dentry->d_inode; @@ -935,9 +937,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)) { -- cgit v1.2.3-70-g09d2 From 43c1c9cd244098012441b90c32304f11f1258d43 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 7 Jun 2011 14:09:30 +0100 Subject: VFS: Reorganise shrink_dcache_for_umount_subtree() after demise of dcache_lock Reorganise shrink_dcache_for_umount_subtree() in light of the demise of dcache_lock. Without that dcache_lock, there is no need for the batching of removal of dentries from the system under it (we wanted to make intensive use of the locked data whilst we held it, but didn't want to hold it for long at a time). This works, provided the preceding patch is correct in its removal of locking on dentry->d_lock on the basis that no one should be locking these dentries any more as the whole superblock is defunct. With this patch, the calls to dentry_lru_del() and __d_shrink() are placed at the point where each dentry is detached handled. It is possible that, as an alternative, the batching should still be done - but only for dentry_lru_del() of all a dentry's children in one go. In such a case, the batching would be done under dcache_lru_lock. Signed-off-by: David Howells Signed-off-by: Al Viro --- fs/dcache.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index 9df8b861e18..2347cdb15ab 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -842,33 +842,21 @@ static void shrink_dcache_for_umount_subtree(struct dentry *dentry) BUG_ON(!IS_ROOT(dentry)); - /* detach this root from the system */ - dentry_lru_del(dentry); - __d_shrink(dentry); - for (;;) { /* descend to the first leaf in the current subtree */ - while (!list_empty(&dentry->d_subdirs)) { - struct dentry *loop; - - /* this is a branch with children - detach all of them - * from the system in one go */ - list_for_each_entry(loop, &dentry->d_subdirs, - d_u.d_child) { - dentry_lru_del(loop); - __d_shrink(loop); - } - - /* move to the first child */ + while (!list_empty(&dentry->d_subdirs)) dentry = list_entry(dentry->d_subdirs.next, struct dentry, d_u.d_child); - } /* consume the dentries from this leaf up through its parents * until we find one with children or run out altogether */ do { struct inode *inode; + /* detach from the system */ + dentry_lru_del(dentry); + __d_shrink(dentry); + if (dentry->d_count != 0) { printk(KERN_ERR "BUG: Dentry %p{i=%lx,n=%s}" -- cgit v1.2.3-70-g09d2 From 2af14162656b81bea9e03e76d7c5f1787cc86ea6 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Wed, 3 Aug 2011 16:21:07 -0700 Subject: fs/dcache.c: fix new kernel-doc warning Fix new kernel-doc warning in fs/dcache.c: Warning(fs/dcache.c:797): No description found for parameter 'sb' Signed-off-by: Randy Dunlap Cc: Al Viro Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/dcache.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index 2347cdb15ab..c83cae19161 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -795,6 +795,7 @@ relock: /** * prune_dcache_sb - shrink the dcache + * @sb: superblock * @nr_to_scan: number of entries to try to free * * Attempt to shrink the superblock dcache LRU by @nr_to_scan entries. This is -- cgit v1.2.3-70-g09d2 From 830c0f0edca67403d361fe976a25b17356c11f19 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 6 Aug 2011 22:41:50 -0700 Subject: vfs: renumber DCACHE_xyz flags, remove some stale ones Gcc tends to generate better code with small integers, including the DCACHE_xyz flag tests - so move the common ones to be first in the list. Also just remove the unused DCACHE_INOTIFY_PARENT_WATCHED and DCACHE_AUTOFS_PENDING values, their users no longer exists in the source tree. And add a "unlikely()" to the DCACHE_OP_COMPARE test, since we want the common case to be a nice straight-line fall-through. Signed-off-by: Linus Torvalds --- fs/dcache.c | 2 +- include/linux/dcache.h | 30 +++++++++++++----------------- 2 files changed, 14 insertions(+), 18 deletions(-) (limited to 'fs/dcache.c') diff --git a/fs/dcache.c b/fs/dcache.c index c83cae19161..a88948b8bd1 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1729,7 +1729,7 @@ seqretry: */ if (read_seqcount_retry(&dentry->d_seq, *seq)) goto seqretry; - if (parent->d_flags & DCACHE_OP_COMPARE) { + if (unlikely(parent->d_flags & DCACHE_OP_COMPARE)) { if (parent->d_op->d_compare(parent, *inode, dentry, i, tlen, tname, name)) diff --git a/include/linux/dcache.h b/include/linux/dcache.h index d37d2a79309..62157c03caf 100644 --- a/include/linux/dcache.h +++ b/include/linux/dcache.h @@ -180,12 +180,12 @@ struct dentry_operations { */ /* d_flags entries */ -#define DCACHE_AUTOFS_PENDING 0x0001 /* autofs: "under construction" */ -#define DCACHE_NFSFS_RENAMED 0x0002 - /* this dentry has been "silly renamed" and has to be deleted on the last - * dput() */ +#define DCACHE_OP_HASH 0x0001 +#define DCACHE_OP_COMPARE 0x0002 +#define DCACHE_OP_REVALIDATE 0x0004 +#define DCACHE_OP_DELETE 0x0008 -#define DCACHE_DISCONNECTED 0x0004 +#define DCACHE_DISCONNECTED 0x0010 /* This dentry is possibly not currently connected to the dcache tree, in * which case its parent will either be itself, or will have this flag as * well. nfsd will not use a dentry with this bit set, but will first @@ -196,22 +196,18 @@ struct dentry_operations { * dentry into place and return that dentry rather than the passed one, * typically using d_splice_alias. */ -#define DCACHE_REFERENCED 0x0008 /* Recently used, don't discard. */ -#define DCACHE_RCUACCESS 0x0010 /* Entry has ever been RCU-visible */ -#define DCACHE_INOTIFY_PARENT_WATCHED 0x0020 - /* Parent inode is watched by inotify */ - -#define DCACHE_COOKIE 0x0040 /* For use by dcookie subsystem */ -#define DCACHE_FSNOTIFY_PARENT_WATCHED 0x0080 - /* Parent inode is watched by some fsnotify listener */ +#define DCACHE_REFERENCED 0x0020 /* Recently used, don't discard. */ +#define DCACHE_RCUACCESS 0x0040 /* Entry has ever been RCU-visible */ #define DCACHE_CANT_MOUNT 0x0100 #define DCACHE_GENOCIDE 0x0200 -#define DCACHE_OP_HASH 0x1000 -#define DCACHE_OP_COMPARE 0x2000 -#define DCACHE_OP_REVALIDATE 0x4000 -#define DCACHE_OP_DELETE 0x8000 +#define DCACHE_NFSFS_RENAMED 0x1000 + /* this dentry has been "silly renamed" and has to be deleted on the last + * dput() */ +#define DCACHE_COOKIE 0x2000 /* For use by dcookie subsystem */ +#define DCACHE_FSNOTIFY_PARENT_WATCHED 0x4000 + /* Parent inode is watched by some fsnotify listener */ #define DCACHE_MOUNTED 0x10000 /* is a mountpoint */ #define DCACHE_NEED_AUTOMOUNT 0x20000 /* handle automount on this dir */ -- cgit v1.2.3-70-g09d2