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>
Wed, 4 Jul 2007 04:47:34 +0000 (00:47 -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 20b234d0eba0e43f55b023b147a22b21b349ac81..656c6236eb1ea1a4b4117f7d73830df43c357d51 100644 (file)
@@ -1004,6 +1004,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)
 {
@@ -1013,24 +1021,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) {
@@ -1085,8 +1075,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;
 }