Unionfs: prevent deadlock with branch-management code.
authorErez_Zadok <ezk@cs.sunysb.edu>
Tue, 3 Jul 2007 13:14:39 +0000 (09:14 -0400)
committerErez Zadok <ezk@cs.sunysb.edu>
Fri, 29 Apr 2011 02:24:13 +0000 (22:24 -0400)
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@cs.sunysb.edu>
fs/unionfs/inode.c

index b2c021445e03c4a3a26492837beadb8ad5441319..401c62ba7a5e7709751aaf3a581de59a6cf14f24 100644 (file)
@@ -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;
 }