| Message ID | 20110404213338.GB984@core.coreip.homeip.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9fb0f14e31b6101a0cc69a333b43541044f9b0a6 |
| Headers | show |
Hi Dmitry, On Mon, Apr 4, 2011 at 2:33 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Fri, Apr 01, 2011 at 11:54:18PM -0700, Jeff Brown wrote: >> + * When the client buffer is full, drain the buffer and enqueue a >> + * SYN_DROPPED event to let the client know that events were dropped >> + * and the last packet was incomplete. We then consume all remaining >> + * events from the dropped packet until the next packet begins. > > I do not think we (kernel) should be doing this. Clients should take > care and not allow overruns but if they happen the pain should be on the > client to restore the context. It has to query the device to get its > current state and it can drop the events till next sync as well. That's fine. We should document SYN_DROPPED and mention that the client should wait until after the next SYN_REPORT to get back in sync. > Therefore I intend to apply the patch in the form below. Looks good to me. It's more like my original patch. Jeff. -- 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
> Input: evdev - indicate buffer overrun with SYN_DROPPED > > From: Jeff Brown <jeffbrown@google.com> > > Add a new EV_SYN code, SYN_DROPPED, to inform the client when input > events have been dropped from the evdev input buffer due to a > buffer overrun. The client should use this event as a hint to > reset its state or ignore all following events until the next > packet begins. > > Signed-off-by: Jeff Brown <jeffbrown@android.com> > [dtor@mail.ru: Implement Henrik's suggestion and drop old events in > case of overflow.] > Signed-off-by: Dmitry Torokhov <dtor@mail.ru> > --- > > Documentation/input/event-codes.txt | 6 ++++++ > drivers/input/evdev.c | 33 +++++++++++++++++++++------------ > include/linux/input.h | 1 + > 3 files changed, 28 insertions(+), 12 deletions(-) > > > diff --git a/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt > index d6732a4..e0326a0 100644 > --- a/Documentation/input/event-codes.txt > +++ b/Documentation/input/event-codes.txt > @@ -79,6 +79,12 @@ defined only by when they are sent in the evdev event stream. > - Used to synchronize and separate touch events. See the > multi-touch-protocol.txt document for more information. > > +* SYN_DROPPED: > + - Used to indicate buffer overrun in the evdev client's event queue. > + Client should ignore all events up to and including next SYN_REPORT > + event and query the device (using EVIOCG* ioctls) to obtain its > + current state. > + > EV_KEY: > ---------- > EV_KEY events take the form KEY_<name> or BTN_<name>. For example, KEY_A is used > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c > index 7f42d3a..f1c0910 100644 > --- a/drivers/input/evdev.c > +++ b/drivers/input/evdev.c > @@ -39,13 +39,13 @@ struct evdev { > }; > > struct evdev_client { > - int head; > - int tail; > + unsigned int head; > + unsigned int tail; > spinlock_t buffer_lock; /* protects access to buffer, head and tail */ > struct fasync_struct *fasync; > struct evdev *evdev; > struct list_head node; > - int bufsize; > + unsigned int bufsize; > struct input_event buffer[]; > }; > > @@ -55,16 +55,25 @@ static DEFINE_MUTEX(evdev_table_mutex); > static void evdev_pass_event(struct evdev_client *client, > struct input_event *event) > { > - /* > - * Interrupts are disabled, just acquire the lock. > - * Make sure we don't leave with the client buffer > - * "empty" by having client->head == client->tail. > - */ > + /* Interrupts are disabled, just acquire the lock. */ > spin_lock(&client->buffer_lock); > - do { > - client->buffer[client->head++] = *event; > - client->head &= client->bufsize - 1; > - } while (client->head == client->tail); > + > + client->buffer[client->head++] = *event; > + client->head &= client->bufsize - 1; > + > + if (unlikely(client->head == client->tail)) { > + /* > + * This effectively "drops" all unconsumed events, leaving > + * EV_SYN/SYN_DROPPED plus the newest event in the queue. > + */ > + client->tail = (client->head - 2) & (client->bufsize - 1); > + > + client->buffer[client->tail].time = event->time; > + client->buffer[client->tail].type = EV_SYN; > + client->buffer[client->tail].code = SYN_DROPPED; > + client->buffer[client->tail].value = 0; > + } > + > spin_unlock(&client->buffer_lock); > > if (event->type == EV_SYN) > diff --git a/include/linux/input.h b/include/linux/input.h > index f3a7794..71d3651 100644 > --- a/include/linux/input.h > +++ b/include/linux/input.h > @@ -167,6 +167,7 @@ struct input_keymap_entry { > #define SYN_REPORT 0 > #define SYN_CONFIG 1 > #define SYN_MT_REPORT 2 > +#define SYN_DROPPED 3 > > /* > * Keys and buttons Acked-by: Henrik Rydberg <rydberg@euromail.se> Thanks, Henrik -- 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/Documentation/input/event-codes.txt b/Documentation/input/event-codes.txt index d6732a4..e0326a0 100644 --- a/Documentation/input/event-codes.txt +++ b/Documentation/input/event-codes.txt @@ -79,6 +79,12 @@ defined only by when they are sent in the evdev event stream. - Used to synchronize and separate touch events. See the multi-touch-protocol.txt document for more information. +* SYN_DROPPED: + - Used to indicate buffer overrun in the evdev client's event queue. + Client should ignore all events up to and including next SYN_REPORT + event and query the device (using EVIOCG* ioctls) to obtain its + current state. + EV_KEY: ---------- EV_KEY events take the form KEY_<name> or BTN_<name>. For example, KEY_A is used diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index 7f42d3a..f1c0910 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -39,13 +39,13 @@ struct evdev { }; struct evdev_client { - int head; - int tail; + unsigned int head; + unsigned int tail; spinlock_t buffer_lock; /* protects access to buffer, head and tail */ struct fasync_struct *fasync; struct evdev *evdev; struct list_head node; - int bufsize; + unsigned int bufsize; struct input_event buffer[]; }; @@ -55,16 +55,25 @@ static DEFINE_MUTEX(evdev_table_mutex); static void evdev_pass_event(struct evdev_client *client, struct input_event *event) { - /* - * Interrupts are disabled, just acquire the lock. - * Make sure we don't leave with the client buffer - * "empty" by having client->head == client->tail. - */ + /* Interrupts are disabled, just acquire the lock. */ spin_lock(&client->buffer_lock); - do { - client->buffer[client->head++] = *event; - client->head &= client->bufsize - 1; - } while (client->head == client->tail); + + client->buffer[client->head++] = *event; + client->head &= client->bufsize - 1; + + if (unlikely(client->head == client->tail)) { + /* + * This effectively "drops" all unconsumed events, leaving + * EV_SYN/SYN_DROPPED plus the newest event in the queue. + */ + client->tail = (client->head - 2) & (client->bufsize - 1); + + client->buffer[client->tail].time = event->time; + client->buffer[client->tail].type = EV_SYN; + client->buffer[client->tail].code = SYN_DROPPED; + client->buffer[client->tail].value = 0; + } + spin_unlock(&client->buffer_lock); if (event->type == EV_SYN) diff --git a/include/linux/input.h b/include/linux/input.h index f3a7794..71d3651 100644 --- a/include/linux/input.h +++ b/include/linux/input.h @@ -167,6 +167,7 @@ struct input_keymap_entry { #define SYN_REPORT 0 #define SYN_CONFIG 1 #define SYN_MT_REPORT 2 +#define SYN_DROPPED 3 /* * Keys and buttons