Hi Henrik,
On Sun, Jun 20, 2010 at 08:48:54PM +0200, Henrik Rydberg wrote:
> @@ -149,11 +148,29 @@ static int evdev_grab(struct evdev *evdev, struct evdev_client *client)
>
> static int evdev_ungrab(struct evdev *evdev, struct evdev_client *client)
> {
> + struct input_dev *dev = evdev->handle.dev;
> + int head, tail;
> +
> if (evdev->grab != client)
> return -EINVAL;
>
> + spin_lock_irq(&dev->event_lock);
> +
> + head = client->head;
> + tail = client->tail;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(client, &evdev->client_list, node) {
> + client->head = head;
> + client->tail = tail;
> + }
> + rcu_read_unlock();
> +
> rcu_assign_pointer(evdev->grab, NULL);
> synchronize_rcu();
You may not call synchronize_rcu() while holding spinlock and with
interrupts off.
> +
> + spin_unlock_irq(&dev->event_lock);
> +
> input_release_device(&evdev->handle);
>
> return 0;
> @@ -162,6 +179,7 @@ static int evdev_ungrab(struct evdev *evdev, struct evdev_client *client)
> static void evdev_attach_client(struct evdev *evdev,
> struct evdev_client *client)
> {
> + client->head = client->tail = evdev->head;
> spin_lock(&evdev->client_lock);
So what happens if we advance evdev->head here but before we added the
new client to the list? I guess we'll have to wait till next event and
then client will read both of them...
Overall I am starting getting concerned about proper isolation between
clients. Right now, if one client stops reading events and another one
issues grab then the first client will only get events that were
accumulated before grab tookm place. With the new shared buffer the
first client may get "grabbed" events if it stop for long enough for
buffer to wrap around.
Do we really same that much memory here? We normally do not have that
many users connected to event devices at once...
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
