GIT: unionfs2-2.6.27.y: Unionfs: prevent deadlock with branch-management code.

Erez Zadok ezk at fsl.cs.sunysb.edu
Thu Aug 12 23:16:43 EDT 2010


commit ce9be7e45499beca9d9b3a0d05c4888a46f91ff9
Author: Erez_Zadok <ezk at cs.sunysb.edu>
Date:   Tue Jul 3 09:14:39 2007 -0400

    Unionfs: prevent deadlock with branch-management code.
    
    Don't grab the superblock read-lock in unionfs_permission, which prevents a
    deadlock with the branch-management "add branch" code (which grabbed the
    write lock).  It is safe to not grab the read lock here, because even with
    branch management taking place, there is no chance that unionfs_permission,
    or anything it calls, will use stale branch information.
    
    Signed-off-by: Erez Zadok <ezk at cs.sunysb.edu>

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index b2c0214..401c62b 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -1017,6 +1017,14 @@ static int inode_permission(struct inode *inode, int mask,
 	return ((retval == -EROFS) ? 0 : retval);	/* ignore EROFS */
 }
 
+/*
+ * Don't grab the superblock read-lock in unionfs_permission, which prevents
+ * a deadlock with the branch-management "add branch" code (which grabbed
+ * the write lock).  It is safe to not grab the read lock here, because even
+ * with branch management taking place, there is no chance that
+ * unionfs_permission, or anything it calls, will use stale branch
+ * information.
+ */
 static int unionfs_permission(struct inode *inode, int mask,
 			      struct nameidata *nd)
 {
@@ -1026,24 +1034,6 @@ static int unionfs_permission(struct inode *inode, int mask,
 	const int is_file = !S_ISDIR(inode->i_mode);
 	const int write_mask = (mask & MAY_WRITE) && !(mask & MAY_READ);
 
-	/*
-	 * If the same process which is pivot_root'ed on a unionfs, tries to
-	 * insert a new branch, then the caller (remount code) already has
-	 * the write lock on this rwsem.  It then calls here to check the
-	 * permission of a new branch to add.  It could get into a self
-	 * deadlock with this attempt to get the read lock (which is crucial
-	 * for dynamic branch-management) unless no one else is waiting on
-	 * this lock.  Essentially this test tries to figure out if the same
-	 * process which also holds a write lock on the rwsem, also tries to
-	 * grab a read lock, and then skip trying to grab this "harmless"
-	 * read lock; otherwise we DO want to grab the read lock, and block
-	 * as needed (dynamic branch management).  (BTW, if there's a better
-	 * way to find out who is the lock owner compared to "current", that
-	 * should be used instead.)
-	 */
-	if (!list_empty(&UNIONFS_SB(inode->i_sb)->rwsem.wait_list))
-		unionfs_read_lock(inode->i_sb);
-
 	bstart = ibstart(inode);
 	bend = ibend(inode);
 	if (bstart < 0 || bend < 0) {
@@ -1098,8 +1088,6 @@ static int unionfs_permission(struct inode *inode, int mask,
 	unionfs_copy_attr_times(inode);
 
 out:
-	if (!list_empty(&UNIONFS_SB(inode->i_sb)->rwsem.wait_list))
-		unionfs_read_unlock(inode->i_sb);
 	unionfs_check_inode(inode);
 	return err;
 }


More information about the unionfs-cvs mailing list