Date:	Tue, 23 Oct 2007 09:21:41 -0400
From:	"Dmitry Torokhov" <dmitry.torokhov@...il.com>
To:	"Ryan Lortie" <desrt@...rt.ca>
Cc:	"Zephaniah E. Hull" <warp@...allh.com>,
	linux-kernel@...r.kernel.org, "Vojtech Pavlik" <vojtech@...e.cz>,
	linux-input <linux-input@...ey.karlin.mff.cuni.cz>
Subject: Re: [PATCH] Input: Support for a less exclusive grab.

Hi Ryan,

On 9/28/07, Ryan Lortie <desrt@...rt.ca> wrote:
>
> Hello.
>
> I have been working on a more flexible system for blocking the delivery
> of input events to other agents in the system.
>
> My approach is basically summed up as follows:
>
>  - split the current purpose of input_handle into two parts
>
>    - input_handle continues to exist as a mechanism for tracking which
>      handlers are currently interested in a particular device
>
>    - a new input_client now exists as a mechanism for tracking
>      tracking who is interested in having events delivered from a
>      particular device.
>
>    As an example, for evdev, one input_handle will exist per event
>    device in /dev/input and one input_client will exist per open().
>
>  - cause input events to be delivered to 'clients' (ie: first argument
>    is an input_client instead of input_handle)
>
>  - add a concept of 'priority'
>
>    - the new input_client structure has a priority field
>
>    - the list of input_client's per device is kept sorted by this field
>
>  - add the ability for the handler 'event' function to block further
>    event propagation.  This is done by making the handler return 'int'
>    instead of 'void'.  0: continue normally.  1: stop.
>
>  - add "set priority" ioctl to evdev
>
>  - add "set requested keys" ioctl to evdev.
>
>    This causes delivery of only certain keycodes to this particular
>    open().  I have no particular interest in doing this sort of masking
>    for other (non-key) events but maybe it makes sense.
>
>  - add "set filter" ioctl to evdev.
>
>    This causes any "requested keys" to be consumed and not further
>    delivered (ie: the event handler for this client will return 1).
>
>  - also, as a small style issue: I made what I believe to be a
>    simplification to the switch() statement in the evdev ioctl code.
>    Whether this is actually a simplification or not is a matter of
>    opinion. :)
>
>  - port the non-evdev handlers (including rfkill) to the new
>    handler/client interface in the most trivial way possible: open the
>    input_client in the places where the old code used to call open with
>    the input_handler.
>
> The motivation is to allow arbitrary things in user-space to have a rich
> support over blocking (possibly a select set of) key events from being
> delivered to anything below a certain 'priority'.
>
> I believe this will solve the problem of X wanting to block key delivery
> to the VT module while still allowing the events to go through to rfkill
> (which presumably will be given a higher priority).
>
> This will also allow hal's "multimedia key" reporting code to be
> improved in two ways: first, it can block the multimedia keys from being
> delivered to X (and causing weird ^[[29~ things to appear in my
> terminals) and second, it results in hal only waking up when an
> "interesting key" is pressed (currently it wakes up on every single
> keypress).
>

I like the idea of limiting number of events that client wants to be
delivered to it. I think we need to extend it from keybit only to
event bitmask + keybit. This way, if keyboard has a scrollwheel or a
touchpad installed, HAL can still specify EV_KEY + KEY_XXX and ignore
all REL_* and ABS_* events.

Priority/filter idea is different matter. I don't think it is a giood
solution. There will always be an "arms race", new applications would
like to get in front of the queue all the time and it will be hard to
manage. X should just keep console open in raw mode and simply ignore
all events coming from it while using evdev driver. I understand that
X's hotplug support is getting there so you should be able to switch
to pure evdev setup pretty easy.

> I have written a patch.  It is "over 40kb" so I don't include it
> directly here.  It is available in its full form (or broken up) at this
> url:
>
> http://desrt.mcmaster.ca/code/input-patch/
>
> This patch is not being proposed for inclusion in its current state.  In
> addition to the many problems that I'm sure I don't even realise, here
> are some that I would specifically like feedback on:
>
>  - how should we decide what gets what priority level?
>

Exactly. You can't predict all future uses. There will always be
someone wanting to get in front of the line.

>  - is it appropriate to have INPUT_PRIORITY_HIGH/LOW/SYSTEM/DEBUG, etc.
>    macros in input.h?
>
>  - why did the old code call flush() on the device before closing when
>    disconnecting?  It seems particularly silly now that with my patch
>    it will be called n times (one for each client)

To remove any force-feedback effects installed by client that is being
disconnected.

>
>  - how are switches supposed to be indented?  the "checkpatch" script
>    in the kernel complained about my style despite following what was
>    already in that file.

I think I fixed this.

>
>  - I haven't tested on big endian or "compat" systems at all.  As far
>    as I know, the relevant code doesn't even compile.  Help here is
>    appreciated (I don't have such a machine!).
>
>  - there is small thread-safety issue with how I handle EVIOCSRKEYS on
>    defined(CONFIG_COMPAT) && defined(__BIG_ENDIAN).  If events are
>    delivered after the copy_from_user but before the word-swapping then
>    events may be inappropriately dropped or delivered.  Do we care?

Input core now has prper locking. You may take device's event_lock to
stop event propagation.

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

Powered by blists - more mailing lists