thaw_bdev() has re-entrancy guards to allow freezes to nest
together. That is, it ensures that the filesystem is not thawed
until the last thaw command is issued. This is needed to prevent the
filesystem from being unfrozen while an existing freezer is still
operating on the filesystem in a frozen state (e.g. dm-snapshot).
Currently, freeze_super() and thaw_super() bypasses these guards,
and as a result manual freezing and unfreezing via the ioctl methods
do not nest correctly. hence mixing userspace directed freezes with
block device level freezes result in inconsistency due to premature
thawing of the filesystem.
Move the re-enterency guards to the superblock and into freeze_super
and thaw_super() so that userspace directed freezes nest correctly
again. Caveat: Things work as expected as long as direct calls to
thaw_super are always in response to a previous sb level freeze. In
other words an unpaired call to thaw_super can still thaw a
filesystem frozen using freeze_bdev (this issue could be addressed
in a follow-up patch if deemed necessary).
This patch retains the bdev level mutex and counter to keep the
"feature" that we can freeze a block device that does not have a
filesystem mounted yet.
IMPORTANT CAVEAT: This patch restores the nesting feature of the fsfreeze ioctl
which got removed by commit 18e9e5104fcd9a973ffe3eed3816c87f2a1b6cd2
("Introduce freeze_super and thaw_super for the fsfreeze ioctl"). It could be
argued that it is too late to fix the userland ABI breakage caused by that
patch and that the current ABI is the one that should be kept. If this is the
respective maintainer(s) opinion this could be modified to not allow fsfreeze
ioctl nesting.
Changes since version 2:
- Fix reference count leak in freeze_super when MS_BORN flag is not set in
the superblock.
- Protect freeze counter using s_umount and get rid of superblock level
fsfreeze mutex.
Cc: Josef Bacik <jbacik@xxxxxxxxxxxx>
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Cc: Jan Kara <jack@xxxxxxx>
Cc: Dave Chinner <dchinner@xxxxxxxxxx>
Signed-off-by: Fernando Luis Vazquez Cao <fernando@xxxxxxxxxxxxx>
---
diff -urNp linux-3.6-rc5-orig/fs/block_dev.c linux-3.6-rc5/fs/block_dev.c
--- linux-3.6-rc5-orig/fs/block_dev.c 2012-09-14 12:48:53.336884715 +0900
+++ linux-3.6-rc5/fs/block_dev.c 2012-09-14 13:51:36.457205813 +0900
@@ -257,16 +257,18 @@ int fsync_bdev(struct block_device *bdev
EXPORT_SYMBOL(fsync_bdev);
/**
- * freeze_bdev -- lock a filesystem and force it into a consistent state
+ * freeze_bdev -- lock a block device
* @bdev: blockdevice to lock
*
- * If a superblock is found on this device, we take the s_umount semaphore
- * on it to make sure nobody unmounts until the snapshot creation is done.
- * The reference counter (bd_fsfreeze_count) guarantees that only the last
- * unfreeze process can unfreeze the frozen filesystem actually when multiple
- * freeze requests arrive simultaneously. It counts up in freeze_bdev() and
- * count down in thaw_bdev(). When it becomes 0, thaw_bdev() will unfreeze
- * actually.
+ * Locks the block device and, if present, the associated filesystem too.
+ *
+ * The reference counter (bd_fsfreeze_count) is used to implement the feature
+ * that allows one to freeze a block device that does not have a filesystem
+ * mounted yet. For filesystems using mount_bdev the kernel takes care of
+ * things by preventing the mount operation from succeeding if the underlying
+ * block device is frozen. Other filesystems should check this counter or risk
+ * a situation where a freeze_bdev user (e.g. dm snapshot) and mount race,
+ * which may lead to inconsistencies.
*/
struct super_block *freeze_bdev(struct block_device *bdev)
{
@@ -274,17 +276,7 @@ struct super_block *freeze_bdev(struct b
int error = 0;
mutex_lock(&bdev->bd_fsfreeze_mutex);
- if (++bdev->bd_fsfreeze_count > 1) {
- /*
- * We don't even need to grab a reference - the first call
- * to freeze_bdev grab an active reference and only the last
- * thaw_bdev drops it.
- */
- sb = get_super(bdev);
- drop_super(sb);
- mutex_unlock(&bdev->bd_fsfreeze_mutex);
- return sb;
- }
+ bdev->bd_fsfreeze_count++;
sb = get_active_super(bdev);
if (!sb)
@@ -297,29 +289,32 @@ struct super_block *freeze_bdev(struct b
return ERR_PTR(error);
}
deactivate_super(sb);
- out:
+out:
sync_blockdev(bdev);
mutex_unlock(&bdev->bd_fsfreeze_mutex);
- return sb; /* thaw_bdev releases s->s_umount */
+ return sb;
}
EXPORT_SYMBOL(freeze_bdev);
/**
- * thaw_bdev -- unlock filesystem
+ * thaw_bdev -- unlock a block device
* @bdev: blockdevice to unlock
* @sb: associated superblock
*
- * Unlocks the filesystem and marks it writeable again after freeze_bdev().
+ * Unlocks the block device and, if present, the associated filesystem too.
*/
int thaw_bdev(struct block_device *bdev, struct super_block *sb)
{
int error = -EINVAL;
mutex_lock(&bdev->bd_fsfreeze_mutex);
+
if (!bdev->bd_fsfreeze_count)
goto out;
- if (--bdev->bd_fsfreeze_count > 0 || !sb) {
+ bdev->bd_fsfreeze_count--;
+
+ if (!sb) {
error = 0;
goto out;
}
diff -urNp linux-3.6-rc5-orig/fs/gfs2/ops_fstype.c linux-3.6-rc5/fs/gfs2/ops_fstype.c
--- linux-3.6-rc5-orig/fs/gfs2/ops_fstype.c 2012-09-14 12:46:15.152871285 +0900
+++ linux-3.6-rc5/fs/gfs2/ops_fstype.c 2012-09-14 13:51:36.457205813 +0900
@@ -1288,11 +1288,6 @@ static struct dentry *gfs2_mount(struct
if (IS_ERR(bdev))
return ERR_CAST(bdev);
- /*
- * once the super is inserted into the list by sget, s_umount
- * will protect the lockfs code from trying to start a snapshot
- * while we are mounting
- */
mutex_lock(&bdev->bd_fsfreeze_mutex);
if (bdev->bd_fsfreeze_count > 0) {
mutex_unlock(&bdev->bd_fsfreeze_mutex);
diff -urNp linux-3.6-rc5-orig/fs/nilfs2/super.c linux-3.6-rc5/fs/nilfs2/super.c
--- linux-3.6-rc5-orig/fs/nilfs2/super.c 2012-09-14 12:46:15.280871314 +0900
+++ linux-3.6-rc5/fs/nilfs2/super.c 2012-09-14 13:51:36.485205815 +0900
@@ -1273,11 +1273,6 @@ nilfs_mount(struct file_system_type *fs_
goto failed;
}
- /*
- * once the super is inserted into the list by sget, s_umount
- * will protect the lockfs code from trying to start a snapshot
- * while we are mounting
- */
mutex_lock(&sd.bdev->bd_fsfreeze_mutex);
if (sd.bdev->bd_fsfreeze_count > 0) {
mutex_unlock(&sd.bdev->bd_fsfreeze_mutex);
diff -urNp linux-3.6-rc5-orig/fs/super.c linux-3.6-rc5/fs/super.c
--- linux-3.6-rc5-orig/fs/super.c 2012-09-14 13:46:38.237179501 +0900
+++ linux-3.6-rc5/fs/super.c 2012-09-14 13:54:12.461207150 +0900
@@ -1041,11 +1041,6 @@ struct dentry *mount_bdev(struct file_sy
if (IS_ERR(bdev))
return ERR_CAST(bdev);
- /*
- * once the super is inserted into the list by sget, s_umount
- * will protect the lockfs code from trying to start a snapshot
- * while we are mounting
- */
mutex_lock(&bdev->bd_fsfreeze_mutex);
if (bdev->bd_fsfreeze_count > 0) {
mutex_unlock(&bdev->bd_fsfreeze_mutex);
@@ -1339,8 +1334,11 @@ static void sb_wait_write(struct super_b
* @sb: the super to lock
*
* Syncs the super to make sure the filesystem is consistent and calls the fs's
- * freeze_fs. Subsequent calls to this without first thawing the fs will return
- * -EBUSY.
+ * freeze_fs. The reference counter (s_freeze_count) guarantees that only the
+ * last unfreeze process can unfreeze the frozen filesystem actually when
+ * multiple freeze requests arrive simultaneously. It counts up in
+ * freeze_super() and counts down in thaw_super(). When it becomes 0,
+ * thaw_super() will execute the unfreeze.
*
* During this function, sb->s_writers.frozen goes through these values:
*
@@ -1365,29 +1363,27 @@ static void sb_wait_write(struct super_b
* freezing. Then we transition to SB_FREEZE_COMPLETE state. This state is
* mostly auxiliary for filesystems to verify they do not modify frozen fs.
*
- * sb->s_writers.frozen is protected by sb->s_umount.
+ * sb->s_writers.frozen and sb->s_freeze_count are protected by sb->s_umount.
*/
int freeze_super(struct super_block *sb)
{
- int ret;
+ int ret = 0;
atomic_inc(&sb->s_active);
down_write(&sb->s_umount);
- if (sb->s_writers.frozen != SB_UNFROZEN) {
- deactivate_locked_super(sb);
- return -EBUSY;
- }
if (!(sb->s_flags & MS_BORN)) {
- up_write(&sb->s_umount);
- return 0; /* sic - it's "nothing to do" */
+ atomic_dec(&sb->s_active);
+ goto out_active; /* sic - it's "nothing to do" */
}
+ if (++sb->s_freeze_count > 1)
+ goto out_deactivate;
+
if (sb->s_flags & MS_RDONLY) {
/* Nothing to do really... */
sb->s_writers.frozen = SB_FREEZE_COMPLETE;
- up_write(&sb->s_umount);
- return 0;
+ goto out_active;
}
/* From now on, no new normal writers can start */
@@ -1419,11 +1415,11 @@ int freeze_super(struct super_block *sb)
if (ret) {
printk(KERN_ERR
"VFS:Filesystem freeze failed\n");
+ sb->s_freeze_count--;
sb->s_writers.frozen = SB_UNFROZEN;
smp_wmb();
wake_up(&sb->s_writers.wait_unfrozen);
- deactivate_locked_super(sb);
- return ret;
+ goto out_deactivate;
}
}
/*
@@ -1431,8 +1427,12 @@ int freeze_super(struct super_block *sb)
* sees write activity when frozen is set to SB_FREEZE_COMPLETE.
*/
sb->s_writers.frozen = SB_FREEZE_COMPLETE;
+out_active:
up_write(&sb->s_umount);
- return 0;
+ return ret;
+out_deactivate:
+ deactivate_locked_super(sb);
+ return ret;
}
EXPORT_SYMBOL(freeze_super);
@@ -1442,6 +1442,10 @@ EXPORT_SYMBOL(freeze_super);
*
* Unlocks the filesystem and marks it writeable again.
*
+ * Returns -EINVAL if @sb is not frozen, 0 if the filesystem specific unfreeze
+ * function was executed and succeeded or the corresponding error code
+ * otherwise. if the unfreeze fails, @sb is left in the frozen state.
+ *
* This is the unlocked version of thaw_super, so it is the caller's job to
* to protect the superblock by grabbing the s_umount semaphore in write mode
* and release it again on return. See thaw_super() for an example.
@@ -1450,11 +1454,14 @@ static int __thaw_super(struct super_blo
{
int error = 0;
- if (sb->s_writers.frozen == SB_UNFROZEN) {
+ if (!sb->s_freeze_count) {
error = -EINVAL;
goto out;
}
+ if (--sb->s_freeze_count > 0)
+ goto out;
+
if (sb->s_flags & MS_RDONLY)
goto out_thaw;
@@ -1463,6 +1470,7 @@ static int __thaw_super(struct super_blo
if (error) {
printk(KERN_ERR
"VFS:Filesystem thaw failed\n");
+ sb->s_freeze_count++;
goto out;
}
}
@@ -1506,6 +1514,7 @@ static void do_thaw_one(struct super_blo
/* We were called from __iterate_supers with superblock lock taken
* so we do not need to do it here. */
+ sb->s_freeze_count = 1; /* avoid multiple calls to __thaw_super */
res = __thaw_super(sb);
if (!res)
deactivate_locked_super(sb);
diff -urNp linux-3.6-rc5-orig/include/linux/fs.h linux-3.6-rc5/include/linux/fs.h
--- linux-3.6-rc5-orig/include/linux/fs.h 2012-09-14 13:46:38.325179510 +0900
+++ linux-3.6-rc5/include/linux/fs.h 2012-09-14 13:51:36.485205815 +0900
@@ -1578,6 +1578,8 @@ struct super_block {
/* Being remounted read-only */
int s_readonly_remount;
+
+ int s_freeze_count; /* nr of nested freezes */
};
/* superblock cache pruning functions */
--
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
