Thread (9 messages) 9 messages, 3 authors, 2018-01-08

Re: [PATCH v5 2/3] input: evdev: Replace timeval with timespec64

From: Deepa Dinamani <hidden>
Date: 2018-01-06 21:43:38
Also in: lkml

On Tue, Jan 2, 2018 at 7:35 AM, Arnd Bergmann [off-list ref] wrote:
On Tue, Jan 2, 2018 at 7:46 AM, Dmitry Torokhov
[off-list ref] wrote:
quoted
On Sun, Dec 17, 2017 at 09:18:43PM -0800, Deepa Dinamani wrote:
quoted
@@ -304,12 +314,11 @@ static void evdev_events(struct input_handle *handle,
 {
      struct evdev *evdev = handle->private;
      struct evdev_client *client;
-     ktime_t ev_time[EV_CLK_MAX];
+     struct timespec64 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);
+     ktime_get_ts64(&ev_time[EV_CLK_MONO]);
+     ktime_get_real_ts64(&ev_time[EV_CLK_REAL]);
+     get_monotonic_boottime64(&ev_time[EV_CLK_BOOT]);
This may result in different ev_time[] members holding different times,
whereas the original code would take one time sample and convert it to
different clocks.
Is this important? On each client we only return one of the two
times, and I would guess that you cannot rely on a correlation
between timestamps on different devices, since the boot and real
offsets can change over time.
Right. I didn't think this was an issue either.
quoted
Also, why can't we keep using ktime_t internally? It is y2038 safe,
right?
Correct, but there may also be a performance difference if we get
a lot of events, not sure if that matters.
quoted
I think you should drop this patch and adjust the 3rd one to
massage the input event timestamp patch to do ktime->timespec64->input
timestamp conversion.
The change in __evdev_queue_syn_dropped still seems useful to me
as ktime_get_*ts64() is a bit more efficient than ktime_get*() followed by
a slow ktime_to_timespec64() or ktime_to_timeval().

For evdev_events(), doing a single ktime_get() followed by a
ktime_to_timespec64/ktime_to_timeval can be faster than three
ktime_get_*ts64 (depending on the hardware clock source), or
it can be slower depending on the CPU and the clocksource
hardware. Again, no idea if this matters at the usual rate of
input events.

I guess dropping the evdev_events() change and replacing it with a
ktime_to_timespec64 change in evdev_pass_values()
would be fine here, it should keep the current performance
behavior and get rid of the timeval.
I was trying to use timespec64 everywhere so that we would not have
conversions back and forth at the input layer.
I dropped the ktime_t conversions for now and merged this patch with
the next one as requested.

Let me know if you would like to keep the changes Arnd preferred above
for __evdev_queue_syn_dropped(). I can submit a separate patch if this
is preferred.

-Deepa
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help