| Message ID | 1438381732-29681-1-git-send-email-aniroop.mathur@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Aniroop, On Sat, Aug 01, 2015 at 03:58:52AM +0530, Aniroop Mathur wrote: > When clock type is changed, previously stored data is flushed > and therfore does not reach to upper layer or application. > Data is critically important along with the timestamp. > So to avoid data loss and send correct timestamp as well, > lets not flush data upon clock type change and > to send correct timestamp, store only monotonic timestamp during write > and change monotonic clock time to desired clock time during read. I wonder if this matters that much. The only time where this change would make difference is when you have a reader thread and then a controlling thread changing clocks. Then you probably do not want to lose events, but you also would not know for given event what kind of timestamp it uses. On the other hand I expect that your application changes clock type the very first thing after opening the event device. In this case the flushing queue does not matter: you are losing events that happened before you opened the device, so it does not really matter if you lose a few more in the beginning. I guess I need better understanding of your use case. Thanks.
Hello Mr. Torokhov, On Tue, Aug 4, 2015 at 10:20 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > Hi Aniroop, > > On Sat, Aug 01, 2015 at 03:58:52AM +0530, Aniroop Mathur wrote: >> When clock type is changed, previously stored data is flushed >> and therfore does not reach to upper layer or application. >> Data is critically important along with the timestamp. >> So to avoid data loss and send correct timestamp as well, >> lets not flush data upon clock type change and >> to send correct timestamp, store only monotonic timestamp during write >> and change monotonic clock time to desired clock time during read. > > I wonder if this matters that much. The only time where this change would make > difference is when you have a reader thread and then a controlling > thread changing clocks. Then you probably do not want to lose events, > but you also would not know for given event what kind of timestamp it > uses. > For this case, I would like to throw some insight. We could lose events and not know what kind of timestamp it uses but we can know if timestamp is proper or not. If frequency of events are very high and clock type is changed in between, it is possible that new timestamp is less than old timestamp. Although it is a correct timestamp as per clock type but for application it would mean that there is some problem with event timestamp because it is ideally impossible that timestamp of new event is less than previous event timestamp. This can happen if events are flushed or not. But I think losing events is a bigger issue than an incorrect timestamp. > On the other hand I expect that your application changes clock type the > very first thing after opening the event device. In this case the > flushing queue does not matter: you are losing events that happened > before you opened the device, so it does not really matter if you lose a > few more in the beginning. > > I guess I need better understanding of your use case. > Before explaining my use case, I would like to tell you about application which checks timestamp strictly. Application Name - CTS Verifier. This is the application developed by Google to check functionality of android device. It is mandatory for every android device in the world to pass all test cases, otherwise device cannot be released in market. In the sensor module, this application performs verifications like EventOrderVerification, EventGapVerification, EventTimestampSynchronizationVerification, etc. EventTimestampSynchronizationVerification verifies that the timestamp of the event is synchronized with SystemClock.elapsedRealtimeNanos(), based on a given threshold. Ref Link: https://source.android.com/compatibility/overview.html http://androidxref.com/5.1.1_r6/xref/cts/tests/tests/hardware/src/android/ hardware/cts/helpers/sensorverification/EventTimestampSynchronizationVerification.java My Use Case: Like cts application, My application checks for sensor events and provide suggestions to correct it at run time. It checks sensor working from "kernel side" by opening up the event device and enable the sensor so that driver starts reporting events. It reads the events and checks if there are any event time synchronization errors, event gap errors, etc are there or not just like cts. Event time synchronization error is checked using api SystemClock.elapsedRealtimeNanos(). Event time synchronization error occurs because event timestamp and timestamp received using SystemClock.elapsedRealtimeNanos() does not match. If event time synchronization error occurs, the application changes the clock type and then check if error is still occuring. Now when clock type is changed, event time synchronization error is removed but event gap error occurs because events are lost. Application working dummy example: 1. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK' 2. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK Application decides to change clock 3. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK 4. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK So, both test cases could not be checked together and thats how I came up with the idea that it is better not to flush the events upon clock change. Similar use case for complete checking of driver, hal, framework where we use original cts application and our application. Similar use case for sensor simulator. Like in sensor domain, there can be more use cases in other domains which other domain team members must be aware of. Apart from above, I have always read and learnt that kernel code should be generic and should work fine for all application uses upon kernel upgrade. I guess if events are lost in previous kernel version, them it should not be lost in new version. So, I realize that it is safer to not flush the events. Also, I think we can add functionality to add an ioctl call using which user space can flush the events or empty the event buffer, whenever they really need to. Regards, Aniroop Mathur > Thanks. > > -- > Dmitry -- 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
On Wed, Aug 5, 2015 at 3:29 AM, Aniroop Mathur <aniroop.mathur@gmail.com> wrote: > Hello Mr. Torokhov, > > On Tue, Aug 4, 2015 at 10:20 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: >> Hi Aniroop, >> >> On Sat, Aug 01, 2015 at 03:58:52AM +0530, Aniroop Mathur wrote: >>> When clock type is changed, previously stored data is flushed >>> and therfore does not reach to upper layer or application. >>> Data is critically important along with the timestamp. >>> So to avoid data loss and send correct timestamp as well, >>> lets not flush data upon clock type change and >>> to send correct timestamp, store only monotonic timestamp during write >>> and change monotonic clock time to desired clock time during read. >> >> I wonder if this matters that much. The only time where this change would make >> difference is when you have a reader thread and then a controlling >> thread changing clocks. Then you probably do not want to lose events, >> but you also would not know for given event what kind of timestamp it >> uses. >> > > For this case, I would like to throw some insight. > We could lose events and not know what kind of timestamp it uses but we can > know if timestamp is proper or not. If frequency of events are very high and > clock type is changed in between, it is possible that new timestamp is less > than old timestamp. Although it is a correct timestamp as per clock type but > for application it would mean that there is some problem with event timestamp > because it is ideally impossible that timestamp of new event is less than > previous event timestamp. This can happen if events are flushed or not. > But I think losing events is a bigger issue than an incorrect timestamp. > >> On the other hand I expect that your application changes clock type the >> very first thing after opening the event device. In this case the >> flushing queue does not matter: you are losing events that happened >> before you opened the device, so it does not really matter if you lose a >> few more in the beginning. >> >> I guess I need better understanding of your use case. >> > > Before explaining my use case, I would like to tell you about application which > checks timestamp strictly. > Application Name - CTS Verifier. > This is the application developed by Google to check functionality of android > device. It is mandatory for every android device in the world to pass all test > cases, otherwise device cannot be released in market. In the sensor module, > this application performs verifications like EventOrderVerification, > EventGapVerification, EventTimestampSynchronizationVerification, etc. > EventTimestampSynchronizationVerification verifies that the timestamp of the > event is synchronized with SystemClock.elapsedRealtimeNanos(), based on a given > threshold. > Ref Link: > https://source.android.com/compatibility/overview.html > http://androidxref.com/5.1.1_r6/xref/cts/tests/tests/hardware/src/android/ > hardware/cts/helpers/sensorverification/EventTimestampSynchronizationVerification.java > > My Use Case: > Like cts application, My application checks for sensor events and provide > suggestions to correct it at run time. It checks sensor working from > "kernel side" by opening up the event device and enable the sensor so that > driver starts reporting events. It reads the events and checks if there are any > event time synchronization errors, event gap errors, etc are there or > not just like cts. > Event time synchronization error is checked using api > SystemClock.elapsedRealtimeNanos(). Event time synchronization error occurs > because event timestamp and timestamp received using > SystemClock.elapsedRealtimeNanos() does not match. > If event time synchronization error occurs, the application changes the clock > type and then check if error is still occuring. Now when clock type is changed, > event time synchronization error is removed but event gap error occurs > because events are lost. > Application working dummy example: > 1. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK' > 2. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK > Application decides to change clock > 3. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK > 4. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK > > So, both test cases could not be checked together and thats how I came up with > the idea that it is better not to flush the events upon clock change. > > Similar use case for complete checking of driver, hal, framework where we use > original cts application and our application. > Similar use case for sensor simulator. > > Like in sensor domain, there can be more use cases in other domains which other > domain team members must be aware of. > > Apart from above, I have always read and learnt that kernel code should be > generic and should work fine for all application uses upon kernel upgrade. > I guess if events are lost in previous kernel version, them it should not be > lost in new version. Sentence Correction: I guess if events are not lost in previous kernel versions, them it should also not be lost in newer versions. > So, I realize that it is safer to not flush the events. > > Also, I think we can add functionality to add an ioctl call using which user > space can flush the events or empty the event buffer, whenever they really > need to. > > Regards, > Aniroop Mathur > >> Thanks. >> >> -- >> Dmitry -- 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
On Wed, Aug 05, 2015 at 03:29:06AM +0530, Aniroop Mathur wrote: > Hello Mr. Torokhov, > > On Tue, Aug 4, 2015 at 10:20 PM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > Hi Aniroop, > > > > On Sat, Aug 01, 2015 at 03:58:52AM +0530, Aniroop Mathur wrote: > >> When clock type is changed, previously stored data is flushed > >> and therfore does not reach to upper layer or application. > >> Data is critically important along with the timestamp. > >> So to avoid data loss and send correct timestamp as well, > >> lets not flush data upon clock type change and > >> to send correct timestamp, store only monotonic timestamp during write > >> and change monotonic clock time to desired clock time during read. > > > > I wonder if this matters that much. The only time where this change would make > > difference is when you have a reader thread and then a controlling > > thread changing clocks. Then you probably do not want to lose events, > > but you also would not know for given event what kind of timestamp it > > uses. > > > > For this case, I would like to throw some insight. > We could lose events and not know what kind of timestamp it uses but we can > know if timestamp is proper or not. If frequency of events are very high and > clock type is changed in between, it is possible that new timestamp is less > than old timestamp. Although it is a correct timestamp as per clock type but > for application it would mean that there is some problem with event timestamp > because it is ideally impossible that timestamp of new event is less than > previous event timestamp. This can happen if events are flushed or not. > But I think losing events is a bigger issue than an incorrect timestamp. Why? I can see that you would not want to lose keyboard events for example, but for them you do not really care about timestamps, but for other devices, like absolute touchpads and touchscreens, dropped events can and do happen (imagine you for some reason get a spike in CPU load and your reader does not schedule quickly enough to drain the event queue). > > > On the other hand I expect that your application changes clock type the > > very first thing after opening the event device. In this case the > > flushing queue does not matter: you are losing events that happened > > before you opened the device, so it does not really matter if you lose a > > few more in the beginning. > > > > I guess I need better understanding of your use case. > > > > Before explaining my use case, I would like to tell you about application which > checks timestamp strictly. > Application Name - CTS Verifier. > This is the application developed by Google to check functionality of android > device. It is mandatory for every android device in the world to pass all test > cases, otherwise device cannot be released in market. In the sensor module, > this application performs verifications like EventOrderVerification, > EventGapVerification, EventTimestampSynchronizationVerification, etc. > EventTimestampSynchronizationVerification verifies that the timestamp of the > event is synchronized with SystemClock.elapsedRealtimeNanos(), based on a given > threshold. > Ref Link: > https://source.android.com/compatibility/overview.html > http://androidxref.com/5.1.1_r6/xref/cts/tests/tests/hardware/src/android/ > hardware/cts/helpers/sensorverification/EventTimestampSynchronizationVerification.java OK. > > My Use Case: > Like cts application, My application checks for sensor events and provide > suggestions to correct it at run time. It checks sensor working from > "kernel side" by opening up the event device and enable the sensor so that > driver starts reporting events. It reads the events and checks if there are any > event time synchronization errors, event gap errors, etc are there or > not just like cts. > Event time synchronization error is checked using api > SystemClock.elapsedRealtimeNanos(). Event time synchronization error occurs > because event timestamp and timestamp received using > SystemClock.elapsedRealtimeNanos() does not match. > If event time synchronization error occurs, the application changes the clock > type and then check if error is still occuring. Now when clock type is changed, > event time synchronization error is removed but event gap error occurs > because events are lost. > Application working dummy example: > 1. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK > 2. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK > Application decides to change clock > 3. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK > 4. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK Now just teach your application that seeing a gap upon clock change is OK and call it a day. > > So, both test cases could not be checked together and thats how I came up with > the idea that it is better not to flush the events upon clock change. > > Similar use case for complete checking of driver, hal, framework where we use > original cts application and our application. > Similar use case for sensor simulator. > > Like in sensor domain, there can be more use cases in other domains which other > domain team members must be aware of. > > Apart from above, I have always read and learnt that kernel code should be > generic and should work fine for all application uses upon kernel upgrade. > I guess if events are lost in previous kernel version, them it should not be > lost in new version. That is not entirely true. We improve behavior of the kernel all the time and in this particular case I consider the behavior an improvement, because it allows application to recognize old vs new timestamps explicitly even in case when thread consuming the events is separate from the thread that is changing clock type. Thanks.
On Wed, Aug 5, 2015 at 4:52 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Wed, Aug 05, 2015 at 03:29:06AM +0530, Aniroop Mathur wrote: >> Hello Mr. Torokhov, >> >> On Tue, Aug 4, 2015 at 10:20 PM, Dmitry Torokhov >> <dmitry.torokhov@gmail.com> wrote: >> > Hi Aniroop, >> > >> > On Sat, Aug 01, 2015 at 03:58:52AM +0530, Aniroop Mathur wrote: >> >> When clock type is changed, previously stored data is flushed >> >> and therfore does not reach to upper layer or application. >> >> Data is critically important along with the timestamp. >> >> So to avoid data loss and send correct timestamp as well, >> >> lets not flush data upon clock type change and >> >> to send correct timestamp, store only monotonic timestamp during write >> >> and change monotonic clock time to desired clock time during read. >> > >> > I wonder if this matters that much. The only time where this change would make >> > difference is when you have a reader thread and then a controlling >> > thread changing clocks. Then you probably do not want to lose events, >> > but you also would not know for given event what kind of timestamp it >> > uses. >> > >> >> For this case, I would like to throw some insight. >> We could lose events and not know what kind of timestamp it uses but we can >> know if timestamp is proper or not. If frequency of events are very high and >> clock type is changed in between, it is possible that new timestamp is less >> than old timestamp. Although it is a correct timestamp as per clock type but >> for application it would mean that there is some problem with event timestamp >> because it is ideally impossible that timestamp of new event is less than >> previous event timestamp. This can happen if events are flushed or not. >> But I think losing events is a bigger issue than an incorrect timestamp. > > Why? I can see that you would not want to lose keyboard events for > example, but for them you do not really care about timestamps, but for > other devices, like absolute touchpads and touchscreens, dropped events > can and do happen (imagine you for some reason get a spike in CPU load > and your reader does not schedule quickly enough to drain the event > queue). > If user touches the screen and no response comes then it does not seem okay as user desires to receive response for every input. So how come losing events is okay ? As i have seen in my device, even when cpu load is completely okay, scheduling gets delayed "sometimes" resulting in delay of event reporting. For faster devices like "sensors", losing or dropping events seems to be critical because it impacts device performance. If someone is playing game using sensors and sensor events gets lost in between then it results in unexpected response. >> >> > On the other hand I expect that your application changes clock type the >> > very first thing after opening the event device. In this case the >> > flushing queue does not matter: you are losing events that happened >> > before you opened the device, so it does not really matter if you lose a >> > few more in the beginning. >> > >> > I guess I need better understanding of your use case. >> > >> >> Before explaining my use case, I would like to tell you about application which >> checks timestamp strictly. >> Application Name - CTS Verifier. >> This is the application developed by Google to check functionality of android >> device. It is mandatory for every android device in the world to pass all test >> cases, otherwise device cannot be released in market. In the sensor module, >> this application performs verifications like EventOrderVerification, >> EventGapVerification, EventTimestampSynchronizationVerification, etc. >> EventTimestampSynchronizationVerification verifies that the timestamp of the >> event is synchronized with SystemClock.elapsedRealtimeNanos(), based on a given >> threshold. >> Ref Link: >> https://source.android.com/compatibility/overview.html >> http://androidxref.com/5.1.1_r6/xref/cts/tests/tests/hardware/src/android/ >> hardware/cts/helpers/sensorverification/EventTimestampSynchronizationVerification.java > > OK. > >> >> My Use Case: >> Like cts application, My application checks for sensor events and provide >> suggestions to correct it at run time. It checks sensor working from >> "kernel side" by opening up the event device and enable the sensor so that >> driver starts reporting events. It reads the events and checks if there are any >> event time synchronization errors, event gap errors, etc are there or >> not just like cts. >> Event time synchronization error is checked using api >> SystemClock.elapsedRealtimeNanos(). Event time synchronization error occurs >> because event timestamp and timestamp received using >> SystemClock.elapsedRealtimeNanos() does not match. >> If event time synchronization error occurs, the application changes the clock >> type and then check if error is still occuring. Now when clock type is changed, >> event time synchronization error is removed but event gap error occurs >> because events are lost. >> Application working dummy example: >> 1. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK >> 2. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK >> Application decides to change clock >> 3. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK >> 4. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK > > Now just teach your application that seeing a gap upon clock change is > OK and call it a day. > However, it is possible to get correct timestamp without event gap. So there will be no issue of either event gap or timestamp. Why it is not possible to follow this approach of fixing both things ? >> >> So, both test cases could not be checked together and thats how I came up with >> the idea that it is better not to flush the events upon clock change. >> >> Similar use case for complete checking of driver, hal, framework where we use >> original cts application and our application. >> Similar use case for sensor simulator. >> >> Like in sensor domain, there can be more use cases in other domains which other >> domain team members must be aware of. >> >> Apart from above, I have always read and learnt that kernel code should be >> generic and should work fine for all application uses upon kernel upgrade. >> I guess if events are lost in previous kernel version, them it should not be >> lost in new version. > > That is not entirely true. We improve behavior of the kernel all the > time and in this particular case I consider the behavior an improvement, > because it allows application to recognize old vs new timestamps > explicitly even in case when thread consuming the events is separate > from the thread that is changing clock type. > > Thanks. > > -- > Dmitry -- 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
On Wed, Aug 05, 2015 at 05:37:57AM +0530, Aniroop Mathur wrote: > On Wed, Aug 5, 2015 at 4:52 AM, Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Wed, Aug 05, 2015 at 03:29:06AM +0530, Aniroop Mathur wrote: > >> Hello Mr. Torokhov, > >> > >> On Tue, Aug 4, 2015 at 10:20 PM, Dmitry Torokhov > >> <dmitry.torokhov@gmail.com> wrote: > >> > Hi Aniroop, > >> > > >> > On Sat, Aug 01, 2015 at 03:58:52AM +0530, Aniroop Mathur wrote: > >> >> When clock type is changed, previously stored data is flushed > >> >> and therfore does not reach to upper layer or application. > >> >> Data is critically important along with the timestamp. > >> >> So to avoid data loss and send correct timestamp as well, > >> >> lets not flush data upon clock type change and > >> >> to send correct timestamp, store only monotonic timestamp during write > >> >> and change monotonic clock time to desired clock time during read. > >> > > >> > I wonder if this matters that much. The only time where this change would make > >> > difference is when you have a reader thread and then a controlling > >> > thread changing clocks. Then you probably do not want to lose events, > >> > but you also would not know for given event what kind of timestamp it > >> > uses. > >> > > >> > >> For this case, I would like to throw some insight. > >> We could lose events and not know what kind of timestamp it uses but we can > >> know if timestamp is proper or not. If frequency of events are very high and > >> clock type is changed in between, it is possible that new timestamp is less > >> than old timestamp. Although it is a correct timestamp as per clock type but > >> for application it would mean that there is some problem with event timestamp > >> because it is ideally impossible that timestamp of new event is less than > >> previous event timestamp. This can happen if events are flushed or not. > >> But I think losing events is a bigger issue than an incorrect timestamp. > > > > Why? I can see that you would not want to lose keyboard events for > > example, but for them you do not really care about timestamps, but for > > other devices, like absolute touchpads and touchscreens, dropped events > > can and do happen (imagine you for some reason get a spike in CPU load > > and your reader does not schedule quickly enough to drain the event > > queue). > > > > If user touches the screen and no response comes then it does not seem okay > as user desires to receive response for every input. So how come losing events > is okay ? Because the device will continue generating events while user is maintaining the touch. But regardless, the "normal" application behavior: 1. Open /dev/input/eventX 2. Select desired clock type for the life of application 3. Retrieve the current device state (optional step) 4. Read event 5. If encountered SYN_DROPPED goto step 3. 6. Act upon event 7. Goto step 4. In this case it does not really matter that we flush the queue on step 2 as we lose the events that were sent before step 1 and in any case you recover by fetching the device state if needed. > As i have seen in my device, even when cpu load is completely okay, > scheduling gets delayed "sometimes" resulting in delay of event reporting. > For faster devices like "sensors", losing or dropping events seems to > be critical > because it impacts device performance. If someone is playing game using sensors > and sensor events gets lost in between then it results in unexpected response. The sensor will continue generating data. The only time when losing events is really important is for "slow" devices like keyboards that generate single event or a pair of events in response to user action. > > > >> > >> > On the other hand I expect that your application changes clock type the > >> > very first thing after opening the event device. In this case the > >> > flushing queue does not matter: you are losing events that happened > >> > before you opened the device, so it does not really matter if you lose a > >> > few more in the beginning. > >> > > >> > I guess I need better understanding of your use case. > >> > > >> > >> Before explaining my use case, I would like to tell you about application which > >> checks timestamp strictly. > >> Application Name - CTS Verifier. > >> This is the application developed by Google to check functionality of android > >> device. It is mandatory for every android device in the world to pass all test > >> cases, otherwise device cannot be released in market. In the sensor module, > >> this application performs verifications like EventOrderVerification, > >> EventGapVerification, EventTimestampSynchronizationVerification, etc. > >> EventTimestampSynchronizationVerification verifies that the timestamp of the > >> event is synchronized with SystemClock.elapsedRealtimeNanos(), based on a given > >> threshold. > >> Ref Link: > >> https://source.android.com/compatibility/overview.html > >> http://androidxref.com/5.1.1_r6/xref/cts/tests/tests/hardware/src/android/ > >> hardware/cts/helpers/sensorverification/EventTimestampSynchronizationVerification.java > > > > OK. > > > >> > >> My Use Case: > >> Like cts application, My application checks for sensor events and provide > >> suggestions to correct it at run time. It checks sensor working from > >> "kernel side" by opening up the event device and enable the sensor so that > >> driver starts reporting events. It reads the events and checks if there are any > >> event time synchronization errors, event gap errors, etc are there or > >> not just like cts. > >> Event time synchronization error is checked using api > >> SystemClock.elapsedRealtimeNanos(). Event time synchronization error occurs > >> because event timestamp and timestamp received using > >> SystemClock.elapsedRealtimeNanos() does not match. > >> If event time synchronization error occurs, the application changes the clock > >> type and then check if error is still occuring. Now when clock type is changed, > >> event time synchronization error is removed but event gap error occurs > >> because events are lost. > >> Application working dummy example: > >> 1. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK > >> 2. data_x1, data_y1, time1... TimeSync - NOT OK, EventGap - OK > >> Application decides to change clock > >> 3. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK > >> 4. data_x1, data_y1, time1... TimeSync - OK, EventGap - NOT OK > > > > Now just teach your application that seeing a gap upon clock change is > > OK and call it a day. > > > > However, it is possible to get correct timestamp without event gap. > So there will be no issue of either event gap or timestamp. > Why it is not possible to follow this approach of fixing both things ? I explained my reason below: with the change as proposed it becomes impossible for an application to reliably detect clock type change in case where thread changing the clock type is different from the thread reading the data. > > >> > >> So, both test cases could not be checked together and thats how I came up with > >> the idea that it is better not to flush the events upon clock change. > >> > >> Similar use case for complete checking of driver, hal, framework where we use > >> original cts application and our application. > >> Similar use case for sensor simulator. > >> > >> Like in sensor domain, there can be more use cases in other domains which other > >> domain team members must be aware of. > >> > >> Apart from above, I have always read and learnt that kernel code should be > >> generic and should work fine for all application uses upon kernel upgrade. > >> I guess if events are lost in previous kernel version, them it should not be > >> lost in new version. > > > > That is not entirely true. We improve behavior of the kernel all the > > time and in this particular case I consider the behavior an improvement, > > because it allows application to recognize old vs new timestamps > > explicitly even in case when thread consuming the events is separate > > from the thread that is changing clock type. > > > > Thanks. > > > > -- > > Dmitry
diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index a18f41b..ab49382 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -111,15 +111,8 @@ 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; - ktime_t time; - - time = client->clk_type == EV_CLK_REAL ? - ktime_get_real() : - client->clk_type == EV_CLK_MONO ? - ktime_get() : - ktime_get_boottime(); - ev.time = ktime_to_timeval(time); + ev.time = ktime_to_timeval(ktime_get()); ev.type = EV_SYN; ev.code = SYN_DROPPED; ev.value = 0; @@ -145,8 +138,6 @@ static void evdev_queue_syn_dropped(struct evdev_client *client) static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid) { - unsigned long flags; - if (client->clk_type == clkid) return 0; @@ -165,19 +156,6 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid) return -EINVAL; } - /* - * Flush pending events and queue SYN_DROPPED event, - * but only if the queue is not empty. - */ - spin_lock_irqsave(&client->buffer_lock, flags); - - if (client->head != client->tail) { - client->packet_head = client->head = client->tail; - __evdev_queue_syn_dropped(client); - } - - spin_unlock_irqrestore(&client->buffer_lock, flags); - return 0; } @@ -209,8 +187,7 @@ static void __pass_event(struct evdev_client *client, } static void evdev_pass_values(struct evdev_client *client, - const struct input_value *vals, unsigned int count, - ktime_t *ev_time) + const struct input_value *vals, unsigned int count) { struct evdev *evdev = client->evdev; const struct input_value *v; @@ -220,7 +197,7 @@ static void evdev_pass_values(struct evdev_client *client, if (client->revoked) return; - event.time = ktime_to_timeval(ev_time[client->clk_type]); + event.time = ktime_to_timeval(ktime_get()); /* Interrupts are disabled, just acquire the lock. */ spin_lock(&client->buffer_lock); @@ -248,22 +225,16 @@ static void evdev_events(struct input_handle *handle, { struct evdev *evdev = handle->private; struct evdev_client *client; - ktime_t ev_time[EV_CLK_MAX]; - - ev_time[EV_CLK_MONO] = ktime_get(); - ev_time[EV_CLK_REAL] = ktime_mono_to_real(ev_time[EV_CLK_MONO]); - ev_time[EV_CLK_BOOT] = ktime_mono_to_any(ev_time[EV_CLK_MONO], - TK_OFFS_BOOT); rcu_read_lock(); client = rcu_dereference(evdev->grab); if (client) - evdev_pass_values(client, vals, count, ev_time); + evdev_pass_values(client, vals, count); else list_for_each_entry_rcu(client, &evdev->client_list, node) - evdev_pass_values(client, vals, count, ev_time); + evdev_pass_values(client, vals, count); rcu_read_unlock(); } @@ -531,6 +502,29 @@ static int evdev_fetch_next_event(struct evdev_client *client, return have_event; } +static void evdev_check_timestamp(struct evdev_client *client, + struct input_event *event) +{ + ktime_t time; + + if (client->clk_type == EV_CLK_MONO) + return; + + time = timeval_to_ktime(event->time); + + switch (client->clk_type) { + + case EV_CLK_REAL: + time = ktime_mono_to_real(time); + break; + case EV_CLK_BOOT: + time = ktime_mono_to_any(time, TK_OFFS_BOOT); + break; + } + + event->time = ktime_to_timeval(time); +} + static ssize_t evdev_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos) { @@ -561,6 +555,8 @@ static ssize_t evdev_read(struct file *file, char __user *buffer, while (read + input_event_size() <= count && evdev_fetch_next_event(client, &event)) { + evdev_check_timestamp(client, &event); + if (input_event_to_user(buffer + read, &event)) return -EFAULT;
When clock type is changed, previously stored data is flushed and therfore does not reach to upper layer or application. Data is critically important along with the timestamp. So to avoid data loss and send correct timestamp as well, lets not flush data upon clock type change and to send correct timestamp, store only monotonic timestamp during write and change monotonic clock time to desired clock time during read. Signed-off-by: Aniroop Mathur <a.mathur@samsung.com> --- drivers/input/evdev.c | 64 ++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 34 deletions(-)