On 9/13/12 2:00 PM, 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 ;). Thanks,
The stupid emergency sysrq thing was my fault (at someone else's suggestion) ;)
It's caused a lot of woe, and hasn't worked for two years. Should we keep it?
-Eric
> 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
