SysRq: print hotkey info while pressing undef key, try 2
PATCH 1/1 SysRq: print hotkey info while pressing undef key, try 2
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
--- sysrq.c | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-)
drivers/char/sysrq.c +26 -12
@@ -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 kestatic 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" );
Quoted 4 lines · Yi Yang
Hot Key Function Description ======= ==================== 0 Changing Loglevel to this value 1 Changing Loglevel to this value
Quoted 8 lines
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
Wouldn't "Change Loglevel to this value" be more consistent with other descriptions?
Or maybe even "Change Kernel Loglevel to <value>"; the repeated "this value" seems somewhat silly.
Quoted 1 line
b Resetting
This is not very descriptive. Maybe "Immediate System Reboot"?
Quoted 1 line
c Trigger a crashdump
Most descriptions are capitalized. So for consistency this should be "Trigger a Crashdump". (Although my personal preference would be to drop the capitalization for all descriptions.)
Quoted 1 line
p Show Regs
Should "Regs" be expanded to "Registers"?
Quoted 1 line
r Keyboard mode set to XLATE
For consistency: "Set Keyboard Mode to XLATE"
Cheers,
Frans Pop
Quoted 16 lines · Frans Pop
Quoted 4 lines · Yi Yang
Hot Key Function Description ======= ==================== 0 Changing Loglevel to this value 1 Changing Loglevel to this valueQuoted 9 lines
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
Quoted 5 lines
Wouldn't "Change Loglevel to this value" be more consistent with other descriptions?
Or maybe even "Change Kernel Loglevel to <value>"; the repeated "this value" seems somewhat silly.
I think so, too, only one line should be enough.
Quoted 21 lines
Quoted 3 lines
b Resetting
This is not very descriptive. Maybe "Immediate System Reboot"?
Quoted 2 lines
c Trigger a crashdump
Most descriptions are capitalized. So for consistency this should be "Trigger a Crashdump". (Although my personal preference would be to drop the capitalization for all descriptions.)
Quoted 2 lines
p Show Regs
Should "Regs" be expanded to "Registers"?
Quoted 9 lines
Quoted 2 lines
r Keyboard mode set to XLATE
For consistency: "Set Keyboard Mode to XLATE"
Cheers,
Frans Pop
These descriptions are from the callers who are from many subsystems, so I can't change it.
Quoted 3 lines · Yi Yang
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.
Quoted 2 lines
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..)
Quoted 3 lines
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.
drivers/char/sysrq.c int __read_mostly __sysrq_enabled = 1; · line 55
Quoted 5 lines
drivers/char/sysrq.c +5
-0+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.
Hi!
Quoted 15 lines
Quoted 3 lines
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.
Quoted 6 lines
Quoted 2 lines
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
Pretty much exact. Thanks for saying this so nicely.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Quoted 10 lines · [email protected] 写道:
Quoted 4 lines · Yi Yang
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:
Quoted 9 lines
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.
Quoted 5 lines
Quoted 4 lines
In addition, that funtion has a big loop with another big loop embedded which is very inefficient, it is intended to skip some hot
Quoted 8 lines
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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 14 lines
drivers/char/sysrq.c int __read_mostly __sysrq_enabled = 1; · line 55
Quoted 8 lines
drivers/char/sysrq.c +5
-0+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.
Hi!
Quoted 10 lines
Quoted 10 lines
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.
Quoted 7 lines
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.
Seems perfectly reasonably to me... parhaps "Full" should be replaced with "Force-oom"...
Quoted 12 lines
Quoted 4 lines
Quoted 3 lines
In addition, that funtion has a big loop with another big loop embedded which is very inefficient, it is intended to skip some hot
Quoted 8 lines
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..)
Quoted 2 lines
To remove a bad loop is just a plus fix. That loop is really inefficient.
No, it is not a fix and no, it is not inefficient.
Quoted 5 lines
Quoted 5 lines
drivers/char/sysrq.c int __read_mostly __sysrq_enabled = 1; · line 55
Quoted 5 lines
drivers/char/sysrq.c +5
-0+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'
This has to go.
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Crutcher Dunnavant
Quoted 40 lines · Pavel Machek
Hi!
Quoted 10 lines
Quoted 10 lines
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.Quoted 7 lines
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.
Seems perfectly reasonably to me... parhaps "Full" should be replaced with "Force-oom"...
Quoted 12 lines
Quoted 4 lines
Quoted 3 lines
In addition, that funtion has a big loop with another big loop embedded which is very inefficient, it is intended to skip some hot
Quoted 8 lines
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..)
Quoted 2 lines
To remove a bad loop is just a plus fix. That loop is really inefficient.
No, it is not a fix and no, it is not inefficient.
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines
drivers/char/sysrq.c int __read_mostly __sysrq_enabled = 1; · line 55
Quoted 5 lines
drivers/char/sysrq.c +5
-0+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' This has to go.
Pavel
Wow. I've not looked at this in a long time.
A. This imp of sysrq was designed to be run time registerable by modules. B. Some keys aren't valid on some arches.
C. If you care about the lookup time for one table scan, sysrq can't help you anyway.
Quoted 4 lines
-- d
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html