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

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

[Bodo Eggert]

Eric Sandeen <sandeen@sandeen.net> wrote:
> Valdis.Kletnieks@vt.edu wrote:
>> On Thu, 15 Jan 2009 21:59:10 CST, Eric Sandeen said:

>>> Oh, actually, I'd think not.  If the freeze was done properly by the
>>> filesystem, all data was flushed, the fs was quiesced, and new IO was
>>> blocked.  pdflush should never be visiting these...
>> 
>> Yes, but a lot of 'if's - and usually you're reaching for sysrq-S precisely
>> *because* you suspect that stuff wasn't happening properly on its own...
> 
> Actually, only one if - if the fs implemented freeze properly.
> 
> Well, the use case I envision here is something like:
> 
> # freeze /my/mount/point/to/fs/to/snapshot
> 
> except oops, that wasn't mounted, and you just froze your root fs.

Maybe freeze should protect against that by requiring to specify the exact
mountpount, unless you say freeze --subdir?

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

[Eric Sandeen]

Bodo Eggert wrote:
> Eric Sandeen <sandeen@sandeen.net> wrote:
>> Valdis.Kletnieks@vt.edu wrote:
>>> On Thu, 15 Jan 2009 21:59:10 CST, Eric Sandeen said:
> 
>>>> Oh, actually, I'd think not.  If the freeze was done properly by the
>>>> filesystem, all data was flushed, the fs was quiesced, and new IO was
>>>> blocked.  pdflush should never be visiting these...
>>> Yes, but a lot of 'if's - and usually you're reaching for sysrq-S precisely
>>> *because* you suspect that stuff wasn't happening properly on its own...
>> Actually, only one if - if the fs implemented freeze properly.
>>
>> Well, the use case I envision here is something like:
>>
>> # freeze /my/mount/point/to/fs/to/snapshot
>>
>> except oops, that wasn't mounted, and you just froze your root fs.
> 
> Maybe freeze should protect against that by requiring to specify the exact
> mountpount, unless you say freeze --subdir?

That's a good idea.  My "freeze" above was a hypothetical tool which
doesn't really exist yet, but should get that enhancement.  :)
(xfs_freeze does not do this checking today, it probably should)

-Eric

[PATCH] Allow SysRq emergency sync to thaw frozen filesystems

[Eric Sandeen]

Now that the filesystem freeze operation has been elevated
to the VFS, and is just an ioctl away, some sort of safety net
for unintentionally frozen root filesystems may be in order.

The timeout thaw originally proposed did not get merged, but
perhaps something like this would be useful in emergencies.

This doesn't have to piggyback on the existing emergency sync
sysrq, but it seems like a reasonable, simple addition to me.

I've tested this on a non-root fs with multiple (nested) freezers,
as well as on a system rendered unresponsive due to a frozen
root fs.

Thanks,
-Eric

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Index: linux-2.6/drivers/char/sysrq.c
===================================================================
--- linux-2.6.orig/drivers/char/sysrq.c
+++ linux-2.6/drivers/char/sysrq.c
@@ -151,6 +151,7 @@ static struct sysrq_key_op sysrq_reboot_
 
 static void sysrq_handle_sync(int key, struct tty_struct *tty)
 {
+	emergency_thaw();
 	emergency_sync();
 }
 static struct sysrq_key_op sysrq_sync_op = {
Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -258,6 +258,29 @@ struct super_block *freeze_bdev(struct b
 }
 EXPORT_SYMBOL(freeze_bdev);
 
+void do_thaw(unsigned long unused)
+{
+	struct super_block *sb;
+	char b[BDEVNAME_SIZE];
+
+	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));
+	}
+	printk(KERN_WARNING "Emergency Thaw complete\n");
+}
+
+/**
+ * emergency_thaw -- force thaw every filesystem
+ *
+ * Used for emergency unfreeze of all filesystems via SysRq
+ */
+void emergency_thaw(void)
+{
+	pdflush_operation(do_thaw, 0);
+}
+
 /**
  * thaw_bdev  -- unlock filesystem
  * @bdev:	blockdevice to unlock
Index: linux-2.6/include/linux/buffer_head.h
===================================================================
--- linux-2.6.orig/include/linux/buffer_head.h
+++ linux-2.6/include/linux/buffer_head.h
@@ -171,6 +171,7 @@ void __wait_on_buffer(struct buffer_head
 wait_queue_head_t *bh_waitq_head(struct buffer_head *bh);
 int fsync_bdev(struct block_device *);
 struct super_block *freeze_bdev(struct block_device *);
+void emergency_thaw(void);
 int thaw_bdev(struct block_device *, struct super_block *);
 int fsync_super(struct super_block *);
 int fsync_no_super(struct block_device *);
Index: linux-2.6/Documentation/sysrq.txt
===================================================================
--- linux-2.6.orig/Documentation/sysrq.txt
+++ linux-2.6/Documentation/sysrq.txt
@@ -101,7 +101,8 @@ On all -  write a character to /proc/sys
 
 'r'     - Turns off keyboard raw mode and sets it to XLATE.
 
-'s'     - Will attempt to sync all mounted filesystems.
+'s'     - Will attempt to sync all mounted filesystems, and unfreeze
+	  any frozen fileystems.
 
 't'     - Will dump a list of current tasks and their information to your
           console.

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

[Andrew Morton]

On Wed, 14 Jan 2009 22:06:17 -0600
Eric Sandeen <sandeen@redhat.com> wrote:

> Now that the filesystem freeze operation has been elevated
> to the VFS, and is just an ioctl away, some sort of safety net
> for unintentionally frozen root filesystems may be in order.
> 
> The timeout thaw originally proposed did not get merged, but
> perhaps something like this would be useful in emergencies.
> 
> This doesn't have to piggyback on the existing emergency sync
> sysrq, but it seems like a reasonable, simple addition to me.
> 
> I've tested this on a non-root fs with multiple (nested) freezers,
> as well as on a system rendered unresponsive due to a frozen
> root fs.

Worried.

Under what operational scenarios is ths feature actually needed/used?

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Index: linux-2.6/drivers/char/sysrq.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/sysrq.c
> +++ linux-2.6/drivers/char/sysrq.c
> @@ -151,6 +151,7 @@ static struct sysrq_key_op sysrq_reboot_
>  
>  static void sysrq_handle_sync(int key, struct tty_struct *tty)
>  {
> +	emergency_thaw();
>  	emergency_sync();
>  }

Kind of weird.   The thaw will happen after/during the sync().

I guess that if the sync is blocked on a frozen fs then things will
sort themselves out.

otoh, if all the pdflush threads are blocked on frozen filesystems
(possible?) then the emergency_thaw() simply won't do anything.

> ===================================================================
> --- linux-2.6.orig/fs/buffer.c
> +++ linux-2.6/fs/buffer.c
> @@ -258,6 +258,29 @@ struct super_block *freeze_bdev(struct b
>  }
>  EXPORT_SYMBOL(freeze_bdev);
>  
> +void do_thaw(unsigned long unused)
> +{
> +	struct super_block *sb;
> +	char b[BDEVNAME_SIZE];
> +
> +	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));

hm, I made the args to bdevname() backwards.  Bad me.

> +	}
> +	printk(KERN_WARNING "Emergency Thaw complete\n");
> +}

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

[Eric Sandeen]

Andrew Morton wrote:
> On Wed, 14 Jan 2009 22:06:17 -0600
> Eric Sandeen <sandeen@redhat.com> wrote:
> 
>> Now that the filesystem freeze operation has been elevated
>> to the VFS, and is just an ioctl away, some sort of safety net
>> for unintentionally frozen root filesystems may be in order.
>>
>> The timeout thaw originally proposed did not get merged, but
>> perhaps something like this would be useful in emergencies.
>>
>> This doesn't have to piggyback on the existing emergency sync
>> sysrq, but it seems like a reasonable, simple addition to me.
>>
>> I've tested this on a non-root fs with multiple (nested) freezers,
>> as well as on a system rendered unresponsive due to a frozen
>> root fs.
> 
> Worried.
> 
> Under what operational scenarios is ths feature actually needed/used?

Well, if you freeze root and do some things that require IO there, you
can get stuck pretty easily

(hacked xfs_io to call the ioctl here)

[root@inode io]# ./xfs_io -r -x -F -c "freeze" /
[root@inode io]# ls
^Z
^C
<tap tap.. uhoh>

>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> Index: linux-2.6/drivers/char/sysrq.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/char/sysrq.c
>> +++ linux-2.6/drivers/char/sysrq.c
>> @@ -151,6 +151,7 @@ static struct sysrq_key_op sysrq_reboot_
>>  
>>  static void sysrq_handle_sync(int key, struct tty_struct *tty)
>>  {
>> +	emergency_thaw();
>>  	emergency_sync();
>>  }
> 
> Kind of weird.   The thaw will happen after/during the sync().

oh, hrm.  Maybe I didn't think enough about how it's handed off to
pdflush; I could rearrange if that makes sense?  Or maybe handing to
pdflush is wrong, it was just so convenient....

> I guess that if the sync is blocked on a frozen fs then things will
> sort themselves out.
> 
> otoh, if all the pdflush threads are blocked on frozen filesystems
> (possible?) then the emergency_thaw() simply won't do anything.

Hm, maybe possible... I'll have to think about that.

Thanks,
-Eric

>> ===================================================================
>> --- linux-2.6.orig/fs/buffer.c
>> +++ linux-2.6/fs/buffer.c
>> @@ -258,6 +258,29 @@ struct super_block *freeze_bdev(struct b
>>  }
>>  EXPORT_SYMBOL(freeze_bdev);
>>  
>> +void do_thaw(unsigned long unused)
>> +{
>> +	struct super_block *sb;
>> +	char b[BDEVNAME_SIZE];
>> +
>> +	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));
> 
> hm, I made the args to bdevname() backwards.  Bad me.
> 
>> +	}
>> +	printk(KERN_WARNING "Emergency Thaw complete\n");
>> +}
> 

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

[Eric Sandeen]

Eric Sandeen wrote:
> Andrew Morton wrote:
>> On Wed, 14 Jan 2009 22:06:17 -0600
>> Eric Sandeen <sandeen@redhat.com> wrote:

...

>>> Index: linux-2.6/drivers/char/sysrq.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/char/sysrq.c
>>> +++ linux-2.6/drivers/char/sysrq.c
>>> @@ -151,6 +151,7 @@ static struct sysrq_key_op sysrq_reboot_
>>>  
>>>  static void sysrq_handle_sync(int key, struct tty_struct *tty)
>>>  {
>>> +	emergency_thaw();
>>>  	emergency_sync();
>>>  }
>> Kind of weird.   The thaw will happen after/during the sync().
> 
> oh, hrm.  Maybe I didn't think enough about how it's handed off to
> pdflush; I could rearrange if that makes sense?  Or maybe handing to
> pdflush is wrong, it was just so convenient....
> 
>> I guess that if the sync is blocked on a frozen fs then things will
>> sort themselves out.
>>
>> otoh, if all the pdflush threads are blocked on frozen filesystems
>> (possible?) then the emergency_thaw() simply won't do anything.
> 
> Hm, maybe possible... I'll have to think about that.

Oh, actually, I'd think not.  If the freeze was done properly by the
filesystem, all data was flushed, the fs was quiesced, and new IO was
blocked.  pdflush should never be visiting these...

In fact emergency sync is kind of orthogonal to emergency thaw, anything
which needs a thaw should never actually need a sync.

-Eric

> Thanks,
> -Eric

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

On Thu, 15 Jan 2009 21:59:10 CST, Eric Sandeen said:

> Oh, actually, I'd think not.  If the freeze was done properly by the
> filesystem, all data was flushed, the fs was quiesced, and new IO was
> blocked.  pdflush should never be visiting these...

Yes, but a lot of 'if's - and usually you're reaching for sysrq-S precisely
*because* you suspect that stuff wasn't happening properly on its own...

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Eric Sandeen]

Valdis.Kletnieks@vt.edu wrote:
> On Thu, 15 Jan 2009 21:59:10 CST, Eric Sandeen said:
> 
>> Oh, actually, I'd think not.  If the freeze was done properly by the
>> filesystem, all data was flushed, the fs was quiesced, and new IO was
>> blocked.  pdflush should never be visiting these...
> 
> Yes, but a lot of 'if's - and usually you're reaching for sysrq-S precisely
> *because* you suspect that stuff wasn't happening properly on its own...

Actually, only one if - if the fs implemented freeze properly.

Well, the use case I envision here is something like:

# freeze /my/mount/point/to/fs/to/snapshot

except oops, that wasn't mounted, and you just froze your root fs.

I was thinking more recovery from admin error, not programming error...

If we're using sysrq to work around any possible programming error, then
we have a pretty tough job to make sure that it always works, no?

-Eric

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

[Dave Kleikamp]

On Thu, 2009-01-15 at 21:49 -0600, Eric Sandeen wrote:
> Andrew Morton wrote:
> > On Wed, 14 Jan 2009 22:06:17 -0600
> > Eric Sandeen <sandeen@redhat.com> wrote:

> >> Index: linux-2.6/drivers/char/sysrq.c
> >> ===================================================================
> >> --- linux-2.6.orig/drivers/char/sysrq.c
> >> +++ linux-2.6/drivers/char/sysrq.c
> >> @@ -151,6 +151,7 @@ static struct sysrq_key_op sysrq_reboot_
> >>  
> >>  static void sysrq_handle_sync(int key, struct tty_struct *tty)
> >>  {
> >> +	emergency_thaw();
> >>  	emergency_sync();
> >>  }
> > 
> > Kind of weird.   The thaw will happen after/during the sync().
> 
> oh, hrm.  Maybe I didn't think enough about how it's handed off to
> pdflush; I could rearrange if that makes sense?  Or maybe handing to
> pdflush is wrong, it was just so convenient....

I don't understand the reason for handing it to pdflush.  I would expect
thaw to be very fast.  Do we expect it to block anywhere, and if so, for
very long?  Not that I see any problem with it.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

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

[Dave Kleikamp]

On Fri, 2009-01-16 at 10:21 -0600, Dave Kleikamp wrote:
> On Thu, 2009-01-15 at 21:49 -0600, Eric Sandeen wrote:

> > oh, hrm.  Maybe I didn't think enough about how it's handed off to
> > pdflush; I could rearrange if that makes sense?  Or maybe handing to
> > pdflush is wrong, it was just so convenient....
> 
> I don't understand the reason for handing it to pdflush.  I would expect
> thaw to be very fast.  Do we expect it to block anywhere, and if so, for
> very long?  Not that I see any problem with it.

Eric pointed out on irc that this is in interrupt context, so even
taking a mutex is not allowed.  I now agree that handing it to pdflush
is reasonable.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

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

[Pavel Machek]

> Now that the filesystem freeze operation has been elevated
> to the VFS, and is just an ioctl away, some sort of safety net
> for unintentionally frozen root filesystems may be in order.
> 
> The timeout thaw originally proposed did not get merged, but
> perhaps something like this would be useful in emergencies.
> 
> This doesn't have to piggyback on the existing emergency sync
> sysrq, but it seems like a reasonable, simple addition to me.
> 
> I've tested this on a non-root fs with multiple (nested) freezers,
> as well as on a system rendered unresponsive due to a frozen
> root fs.

Emergency Sync should not do this. Invent another key.

...because otherwise, if you hit emergency sync but the system is
still alive and relies on filesystem freezing, bad stuff will happen.

								Pavel
> ---
> 
> Index: linux-2.6/drivers/char/sysrq.c
> ===================================================================
> --- linux-2.6.orig/drivers/char/sysrq.c
> +++ linux-2.6/drivers/char/sysrq.c
> @@ -151,6 +151,7 @@ static struct sysrq_key_op sysrq_reboot_
>  
>  static void sysrq_handle_sync(int key, struct tty_struct *tty)
>  {
> +	emergency_thaw();
>  	emergency_sync();
>  }
>  static struct sysrq_key_op sysrq_sync_op = {
> Index: linux-2.6/fs/buffer.c
> ===================================================================
> --- linux-2.6.orig/fs/buffer.c
> +++ linux-2.6/fs/buffer.c
> @@ -258,6 +258,29 @@ struct super_block *freeze_bdev(struct b
>  }
>  EXPORT_SYMBOL(freeze_bdev);
>  
> +void do_thaw(unsigned long unused)
> +{
> +	struct super_block *sb;
> +	char b[BDEVNAME_SIZE];
> +
> +	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));
> +	}
> +	printk(KERN_WARNING "Emergency Thaw complete\n");
> +}
> +
> +/**
> + * emergency_thaw -- force thaw every filesystem
> + *
> + * Used for emergency unfreeze of all filesystems via SysRq
> + */
> +void emergency_thaw(void)
> +{
> +	pdflush_operation(do_thaw, 0);
> +}
> +
>  /**
>   * thaw_bdev  -- unlock filesystem
>   * @bdev:	blockdevice to unlock
> Index: linux-2.6/include/linux/buffer_head.h
> ===================================================================
> --- linux-2.6.orig/include/linux/buffer_head.h
> +++ linux-2.6/include/linux/buffer_head.h
> @@ -171,6 +171,7 @@ void __wait_on_buffer(struct buffer_head
>  wait_queue_head_t *bh_waitq_head(struct buffer_head *bh);
>  int fsync_bdev(struct block_device *);
>  struct super_block *freeze_bdev(struct block_device *);
> +void emergency_thaw(void);
>  int thaw_bdev(struct block_device *, struct super_block *);
>  int fsync_super(struct super_block *);
>  int fsync_no_super(struct block_device *);
> Index: linux-2.6/Documentation/sysrq.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/sysrq.txt
> +++ linux-2.6/Documentation/sysrq.txt
> @@ -101,7 +101,8 @@ On all -  write a character to /proc/sys
>  
>  'r'     - Turns off keyboard raw mode and sets it to XLATE.
>  
> -'s'     - Will attempt to sync all mounted filesystems.
> +'s'     - Will attempt to sync all mounted filesystems, and unfreeze
> +	  any frozen fileystems.
>  
>  't'     - Will dump a list of current tasks and their information to your
>            console.
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 629 bytes --]

On Fri, 16 Jan 2009 09:48:28 +0100, Pavel Machek said:

> Emergency Sync should not do this. Invent another key.
> 
> ...because otherwise, if you hit emergency sync but the system is
> still alive and relies on filesystem freezing, bad stuff will happen.

Under what conditions would a system be alive and relying on freezing,
*and* an emergency thaw would be worse than whatever reason you're doing
an emergency sync?

Hmm.. guess you *could* get into trouble if you tried to do a Sysrq-[not-s]
and hit the wrong key - but you have the same danger if you have *any*
sysrq- invoking an emergency_thaw and hit it by accident...


[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Eric Sandeen]

Valdis.Kletnieks@vt.edu wrote:
> On Fri, 16 Jan 2009 09:48:28 +0100, Pavel Machek said:
> 
>> Emergency Sync should not do this. Invent another key.
>>
>> ...because otherwise, if you hit emergency sync but the system is
>> still alive and relies on filesystem freezing, bad stuff will happen.
> 
> Under what conditions would a system be alive and relying on freezing,
> *and* an emergency thaw would be worse than whatever reason you're doing
> an emergency sync?
> 
> Hmm.. guess you *could* get into trouble if you tried to do a Sysrq-[not-s]
> and hit the wrong key - but you have the same danger if you have *any*
> sysrq- invoking an emergency_thaw and hit it by accident...

I could certainly use up another key ('z' is available for unfreeZe) but
I have the same question; under what conditions do you expect to need an
emergency sync and also need to maintain frozen filesystems as frozen?

>From a maximum flexibility and control perspective, it'd be better to
have them separated I suppose.  Is it worth using up another available key?

-Eric

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

[Pavel Machek]

On Fri 2009-01-16 10:17:09, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 16 Jan 2009 09:48:28 +0100, Pavel Machek said:
> 
> > Emergency Sync should not do this. Invent another key.
> > 
> > ...because otherwise, if you hit emergency sync but the system is
> > still alive and relies on filesystem freezing, bad stuff will happen.
> 
> Under what conditions would a system be alive and relying on freezing,
> *and* an emergency thaw would be worse than whatever reason you're doing
> an emergency sync?

I sometimes hit emergency sync on perfectly healthy system... like
before "insmod shiny-new-guaranteed-to-work-module"... I believe
sysrq-s should not have sideffects.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

[Theodore Tso]

On Fri, Jan 16, 2009 at 10:17:09AM -0500, Valdis.Kletnieks@vt.edu wrote:
> On Fri, 16 Jan 2009 09:48:28 +0100, Pavel Machek said:
> 
> > Emergency Sync should not do this. Invent another key.
> > 
> > ...because otherwise, if you hit emergency sync but the system is
> > still alive and relies on filesystem freezing, bad stuff will happen.
> 
> Under what conditions would a system be alive and relying on freezing,
> *and* an emergency thaw would be worse than whatever reason you're doing
> an emergency sync?
> 
> Hmm.. guess you *could* get into trouble if you tried to do a Sysrq-[not-s]
> and hit the wrong key - but you have the same danger if you have *any*
> sysrq- invoking an emergency_thaw and hit it by accident...

My biggest complaint is that the two operations are largely
orthogonal.  Emergency sync and unfreeze are two very different
operations, and while emergency sync is largely harmless, it just
seems really unclean to combine the two.  For one thing, it'll be
extremely non-obvious that emergency sync implies unfreeze, and
changing the sysrq help to say emergency-Sync-and-unfreeze just
screams "kludge"....

						- Ted

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

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 745 bytes --]

On Fri, 16 Jan 2009 10:40:26 EST, Theodore Tso said:

> My biggest complaint is that the two operations are largely
> orthogonal.  Emergency sync and unfreeze are two very different
> operations, and while emergency sync is largely harmless, it just
> seems really unclean to combine the two.  For one thing, it'll be
> extremely non-obvious that emergency sync implies unfreeze, and
> changing the sysrq help to say emergency-Sync-and-unfreeze just
> screams "kludge"....

Fair enough - as it is, I usually end up doing sysrq-s, -s, -u, -s, -b
anyhow because each sysrq- does one little thing, adding another little
thing on some other control-meta-alt-cokebottle key *is* probably the
right choice, we still have a few keyscan codes left.. ;)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

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

[Eric Sandeen]

Theodore Tso wrote:
> On Fri, Jan 16, 2009 at 10:17:09AM -0500, Valdis.Kletnieks@vt.edu wrote:
>> On Fri, 16 Jan 2009 09:48:28 +0100, Pavel Machek said:
>>
>>> Emergency Sync should not do this. Invent another key.
>>>
>>> ...because otherwise, if you hit emergency sync but the system is
>>> still alive and relies on filesystem freezing, bad stuff will happen.
>> Under what conditions would a system be alive and relying on freezing,
>> *and* an emergency thaw would be worse than whatever reason you're doing
>> an emergency sync?
>>
>> Hmm.. guess you *could* get into trouble if you tried to do a Sysrq-[not-s]
>> and hit the wrong key - but you have the same danger if you have *any*
>> sysrq- invoking an emergency_thaw and hit it by accident...
> 
> My biggest complaint is that the two operations are largely
> orthogonal.  Emergency sync and unfreeze are two very different
> operations, and while emergency sync is largely harmless, it just
> seems really unclean to combine the two.  For one thing, it'll be
> extremely non-obvious that emergency sync implies unfreeze, and
> changing the sysrq help to say emergency-Sync-and-unfreeze just
> screams "kludge"....
> 
> 						- Ted

Yeah, they really are orthogonal, it's true.  Ok, if people are willing
to give up 'z' I'll move it there.

-Eric