Re: Possible bug in keyboard.c (2.6.10)

[Vojtech Pavlik]

On Fri, Jan 28, 2005 at 01:39:08AM +0100, Roman Zippel wrote:
> Hi,
> 
> On Thu, 27 Jan 2005, Andries Brouwer wrote:
> 
> > In short - raw mode in 2.6 is badly broken.
> 
> I think not just that. The whole keyboard input layer needs some serious 
> review. Just the complete lack of any locking is frightening, I'd really 
> like to know how the input layer could become the standard.

I'm very sorry about the locking, but the thing grew up in times of
kernel 2.0, which didn't require any locking. There are a few possible
races with device registration/unregistration, and it's on my list to
fix that, however under normal operation there shouldn't be any need for
locks, as there are no complex structures built that'd become
inconsistent. 

If you find scenarios which will lead to trouble in the event delivery
system, please tell me, and I'll try to fix that as soon as possible.

> I tried to find a few times to find any discussion about the input
> layer design, but I couldn't find anything.

You have to search in very old archives. There was quite a lot of it,
and it was going off on a lot of tangents. In the end, I just wrote it.

> Some of my favourites in the input layer:
> - the keyboard sound/led handling: the keyboard driver basically fakes 
> events for the device and input_event() is "clever" enough to also tell 
> the device about it. This is quite an abuse of event system for general 
> device/driver communication.

The intention here is that we have two types of events, input and
output. Most events are input (REL, ABS, ...), while some travel the
opposite direction. For simplicity, the interface is the same -
input_event(), which then, based on the event type decides where to
forward it - whether up or down the stream (or both, where other users
of the device may be interested in the change).

> - a single input device structure for all types: this structure is quite 
> big, where most of its contents is irrelevant for most devices.

I actually think this is a big plus. 

Real word devices cannot be confined into predefined structures, as
hardware develops, mice get more buttons, wheels, force feedback, 

The structure, if the size is a problem, can be made smaller by having
the larger bitmaps allocated separately.

> - fine grained matching/filtering: I have no idea why the input layer has 
> to do this down to the single event instead of just the event type.

I wonder what do you mean by this, the layer itself doesn't have any
codepaths dependent directly on event code, just on the types.

If you wonder why the input_event() function checks whether the event
generated by a device really is possible for that device and ignores it
if not, that's to make the drivers life easier by, in the example of a
PS/2 mouse always reporting the state of the middle button, even when
the PS/2 mouse is a 2-button mouse. The driver only needs to say that
the middle button doesn't exist in the bitmap setup and the packet
processing routine doesn't need to care about it.

And if you wonder whether struct input_dev needs to even know what event
codes for each type are generated by the device - that's there to tell
the event handlers (whether kernel or userspace), so they will know what
to expect and can make decisions based on it.

> Vojtech, could you please explain a bit the reason for the above and what 
> are your plans to e.g. fix the locking?

-- 
Vojtech Pavlik
SuSE Labs, SuSE CR
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/