* [PATCH] [v2]Input: evdev - drop partial events after emptying the buffer
@ 2016-01-04 16:28 Aniroop Mathur
2016-01-08 5:04 ` Peter Hutterer
0 siblings, 1 reply; 4+ messages in thread
From: Aniroop Mathur @ 2016-01-04 16:28 UTC (permalink / raw)
To: dmitry.torokhov, benjamin.tissoires, peter.hutterer
Cc: dh.herrmann, rydberg, aniroop.mathur, linux-input, linux-kernel,
Aniroop Mathur
This patch introduces concept to drop partial events in evdev handler
itself after emptying the buffer which are dropped by all evdev
clients in userspace after SYN_DROPPED occurs.
This in turn saves space of partial events in evdev handler buffer
and reduces evdev client reading requests.
Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
---
drivers/input/evdev.c | 56 ++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 47 insertions(+), 9 deletions(-)
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e9ae3d5..0a5ead8 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -58,6 +58,7 @@ struct evdev_client {
struct list_head node;
unsigned int clk_type;
bool revoked;
+ bool drop_pevent; /* specifies whether partial events need to be dropped */
unsigned long *evmasks[EV_CNT];
unsigned int bufsize;
struct input_event buffer[];
@@ -192,6 +193,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
{
unsigned long flags;
unsigned int clk_type;
+ struct input_event ev;
switch (clkid) {
@@ -218,8 +220,26 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
spin_lock_irqsave(&client->buffer_lock, flags);
if (client->head != client->tail) {
- client->packet_head = client->head = client->tail;
+
+ /* Store previous event */
+ client->head--;
+ client->head &= client->bufsize - 1;
+ ev = client->buffer[client->head];
+
+ client->packet_head = client->head = client->tail = 0;
__evdev_queue_syn_dropped(client);
+
+ /*
+ * If last packet is fully stored, queue SYN_REPORT
+ * so that clients do not ignore next full packet.
+ * And if last packet is NOT fully stored,
+ * set drop_pevent to true to ignore partial events.
+ */
+ if (ev.type == EV_SYN && ev.code == SYN_REPORT) {
+ ev.time = client->buffer[client->head - 1].time;
+ client->buffer[client->head++] = ev;
+ } else if (ev.type != EV_SYN && ev.code != SYN_REPORT)
+ client->drop_pevent = true;
}
spin_unlock_irqrestore(&client->buffer_lock, flags);
@@ -236,17 +256,27 @@ static void __pass_event(struct evdev_client *client,
if (unlikely(client->head == client->tail)) {
/*
- * This effectively "drops" all unconsumed events, leaving
- * EV_SYN/SYN_DROPPED plus the newest event in the queue.
+ * This effectively "drops" all unconsumed events, storing
+ * EV_SYN/SYN_DROPPED and the newest event in the queue but
+ * only if it is EV_SYN/SYN_REPORT so that clients can read
+ * the next full event packet. And set drop_pevent to true
+ * if last event packet is not stored completely in buffer
+ * to ignore upcoming partial events.
*/
- 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;
+ client->tail = client->head = client->packet_head;
- client->packet_head = client->tail;
+ client->buffer[client->head].time = event->time;
+ client->buffer[client->head].type = EV_SYN;
+ client->buffer[client->head].code = SYN_DROPPED;
+ client->buffer[client->head].value = 0;
+ client->head = (client->head + 1) & (client->bufsize -1);
+
+ if (event->type == EV_SYN && event->code == SYN_REPORT) {
+ client->buffer[client->head++] = *event;
+ client->head &= client->bufsize - 1;
+ } else if (event->type != EV_SYN && event->code != SYN_REPORT)
+ client->drop_pevent = true;
}
if (event->type == EV_SYN && event->code == SYN_REPORT) {
@@ -284,6 +314,14 @@ static void evdev_pass_values(struct evdev_client *client,
wakeup = true;
}
+ /* drop partial events of last packet but queue SYN_REPORT */
+ if (client->drop_pevent) {
+ if (v->type == EV_SYN && v->code == SYN_REPORT)
+ client->drop_pevent = false;
+ else
+ continue;
+ }
+
event.type = v->type;
event.code = v->code;
event.value = v->value;
--
2.6.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] [v2]Input: evdev - drop partial events after emptying the buffer
2016-01-04 16:28 [PATCH] [v2]Input: evdev - drop partial events after emptying the buffer Aniroop Mathur
@ 2016-01-08 5:04 ` Peter Hutterer
2016-01-08 5:46 ` Aniroop Mathur
0 siblings, 1 reply; 4+ messages in thread
From: Peter Hutterer @ 2016-01-08 5:04 UTC (permalink / raw)
To: Aniroop Mathur
Cc: dmitry.torokhov, benjamin.tissoires, dh.herrmann, rydberg,
aniroop.mathur, linux-input, linux-kernel
On Mon, Jan 04, 2016 at 09:58:48PM +0530, Aniroop Mathur wrote:
> This patch introduces concept to drop partial events in evdev handler
> itself after emptying the buffer which are dropped by all evdev
> clients in userspace after SYN_DROPPED occurs.
> This in turn saves space of partial events in evdev handler buffer
> and reduces evdev client reading requests.
>
> Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
> ---
> drivers/input/evdev.c | 56 ++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 47 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index e9ae3d5..0a5ead8 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -58,6 +58,7 @@ struct evdev_client {
> struct list_head node;
> unsigned int clk_type;
> bool revoked;
> + bool drop_pevent; /* specifies whether partial events need to be dropped */
> unsigned long *evmasks[EV_CNT];
> unsigned int bufsize;
> struct input_event buffer[];
> @@ -192,6 +193,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
> {
> unsigned long flags;
> unsigned int clk_type;
> + struct input_event ev;
>
> switch (clkid) {
>
> @@ -218,8 +220,26 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
> spin_lock_irqsave(&client->buffer_lock, flags);
>
> if (client->head != client->tail) {
> - client->packet_head = client->head = client->tail;
> +
> + /* Store previous event */
> + client->head--;
> + client->head &= client->bufsize - 1;
> + ev = client->buffer[client->head];
> +
> + client->packet_head = client->head = client->tail = 0;
> __evdev_queue_syn_dropped(client);
> +
> + /*
> + * If last packet is fully stored, queue SYN_REPORT
> + * so that clients do not ignore next full packet.
> + * And if last packet is NOT fully stored,
> + * set drop_pevent to true to ignore partial events.
> + */
> + if (ev.type == EV_SYN && ev.code == SYN_REPORT) {
> + ev.time = client->buffer[client->head - 1].time;
if client->head happens to be 0, your index won't be what it should be :)
you need the &= here and again after the next line:
> + client->buffer[client->head++] = ev;
> + } else if (ev.type != EV_SYN && ev.code != SYN_REPORT)
> + client->drop_pevent = true;
> }
>
> spin_unlock_irqrestore(&client->buffer_lock, flags);
> @@ -236,17 +256,27 @@ static void __pass_event(struct evdev_client *client,
>
> if (unlikely(client->head == client->tail)) {
> /*
> - * This effectively "drops" all unconsumed events, leaving
> - * EV_SYN/SYN_DROPPED plus the newest event in the queue.
> + * This effectively "drops" all unconsumed events, storing
> + * EV_SYN/SYN_DROPPED and the newest event in the queue but
> + * only if it is EV_SYN/SYN_REPORT so that clients can read
> + * the next full event packet. And set drop_pevent to true
> + * if last event packet is not stored completely in buffer
> + * to ignore upcoming partial events.
> */
> - 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;
> + client->tail = client->head = client->packet_head;
>
> - client->packet_head = client->tail;
> + client->buffer[client->head].time = event->time;
> + client->buffer[client->head].type = EV_SYN;
> + client->buffer[client->head].code = SYN_DROPPED;
> + client->buffer[client->head].value = 0;
> + client->head = (client->head + 1) & (client->bufsize -1);
> +
> + if (event->type == EV_SYN && event->code == SYN_REPORT) {
> + client->buffer[client->head++] = *event;
> + client->head &= client->bufsize - 1;
> + } else if (event->type != EV_SYN && event->code != SYN_REPORT)
> + client->drop_pevent = true;
given that this condition is almost exactly the same as above, you should
share the code. you should also look into checking whether this can be moved
to __evdev_queue_syn_dropped() anyway, it seems like it can with little
effort.
The protocol itself looks correct now though.
Cheers,
Peter
> }
>
> if (event->type == EV_SYN && event->code == SYN_REPORT) {
> @@ -284,6 +314,14 @@ static void evdev_pass_values(struct evdev_client *client,
> wakeup = true;
> }
>
> + /* drop partial events of last packet but queue SYN_REPORT */
> + if (client->drop_pevent) {
> + if (v->type == EV_SYN && v->code == SYN_REPORT)
> + client->drop_pevent = false;
> + else
> + continue;
> + }
> +
> event.type = v->type;
> event.code = v->code;
> event.value = v->value;
> --
> 2.6.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [v2]Input: evdev - drop partial events after emptying the buffer
2016-01-08 5:04 ` Peter Hutterer
@ 2016-01-08 5:46 ` Aniroop Mathur
2016-01-08 5:52 ` Peter Hutterer
0 siblings, 1 reply; 4+ messages in thread
From: Aniroop Mathur @ 2016-01-08 5:46 UTC (permalink / raw)
To: Peter Hutterer
Cc: Dmitry Torokhov, Benjamin Tissoires, David Herrmann,
Henrik Rydberg, linux-input, linux-kernel, Aniroop Mathur
On Fri, Jan 8, 2016 at 10:34 AM, Peter Hutterer
<peter.hutterer@who-t.net> wrote:
> On Mon, Jan 04, 2016 at 09:58:48PM +0530, Aniroop Mathur wrote:
>> This patch introduces concept to drop partial events in evdev handler
>> itself after emptying the buffer which are dropped by all evdev
>> clients in userspace after SYN_DROPPED occurs.
>> This in turn saves space of partial events in evdev handler buffer
>> and reduces evdev client reading requests.
>>
>> Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
>> ---
>> drivers/input/evdev.c | 56 ++++++++++++++++++++++++++++++++++++++++++---------
>> 1 file changed, 47 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>> index e9ae3d5..0a5ead8 100644
>> --- a/drivers/input/evdev.c
>> +++ b/drivers/input/evdev.c
>> @@ -58,6 +58,7 @@ struct evdev_client {
>> struct list_head node;
>> unsigned int clk_type;
>> bool revoked;
>> + bool drop_pevent; /* specifies whether partial events need to be dropped */
>> unsigned long *evmasks[EV_CNT];
>> unsigned int bufsize;
>> struct input_event buffer[];
>> @@ -192,6 +193,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
>> {
>> unsigned long flags;
>> unsigned int clk_type;
>> + struct input_event ev;
>>
>> switch (clkid) {
>>
>> @@ -218,8 +220,26 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
>> spin_lock_irqsave(&client->buffer_lock, flags);
>>
>> if (client->head != client->tail) {
>> - client->packet_head = client->head = client->tail;
>> +
>> + /* Store previous event */
>> + client->head--;
>> + client->head &= client->bufsize - 1;
>> + ev = client->buffer[client->head];
>> +
>> + client->packet_head = client->head = client->tail = 0;
>> __evdev_queue_syn_dropped(client);
>> +
>> + /*
>> + * If last packet is fully stored, queue SYN_REPORT
>> + * so that clients do not ignore next full packet.
>> + * And if last packet is NOT fully stored,
>> + * set drop_pevent to true to ignore partial events.
>> + */
>> + if (ev.type == EV_SYN && ev.code == SYN_REPORT) {
>> + ev.time = client->buffer[client->head - 1].time;
>
> if client->head happens to be 0, your index won't be what it should be :)
> you need the &= here and again after the next line:
>
I have set head to 0 after which syn_dropped is queued,
so head will be 1 always and hence it seems no need to take
care of head overflow here as min bufsize is 64.
>> + client->buffer[client->head++] = ev;
>
>
>
>> + } else if (ev.type != EV_SYN && ev.code != SYN_REPORT)
>> + client->drop_pevent = true;
>> }
>>
>> spin_unlock_irqrestore(&client->buffer_lock, flags);
>> @@ -236,17 +256,27 @@ static void __pass_event(struct evdev_client *client,
>>
>> if (unlikely(client->head == client->tail)) {
>> /*
>> - * This effectively "drops" all unconsumed events, leaving
>> - * EV_SYN/SYN_DROPPED plus the newest event in the queue.
>> + * This effectively "drops" all unconsumed events, storing
>> + * EV_SYN/SYN_DROPPED and the newest event in the queue but
>> + * only if it is EV_SYN/SYN_REPORT so that clients can read
>> + * the next full event packet. And set drop_pevent to true
>> + * if last event packet is not stored completely in buffer
>> + * to ignore upcoming partial events.
>> */
>> - 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;
>> + client->tail = client->head = client->packet_head;
>>
>> - client->packet_head = client->tail;
>> + client->buffer[client->head].time = event->time;
>> + client->buffer[client->head].type = EV_SYN;
>> + client->buffer[client->head].code = SYN_DROPPED;
>> + client->buffer[client->head].value = 0;
>> + client->head = (client->head + 1) & (client->bufsize -1);
>> +
>> + if (event->type == EV_SYN && event->code == SYN_REPORT) {
>> + client->buffer[client->head++] = *event;
>> + client->head &= client->bufsize - 1;
>> + } else if (event->type != EV_SYN && event->code != SYN_REPORT)
>> + client->drop_pevent = true;
>
> given that this condition is almost exactly the same as above, you should
> share the code. you should also look into checking whether this can be moved
> to __evdev_queue_syn_dropped() anyway, it seems like it can with little
> effort.
>
> The protocol itself looks correct now though.
>
Thank you Mr. Peter.
I have already sent the v4 which moved the code to
__evdev_queue_syn_dropped() function. Please have a look.
Regards,
Aniroop
> Cheers,
> Peter
>
>> }
>>
>> if (event->type == EV_SYN && event->code == SYN_REPORT) {
>> @@ -284,6 +314,14 @@ static void evdev_pass_values(struct evdev_client *client,
>> wakeup = true;
>> }
>>
>> + /* drop partial events of last packet but queue SYN_REPORT */
>> + if (client->drop_pevent) {
>> + if (v->type == EV_SYN && v->code == SYN_REPORT)
>> + client->drop_pevent = false;
>> + else
>> + continue;
>> + }
>> +
>> event.type = v->type;
>> event.code = v->code;
>> event.value = v->value;
>> --
>> 2.6.2
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] [v2]Input: evdev - drop partial events after emptying the buffer
2016-01-08 5:46 ` Aniroop Mathur
@ 2016-01-08 5:52 ` Peter Hutterer
0 siblings, 0 replies; 4+ messages in thread
From: Peter Hutterer @ 2016-01-08 5:52 UTC (permalink / raw)
To: Aniroop Mathur
Cc: Dmitry Torokhov, Benjamin Tissoires, David Herrmann,
Henrik Rydberg, linux-input, linux-kernel, Aniroop Mathur
On Fri, Jan 08, 2016 at 11:16:49AM +0530, Aniroop Mathur wrote:
> On Fri, Jan 8, 2016 at 10:34 AM, Peter Hutterer
> <peter.hutterer@who-t.net> wrote:
> > On Mon, Jan 04, 2016 at 09:58:48PM +0530, Aniroop Mathur wrote:
> >> This patch introduces concept to drop partial events in evdev handler
> >> itself after emptying the buffer which are dropped by all evdev
> >> clients in userspace after SYN_DROPPED occurs.
> >> This in turn saves space of partial events in evdev handler buffer
> >> and reduces evdev client reading requests.
> >>
> >> Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
> >> ---
> >> drivers/input/evdev.c | 56 ++++++++++++++++++++++++++++++++++++++++++---------
> >> 1 file changed, 47 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> >> index e9ae3d5..0a5ead8 100644
> >> --- a/drivers/input/evdev.c
> >> +++ b/drivers/input/evdev.c
> >> @@ -58,6 +58,7 @@ struct evdev_client {
> >> struct list_head node;
> >> unsigned int clk_type;
> >> bool revoked;
> >> + bool drop_pevent; /* specifies whether partial events need to be dropped */
> >> unsigned long *evmasks[EV_CNT];
> >> unsigned int bufsize;
> >> struct input_event buffer[];
> >> @@ -192,6 +193,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
> >> {
> >> unsigned long flags;
> >> unsigned int clk_type;
> >> + struct input_event ev;
> >>
> >> switch (clkid) {
> >>
> >> @@ -218,8 +220,26 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
> >> spin_lock_irqsave(&client->buffer_lock, flags);
> >>
> >> if (client->head != client->tail) {
> >> - client->packet_head = client->head = client->tail;
> >> +
> >> + /* Store previous event */
> >> + client->head--;
> >> + client->head &= client->bufsize - 1;
> >> + ev = client->buffer[client->head];
> >> +
> >> + client->packet_head = client->head = client->tail = 0;
> >> __evdev_queue_syn_dropped(client);
> >> +
> >> + /*
> >> + * If last packet is fully stored, queue SYN_REPORT
> >> + * so that clients do not ignore next full packet.
> >> + * And if last packet is NOT fully stored,
> >> + * set drop_pevent to true to ignore partial events.
> >> + */
> >> + if (ev.type == EV_SYN && ev.code == SYN_REPORT) {
> >> + ev.time = client->buffer[client->head - 1].time;
> >
> > if client->head happens to be 0, your index won't be what it should be :)
> > you need the &= here and again after the next line:
> >
>
> I have set head to 0 after which syn_dropped is queued,
> so head will be 1 always and hence it seems no need to take
> care of head overflow here as min bufsize is 64.
>
> >> + client->buffer[client->head++] = ev;
> >
> >
> >
> >> + } else if (ev.type != EV_SYN && ev.code != SYN_REPORT)
> >> + client->drop_pevent = true;
> >> }
> >>
> >> spin_unlock_irqrestore(&client->buffer_lock, flags);
> >> @@ -236,17 +256,27 @@ static void __pass_event(struct evdev_client *client,
> >>
> >> if (unlikely(client->head == client->tail)) {
> >> /*
> >> - * This effectively "drops" all unconsumed events, leaving
> >> - * EV_SYN/SYN_DROPPED plus the newest event in the queue.
> >> + * This effectively "drops" all unconsumed events, storing
> >> + * EV_SYN/SYN_DROPPED and the newest event in the queue but
> >> + * only if it is EV_SYN/SYN_REPORT so that clients can read
> >> + * the next full event packet. And set drop_pevent to true
> >> + * if last event packet is not stored completely in buffer
> >> + * to ignore upcoming partial events.
> >> */
> >> - 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;
> >> + client->tail = client->head = client->packet_head;
> >>
> >> - client->packet_head = client->tail;
> >> + client->buffer[client->head].time = event->time;
> >> + client->buffer[client->head].type = EV_SYN;
> >> + client->buffer[client->head].code = SYN_DROPPED;
> >> + client->buffer[client->head].value = 0;
> >> + client->head = (client->head + 1) & (client->bufsize -1);
> >> +
> >> + if (event->type == EV_SYN && event->code == SYN_REPORT) {
> >> + client->buffer[client->head++] = *event;
> >> + client->head &= client->bufsize - 1;
> >> + } else if (event->type != EV_SYN && event->code != SYN_REPORT)
> >> + client->drop_pevent = true;
> >
> > given that this condition is almost exactly the same as above, you should
> > share the code. you should also look into checking whether this can be moved
> > to __evdev_queue_syn_dropped() anyway, it seems like it can with little
> > effort.
> >
> > The protocol itself looks correct now though.
> >
>
> Thank you Mr. Peter.
> I have already sent the v4 which moved the code to
> __evdev_queue_syn_dropped() function. Please have a look.
aargh. I had both in my inbox and then replied to the wrong version anyway.
sorry.
Cheers,
Peter
>
> Regards,
> Aniroop
>
> > Cheers,
> > Peter
> >
> >> }
> >>
> >> if (event->type == EV_SYN && event->code == SYN_REPORT) {
> >> @@ -284,6 +314,14 @@ static void evdev_pass_values(struct evdev_client *client,
> >> wakeup = true;
> >> }
> >>
> >> + /* drop partial events of last packet but queue SYN_REPORT */
> >> + if (client->drop_pevent) {
> >> + if (v->type == EV_SYN && v->code == SYN_REPORT)
> >> + client->drop_pevent = false;
> >> + else
> >> + continue;
> >> + }
> >> +
> >> event.type = v->type;
> >> event.code = v->code;
> >> event.value = v->value;
> >> --
> >> 2.6.2
> >>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-08 5:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-04 16:28 [PATCH] [v2]Input: evdev - drop partial events after emptying the buffer Aniroop Mathur
2016-01-08 5:04 ` Peter Hutterer
2016-01-08 5:46 ` Aniroop Mathur
2016-01-08 5:52 ` Peter Hutterer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.