On Thu, Jan 14, 2016 at 12:22 AM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Wed, Jan 13, 2016 at 05:27:41PM +0530, Aniroop Mathur wrote:
>> If last event in old queue that was dropped was EV_SYN/SYN_REPORT, then
>> lets generate EV_SYN/SYN_REPORT immediately after queing EV_SYN/SYN_DROPPED
>> so that clients would not ignore next valid full packet events.
>>
>> Signed-off-by: Aniroop Mathur <a.mathur@xxxxxxxxxxx>
>> ---
>> drivers/input/evdev.c | 45 +++++++++++++++++++++++++++++++++------------
>> 1 file changed, 33 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
>> index e9ae3d5..0bc7b98 100644
>> --- a/drivers/input/evdev.c
>> +++ b/drivers/input/evdev.c
>> @@ -156,7 +156,12 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>> static void __evdev_queue_syn_dropped(struct evdev_client *client)
>> {
>> struct input_event ev;
>> + struct input_event *prev_ev;
>> ktime_t time;
>> + unsigned int mask = client->bufsize - 1;
>> +
>> + /* store previous event */
>> + prev_ev = &client->buffer[(client->head - 1) & mask];
>
> How do you know that previous event is valid/exists? In fact, when we
> are dropping events due to the full queue, you will be referencing the
> newest event being processed, not the previous event.
>
> I also wonder if this code is safe with regard to __evdev_flush_queue()
> that is dropping bunch of events and possible empty SYN_REPORT groups.
>
Yes, right.
Sorry, I could not understand what you meant on the first read.
You were asking to validate head for last event dropped in queue,
but I understood something else.
Thanks for making me find the problem.
Now, I have corrected it and sent the new patch v6 and v7.
v6:
Corrected head index to store last event properly
About __evdev_flush_queue():
As input core passes packets (not single events) to evdev handler so
after flushing queue for particular events, there will always be syn_report
in the buffer at the end. And if flush request is for ev_syn, then
syn_report will could not be queued after syn_dropped, anyways.
Only problematic case will be if last event stored in the queue was not
syn_report and other events are filtered out such that last event now becomes
syn_report. But I cannot think of a case when last event stored in the buffer
was not syn_report during flushing the queue.
Still, if such case exist, we can take care of it:
How about we store the last event occurred before flushing the queue and
decides of the basis of this event?
v7:
Included fix during pass_events and during clock change request.
It does not consider ev_dev_flush_queue() case.
So, if you find no problem for the case __evdev_flush_queue, we
can use v6, otherwise v7.
BR,
Aniroop Mathur
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
