[Unionfs] Unionfs2 cause of crash?

[Erez Zadok]

In message <46861944.4080901 at euskalnet.net>, =?UTF-8?B?Um9tYW4gTXXDsW96?= writes:
> Thank you very much. I think that to have the latest unionfs patch for 
> the latest stable kernel release will be helpfull.
> 
> The patch applies quite cleanly to 2.6.21.5-rt18:
> 
> patching file Documentation/filesystems/00-INDEX
[...]

> However, building the kernel now fails with:
> 
> fs/unionfs/inode.c: In function â€˜unionfs_permissionâ€™:
> fs/unionfs/inode.c:1044: error: â€˜struct rw_semaphoreâ€™ has no member 
> named â€˜wait_listâ€™
> fs/unionfs/inode.c:1101: error: â€˜struct rw_semaphoreâ€™ has no member 
> named â€˜wait_listâ€™
> 
> This is a vanilla 2.6.21.5 patched with 2.6.21.5-rt18, squashfs patch 
> for 2.6.20, unionfs 2.6.21.5-u1 and bootsplash-3.1.6-2.6.21
> 
> Best regards,
> Roman

Roman, I got fix for you.  Turns out that the rt18 patcheset, which is
*quite* invasive, changes the semantics of read-write semaphores.  We
changed our code to better work with rt18 as well as vanilla kernels.  You
can get the new patch here:

<ftp://ftp.filesystems.org/pub/unionfs/unionfs-2.x/linux-2.6.21.5-u2.diff.gz>

or, if you already have the -u1 patch applied, you can apply this small
patch against a 2.6.21.5 kernel with -u1 and it should apply and compile
fine.

Enjoy,
Erez.

##############################################################################

diff --git a/fs/unionfs/inode.c b/fs/unionfs/inode.c
index 20b234d..656c623 100644
--- a/fs/unionfs/inode.c
+++ b/fs/unionfs/inode.c
@@ -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;
 }