Re: 2.6.6-mm2 : Hitting Num Lock kills keyboard

[Vojtech Pavlik <vojtech@suse.cz>]

On Fri, May 14, 2004 at 12:32:52AM -0500, Dmitry Torokhov wrote:

> > > 	3) They light up a led on the keyboard,
> > 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > 
> > atkbd tries to switch leds but one tasklet can not interrupt another so
> > it deadlocks...
> > 
> > You need to revert just the patch below, not entire bk-input
> 
> Guys,
> 
> I need some guidance. The issue can be resolved in several ways but I am not
> sure which one is better:
> 
> - just revert the patch and continue doing everything in IRQ context;
> - move IRQ/tasklet split higher, into atkbd and psmouse-base respectively.
>   Every device would have it's own ring buffer, tasklet will be fired off
>   for incoming data only (i.e. ACKs and command response will be processed
>   right there in IRQ context);
> - move drivers/char/keyboard.c from tasklet to schedule_work. Need also
>   take care of others who are using keyboard_tasklet (qtronix, scan_keyb,
>   ec3104_keyb);
> - have atkbd use schedule_work to do LED and repeat rate setting.
> 
> What would you suggest?
 
Now that I'm considering it all, I think serio->interrupt function
should indeed be called within an interrupt context. The name should be
a big enough warning not to do too much processing (and definitely no
communication with the device other than doing something with the
received byte) there.

The input core and the handlers were intended to be lightweight enough
so that they could be executed from an interrupt handler as well.

Are we really spending too much time in the interrupt there? It's well
possible, but it'd be good if it could be evaluated. It should be rather
easy to add a rdtsc before an after the invocation of serio->interrupt
from i8042.c.

That should give us the answer about what to do - which parts of the
input processing can stay in the interrupt (because that is simple), and
what needs to be postponed to a tasklet or schedule_work().

Each of your options seems reasonable. I think moving keyboard.c to
schedule_work is a definitely good idea, but we may want to do more than
that.

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR