[PATCH] Input: evdev: avoid storing newest SYN_REPORT when dropping all events

[PATCH] Input: evdev: avoid storing newest SYN_REPORT when dropping all events

[Aniroop Mathur]

The newest event can be SYN_REPORT in case of dropping all old events when
buffer is full, but as now there is no device data available to read so
lets drop SYN_REPORT as well and store only SYN_DROPPED.

Signed-off-by: Aniroop Mathur <a.mathur@samsung.com>
---
 drivers/input/evdev.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e9ae3d5..e270219 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -228,7 +228,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
 	return 0;
 }
 
-static void __pass_event(struct evdev_client *client,
+static int __pass_event(struct evdev_client *client,
 			 const struct input_event *event)
 {
 	client->buffer[client->head++] = *event;
@@ -247,12 +247,24 @@ static void __pass_event(struct evdev_client *client,
 		client->buffer[client->tail].value = 0;
 
 		client->packet_head = client->tail;
+
+		/*
+		 * Do not store SYN_REPORT as there is no device data in buffer
+		 * and return to avoid wakeup and changing packet_head.
+		 */
+		if (event->type == EV_SYN && event->code == SYN_REPORT) {
+			client->head--;
+			client->head &= client->bufsize - 1;
+			return 1;
+		}
 	}
 
 	if (event->type == EV_SYN && event->code == SYN_REPORT) {
 		client->packet_head = client->head;
 		kill_fasync(&client->fasync, SIGIO, POLL_IN);
 	}
+
+	return 0;
 }
 
 static void evdev_pass_values(struct evdev_client *client,
@@ -287,7 +299,8 @@ static void evdev_pass_values(struct evdev_client *client,
 		event.type = v->type;
 		event.code = v->code;
 		event.value = v->value;
-		__pass_event(client, &event);
+		if (__pass_event(client, &event))
+			wakeup = false;
 	}
 
 	spin_unlock(&client->buffer_lock);
-- 
2.6.2

Re: [PATCH] Input: evdev: avoid storing newest SYN_REPORT when dropping all events

[Dmitry Torokhov]

Hi Aniroop,

On Thu, Dec 24, 2015 at 11:25 AM, Aniroop Mathur <a.mathur@samsung.com> wrote:
> The newest event can be SYN_REPORT in case of dropping all old events when
> buffer is full, but as now there is no device data available to read so
> lets drop SYN_REPORT as well and store only SYN_DROPPED.
>

We can not drop the SYN_REPORT event in this case as userspace needs
it to determine if the events that are coming after SYN_DROPPED are
part of a full packet or if they belong to a partial packet. From
Documentation/input/event-codes.txt:

* 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.

Thanks.

-- 
Dmitry

Re: [PATCH] Input: evdev: avoid storing newest SYN_REPORT when dropping all events

[Aniroop Mathur]

Hello Mr. Torokhov,

On Tue, Dec 29, 2015 at 5:11 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Aniroop,
>
> On Thu, Dec 24, 2015 at 11:25 AM, Aniroop Mathur <a.mathur@samsung.com> wrote:
>> The newest event can be SYN_REPORT in case of dropping all old events when
>> buffer is full, but as now there is no device data available to read so
>> lets drop SYN_REPORT as well and store only SYN_DROPPED.
>>
>
> We can not drop the SYN_REPORT event in this case as userspace needs
> it to determine if the events that are coming after SYN_DROPPED are
> part of a full packet or if they belong to a partial packet. From
> Documentation/input/event-codes.txt:
>
> * 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.
>

Okay I see.

In case of buffer overrun, how about handling the partial packet case
in evdev handler itself and free evdev client from handling it ?

Normally, evdev buffer contain only full packets. But in case of dropping
events, buffer would contain a partial packet almost all times and hence
evdev client would have to handle the case of partial packet and
as documentation says, client will ignore the events until new packet
starts. So as a new protocol, after buffer overrun occurs, evdev handler
can ignore the events until SYN_REPORT occurs and then start storing
events. This way buffer will contain full packet as always and reading
would be simplified for evdev clients.
Accordingly, document can also be updated.

Please let me know your opinion about it.

Thanks and Regards,
Aniroop Mathur


> Thanks.
>
> --
> Dmitry