On 09/24/2010 11:08 PM, Dmitry Torokhov wrote:
> On Sat, Sep 25, 2010 at 02:14:32AM +0200, Maxim Levitsky wrote:
>   
>> On Fri, 2010-09-24 at 15:58 -0500, Jason Wessel wrote:
>>     
>>> On 09/24/2010 03:50 PM, Maxim Levitsky wrote:
>>>       
>>>>   
>>>>         
>>>>> [Dropped nouveau list, because this is offtopic there]
>>>>>
>>>>> I pretty much got to the bottom of this.
>>>>> There are 2 separate issues:
>>>>>
>>>>>
>>>>> 1. SysRq handler is now a input 'filter', which means that it can 'eat'
>>>>> input events, so they don't show up on input bus.
>>>>> It does so while sysrq key is down.
>>>>> So sysrq and 'g' events never reach the kernel kbd driver and therefore
>>>>> the hack to release them doesn't work.
>>>>>
>>>>> 2. The kbd_clear_keys_helper injects the keyup events alright, but it
>>>>> doesn't inject SYN events, and therefore X evdev driver doesn't pick
>>>>> these injected events untill next SYN event. 
>>>>>
>>>>> This patch makes key release work in expense of showing sysrq key to userspace, which isn't that good,
>>>>> because now Alt+SysRQ causes a screen capture by default.
>>>>> In my opinion the sysrq filter should stay.
>>>>> We should just make kdb hook into atkbd and do the key release there.
>>>>> This should both result in cleaner/more robust code, and make this issue disappear.
>>>>> I'll look at doing that.
>>>>>
>>>>>
>>>>> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
>>>>> index 0c6c641..7df6af5 100644
>>>>> --- a/drivers/char/keyboard.c
>>>>> +++ b/drivers/char/keyboard.c
>>>>> @@ -368,6 +368,7 @@ static int kbd_clear_keys_helper(struct input_handle *handle, void *data)
>>>>>  {
>>>>>  	unsigned int *keycode = data;
>>>>>  	input_inject_event(handle, EV_KEY, *keycode, 0);
>>>>> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
>>>>>  	return 0;
>>>>>  }
>>>>>           

So I kept this piece, in the 0002 patch which is attached.   I revised
the keyboard/input series at:

http://git.kernel.org/?p=linux/kernel/git/jwessel/linux-2.6-kgdb.git;a=shortlog;h=refs/heads/for_input

>>>>>  
>>>>> diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
>>>>> index ef31bb8..db1eb12 100644
>>>>> --- a/drivers/char/sysrq.c
>>>>> +++ b/drivers/char/sysrq.c
>>>>> @@ -601,7 +601,7 @@ static bool sysrq_filter(struct input_handle *handle, unsigned int type,
>>>>>  	}
>>>>>  
>>>>>  out:
>>>>> -	return sysrq_down;
>>>>> +	return 0;
>>>>>  }
>>>>>           

I took some time to look more closely at the problem, and I believe we
still want to filter out all these input events.

>   
>> The right solution in my opinion is to make atkbd register with kgdb and
>> provide to it, the polling keyboard IO services, and also take care of
>> releasing the keys as soon as debug mode is entered or exited.
>>     
>
> Disagree. We may support different keyboard devices with kdb or use one
> keyboard (USB) to drop into debugger and then switch to atkbd...
>
> I think we need to move Jason's code from drivers/char/keyboard.c to
> drivers/char/sysrq.c and have it track keys that have been kept pressed
> before entering sysrq mode and release them when leaving the mode.
>
> We also need to teach sysrq that some handlers need resetting of the
> keyboard state.
>
>   

I found the root of the problem was the low level keyboard bit mask
getting out of sync in the input layer.   Perhaps Dmitry can have a look
at the 3rd patch in the series, which addresses the problem.  

I also put these patches into kgdb-next.

I believe it fixes the problems Maxim encountered, or at least the
problems I observed independently with stuck keys and the lack of the
ability to correctly use alt-PrintScreen.

If you are willing Maxim, can you give these patches a go?

Thanks,
Jason.