| Message ID | 1397156944-5991-1-git-send-email-dh.herrmann@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Apr 10, 2014 at 09:09:04PM +0200, David Herrmann wrote: > We currently flush input events in the outgoing client queue on most > EVIOCG* ioctls so userspace doesn't get events twice (once during the > ioctl and once during a later read()). > > We introduced this in: > commit 483180281f0ac60d1138710eb21f4b9961901294 > Author: David Herrmann <dh.herrmann@gmail.com> > Date: Sun Apr 7 21:13:19 2013 -0700 > > Input: evdev - flush queues during EVIOCGKEY-like ioctls > > However, we didn't modify the EVIOCGABS handler as we considered ABS > values to change fast enough that flushing the queue seems irrelevant. But > as it turns out, the ABS SLOT events suffer from the same issues. Hence, > also flush the input queue from ABS values on EVIOCGABS. > > Reported-by: Peter Hutterer <peter.hutterer@who-t.net> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com> > --- > Hi > > This is untested as I don't have any multitouch hardware right now. Peter, can > you give this a try? How are you planning to handle the slot-based events? We'd either need to add something similar (but more complex) to evdev_handle_mt_request or rely on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. I'd prefer the former, the latter is yet more behaviour that's easy to get wrong. Cheers, Peter > > Thanks > David > > drivers/input/evdev.c | 63 ++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 45 insertions(+), 18 deletions(-) > > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c > index a06e125..fc55c19 100644 > --- a/drivers/input/evdev.c > +++ b/drivers/input/evdev.c > @@ -55,8 +55,11 @@ struct evdev_client { > struct input_event buffer[]; > }; > > -/* flush queued events of type @type, caller must hold client->buffer_lock */ > -static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) > +/* Flush queued events of given type @type and code @code. A negative code > + * is interpreted as catch-all. Caller must hold client->buffer_lock. */ > +static void __evdev_flush_queue(struct evdev_client *client, > + unsigned int type, > + int code) > { > unsigned int i, head, num; > unsigned int mask = client->bufsize - 1; > @@ -75,7 +78,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) > ev = &client->buffer[i]; > is_report = ev->type == EV_SYN && ev->code == SYN_REPORT; > > - if (ev->type == type) { > + if (ev->type == type && (code < 0 || ev->code == code)) { > /* drop matched entry */ > continue; > } else if (is_report && !num) { > @@ -774,7 +777,7 @@ static int evdev_handle_get_val(struct evdev_client *client, > > spin_unlock(&dev->event_lock); > > - __evdev_flush_queue(client, type); > + __evdev_flush_queue(client, type, -1); > > spin_unlock_irq(&client->buffer_lock); > > @@ -787,6 +790,40 @@ static int evdev_handle_get_val(struct evdev_client *client, > return ret; > } > > +static int evdev_handle_get_abs(struct evdev_client *client, > + struct input_dev *dev, > + unsigned int code, > + unsigned int size, > + void __user *p) > +{ > + struct input_absinfo abs; > + int ret; > + > + if (!dev->absinfo) > + return -EINVAL; > + > + /* see evdev_handle_get_val() for locking order */ > + > + spin_lock_irq(&dev->event_lock); > + spin_lock(&client->buffer_lock); > + > + abs = dev->absinfo[code]; > + > + spin_unlock(&dev->event_lock); > + > + __evdev_flush_queue(client, EV_ABS, code); > + > + spin_unlock_irq(&client->buffer_lock); > + > + ret = copy_to_user(p, &abs, min_t(size_t, > + size, > + sizeof(struct input_absinfo))); > + if (ret < 0) > + evdev_queue_syn_dropped(client); > + > + return ret; > +} > + > static int evdev_handle_mt_request(struct input_dev *dev, > unsigned int size, > int __user *ip) > @@ -972,20 +1009,10 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, > _IOC_NR(cmd) & EV_MAX, size, > p, compat_mode); > > - if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) { > - > - if (!dev->absinfo) > - return -EINVAL; > - > - t = _IOC_NR(cmd) & ABS_MAX; > - abs = dev->absinfo[t]; > - > - if (copy_to_user(p, &abs, min_t(size_t, > - size, sizeof(struct input_absinfo)))) > - return -EFAULT; > - > - return 0; > - } > + if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) > + return evdev_handle_get_abs(client, dev, > + _IOC_NR(cmd) & ABS_MAX, > + size, p); > } > > if (_IOC_DIR(cmd) == _IOC_WRITE) { > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Peter On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer <peter.hutterer@who-t.net> wrote: > How are you planning to handle the slot-based events? We'd either need to > add something similar (but more complex) to evdev_handle_mt_request or rely > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. > I'd prefer the former, the latter is yet more behaviour that's easy to get > wrong. This is all racy.. We _really_ need an ioctl to receive _all_ ABS information atomically. I mean, there's no way we can know the user's state from the kernel. Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the whole ABS queue. Problem is, the user has to call the ioctl for _each_ available MT code and events might get queued in between. So yeah, this patch doesn't help much.. I have no better idea than adding a new EVIOCGABS call that retrieves ABS values for all slots atomically (and for all other axes..). No idea how to properly fix the old ioctls. Thanks David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote: > Hi Peter > > On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer > <peter.hutterer@who-t.net> wrote: > > How are you planning to handle the slot-based events? We'd either need to > > add something similar (but more complex) to evdev_handle_mt_request or rely > > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. > > I'd prefer the former, the latter is yet more behaviour that's easy to get > > wrong. > > This is all racy.. > > We _really_ need an ioctl to receive _all_ ABS information atomically. > I mean, there's no way we can know the user's state from the kernel. > Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the > whole ABS queue. Problem is, the user has to call the ioctl for _each_ > available MT code and events might get queued in between. So yeah, > this patch doesn't help much.. > > I have no better idea than adding a new EVIOCGABS call that retrieves > ABS values for all slots atomically (and for all other axes..). No > idea how to properly fix the old ioctls. bonus points for making that ioctl fetch the state of the last SYN_DROPPED and leave the events since in the client buffer. That way we can smooth over SYN_DROPPED and lose less information. Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote: > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote: > > Hi Peter > > > > On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer > > <peter.hutterer@who-t.net> wrote: > > > How are you planning to handle the slot-based events? We'd either need to > > > add something similar (but more complex) to evdev_handle_mt_request or rely > > > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. > > > I'd prefer the former, the latter is yet more behaviour that's easy to get > > > wrong. > > > > This is all racy.. > > > > We _really_ need an ioctl to receive _all_ ABS information atomically. > > I mean, there's no way we can know the user's state from the kernel. > > Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the > > whole ABS queue. Problem is, the user has to call the ioctl for _each_ > > available MT code and events might get queued in between. So yeah, > > this patch doesn't help much.. > > > > I have no better idea than adding a new EVIOCGABS call that retrieves > > ABS values for all slots atomically (and for all other axes..). No > > idea how to properly fix the old ioctls. > > bonus points for making that ioctl fetch the state of the last SYN_DROPPED > and leave the events since in the client buffer. That way we can smooth over > SYN_DROPPED and lose less information. to expand on this, something like the below would work from userspace: 1. userspace opens fd, EVIOCGBIT for everything 2. userspace calls EVIOCGABSATOMIC 3. kernel empties the event queue, flags the client as capable 4. kernel copies current device state into client-specific struct 5. kernel replies with that device state to the ioctl 6. client reads events .. 7. kernel sees a SYN_DROPPED for this client. Takes a snapshot of the device for the client, empties the buffer, leaves SYN_DROPPED in the buffer (current behaviour) 8. client reads SYN_DROPPED, calls EVIOCGABSATOMIC 9. kernel replies with the snapshot state, leaves the event buffer otherwise unmodified 10. client reads all events after SYN_DROPPED, gets a smooth continuation 11. goto 6 if the buffer overflows multiple times, repeat 7 so that the snapshot state is always the last SYN_DROPPED state. well, technically the state should be the state of the device at the first SYN_REPORT after the last SYN_DROPPED, since the current API says that interrupted event is incomplete. there are two oddities here: 1. the first ioctl will have to flush the buffer to guarantee consistent state, though you could even avoid that by taking a snapshot of the device on open(). though that comes with a disadvantage, you don't know if the client supports the new approach so you're wasting effort and memory here. 2. I'm not quite sure how to handle multiple repeated calls short of updating the client-specific snapshot with every event as it is read successfully. any comments? Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 03:38:49PM +1000, Peter Hutterer wrote: > On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote: > > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote: > > > Hi Peter > > > > > > On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer > > > <peter.hutterer@who-t.net> wrote: > > > > How are you planning to handle the slot-based events? We'd either need to > > > > add something similar (but more complex) to evdev_handle_mt_request or rely > > > > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. > > > > I'd prefer the former, the latter is yet more behaviour that's easy to get > > > > wrong. > > > > > > This is all racy.. > > > > > > We _really_ need an ioctl to receive _all_ ABS information atomically. > > > I mean, there's no way we can know the user's state from the kernel. > > > Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the > > > whole ABS queue. Problem is, the user has to call the ioctl for _each_ > > > available MT code and events might get queued in between. So yeah, > > > this patch doesn't help much.. > > > > > > I have no better idea than adding a new EVIOCGABS call that retrieves > > > ABS values for all slots atomically (and for all other axes..). No > > > idea how to properly fix the old ioctls. > > > > bonus points for making that ioctl fetch the state of the last SYN_DROPPED > > and leave the events since in the client buffer. That way we can smooth over > > SYN_DROPPED and lose less information. > > to expand on this, something like the below would work from userspace: > > 1. userspace opens fd, EVIOCGBIT for everything > 2. userspace calls EVIOCGABSATOMIC > 3. kernel empties the event queue, flags the client as capable > 4. kernel copies current device state into client-specific struct > 5. kernel replies with that device state to the ioctl > 6. client reads events > .. > 7. kernel sees a SYN_DROPPED for this client. Takes a snapshot of the device > for the client, empties the buffer, leaves SYN_DROPPED in the buffer > (current behaviour) > 8. client reads SYN_DROPPED, calls EVIOCGABSATOMIC > 9. kernel replies with the snapshot state, leaves the event buffer otherwise > unmodified > 10. client reads all events after SYN_DROPPED, gets a smooth continuation > 11. goto 6 > > if the buffer overflows multiple times, repeat 7 so that the snapshot state > is always the last SYN_DROPPED state. well, technically the state should be > the state of the device at the first SYN_REPORT after the last SYN_DROPPED, > since the current API says that interrupted event is incomplete. > > there are two oddities here: > 1. the first ioctl will have to flush the buffer to guarantee consistent state, > though you could even avoid that by taking a snapshot of the device on > open(). though that comes with a disadvantage, you don't know if the client > supports the new approach so you're wasting effort and memory here. > 2. I'm not quite sure how to handle multiple repeated calls short of > updating the client-specific snapshot with every event as it is read > successfully. > > any comments? Do we really need to optimize the case when we are dropping events? Thanks.
On Tue, Apr 22, 2014 at 10:46:47PM -0700, Dmitry Torokhov wrote: > On Wed, Apr 23, 2014 at 03:38:49PM +1000, Peter Hutterer wrote: > > On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote: > > > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote: > > > > Hi Peter > > > > > > > > On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer > > > > <peter.hutterer@who-t.net> wrote: > > > > > How are you planning to handle the slot-based events? We'd either need to > > > > > add something similar (but more complex) to evdev_handle_mt_request or rely > > > > > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. > > > > > I'd prefer the former, the latter is yet more behaviour that's easy to get > > > > > wrong. > > > > > > > > This is all racy.. > > > > > > > > We _really_ need an ioctl to receive _all_ ABS information atomically. > > > > I mean, there's no way we can know the user's state from the kernel. > > > > Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the > > > > whole ABS queue. Problem is, the user has to call the ioctl for _each_ > > > > available MT code and events might get queued in between. So yeah, > > > > this patch doesn't help much.. > > > > > > > > I have no better idea than adding a new EVIOCGABS call that retrieves > > > > ABS values for all slots atomically (and for all other axes..). No > > > > idea how to properly fix the old ioctls. > > > > > > bonus points for making that ioctl fetch the state of the last SYN_DROPPED > > > and leave the events since in the client buffer. That way we can smooth over > > > SYN_DROPPED and lose less information. > > > > to expand on this, something like the below would work from userspace: > > > > 1. userspace opens fd, EVIOCGBIT for everything > > 2. userspace calls EVIOCGABSATOMIC > > 3. kernel empties the event queue, flags the client as capable > > 4. kernel copies current device state into client-specific struct > > 5. kernel replies with that device state to the ioctl > > 6. client reads events > > .. > > 7. kernel sees a SYN_DROPPED for this client. Takes a snapshot of the device > > for the client, empties the buffer, leaves SYN_DROPPED in the buffer > > (current behaviour) > > 8. client reads SYN_DROPPED, calls EVIOCGABSATOMIC > > 9. kernel replies with the snapshot state, leaves the event buffer otherwise > > unmodified > > 10. client reads all events after SYN_DROPPED, gets a smooth continuation > > 11. goto 6 > > > > if the buffer overflows multiple times, repeat 7 so that the snapshot state > > is always the last SYN_DROPPED state. well, technically the state should be > > the state of the device at the first SYN_REPORT after the last SYN_DROPPED, > > since the current API says that interrupted event is incomplete. > > > > there are two oddities here: > > 1. the first ioctl will have to flush the buffer to guarantee consistent state, > > though you could even avoid that by taking a snapshot of the device on > > open(). though that comes with a disadvantage, you don't know if the client > > supports the new approach so you're wasting effort and memory here. > > 2. I'm not quite sure how to handle multiple repeated calls short of > > updating the client-specific snapshot with every event as it is read > > successfully. > > > > any comments? > > Do we really need to optimize the case when we are dropping events? It happens frequently, to the point where on some laptops you're pretty much guaranteed to get SYN_DROPPED events on resume and sometimes even during normal multi-finger user. I don't have any measurements on how many events are dropped on average. Could be one or two, could be several buffer sizes, I honestly don't know. Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 03:55:28PM +1000, Peter Hutterer wrote: > On Tue, Apr 22, 2014 at 10:46:47PM -0700, Dmitry Torokhov wrote: > > On Wed, Apr 23, 2014 at 03:38:49PM +1000, Peter Hutterer wrote: > > > On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote: > > > > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote: > > > > > Hi Peter > > > > > > > > > > On Tue, Apr 22, 2014 at 6:15 AM, Peter Hutterer > > > > > <peter.hutterer@who-t.net> wrote: > > > > > > How are you planning to handle the slot-based events? We'd either need to > > > > > > add something similar (but more complex) to evdev_handle_mt_request or rely > > > > > > on the caller to call the whole EV_ABS range and ditch anything ABS_MT_. > > > > > > I'd prefer the former, the latter is yet more behaviour that's easy to get > > > > > > wrong. > > > > > > > > > > This is all racy.. > > > > > > > > > > We _really_ need an ioctl to receive _all_ ABS information atomically. > > > > > I mean, there's no way we can know the user's state from the kernel. > > > > > Even if the user resyncs via EVIOCGMTSLOTS, we can never flush the > > > > > whole ABS queue. Problem is, the user has to call the ioctl for _each_ > > > > > available MT code and events might get queued in between. So yeah, > > > > > this patch doesn't help much.. > > > > > > > > > > I have no better idea than adding a new EVIOCGABS call that retrieves > > > > > ABS values for all slots atomically (and for all other axes..). No > > > > > idea how to properly fix the old ioctls. > > > > > > > > bonus points for making that ioctl fetch the state of the last SYN_DROPPED > > > > and leave the events since in the client buffer. That way we can smooth over > > > > SYN_DROPPED and lose less information. > > > > > > to expand on this, something like the below would work from userspace: > > > > > > 1. userspace opens fd, EVIOCGBIT for everything > > > 2. userspace calls EVIOCGABSATOMIC > > > 3. kernel empties the event queue, flags the client as capable > > > 4. kernel copies current device state into client-specific struct > > > 5. kernel replies with that device state to the ioctl > > > 6. client reads events > > > .. > > > 7. kernel sees a SYN_DROPPED for this client. Takes a snapshot of the device > > > for the client, empties the buffer, leaves SYN_DROPPED in the buffer > > > (current behaviour) > > > 8. client reads SYN_DROPPED, calls EVIOCGABSATOMIC > > > 9. kernel replies with the snapshot state, leaves the event buffer otherwise > > > unmodified > > > 10. client reads all events after SYN_DROPPED, gets a smooth continuation > > > 11. goto 6 > > > > > > if the buffer overflows multiple times, repeat 7 so that the snapshot state > > > is always the last SYN_DROPPED state. well, technically the state should be > > > the state of the device at the first SYN_REPORT after the last SYN_DROPPED, > > > since the current API says that interrupted event is incomplete. > > > > > > there are two oddities here: > > > 1. the first ioctl will have to flush the buffer to guarantee consistent state, > > > though you could even avoid that by taking a snapshot of the device on > > > open(). though that comes with a disadvantage, you don't know if the client > > > supports the new approach so you're wasting effort and memory here. > > > 2. I'm not quite sure how to handle multiple repeated calls short of > > > updating the client-specific snapshot with every event as it is read > > > successfully. > > > > > > any comments? > > > > Do we really need to optimize the case when we are dropping events? > > It happens frequently, to the point where on some laptops you're pretty much > guaranteed to get SYN_DROPPED events on resume and sometimes even during > normal multi-finger user. > > I don't have any measurements on how many events are dropped on average. > Could be one or two, could be several buffer sizes, I honestly don't know. I think we need to figure this out. The idea is that dropping events should be an exception, not a rule.
On Tue, Apr 22, 2014 at 11:07:47PM -0700, Dmitry Torokhov wrote: > On Wed, Apr 23, 2014 at 03:55:28PM +1000, Peter Hutterer wrote: > > On Tue, Apr 22, 2014 at 10:46:47PM -0700, Dmitry Torokhov wrote: > > > On Wed, Apr 23, 2014 at 03:38:49PM +1000, Peter Hutterer wrote: > > > > On Wed, Apr 23, 2014 at 10:21:03AM +1000, Peter Hutterer wrote: > > > > > On Tue, Apr 22, 2014 at 08:21:54AM +0200, David Herrmann wrote: .. > > > > > > > > any comments? > > > > > > Do we really need to optimize the case when we are dropping events? > > > > It happens frequently, to the point where on some laptops you're pretty much > > guaranteed to get SYN_DROPPED events on resume and sometimes even during > > normal multi-finger user. > > > > I don't have any measurements on how many events are dropped on average. > > Could be one or two, could be several buffer sizes, I honestly don't know. > > I think we need to figure this out. The idea is that dropping events > should be an exception, not a rule. increase the buffer size for the devices I guess, with some heuristics maybe. you could dynamically grow the buffer in the kernel, if the buffer gets full or close to full, grow it. but really, this is a moving target, eventually you will get the SYN_DROPPED. if a client sleeps for a second or more, the events from a second ago are likely useless anyway, so the need for an atomic sync exists regardless. fwiw, I can live with a atomic sync that clears the client buffer provided I get the correct state. any optimisation on top of that can be done afterwards. Cheers, Peter -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index a06e125..fc55c19 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -55,8 +55,11 @@ struct evdev_client { struct input_event buffer[]; }; -/* flush queued events of type @type, caller must hold client->buffer_lock */ -static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) +/* Flush queued events of given type @type and code @code. A negative code + * is interpreted as catch-all. Caller must hold client->buffer_lock. */ +static void __evdev_flush_queue(struct evdev_client *client, + unsigned int type, + int code) { unsigned int i, head, num; unsigned int mask = client->bufsize - 1; @@ -75,7 +78,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type) ev = &client->buffer[i]; is_report = ev->type == EV_SYN && ev->code == SYN_REPORT; - if (ev->type == type) { + if (ev->type == type && (code < 0 || ev->code == code)) { /* drop matched entry */ continue; } else if (is_report && !num) { @@ -774,7 +777,7 @@ static int evdev_handle_get_val(struct evdev_client *client, spin_unlock(&dev->event_lock); - __evdev_flush_queue(client, type); + __evdev_flush_queue(client, type, -1); spin_unlock_irq(&client->buffer_lock); @@ -787,6 +790,40 @@ static int evdev_handle_get_val(struct evdev_client *client, return ret; } +static int evdev_handle_get_abs(struct evdev_client *client, + struct input_dev *dev, + unsigned int code, + unsigned int size, + void __user *p) +{ + struct input_absinfo abs; + int ret; + + if (!dev->absinfo) + return -EINVAL; + + /* see evdev_handle_get_val() for locking order */ + + spin_lock_irq(&dev->event_lock); + spin_lock(&client->buffer_lock); + + abs = dev->absinfo[code]; + + spin_unlock(&dev->event_lock); + + __evdev_flush_queue(client, EV_ABS, code); + + spin_unlock_irq(&client->buffer_lock); + + ret = copy_to_user(p, &abs, min_t(size_t, + size, + sizeof(struct input_absinfo))); + if (ret < 0) + evdev_queue_syn_dropped(client); + + return ret; +} + static int evdev_handle_mt_request(struct input_dev *dev, unsigned int size, int __user *ip) @@ -972,20 +1009,10 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd, _IOC_NR(cmd) & EV_MAX, size, p, compat_mode); - if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) { - - if (!dev->absinfo) - return -EINVAL; - - t = _IOC_NR(cmd) & ABS_MAX; - abs = dev->absinfo[t]; - - if (copy_to_user(p, &abs, min_t(size_t, - size, sizeof(struct input_absinfo)))) - return -EFAULT; - - return 0; - } + if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) + return evdev_handle_get_abs(client, dev, + _IOC_NR(cmd) & ABS_MAX, + size, p); } if (_IOC_DIR(cmd) == _IOC_WRITE) {
We currently flush input events in the outgoing client queue on most EVIOCG* ioctls so userspace doesn't get events twice (once during the ioctl and once during a later read()). We introduced this in: commit 483180281f0ac60d1138710eb21f4b9961901294 Author: David Herrmann <dh.herrmann@gmail.com> Date: Sun Apr 7 21:13:19 2013 -0700 Input: evdev - flush queues during EVIOCGKEY-like ioctls However, we didn't modify the EVIOCGABS handler as we considered ABS values to change fast enough that flushing the queue seems irrelevant. But as it turns out, the ABS SLOT events suffer from the same issues. Hence, also flush the input queue from ABS values on EVIOCGABS. Reported-by: Peter Hutterer <peter.hutterer@who-t.net> Signed-off-by: David Herrmann <dh.herrmann@gmail.com> --- Hi This is untested as I don't have any multitouch hardware right now. Peter, can you give this a try? Thanks David drivers/input/evdev.c | 63 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 18 deletions(-)