Re: [PATCH V4] Allow SysRq emergency thaw to thaw frozen filesystems

[Eric Sandeen <sandeen@redhat.com>]

Andrew Morton wrote:
> On Mon, 02 Feb 2009 16:55:18 -0600
> Eric Sandeen <sandeen@redhat.com> wrote:
> 
>> (this version fixes a couple small issues raised by Randy Dunlap)
>> (and adds sb_lock locking I forgot, as akpm pointed out)
>> (Randy's CONFIG_BLOCK fix should still apply over this)
> 
> <converts it to a delta so I can see what you did>

Sorry, didn't know if you want deltas on top of old patches or new
respun patches.  Guess now I know :)

> --- a/fs/buffer.c~allow-sysrq-emergency-thaw-to-thaw-frozen-filesystems-v4
> +++ a/fs/buffer.c
> @@ -263,11 +263,13 @@ void do_thaw_all(unsigned long unused)
>  	struct super_block *sb;
>  	char b[BDEVNAME_SIZE];
>  
> +	spin_lock(&sb_lock);
>  	list_for_each_entry(sb, &super_blocks, s_list) {
>  		while (sb->s_bdev && !thaw_bdev(sb->s_bdev, sb))
>  			printk(KERN_WARNING "Emergency Thaw on %s\n",
>  			       bdevname(sb->s_bdev, b));
>  	}
> +	spin_unlock(&sb_lock);
>  	printk(KERN_WARNING "Emergency Thaw complete\n");
>  }
>  
> _
> 
> 
> Can't call thaw_bdev() under spinlock.

Oh, sigh, I've come full circle to what I started with that didn't work,
didn't I.

> If we're going to do this, I think it will need the whole
> sb_lock/s_count/s_umount song-n-dance.
> 
> It's a pretty common operation.  What you want is, I think, identical
> to sync_supers(), only with one line changed.
> 
> so we could do
> 
> void apply_to_all_supers(void (fn)(struct super_block *))
> {
> 	struct super_block *sb;
> 
> 	spin_lock(&sb_lock);
> restart:
> 	list_for_each_entry(sb, &super_blocks, s_list) {
> 		if (sb->s_dirt) {
> 			sb->s_count++;
> 			spin_unlock(&sb_lock);
> 			down_read(&sb->s_umount);
> 			(*fn)(sb);
> 			up_read(&sb->s_umount);
> 			spin_lock(&sb_lock);
> 			if (__put_super_and_need_restart(sb))
> 				goto restart;
> 		}
> 	}
> 	spin_unlock(&sb_lock);
> 
> }
> 
> That isn't quite sufficient to use for get_super(), but I think it
> could be made so.
> 
> Ditto user_get_super().
> 
> Ditto do_emergency_remount()
> 
> But that's a separate little project for someone.  For the purposes of
> this patch I guess you could do yet another copy-n-paste.

Ok, thanks.  Sorry for dragging this out through 5 revisions :(

-Eric