The general reaction was that the changes were unnecessary. Valdis Kletnieks pointed out, "you're optimizing code that hopefully never gets executed, and even if it does, you have the optimization *backwards*. If you're worried about the efficiency, trim it down to output 3 lines - do you realize how many instructions it takes in the VGA and fb drivers to actually *output* all these lines?" He added that people using SysRq for debugging usually fall into two classes and don't require lengthy online help, "1) he has been told 'recreate the problem, hit alt-sysreq-cokebottle, and send me the results', or 2) he's already read the file in Documentation/ and just needs a reminder."
From: Yi Yang <yang.y.yi@...>
Subject: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Date: Oct 17, 11:22 am 2007
Compared with last patch, this has a little change according to
Andreas and Pavel's feedbacks.
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as "Changing Loglevel", just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch adds this online help function, it'll print the whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is below on pressing an undefined hot key before this patch:
SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks
But after applying this patch, the output is:
SysRq :
this hot key isn't defined.
SysRq Help Information:
Hot Key Function Description
======= ====================
0 Changing Loglevel to this value
1 Changing Loglevel to this value
2 Changing Loglevel to this value
3 Changing Loglevel to this value
4 Changing Loglevel to this value
5 Changing Loglevel to this value
6 Changing Loglevel to this value
7 Changing Loglevel to this value
8 Changing Loglevel to this value
9 Changing Loglevel to this value
a Not defined
b Resetting
c Trigger a crashdump
d Not defined
e Terminate All Tasks
f Manual OOM execution
g Not defined
h Not defined
i Kill All Tasks
j Not defined
k SAK
l Not defined
m Show Memory
n Nice All RT Tasks
o Power Off
p Show Regs
q Show Pending Timers
r Keyboard mode set to XLATE
s Emergency Sync
t Show State
u Emergency Remount R/O
v Not defined
w Show Blocked State
x Not defined
y Not defined
z Not defined
Signed-off-by: Yi Yang
---
sysrq.c | 38 ++++++++++++++++++++++++++------------
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.000000000 +0800
+++ b/drivers/char/sysrq.c 2007-10-17 17:52:45.000000000 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -81,7 +91,7 @@ static void sysrq_handle_loglevel(int ke
static struct sysrq_key_op sysrq_loglevel_op = {
.handler = sysrq_handle_loglevel,
.help_msg = "loglevel0-8",
- .action_msg = "Changing Loglevel",
+ .action_msg = "Changing Loglevel to this value",
.enable_mask = SYSRQ_ENABLE_LOG,
};
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(&sysrq_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO "SysRq : ");
+ printk(KERN_INFO "SysRq : \n");
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,18 +436,22 @@ void __handle_sysrq(int key, struct tty_
printk("This sysrq operation is disabled.\n");
}
} else {
- printk("HELP : ");
- /* Only print the help msg once per handler */
+ /*
+ * Print SysRq hot key list
+ */
+ printk(KERN_INFO "this hot key isn't defined.\n\n");
+ printk(KERN_INFO "SysRq Help Information:\n\n");
+ printk(KERN_INFO "Hot Key Function Description\n");
+ printk(KERN_INFO "======= ====================\n");
for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
if (sysrq_key_table[i]) {
- int j;
-
- for (j = 0; sysrq_key_table[i] !=
- sysrq_key_table[j]; j++)
- ;
- if (j != i)
- continue;
- printk("%s ", sysrq_key_table[i]->help_msg);
+ printk(KERN_INFO " %c %s\n",
+ sysrq_hot_key_table[i],
+ sysrq_key_table[i]->action_msg);
+ } else {
+ printk(KERN_INFO " %c %s\n",
+ sysrq_hot_key_table[i],
+ "Not defined");
}
}
printk("\n");
-
From: <Valdis.Kletnieks@...>
Subject: Re: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Date: Oct 17, 4:09 pm 2007
On Wed, 17 Oct 2007 23:22:58 +0800, Yi Yang said:
> SysRq has already provided a similiar help before this patch, but it
> is not so clear that the user doesn't know what happened and what
> he/she should do.
The person is in one of two states:
1) He has been told "recreate the problem, hit alt-sysreq-cokebottle,
and send me the results". He has a mission, and the only feedback he
needs is (a) that he hit cokebottle and not pepsibottle, and (b) the
resulting output.
2) He's already read the file in Documentation/ and just needs a reminder.
In this case, the fact it's only 2 or 3 lines and doesn't scroll other stuff
out of sight is more important.
> In addition, that funtion has a big loop with another big loop
> embedded which is very inefficient, it is intended to skip some hot
You're optimizing code that hopefully never gets executed, and even if
it does, you have the optimization *backwards*. If you're worried about
the efficiency, trim it down to output 3 lines - do you realize how many
instructions it takes in the VGA and fb drivers to actually *output* all
these lines? (Seriously - I had a 1.6Ghz P4 laptop, where scrolling the
screen with vga=791 actually ran so slowly that it horqued up the timer
initialization code. *That* was a fun bug to figure out..)
> key help info for such a function as "Changing Loglevel", just print
> a help info for this, that is very unnecessary. In fact, the key '0'
> - '8' have different results the user should know.
And ironically enough, you then output the same exact text for all levels.
> +static char __read_mostly sysrq_hot_key_table[36] = {
> + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
> + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
> + 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
> + 'u', 'v', 'w', 'x', 'y', 'z'
The lists of 'Not defined' tends to scroll the screen away. The old code
instead focuses on listing the things you *can* do. If I'm looking at the
help output, I don't care that 'g' is not defined. I need to be reminded
that 'p' is 'showPc' and D is show-all-locks.
From: Yi Yang <yang.y.yi@...>
Subject: Re: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Date: Oct 18, 10:34 am 2007
Valdis.Kletnieks@vt.edu 写道:
> On Wed, 17 Oct 2007 23:22:58 +0800, Yi Yang said:
>
>
>> SysRq has already provided a similiar help before this patch, but it
>> is not so clear that the user doesn't know what happened and what
>> he/she should do.
>>
>
> The person is in one of two states:
>
> 1) He has been told "recreate the problem, hit alt-sysreq-cokebottle,
> and send me the results". He has a mission, and the only feedback he
> needs is (a) that he hit cokebottle and not pepsibottle, and (b) the
> resulting output.
>
> 2) He's already read the file in Documentation/ and just needs a reminder.
> In this case, the fact it's only 2 or 3 lines and doesn't scroll other stuff
> out of sight is more important.
>
Screen scroll isn't a problem.
"SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks"
The above help information isn't very user-friendly.
>
>> In addition, that funtion has a big loop with another big loop
>> embedded which is very inefficient, it is intended to skip some hot
>>
>
> You're optimizing code that hopefully never gets executed, and even if
> it does, you have the optimization *backwards*. If you're worried about
> the efficiency, trim it down to output 3 lines - do you realize how many
> instructions it takes in the VGA and fb drivers to actually *output* all
> these lines? (Seriously - I had a 1.6Ghz P4 laptop, where scrolling the
> screen with vga=791 actually ran so slowly that it horqued up the timer
> initialization code. *That* was a fun bug to figure out..)
>
To remove a bad loop is just a plus fix. That loop is really inefficient.
>
>> key help info for such a function as "Changing Loglevel", just print
>> a help info for this, that is very unnecessary. In fact, the key '0'
>> - '8' have different results the user should know.
>>
>
> And ironically enough, you then output the same exact text for all levels.
>
Yes, only one line for them is better.
>
>
>> +static char __read_mostly sysrq_hot_key_table[36] = {
>> + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
>> + 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
>> + 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
>> + 'u', 'v', 'w', 'x', 'y', 'z'
>>
>
> The lists of 'Not defined' tends to scroll the screen away. The old code
> instead focuses on listing the things you *can* do. If I'm looking at the
> help output, I don't care that 'g' is not defined. I need to be reminded
> that 'p' is 'showPc' and D is show-all-locks.
>
You're right, "Not defined" is meaningless for the common users. I'll
submit a new revision
to fix your concerns.
Thank you very much.
-
Cokebottle?
what the hell is a cokebottle?
I was wondering when someone would ask that...
I remember hearing (apocryphal, it turns out) legend that on the old LISP machines with the so-called "space cadet" keyboards, if you pressed Ctrl-Alt-Shift-Meta-Super-Hyper-1 you'd get a symbol that looked like a Coke bottle. There's some info in the Jargon File and in this old thread.
I found another recent appearance here. Clearly still around among a certain crowd. :-)
--
Program Intellivision and play Space Patrol!