[PATCH review 2/7] dcache: Reduce the scope of i_lock in d_splice_alias

Eric W. Biederman ebiederm at xmission.com
Sat Aug 15 18:36:41 UTC 2015

i_lock is only needed until __d_find_any_alias calls dget on the alias
dentry.  After that the reference to new ensures that dentry_kill and
d_delete will not remove the inode from the dentry, and remove the
dentry from the inode->d_entry list.

The inode i_lock came to be held over the the __d_move calls in
d_splice_alias through a series of introduction of locks with
increasing smaller scope.  First it was the dcache_lock, then
it was the dcache_inode_lock, and finally inode->i_lock.

Furthermore inode->i_lock is not held over any other calls
to d_move or __d_move so it can not provide any meaningful
rename protection.

Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
 fs/dcache.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index f762e76e85cc..53b7f1e63beb 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2715,7 +2715,7 @@ struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2)
  * This helper attempts to cope with remotely renamed directories
  * It assumes that the caller is already holding
- * dentry->d_parent->d_inode->i_mutex, inode->i_lock and rename_lock
+ * dentry->d_parent->d_inode->i_mutex, and rename_lock
  * Note: If ever the locking in lock_rename() changes, then please
  * remember to update this too...
@@ -2741,7 +2741,6 @@ out_unalias:
 	__d_move(alias, dentry, false);
 	ret = 0;
-	spin_unlock(&inode->i_lock);
 	if (m2)
 	if (m1)
@@ -2787,10 +2786,11 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
 	if (S_ISDIR(inode->i_mode)) {
 		struct dentry *new = __d_find_any_alias(inode);
 		if (unlikely(new)) {
+			/* The reference to new ensures it remains an alias */
+			spin_unlock(&inode->i_lock);
 			if (unlikely(d_ancestor(new, dentry))) {
-				spin_unlock(&inode->i_lock);
 				new = ERR_PTR(-ELOOP);
@@ -2809,7 +2809,6 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
 			} else {
 				__d_move(new, dentry, false);
-				spin_unlock(&inode->i_lock);
 				security_d_instantiate(new, inode);

More information about the Containers mailing list