[patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Pavel Machek]

On Wed 2006-07-12 00:21:31, Roman Zippel wrote:
> Hi,
> 
> On Tue, 11 Jul 2006, Pavel Machek wrote:
> 
> > > Apparently it changes existing well documented behaviour, which is a 
> > > really bad idea.
> > 
> > _well documented_? Where was it documented? Anyway, 2.6.17 behaviour
> > does not work on _many_ keyboards, like for example thinkpad x32...
> 
> Documentation/sysrq.txt and this was working on _many_ more keyboards just 
> fine.
> The fact is this patch changes existing behaviour, it either needs to be
> fixed or reverted. Adding new features is one thing, breaking existing 
> features is not acceptable without a very good reason.

Your "well documented" is sentence "you may have better luck
with"... okay, but we now have better sentence. Document it better.

BTW I believe that original way (alt down, sysrq down, b down) still
works before and after the patch.

Here's patch that updates docs with now-working trick.

Signed-off-by: Pavel Machek <pavel@suse.cz> 

diff --git a/Documentation/sysrq.txt b/Documentation/sysrq.txt
index e0188a2..58e04c0 100644
--- a/Documentation/sysrq.txt
+++ b/Documentation/sysrq.txt
@@ -43,7 +43,7 @@ On x86   - You press the key combo 'ALT-
            keyboards may not have a key labeled 'SysRq'. The 'SysRq' key is
            also known as the 'Print Screen' key. Also some keyboards cannot
 	   handle so many keys being pressed at the same time, so you might
-	   have better luck with "press Alt", "press SysRq", "release Alt",
+	   have better luck with "press Alt", "press SysRq", "release SysRq",
 	   "press <command key>", release everything.
 
 On SPARC - You press 'ALT-STOP-<command key>', I believe.


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

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Roman Zippel]

Hi,

On Wed, 12 Jul 2006, Pavel Machek wrote:

> BTW I believe that original way (alt down, sysrq down, b down) still
> works before and after the patch.

No, it doesn't.

> Here's patch that updates docs with now-working trick.

NACK.

bye, Roman

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Andrew Morton]

Roman Zippel <zippel@linux-m68k.org> wrote:
>
> Hi,
> 
> On Wed, 12 Jul 2006, Pavel Machek wrote:
> 
> > BTW I believe that original way (alt down, sysrq down, b down) still
> > works before and after the patch.
> 
> No, it doesn't.
> 
> > Here's patch that updates docs with now-working trick.
> 
> NACK.
> 

Nack your nack!  The patch in 2.6.18-rc1 makes sysrq work on machines on
which it *did not work at all*.  If that makes it harder to type but still
possible to type on other machines, well, we win.

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Roman Zippel]

Hi,

On Tue, 11 Jul 2006, Andrew Morton wrote:

> > On Wed, 12 Jul 2006, Pavel Machek wrote:
> > 
> > > BTW I believe that original way (alt down, sysrq down, b down) still
> > > works before and after the patch.
> > 
> > No, it doesn't.
> > 
> > > Here's patch that updates docs with now-working trick.
> > 
> > NACK.
> > 
> 
> Nack your nack!  The patch in 2.6.18-rc1 makes sysrq work on machines on
> which it *did not work at all*.  If that makes it harder to type but still
> possible to type on other machines, well, we win.

Why can't we even _try_ to preserve compatibility? :-(

bye, Roman

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Andrew Morton]

Roman Zippel <zippel@linux-m68k.org> wrote:
>
> Hi,
> 
> On Tue, 11 Jul 2006, Andrew Morton wrote:
> 
> > > On Wed, 12 Jul 2006, Pavel Machek wrote:
> > > 
> > > > BTW I believe that original way (alt down, sysrq down, b down) still
> > > > works before and after the patch.
> > > 
> > > No, it doesn't.
> > > 
> > > > Here's patch that updates docs with now-working trick.
> > > 
> > > NACK.
> > > 
> > 
> > Nack your nack!  The patch in 2.6.18-rc1 makes sysrq work on machines on
> > which it *did not work at all*.  If that makes it harder to type but still
> > possible to type on other machines, well, we win.
> 
> Why can't we even _try_ to preserve compatibility? :-(
> 

It would of course be good if we could do that.  If it's impossible to
retain the old behaviour without breaking those oddball keyboards then
we're screwed.

IOW, someone needs to find a way to make the new code work like the old
code without re-breaking Pavel's keyboard.  But the bitchin-to-patchin
ratio here seems to exclude that outcome.

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Roman Zippel]

Hi,

On Tue, 11 Jul 2006, Andrew Morton wrote:

> > > > > BTW I believe that original way (alt down, sysrq down, b down) still
> > > > > works before and after the patch.
> > > > 
> > > > No, it doesn't.
> > > > 
> > > > > Here's patch that updates docs with now-working trick.
> > > > 
> > > > NACK.
> > > > 
> > > 
> > > Nack your nack!  The patch in 2.6.18-rc1 makes sysrq work on machines on
> > > which it *did not work at all*.  If that makes it harder to type but still
> > > possible to type on other machines, well, we win.
> > 
> > Why can't we even _try_ to preserve compatibility? :-(
> > 
> 
> It would of course be good if we could do that.  If it's impossible to
> retain the old behaviour without breaking those oddball keyboards then
> we're screwed.
> 
> IOW, someone needs to find a way to make the new code work like the old
> code without re-breaking Pavel's keyboard.  But the bitchin-to-patchin
> ratio here seems to exclude that outcome.

Traditionally that responsibility is in the hands of whose who break it in 
the first place, so this patch needs to be either reverted or fixed 
(preferably before 2.18).

bye, Roman

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Andrew Morton]

On Wed, 12 Jul 2006 02:52:24 +0200 (CEST)
Roman Zippel <zippel@linux-m68k.org> wrote:

> > IOW, someone needs to find a way to make the new code work like the old
> > code without re-breaking Pavel's keyboard.  But the bitchin-to-patchin
> > ratio here seems to exclude that outcome.
> 
> Traditionally that responsibility is in the hands of whose who break it in 
> the first place

If that person cannot reproduce the problem but another skilled kernel
developer can then it would make sense for he-who-can-reproduce-it to do
some work.

Still, I doubt if that's the case here.


Is the below correct?

Old behaviour:

	a) press alt
	b) press sysrq
	c) release alt
	d) press T
	e) release T
	f) release sysrq

New behaviour:

	a) press alt
	b) press sysrq
	c) release sysrq
	d) press T
	e) release T
	f) release alt

If so, then the old behaviour was weird and the new behaviour is sensible. 
What, actually, is the problem?

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Fredrik Roubert]

On Wed 12 Jul 02:16 CEST 2006, Roman Zippel wrote:

> > Nack your nack!  The patch in 2.6.18-rc1 makes sysrq work on machines on
> > which it *did not work at all*.  If that makes it harder to type but still
> > possible to type on other machines, well, we win.
>
> Why can't we even _try_ to preserve compatibility? :-(

First, please note that the documented behaviour (You press the key
combo 'ALT-SysRq-<command key>'.) still works.

The problem at hand is that not all keyboards can handle this many keys
pressed at once, and that there also are keyboards broken in other ways.

The work-around suggested in the documentation ([Y]ou might have better
luck with "press Alt", "press SysRq", "release Alt", "press <command
key>", release everything.) does not work with keyboards that sends the
make and break codes for SysRq immediately after another, and this was
the reason for changing the behaviour (for broken keyboards) in
2.6.18-rc1. The new behaviour works with every keyboard the people
involved in this discussion has heard of.

That the documentation wasn't updated with the new work-around key
combination for broken keyboards was a mistake.

Cheers // Fredrik Roubert

-- 
Visserij 192  |  +32 473 344527 / +46 708 776974
BE-9000 Gent  |  http://www.df.lth.se/~roubert/

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Pavel Machek]

Hi!

> > > IOW, someone needs to find a way to make the new code work like the old
> > > code without re-breaking Pavel's keyboard.  But the bitchin-to-patchin
> > > ratio here seems to exclude that outcome.
> > 
> > Traditionally that responsibility is in the hands of whose who break it in 
> > the first place
> 
> If that person cannot reproduce the problem but another skilled kernel
> developer can then it would make sense for he-who-can-reproduce-it to do
> some work.
> 
> Still, I doubt if that's the case here.
> 
> 
> Is the below correct?
> 
> Old behaviour:
> 
> 	a) press alt
> 	b) press sysrq
> 	c) release alt
> 	d) press T
> 	e) release T
> 	f) release sysrq
> 
> New behaviour:
> 
> 	a) press alt
> 	b) press sysrq
> 	c) release sysrq
> 	d) press T
> 	e) release T
> 	f) release alt

Plus there was "very old" behaviour:

a) press alt
b) press sysrq
c) press T
d) release T
e) release sysrq
f) release alt

...that worked along with "old" behaviour....

> If so, then the old behaviour was weird and the new behaviour is sensible. 
> What, actually, is the problem?

I'd agree. ...and was _real_ weird.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Roman Zippel]

Hi,

On Tue, 11 Jul 2006, Andrew Morton wrote:

> > Traditionally that responsibility is in the hands of whose who break it in 
> > the first place
> 
> If that person cannot reproduce the problem but another skilled kernel
> developer can then it would make sense for he-who-can-reproduce-it to do
> some work.
> 
> Still, I doubt if that's the case here.
> 
> 
> Is the below correct?
> 
> Old behaviour:
> 
> 	a) press alt
> 	b) press sysrq
> 	c) release alt
> 	d) press T
> 	e) release T
> 	f) release sysrq
> 
> New behaviour:
> 
> 	a) press alt
> 	b) press sysrq
> 	c) release sysrq
> 	d) press T
> 	e) release T
> 	f) release alt
> 
> If so, then the old behaviour was weird and the new behaviour is sensible. 

It may be weird, but it was documented and people know about it.

> What, actually, is the problem?

It changes the behaviour, it will annoy the hell out of people like me who 
have to deal with different kernels and expect this to just work. :-(
Since then has it been acceptable to just go ahead and break stuff? This 
problem doesn't really look unsolvable, so why is my request to fix the 
damn thing so unreasonable?

bye, Roman

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Roman Zippel]

Hi,

On Wed, 12 Jul 2006, Fredrik Roubert wrote:

> The work-around suggested in the documentation ([Y]ou might have better
> luck with "press Alt", "press SysRq", "release Alt", "press <command
> key>", release everything.) does not work with keyboards that sends the
> make and break codes for SysRq immediately after another, and this was
> the reason for changing the behaviour (for broken keyboards) in
> 2.6.18-rc1. The new behaviour works with every keyboard the people
> involved in this discussion has heard of.

Could you please try to get both methods working?

bye, Roman

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Paulo Marques]

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

Roman Zippel wrote:
> On Tue, 11 Jul 2006, Andrew Morton wrote:
> [...]
>> What, actually, is the problem?
> 
> It changes the behaviour, it will annoy the hell out of people like me who 
> have to deal with different kernels and expect this to just work. :-(
> Since then has it been acceptable to just go ahead and break stuff? This 
> problem doesn't really look unsolvable, so why is my request to fix the 
> damn thing so unreasonable?

Ok, what about this one?

I don't have time to test it (it compiles, at least), but it seems the 
logic is pretty clear: once you have pressed both "Alt" and "SysRq" 
sysrq mode becomes active until you release *both* keys. In this mode 
any regular key press triggers handle_sysrq.

This allows for all the combinations mentioned before in this thread and 
makes the logic simpler, IMHO.

-- 
Paulo Marques - www.grupopie.com

Pointy-Haired Boss: I don't see anything that could stand in our way.
            Dilbert: Sanity? Reality? The laws of physics?

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1462 bytes --]

Subject: allow the old behavior of Alt+SysRq+<key>

This should allow any order of Alt + SysRq press followed by any key
while still holding one of SysRq or Alt.

Signed-off-by: Paulo Marques <pmarques@grupopie.com>

--- ./drivers/char/keyboard.c.orig	2006-07-12 13:03:32.000000000 +0100
+++ ./drivers/char/keyboard.c	2006-07-12 14:18:52.000000000 +0100
@@ -150,7 +150,7 @@ unsigned char kbd_sysrq_xlate[KEY_MAX +
         "230\177\000\000\213\214\000\000\000\000\000\000\000\000\000\000" /* 0x50 - 0x5f */
         "\r\000/";                                      /* 0x60 - 0x6f */
 static int sysrq_down;
-static int sysrq_alt_use;
+static int sysrq_active;
 #endif
 static int sysrq_alt;

@@ -1164,16 +1164,17 @@ static void kbd_keycode(unsigned int key
 				printk(KERN_WARNING "keyboard.c: can't emulate rawmode for keycode %d\n", keycode);

 #ifdef CONFIG_MAGIC_SYSRQ	       /* Handle the SysRq Hack */
-	if (keycode == KEY_SYSRQ && (sysrq_down || (down == 1 && sysrq_alt))) {
-		if (!sysrq_down) {
-			sysrq_down = down;
-			sysrq_alt_use = sysrq_alt;
-		}
+	if (keycode == KEY_SYSRQ) {
+		sysrq_down = down;
 		return;
 	}
-	if (sysrq_down && !down && keycode == sysrq_alt_use)
-		sysrq_down = 0;
-	if (sysrq_down && down && !rep) {
+
+	if (sysrq_down && sysrq_alt)
+		sysrq_active = 1;
+	else if (!sysrq_down && !sysrq_alt)
+		sysrq_active = 0;
+
+	if (sysrq_active && down && !rep) {
 		handle_sysrq(kbd_sysrq_xlate[keycode], regs, tty);
 		return;
 	}

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Randy.Dunlap]

On Wed, 12 Jul 2006 09:26:28 +0200 Fredrik Roubert wrote:

> On Wed 12 Jul 02:16 CEST 2006, Roman Zippel wrote:
> 
> > > Nack your nack!  The patch in 2.6.18-rc1 makes sysrq work on machines on
> > > which it *did not work at all*.  If that makes it harder to type but still
> > > possible to type on other machines, well, we win.
> >
> > Why can't we even _try_ to preserve compatibility? :-(
> 
> First, please note that the documented behaviour (You press the key
> combo 'ALT-SysRq-<command key>'.) still works.
> 
> The problem at hand is that not all keyboards can handle this many keys
> pressed at once, and that there also are keyboards broken in other ways.
> 
> The work-around suggested in the documentation ([Y]ou might have better
> luck with "press Alt", "press SysRq", "release Alt", "press <command
> key>", release everything.) does not work with keyboards that sends the
> make and break codes for SysRq immediately after another, and this was
> the reason for changing the behaviour (for broken keyboards) in
> 2.6.18-rc1. The new behaviour works with every keyboard the people
> involved in this discussion has heard of.
> 
> That the documentation wasn't updated with the new work-around key
> combination for broken keyboards was a mistake.

In the past, I've had/used several huge-company black
keyboards that didn't work (recognize) Alt-SysRq-X.

Should they work now?  (I don't currently have one.)
It would be very nice if they do.

---
~Randy

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Paulo Marques]

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

Paulo Marques wrote:
> Roman Zippel wrote:
>> On Tue, 11 Jul 2006, Andrew Morton wrote:
>> [...]
>>> What, actually, is the problem?
>>
>> It changes the behaviour, it will annoy the hell out of people like me 
>> who have to deal with different kernels and expect this to just work. :-(
>> Since then has it been acceptable to just go ahead and break stuff? 
>> This problem doesn't really look unsolvable, so why is my request to 
>> fix the damn thing so unreasonable?
> 
> Ok, what about this one?

It doesn't work :P

Ok, I've tested it this time and this new one works as expected. I can
use any of the sequences discussed and I can produce a SysRq every time.
Still, just pressing SysRq or any sequence that doesn't start with 
"press Alt -> press SysRq" seems unaffected.

-- 
Paulo Marques - www.grupopie.com

Pointy-Haired Boss: I don't see anything that could stand in our way.
            Dilbert: Sanity? Reality? The laws of physics?


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 1586 bytes --]

Subject: allow the old behavior of Alt+SysRq+<key>

This should allow any order of Alt + SysRq press followed by any key
while still holding one of SysRq or Alt.

Signed-off-by: Paulo Marques <pmarques@grupopie.com>

--- ./drivers/char/keyboard.c.orig	2006-07-12 13:03:32.000000000 +0100
+++ ./drivers/char/keyboard.c	2006-07-12 20:39:21.000000000 +0100
@@ -150,7 +150,7 @@ unsigned char kbd_sysrq_xlate[KEY_MAX +
         "230\177\000\000\213\214\000\000\000\000\000\000\000\000\000\000" /* 0x50 - 0x5f */
         "\r\000/";                                      /* 0x60 - 0x6f */
 static int sysrq_down;
-static int sysrq_alt_use;
+static int sysrq_active;
 #endif
 static int sysrq_alt;

@@ -1164,16 +1164,24 @@ static void kbd_keycode(unsigned int key
 				printk(KERN_WARNING "keyboard.c: can't emulate rawmode for keycode %d\n", keycode);

 #ifdef CONFIG_MAGIC_SYSRQ	       /* Handle the SysRq Hack */
-	if (keycode == KEY_SYSRQ && (sysrq_down || (down == 1 && sysrq_alt))) {
-		if (!sysrq_down) {
-			sysrq_down = down;
-			sysrq_alt_use = sysrq_alt;
+	if (keycode == KEY_SYSRQ) {
+		if (down) {
+			if(sysrq_alt)
+				sysrq_down = down;
+		} else {
+			sysrq_down = 0;
 		}
-		return;
 	}
-	if (sysrq_down && !down && keycode == sysrq_alt_use)
-		sysrq_down = 0;
-	if (sysrq_down && down && !rep) {
+
+	if (sysrq_down && sysrq_alt)
+		sysrq_active = 1;
+	else if (!sysrq_down && !sysrq_alt)
+		sysrq_active = 0;
+
+	if (keycode == KEY_SYSRQ && sysrq_active)
+		return;
+
+	if (sysrq_active && down && !rep) {
 		handle_sysrq(kbd_sysrq_xlate[keycode], regs, tty);
 		return;
 	}

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Dmitry Torokhov]

Hi,

On 7/12/06, Paulo Marques <pmarques@grupopie.com> wrote:
>
> Ok, I've tested it this time and this new one works as expected. I can
> use any of the sequences discussed and I can produce a SysRq every time.
> Still, just pressing SysRq or any sequence that doesn't start with
> "press Alt -> press SysRq" seems unaffected.
>

I like this, however:

> +       if (keycode == KEY_SYSRQ) {
> +               if (down) {
> +                       if(sysrq_alt)
> +                               sysrq_down = down;
> +               } else {
> +                       sysrq_down = 0;

Are you sure? This will set sysrq_down only if ALT has already been
pressed. If SysRq does not autorepeat and it is pressed first we won't
ever see sysrq_down. Am I missing something?

> +
> +       if (sysrq_down && sysrq_alt)
> +               sysrq_active = 1;
> +       else if (!sysrq_down && !sysrq_alt)
> +               sysrq_active = 0;
> +
> +       if (keycode == KEY_SYSRQ && sysrq_active)
> +               return;

What about alt? I think that "if (...) sysrq_active = 1;" statement
should go down, below handle_sysrq block.

> +
> +       if (sysrq_active && down && !rep) {
>                handle_sysrq(kbd_sysrq_xlate[keycode], regs, tty);
>                return;
>        }
>

We also need to check if emulate_raw() needs to be adjusted...

-- 
Dmitry

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Bodo Eggert]

Paulo Marques <pmarques@grupopie.com> wrote:
> Roman Zippel wrote:
>> On Tue, 11 Jul 2006, Andrew Morton wrote:
>> [...]
>>> What, actually, is the problem?
>> 
>> It changes the behaviour, it will annoy the hell out of people like me who
>> have to deal with different kernels and expect this to just work. :-(
>> Since then has it been acceptable to just go ahead and break stuff? This
>> problem doesn't really look unsolvable, so why is my request to fix the
>> damn thing so unreasonable?
> 
> Ok, what about this one?
> 
> I don't have time to test it (it compiles, at least), but it seems the
> logic is pretty clear: once you have pressed both "Alt" and "SysRq"
> sysrq mode becomes active until you release *both* keys. In this mode
> any regular key press triggers handle_sysrq.
> 
> This allows for all the combinations mentioned before in this thread and
> makes the logic simpler, IMHO.

Why don't you use a bitmask?
alt-sysrq down -> val  =  0b11
sysrq up       -> val &= ~0b01
alt up         -> val &= ~0b10

test is_sysrq == !!val
-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.

http://david.woodhou.se/why-not-spf.html

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Paulo Marques]

Dmitry Torokhov wrote:
> Hi,
> 
> On 7/12/06, Paulo Marques <pmarques@grupopie.com> wrote:
> 
>> Ok, I've tested it this time and this new one works as expected. I can
>> use any of the sequences discussed and I can produce a SysRq every time.
>> Still, just pressing SysRq or any sequence that doesn't start with
>> "press Alt -> press SysRq" seems unaffected.
> 
> I like this, however:
> 
>> +       if (keycode == KEY_SYSRQ) {
>> +               if (down) {
>> +                       if(sysrq_alt)
>> +                               sysrq_down = down;
>> +               } else {
>> +                       sysrq_down = 0;
> 
> Are you sure? This will set sysrq_down only if ALT has already been
> pressed. If SysRq does not autorepeat and it is pressed first we won't
> ever see sysrq_down. Am I missing something?

This was done on purpose, yes. My first thought was to allow any order, 
but this could cause strange complications like: if I just press 
PrintScreen when is it ok to forward that keypress before I press Alt? 
The Alt afterwards arrives to late to not have messed the user space 
application already.

Anyway, none of the sequences posted required this, and it is more 
intuitive to press Alt first anyway.

>> +
>> +       if (sysrq_down && sysrq_alt)
>> +               sysrq_active = 1;
>> +       else if (!sysrq_down && !sysrq_alt)
>> +               sysrq_active = 0;
>> +
>> +       if (keycode == KEY_SYSRQ && sysrq_active)
>> +               return;
> 
> What about alt? I think that "if (...) sysrq_active = 1;" statement
> should go down, below handle_sysrq block.

It can not go down, or when you press Alt and then SysRq, the first 
SysRq down event will get through without returning.

alt is handled a little higher in that function outside the #IFDEF. This 
is because emulate_raw should work even if we don't support magic sysrq.

The logic here is pretty simple: if we press Alt and then SysRq we enter 
sysrq_active mode. We only come out of that mode when we release both 
keys. Releasing any one of them individually is fine.

Any KEY_SYSRQ repetitions or releases while on this mode are not 
processed any further.

Oops, I just realised that if I release Alt first and then SysRq, the 
SysRq release will get through because at that point we're already out 
of sysrq_active mode. This should be easy to fix, though. If this is a 
problem, I can send a new patch tomorrow (with some more comments in 
there, too).

>> +
>> +       if (sysrq_active && down && !rep) {
>>                handle_sysrq(kbd_sysrq_xlate[keycode], regs, tty);
>>                return;
>>        }
> 
> We also need to check if emulate_raw() needs to be adjusted...

I looked at that very quickly, to be honest, but couldn't understand it 
entirely. This code:

1087 if (keycode == KEY_SYSRQ && sysrq_alt) {
1088   put_queue(vc, 0x54 | up_flag);
1089   return 0;
1090 }

seems to be supposed to cancel the Alt "press" by sending a Alt 
"release" to handle the sequence press alt -> press sysrq without 
affecting the application. However, this is just a guess. I couldn't 
find out what that magic 0x54 meant or why this would be enough wether 
we pressed the left or the right alt...

Then there are other things that I don't understand there: I don't see 
any code to filter out the keys we press ('T','P', etc.) while using 
SysRq magic if we are in raw mode. emulate_raw will happily call 
put_queue on them before we have a chance to bail out.

Maybe we should just stop calling emulate_raw while sysrq_active is 
active. This way, after we press Alt + SysRq, every keypress would be 
processed as a magic sysrq and not handled by any other code until we 
release both keys.

Does this sound reasonable?

-- 
Paulo Marques - www.grupopie.com

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Paulo Marques]

Bodo Eggert wrote:
> Paulo Marques <pmarques@grupopie.com> wrote:
>[...]
>>This allows for all the combinations mentioned before in this thread and
>>makes the logic simpler, IMHO.
> 
> Why don't you use a bitmask?
> alt-sysrq down -> val  =  0b11
> sysrq up       -> val &= ~0b01
> alt up         -> val &= ~0b10
> 
> test is_sysrq == !!val

It can be done, but it doesn't seem to buy you much.

The sysrq_alt variable is used wether we use magic sysrq or not, so we 
must keep it anyway. This var doesn't do what your high bit does, 
because in the bit mask this bit only goes on when both are pressed (not 
just alt).

alt up is actually 2 different keys (left and right). To detect it, we 
either copy the same "if" that is outside the #ifdef or we try to follow 
the state of sysrq_alt to detect the change from low to high :P

Anyway, I think the code can be simplified further, though, and it might 
involve a similar trick. And it definitely needs some more comments in 
there ;)

I'll play with it some more and try to produce a better patch.

-- 
Paulo Marques

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Pavel Machek]

Hi!

> Maybe we should just stop calling emulate_raw while sysrq_active is 
> active. This way, after we press Alt + SysRq, every keypress would be 
> processed as a magic sysrq and not handled by any other code until we 
> release both keys.

I guess so. Magic sysrq should be hidden from applications, even
applications using raw mode.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [patch] Re: Magic Alt-SysRq change in 2.6.18-rc1

[Dmitry Torokhov]

On 7/12/06, Paulo Marques <pmarques@grupopie.com> wrote:
> >
> > We also need to check if emulate_raw() needs to be adjusted...
>
> I looked at that very quickly, to be honest, but couldn't understand it
> entirely. This code:
>
> 1087 if (keycode == KEY_SYSRQ && sysrq_alt) {
> 1088   put_queue(vc, 0x54 | up_flag);
> 1089   return 0;
> 1090 }
>
> seems to be supposed to cancel the Alt "press" by sending a Alt
> "release" to handle the sequence press alt -> press sysrq without
> affecting the application. However, this is just a guess. I couldn't
> find out what that magic 0x54 meant or why this would be enough wether
> we pressed the left or the right alt...
>

Real AT keyboards (that's what we are trying to emulate here) send
0x54 scancode (and 0xB4 break code) if PrintScreen/SysRq key is
pressed while holding Alt (left or right). Otherwise it sends e0 2a e0
37. That's it.

> Then there are other things that I don't understand there: I don't see
> any code to filter out the keys we press ('T','P', etc.) while using
> SysRq magic if we are in raw mode. emulate_raw will happily call
> put_queue on them before we have a chance to bail out.
>
> Maybe we should just stop calling emulate_raw while sysrq_active is
> active. This way, after we press Alt + SysRq, every keypress would be
> processed as a magic sysrq and not handled by any other code until we
> release both keys.

We never supressed sysrq scancodes from reaching userspace if keyboard
is in raw mode (2.4 works the same). Plus you won't be able to supress
it for keyboards working in hadware raw mode (kbd_rawcode is called
even before we get to detecting SysRq state). Also users getting
keyboard events from alternative interfaces (such as evdev) won't be
affected anyway so I think we shoudl leave it as is.

-- 
Dmitry