Hi Josef,
On 2012/09/14 04:00, Josef Bacik wrote:
On Thu, Sep 13, 2012 at 05:08:19AM -0600, Fernando Luis Vázquez Cao wrote:
It makes no sense having the emergency thaw code in fs/buffer.c when all of
it's operations are one superblocks and the code it executes is all in
fs/super.c. Move the code there and clean it up.
Cc: Josef Bacik <jbacik@xxxxxxxxxxxx>
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Reviewed-by: Jan Kara <jack@xxxxxxx>
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Signed-off-by: Fernando Luis Vazquez Cao <fernando@xxxxxxxxxxxxx>
---
diff -urNp linux-3.6-rc5-orig/fs/buffer.c linux-3.6-rc5/fs/buffer.c
--- linux-3.6-rc5-orig/fs/buffer.c 2012-09-12 20:44:13.226112590 +0900
+++ linux-3.6-rc5/fs/buffer.c 2012-09-12 20:50:25.406058417 +0900
@@ -511,52 +511,6 @@ repeat:
return err;
}
-static int thaw_super_emergency(struct super_block *sb)
-{
- int res;
- /* We were called from __iterate_supers with superblock lock taken
- * so we do not need to do it here. */
- res = __thaw_super(sb);
- if (!res)
- deactivate_locked_super(sb);
- else
- up_write(&sb->s_umount);
- return res;
-}
-
-static void do_thaw_one(struct super_block *sb, void *unused)
-{
- if (sb->s_bdev) {
- char b[BDEVNAME_SIZE];
- printk(KERN_WARNING "Emergency Thaw on %s.\n",
- bdevname(sb->s_bdev, b));
- }
- while (!thaw_super_emergency(sb));
-}
-
-static void do_thaw_all(struct work_struct *work)
-{
- __iterate_supers(do_thaw_one, NULL, true);
- kfree(work);
- printk(KERN_WARNING "Emergency Thaw complete\n");
-}
-
-/**
- * emergency_thaw_all -- forcibly thaw every frozen filesystem
- *
- * Used for emergency unfreeze of all filesystems via SysRq
- */
-void emergency_thaw_all(void)
-{
- struct work_struct *work;
-
- work = kmalloc(sizeof(*work), GFP_ATOMIC);
- if (work) {
- INIT_WORK(work, do_thaw_all);
- schedule_work(work);
- }
-}
-
/**
* sync_mapping_buffers - write out & wait upon a mapping's "associated" buffers
* @mapping: the mapping which wants those buffers written
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-12 20:24:10.474041390 +0900
+++ linux-3.6-rc5/fs/super.c 2012-09-12 20:50:42.546044906 +0900
@@ -1475,3 +1475,49 @@ int thaw_super(struct super_block *sb)
return res;
}
EXPORT_SYMBOL(thaw_super);
+
+static int thaw_super_emergency(struct super_block *sb)
+{
+ int res;
+ /* We were called from __iterate_supers with superblock lock taken
+ * so we do not need to do it here. */
+ res = __thaw_super(sb);
+ if (!res)
+ deactivate_locked_super(sb);
+ else
+ up_write(&sb->s_umount);
+ return res;
+}
So unless I'm missing something this is wrong. We do __iterate_supers() which
does down_write(sb) and then call into this. Lets imagine a perfect world where
the sb was only frozen once. So we go into __thaw_super() and return 0 because
we were successfull and do deactivate_locked_super() which does
up_write(s_umount), and then we loop because we want to get an -EINVAL to know
we completely unfroze, so we call into __thaw_super(sb) without s_umount held
and then we get our error and do up_write(s_umount) _again_. So this needs to
be reworked to be correct ;).
You are right. I had fixed that locally, but it seems that I sent an
old version of patches 4 and 6.
The current version turns
while (!thaw_super_emergency(sb));
into
thaw_super_emergency(sb);
and sets sb->s_freeze_count to 1 before calling __thaw_super(),
which should address the problem you pointed out.
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
