Re: [PATCH 2/8] fsfreeze: emergency thaw will deadlock on s_umount

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2012/07/13 22:17, Jan Kara wrote:
On Thu 12-07-12 18:04:09, Fernando Luis Vázquez Cao wrote:
diff -urNp vfs-orig/fs/super.c vfs/fs/super.c
--- vfs-orig/fs/super.c	2012-07-04 18:57:54.000000000 +0900
+++ vfs/fs/super.c	2012-07-12 14:21:09.736566768 +0900
@@ -1221,23 +1221,25 @@ int freeze_super(struct super_block *sb)
  EXPORT_SYMBOL(freeze_super);
/**
- * thaw_super -- unlock filesystem
+ * __thaw_super -- unlock filesystem
   * @sb: the super to thaw
+ * @emergency:	emergency thaw
   *
   * Unlocks the filesystem and marks it writeable again after freeze_super().
+ * This is the unlocked version of thaw_super and has to be called with the
+ * sb->s_umount lock held in the non-emergency thaw case.
   */
-int thaw_super(struct super_block *sb)
+static int __thaw_super(struct super_block *sb, int emergency)
  {
-	int error;
+	int error = 0;
- down_write(&sb->s_umount);
  	if (sb->s_frozen == SB_UNFROZEN) {
-		up_write(&sb->s_umount);
-		return -EINVAL;
+		error = -EINVAL;
+		goto out;
  	}
if (sb->s_flags & MS_RDONLY)
-		goto out;
+		goto out_thaw;
if (sb->s_op->unfreeze_fs) {
  		error = sb->s_op->unfreeze_fs(sb);
@@ -1245,17 +1247,52 @@ int thaw_super(struct super_block *sb)
  			printk(KERN_ERR
  				"VFS:Filesystem thaw failed\n");
  			sb->s_frozen = SB_FREEZE_TRANS;
-			up_write(&sb->s_umount);
-			return error;
+			goto out;
  		}
  	}
-out:
+out_thaw:
  	sb->s_frozen = SB_UNFROZEN;
  	smp_wmb();
  	wake_up(&sb->s_wait_unfrozen);
-	deactivate_locked_super(sb);
- return 0;
+	/*
+	 * When called from emergency scope, we cannot grab the s_umount lock
+	 * so we cannot deactivate the superblock. This may leave unbalanced
+	 * superblock references which could prevent unmount, but given this is
+	 * an emergency operation....
+	 */
+	if (!emergency)
+		deactivate_locked_super(sb);
+out:
+	return error;
+}
   This is just wrong. deactivate_locked_super() will unlock the superblock
for you (and may even free it). So just do this in thaw_super() instead of
up_write(&sb->s_umount) which is bogus there. That will also allow you to
get rid of that ugly 'emergency' argument...

I just wanted to let you know that I rewrote this patch from scratch
taking a different approach which allows me to get locking right
in the emergency thaw case too. I will be sending the updated
patch set later today.

Thanks,
Fernando
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux