Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH/RFC] Resolve 2 year old issue with different demands on EVIOCGRAB

215 views
Skip to first unread message

Neil Brown

unread,
Aug 14, 2008, 10:10:06 PM8/14/08
to

I'll let the comments in the patch below to most of the talking.
This came up because I wanted to use EVIOCGRAB in some code on an
Openmoko Freerunner, and found that EVIOCGRAG does different things on
that kernel to elsewhere. I looked into why, and found that there was
a good reason but that the issues hadn't been completely resolved. I
hope to help resolve the issues so that EVIOCGRAB can behave the same
everywhere, and still meet everybody's needs.

I would have Cc:ed to Magnus Vigerlof who wrote the original patch on
which this is based, but his Email address doesn't appear in lkml.org.

NeilBrown

Support multiple styles of input device GRABbing with EVIOCGRAB

From: NeilBrown <ne...@suse.de>

This is a 2 year old issue (almost to the day!) which does not
appear to have been properly resolved yet.
See http://lkml.org/lkml/2006/8/12/64

An input device (e.g. touch pad, key board) can be attached to
multiple handlers (/dev/mouseX, console, etc) one of which can be
evdev which provides the /dev/input/event* files.
evdev can support multiple clients on a single device, as multiple
processes can open /dev/input/eventX. They each get to see all the
events.

EVIOCGRAB is an ioctl on an evdev device which causes all events to go
to that handler, and that client, exclusively.

It is being used for two distinct purposes.

1/ Stop events going to other handlers, so that e.g. absolute touchpad
events don't get mapped to relative mouse events. Keyboard events
processed by X server don't get processed by console and control-C
kills the X server.
2/ Stop events going to any other client, so they can temporarily be
used for a different purpose. e.g. GRAB all events when putting a
device partially to sleep (e.g. screen blank) so the events can
cause a wakeup without being processed by any application.

So there are three levels of grab that are required:

0/ no grab : all events go to all handlers and all clients
1/ evdev grab: events only go to evdev, but can then go to any
client of evdev
2/ total grab: events only go to the specific evdev client that
requested the grab.


0 is the default
mainline allows you to select 2 but not 1.
There is a patch floating around (in the above mail thread, and
in the Openmoko kernel for example) that allows you to select 1, but not 2.

It seems that the "obvious" thing to do is to interpret the argument
to EVIOCGRAB to select between these options.
We cannot use 0, 1, 2 as above - despite the fact that it makes sense
- because that would be a regression in mainline: working programs could break.
Maybe it's best to use 0, 2, 1 and document it clearly. The chance of
anyone using 2 already has got to be very close to zero.

With this patch in place, clients that are interested in purpose '1'
above would need to be changed to pass '2' to EVIOCGRAB, then (I
think) everyone would be happy.

Note that this patch causes any value other than 0, 1, 2 to return
EINVAL. I believe this is appropriate defensive practice in general
to allow for future enhancements. However it does mean that any
current code which uses a value other than 0 or 1 would break. Is
there any such code?

Note also that it is not possible to switch between '1' and '2'. A
client must always release the grab first. Such a switch could be
implemented, but there doesn't seem much point.

Signed-off-by: NeilBrown <ne...@suse.de>

---

drivers/input/evdev.c | 53 ++++++++++++++++++++++++++++++++++++++++---------
include/linux/input.h | 10 ++++++++-
2 files changed, 52 insertions(+), 11 deletions(-)


diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 2d65411..ff4169f 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -29,6 +29,7 @@ struct evdev {
struct input_handle handle;
wait_queue_head_t wait;
struct evdev_client *grab;
+ int grabcnt;
struct list_head client_list;
spinlock_t client_lock; /* protects client_list */
struct mutex mutex;
@@ -39,6 +40,7 @@ struct evdev_client {
struct input_event buffer[EVDEV_BUFFER_SIZE];
int head;
int tail;
+ int grab;
spinlock_t buffer_lock; /* protects access to buffer, head and tail */
struct fasync_struct *fasync;
struct evdev *evdev;
@@ -129,17 +131,38 @@ static void evdev_free(struct device *dev)
}

/*
+ * Grabs an underlying input device for use by evdev only.
+ */
+static int evdev_shared_grab(struct evdev *evdev, struct evdev_client *client)
+{
+ int error;
+
+ if (client->grab)
+ return -EBUSY;
+
+ if (!evdev->grabcnt) {
+ error = input_grab_device(&evdev->handle);
+ if (error)
+ return error;
+ }
+ evdev->grabcnt++;
+ client->grab = 1;
+
+ return 0;
+}
+
+/*
* Grabs an event device (along with underlying input device).
* This function is called with evdev->mutex taken.
*/
-static int evdev_grab(struct evdev *evdev, struct evdev_client *client)
+static int evdev_exclusive_grab(struct evdev *evdev, struct evdev_client *client)
{
int error;

if (evdev->grab)
return -EBUSY;

- error = input_grab_device(&evdev->handle);
+ error = evdev_shared_grab(evdev, client);
if (error)
return error;

@@ -151,12 +174,17 @@ static int evdev_grab(struct evdev *evdev, struct evdev_client *client)

static int evdev_ungrab(struct evdev *evdev, struct evdev_client *client)
{
- if (evdev->grab != client)
+ if (!client->grab)
return -EINVAL;

- rcu_assign_pointer(evdev->grab, NULL);
- synchronize_rcu();
- input_release_device(&evdev->handle);
+ if (evdev->grab == client) {
+ rcu_assign_pointer(evdev->grab, NULL);
+ synchronize_rcu();
+ }
+
+ if (!--evdev->grabcnt && evdev->exist)
+ input_release_device(&evdev->handle);
+ client->grab = 0;

return 0;
}
@@ -231,7 +259,7 @@ static int evdev_release(struct inode *inode, struct file *file)
struct evdev *evdev = client->evdev;

mutex_lock(&evdev->mutex);
- if (evdev->grab == client)
+ if (client->grab)
evdev_ungrab(evdev, client);
mutex_unlock(&evdev->mutex);

@@ -721,10 +749,15 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
return 0;

case EVIOCGRAB:
- if (p)
- return evdev_grab(evdev, client);
- else
+ switch ((unsigned long)p) {
+ case EVG_NONE:
return evdev_ungrab(evdev, client);
+ case EVG_EXCLUSIVE:
+ return evdev_exclusive_grab(evdev, client);
+ case EVG_SHARED:
+ return evdev_shared_grab(evdev, client);
+ default: return -EINVAL;
+ }

default:

diff --git a/include/linux/input.h b/include/linux/input.h
index a5802c9..d1e6a8a 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -79,7 +79,7 @@ struct input_absinfo {
#define EVIOCRMFF _IOW('E', 0x81, int) /* Erase a force effect */
#define EVIOCGEFFECTS _IOR('E', 0x84, int) /* Report number of effects playable at the same time */

-#define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device */
+#define EVIOCGRAB _IOW('E', 0x90, int) /* Grab/Release device. See GRAB types below */

/*
* Event types
@@ -108,6 +108,14 @@ struct input_absinfo {
#define SYN_CONFIG 1

/*
+ * GRAB types
+ */
+#define EVG_NONE 0 /* release any active GRAB */
+#define EVG_EXCLUSIVE 1 /* No other handler or evdev gets events */
+#define EVG_SHARED 2 /* No other handler gets events, but other
+ * evdev clients still do.
+ */
+/*
* Keys and buttons
*
* Most of the keys/buttons are modeled after USB HUT 1.12
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Dmitry Torokhov

unread,
Aug 14, 2008, 10:30:12 PM8/14/08
to
Hi Neil,

On Fri, Aug 15, 2008 at 12:02:09PM +1000, Neil Brown wrote:
>
> I'll let the comments in the patch below to most of the talking.
> This came up because I wanted to use EVIOCGRAB in some code on an
> Openmoko Freerunner, and found that EVIOCGRAG does different things on
> that kernel to elsewhere. I looked into why, and found that there was
> a good reason but that the issues hadn't been completely resolved. I
> hope to help resolve the issues so that EVIOCGRAB can behave the same
> everywhere, and still meet everybody's needs.
>
> I would have Cc:ed to Magnus Vigerlof who wrote the original patch on
> which this is based, but his Email address doesn't appear in lkml.org.
>

I don't think this is a viable solution - there are other "good"
handlers beisdes evdev, such as rfkill-input, which will still get
disabled by the "lightweight" grabs.

Overall I think it is application's responsibility to not use
multiplexing devices if they use evdev, bit I can consider adding a new
interface (maybe another ioctl) that would disable event delivery though
certain interfaces for a device.

--
Dmitry

Neil Brown

unread,
Aug 17, 2008, 9:00:15 PM8/17/08
to
On Thursday August 14, dmitry....@gmail.com wrote:
> Hi Neil,
>
> On Fri, Aug 15, 2008 at 12:02:09PM +1000, Neil Brown wrote:
> >
> > I'll let the comments in the patch below to most of the talking.
> > This came up because I wanted to use EVIOCGRAB in some code on an
> > Openmoko Freerunner, and found that EVIOCGRAG does different things on
> > that kernel to elsewhere. I looked into why, and found that there was
> > a good reason but that the issues hadn't been completely resolved. I
> > hope to help resolve the issues so that EVIOCGRAB can behave the same
> > everywhere, and still meet everybody's needs.
> >
> > I would have Cc:ed to Magnus Vigerlof who wrote the original patch on
> > which this is based, but his Email address doesn't appear in lkml.org.
> >
>
> I don't think this is a viable solution - there are other "good"
> handlers beisdes evdev, such as rfkill-input, which will still get
> disabled by the "lightweight" grabs.
>
> Overall I think it is application's responsibility to not use
> multiplexing devices if they use evdev, bit I can consider adding a new
> interface (maybe another ioctl) that would disable event delivery though
> certain interfaces for a device.

Hi,

I think you are saying that if the X server (for example) uses evdev
at all, it should use evdev exclusively for getting events, and not
use /dev/mice or whatever the equivalent is for keyboard (/dev/tty??).
But the X server still needs to know a little bit about /dev/tty to
make sure that control-C doesn't get delivered the wrong way. That's
awkward.
It also negates much of the power of the input layer (easy hot-plug).
I don't much like that approach.

I think your 'can consider' option involves the application (X
server) saying "I want to use both /dev/input/eventX and
/dev/input/mice, so I'll break the connection between those so as not
to cause problems".

I can probably work with that, though it isn't clear how to request
this break. Should we tell /dev/input/mice not to listen on (the
device served by) /dev/input/eventX, or tell (the device served by)
/dev/input/eventX not to pass events to /dev/input/mice.
The first seems to make more sense to me as /dev/input/mice is in a
position to know about devices and disconnect individual devices.
But how would you tell it, and what name would you use? Any how
would the device get back under the control of /dev/input/mice when
the X server lets go of it?

As I've been thinking through this problem I keep coming back to
/dev/input/mice and /dev/tty. They are where the issue lies. They
are both a strength and a weakness. It seems to me that the right
approach might be to special-case them.

i.e. keep the three-level grab that I proposed before (none, shared,
exclusive), but change the 'shared' option to mean
Send the events to everybody except /dev/input/mice or /dev/tty

This would be implemented by those to drivers declaring themselves as
"hotplug muxing" drivers which are only really interested in events
that no-one else wants.
There would then need to be two flavours of input_grab_device, one
which grabs it for the given handle, and one which simply disables
passing events to "hotplug muxing" drivers.

Would you be happy with that degree of special-casing? If so I can
try to put together a patch. I suspect an extra field in 'struct
input_handle' to say "this is a muxing device", then input_pass_event
checks if the device has a shared-grab and if it does, skip any
handler which is muxing.

NeilBrown

Dmitry Torokhov

unread,
Aug 18, 2008, 11:50:07 AM8/18/08
to
On Mon, Aug 18, 2008 at 10:51:33AM +1000, Neil Brown wrote:
> On Thursday August 14, dmitry....@gmail.com wrote:
> > Hi Neil,
> >
> > On Fri, Aug 15, 2008 at 12:02:09PM +1000, Neil Brown wrote:
> > >
> > > I'll let the comments in the patch below to most of the talking.
> > > This came up because I wanted to use EVIOCGRAB in some code on an
> > > Openmoko Freerunner, and found that EVIOCGRAG does different things on
> > > that kernel to elsewhere. I looked into why, and found that there was
> > > a good reason but that the issues hadn't been completely resolved. I
> > > hope to help resolve the issues so that EVIOCGRAB can behave the same
> > > everywhere, and still meet everybody's needs.
> > >
> > > I would have Cc:ed to Magnus Vigerlof who wrote the original patch on
> > > which this is based, but his Email address doesn't appear in lkml.org.
> > >
> >
> > I don't think this is a viable solution - there are other "good"
> > handlers beisdes evdev, such as rfkill-input, which will still get
> > disabled by the "lightweight" grabs.
> >
> > Overall I think it is application's responsibility to not use
> > multiplexing devices if they use evdev, bit I can consider adding a new
> > interface (maybe another ioctl) that would disable event delivery though
> > certain interfaces for a device.
>
> Hi,
>
> I think you are saying that if the X server (for example) uses evdev
> at all, it should use evdev exclusively for getting events, and not
> use /dev/mice or whatever the equivalent is for keyboard (/dev/tty??).

Not quite, but generally yes - using multiplexing devices makes the
task of filtering out duplicate events much harder so it is better not
to use them. You can use /dev/input/eventX and /dev/input/mouseX
pretty easily but /dev/input/eventX and /dev/input/mice is hard to use
together.

> But the X server still needs to know a little bit about /dev/tty to
> make sure that control-C doesn't get delivered the wrong way. That's
> awkward.

Does it need to do anything besides switching VC into the raw mode?

> It also negates much of the power of the input layer (easy hot-plug).
> I don't much like that approach.
>

I think this is the only sensible approach though. X needs to have
native hotplug capabilities anyway because of all these new mice that
have bazillion of buttons on them that PS/2 emulation simply can not
support. And once you have hotplug support in X and don't rely on
myultiplexors anymore you can remove bunch of things, like grabbing
devices in one fashion or another and you can even keep the devices
open while switching to the text mode - no need to close and reopen
them all the time.

> I think your 'can consider' option involves the application (X
> server) saying "I want to use both /dev/input/eventX and
> /dev/input/mice, so I'll break the connection between those so as not
> to cause problems".
>

Are there other applications besides X that have this kind of issue?
Hmm, GPM - hotplug scripts can probably just restart it any time they
see a mouse-like device appearing or going away.. DirectFB?

I think that the best solution is to fix applications, rather than
special case multiplexing devices like you suggested below.

Thanks.

--
Dmitry

Neil Brown

unread,
Aug 20, 2008, 8:20:13 PM8/20/08
to
On Monday August 18, dmitry....@gmail.com wrote:
> On Mon, Aug 18, 2008 at 10:51:33AM +1000, Neil Brown wrote:
> > But the X server still needs to know a little bit about /dev/tty to
> > make sure that control-C doesn't get delivered the wrong way. That's
> > awkward.
>
> Does it need to do anything besides switching VC into the raw mode?

Probably not, no.

>
> > It also negates much of the power of the input layer (easy hot-plug).
> > I don't much like that approach.
> >
>
> I think this is the only sensible approach though. X needs to have
> native hotplug capabilities anyway because of all these new mice that
> have bazillion of buttons on them that PS/2 emulation simply can not
> support. And once you have hotplug support in X and don't rely on
> myultiplexors anymore you can remove bunch of things, like grabbing
> devices in one fashion or another and you can even keep the devices
> open while switching to the text mode - no need to close and reopen
> them all the time.

So your position is that for anything non-trivial, the clever stuff in
the input layer like multiplexing and simulating PS2 mice is not
sufficiently powerful and not worth fixing, and it should all be left
to userspace. Correct?
I must confess a certain sympathy for that position, but it does make
solving my current problem harder - which dampens my enthusiasm :-)

So: The X server need to support hotplug of input devices.
Apparently there is code out there (for nearly 2 years) but it doesn't
seem to be in a release yet.
Quoting http://wiki.x.org/wiki/XInputHotplug

The X.org server supports hotplugging input devices since November
2006
http://lists.freedesktop.org/archives/xorg/2006-October/019007.html
(X11R7.2 will NOT have hotplug support yet).

So both the kernel and the X server could provide the hotplug support
required, but neither are really quite usable at the moment for very
different reasons. Sad.
Options...
First: problem description.
Device with a touchscreen wants:
- touch screen events to be delivered as absolute events to X
server.
- Other programs to be able to monitor the touch screen to
e.g. detect activity independently of whether an X server is in
use or direct fbdev access is happening (e.g. Qtopia).
- hot-plugged mice (e.g. bluetooth) should be recognised by
X server.

We could:
1/ Wait for X11 XInputHotplug to be released.
2/ Hack a little input driver for X which somehow finds
mouse devices and interprets them.
3/ Use kernel's input multiplexing and:
3a/ change EVIOCGRAB to not exclude other evdev devices
3b/ change EVIOCGRAB to optionally not exclude other evdev devices.
3c/ somehow convince mousedev never to listen on the touchscreen.

Did I miss anything?

3a is what the openmoko kernel (and presumably others) do today. But
it breaks backwards compatibility .
3b is what I suggested but you don't like.
1 does not provide a solution in a reasonable time frame
2 is likely to be very messy and error prone

So I'm now wondering about 3c.
While I can understand the value of pretending that a touch-pad looks
like a mouse, a touch-screen is a very different thing. I don't
think it would ever make sense for a touch screen to generate
relative events.
So how might we make our touchscreen appear uninteresting to
mousedev?

We could get it to return some other 'key' rather one of
BTN_TOUCH BTN_TOOL_FINGER BTN_LEFT

Maybe BTN_STYLUS. Given that the touchpad is designed for finger
usage it is a bit of a lie. But it would work.

This is really sidestepping the issue. We really want to be able to
say "This is a device where relative events are completely
meaningless". I suspect that isn't going to happen though.

tslib doesn't seem to care much about what key is sent. It only
looks for BTN_TOUCH, but all it sees is when the value goes to zero
as a "finger is removed" event. It equally gets that from
ABS_PRESSURE going to zero.

So if I just arranged for the touchscreen to deliver BTN_STYLUS
instead of BTN_TOUCH it would hide it from all the mousedev devices
(which I think is the correct thing) and mean that the Xserver
doesn't need to use EVIOCGRAB, so we don't need to break it in the
kernel.

OK, I think I can go forward with that. Thanks for listening.

NeilBrown

Dmitry Torokhov

unread,
Aug 22, 2008, 4:20:11 PM8/22/08
to
On Thu, Aug 21, 2008 at 10:10:33AM +1000, Neil Brown wrote:
> On Monday August 18, dmitry....@gmail.com wrote:
> > On Mon, Aug 18, 2008 at 10:51:33AM +1000, Neil Brown wrote:
> > > But the X server still needs to know a little bit about /dev/tty to
> > > make sure that control-C doesn't get delivered the wrong way. That's
> > > awkward.
> >
> > Does it need to do anything besides switching VC into the raw mode?
>
> Probably not, no.
>
> >
> > > It also negates much of the power of the input layer (easy hot-plug).
> > > I don't much like that approach.
> > >
> >
> > I think this is the only sensible approach though. X needs to have
> > native hotplug capabilities anyway because of all these new mice that
> > have bazillion of buttons on them that PS/2 emulation simply can not
> > support. And once you have hotplug support in X and don't rely on
> > myultiplexors anymore you can remove bunch of things, like grabbing
> > devices in one fashion or another and you can even keep the devices
> > open while switching to the text mode - no need to close and reopen
> > them all the time.
>
> So your position is that for anything non-trivial, the clever stuff in
> the input layer like multiplexing and simulating PS2 mice is not
> sufficiently powerful and not worth fixing, and it should all be left
> to userspace. Correct?

Yes, due to limitations of the PS/2 protocol itself among other
things. I am sure Vojtech will correct me but I believe he made
/dev/input/mice as a stop-gap measure until X is ready... well, it was
8 some years ago ;(

> I must confess a certain sympathy for that position, but it does make
> solving my current problem harder - which dampens my enthusiasm :-)
>

Yeah, I can understand that.

All these solutions require changes to userspace/drivers. I feel that
this time is better spent on the X itself rather on various drivers
re-implementing the same thing and stumbling over the same problems.

> While I can understand the value of pretending that a touch-pad looks
> like a mouse, a touch-screen is a very different thing. I don't
> think it would ever make sense for a touch screen to generate
> relative events.

It probaibly was a mistake to make touchscreens go through
/dev/input/mice. Again, the adea was to allow limitied use until
proper driver is installed.

> So how might we make our touchscreen appear uninteresting to
> mousedev?
>
> We could get it to return some other 'key' rather one of
> BTN_TOUCH BTN_TOOL_FINGER BTN_LEFT
>
> Maybe BTN_STYLUS. Given that the touchpad is designed for finger
> usage it is a bit of a lie. But it would work.
>
> This is really sidestepping the issue. We really want to be able to
> say "This is a device where relative events are completely
> meaningless". I suspect that isn't going to happen though.
>
> tslib doesn't seem to care much about what key is sent. It only
> looks for BTN_TOUCH, but all it sees is when the value goes to zero
> as a "finger is removed" event. It equally gets that from
> ABS_PRESSURE going to zero.
>
> So if I just arranged for the touchscreen to deliver BTN_STYLUS
> instead of BTN_TOUCH it would hide it from all the mousedev devices
> (which I think is the correct thing) and mean that the Xserver
> doesn't need to use EVIOCGRAB, so we don't need to break it in the
> kernel.
>
> OK, I think I can go forward with that. Thanks for listening.
>

Maybe openmoko could just patch mousedev to ignore touchscreens for
now, until X is fixed?

--
Dmitry

Neil Brown

unread,
Aug 24, 2008, 7:50:06 PM8/24/08
to
On Friday August 22, dmitry....@gmail.com wrote:
>
> Maybe openmoko could just patch mousedev to ignore touchscreens for
> now, until X is fixed?

Maybe. And in fact it already is!
As I delved further in to this issue, I find that not only is there a
patch in tslib to use EVIOCGRAB, but there is also a patch in the
kernel to exclude the "ABS_X + ABS_Y + BTN_TOUCH" case from mousedev,
making the EVIOCGRAB patch pointless.
And there might even be another patch in the kdrive Xserver to use
EVIOCGRAB in a different context (which might not actually be being
used, I'm not sure). A twisted maze of little passages, all
twisted. :-)

But I am firmly against carrying patches like this which cannot go
upstream. They can lead to subtle incompatibilities and can be hard
to get rid of. I much prefer a solution that can work for everyone,
and I currently think the BTN_TOUCH -> BTN_STYLUS change is that
solution. It remains to be seen which I can push into the openmoko
source tree :-)

Thanks,
NeilBrown

0 new messages