Re: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2

[Crutcher Dunnavant <crutcher@gmail.com>]

Crutcher Dunnavant

On Oct 18, 2007, at 10:10 AM, Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
>
>>> 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.
>
> Seems perfectly reasonably to me... parhaps "Full" should be replaced
> with "Force-oom"...
>
>>>> 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.
>
> No, it is not a fix and no, it is not inefficient.
>
>>>> +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.
>
> -- d
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html