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 ;). Thanks,
Josef
--
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
