On Tue, Jan 12, 2016 at 11:23 PM, Dmitry Torokhov
[off-list ref] wrote:

On Tue, Jan 12, 2016 at 12:09 AM, Aniroop Mathur
[off-list ref] wrote:

quoted

On Tue, Jan 12, 2016 at 6:22 AM, Peter Hutterer
[off-list ref] wrote:

quoted

On Mon, Jan 11, 2016 at 07:46:53PM +0530, Aniroop Mathur wrote:

quoted

On Mon, Jan 11, 2016 at 9:41 AM, Peter Hutterer
[off-list ref] wrote:

quoted

On Tue, Jan 05, 2016 at 03:26:41AM +0530, Aniroop Mathur wrote:

quoted

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 and this in turn reduces
evdev client reading requests plus saves memory space filled by partial
events in evdev handler buffer.
Also, this patch prevents dropping of full packet by evdev client after
SYN_DROPPED occurs in case last stored event was SYN_REPORT.
(like after clock change request)

this patch looks technically correct now, thanks. but tbh, especially after
reading the EVIOCGBUFSIZE patch I'm wondering why you need to optimise this
path. ignoring events after a SYN_DROPPED is easy in userspace, and you'll
need the code to do so anyway because even with your patch, there is no API
guarantee that this will always happen - if you rely on it, your code may
break on a future kernel.

From userland, this patch has no real effect, it only slightly reduces the
chance that you get a SYN_DROPPED while reading events after a SYN_DROPPED
has already occured. And if that's a problem, fixing the kernel is likely
the wrong solution anyway. so yeah, still in doubt about whether this patch
provides any real benefit.

Hello Mr. Peter,

I'm sorry that I'm really not able to understand you fully above.
Please, provide your opinion after seeing below reason of this patch and
elaborate more on above, if still required.

As you can understand by seeing the patch code, this patch is required for
three reasons as follows:

1. To fix bug of dropping full valid packet events by userspace clients in case
last packet was fully stored and then syn_dropped occurs.

For example:
Consider kernel buf contains:
... SYN_REPORT REL_X, REL_Y, SYN_REPORT <one event space>
Now consider case that clock type is changed, so these events will be dropped
and syn_dropped will be queued.
OR
consider second case that new first packet event occur and that is stored in
last event space left, so all stored events will be dropped and syn_dropped
will be queued + newest event as per current code.
So now for first case, kernel buf contains: SYN_DROPPED
and for second case, kernel buf contains: SYN_DROPPED REL_X
Next consider that full packet is finished storing for both cases and
user-space is notified that events are ready to be read.
So now kernel buf contains: SYN_DROPPED REL_X REL_Y SYN_REPORT
But this new valid full packet event will be ignored by the userspace client
as mentioned in documentation that userspace clients ignore all events up to
and including next SYN_REPORT. As you know, this valid full event should not
be ignored by the userspace. So this patch fixes this bug.

How about 1st point above? (a bug fix)

OK, I can see that we might want to generate EV_SYN/SYN_REPORT
immediately after queuing EV_SYN/SYN_DROPPED if last event in the old
queue that was dropped was EV_SYN/SYN_REPORT. This might be borderline
useful in case when we switch clock type and then user presses mouse
button to make sure button press is not ignored.

The rest of the changes I'd drop.

Thank you, Mr. Torokhov.
I've sent the v5 which included this fix only.
Title: Input: evdev: fix bug of dropping full valid packet after syn_dropped

BR,
Aniroop Mathur

Thanks.

--
Dmitry