[PATCH] Input: evdev - use monotonic clock for event timestamps

[PATCH] Input: evdev - use monotonic clock for event timestamps

[Daniel Kurtz]

Using wallclock time for event timestamps subjects inter-event timing to
ntp and other clock adjustments.  This complicates userspace drivers
that use these timestamps to calculate velocities, or while processing
state transitions.

Instead, use the kernel monotonic clock for event timestamps, which is
at least guaranteed never to go backwards.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/evdev.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 4cf2534..118f936 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -94,8 +94,11 @@ static void evdev_event(struct input_handle *handle,
 	struct evdev *evdev = handle->private;
 	struct evdev_client *client;
 	struct input_event event;
+	struct timespec now;
 
-	do_gettimeofday(&event.time);
+	getrawmonotonic(&now);
+	event.time.tv_sec = now.tv_sec;
+	event.time.tv_usec = now.tv_nsec/1000;
 	event.type = type;
 	event.code = code;
 	event.value = value;
-- 
1.7.3.1

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Henrik Rydberg]

Hi Daniel,

> Using wallclock time for event timestamps subjects inter-event timing to
> ntp and other clock adjustments.  This complicates userspace drivers
> that use these timestamps to calculate velocities, or while processing
> state transitions.
> 
> Instead, use the kernel monotonic clock for event timestamps, which is
> at least guaranteed never to go backwards.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/input/evdev.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 4cf2534..118f936 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -94,8 +94,11 @@ static void evdev_event(struct input_handle *handle,
>  	struct evdev *evdev = handle->private;
>  	struct evdev_client *client;
>  	struct input_event event;
> +	struct timespec now;
>  
> -	do_gettimeofday(&event.time);
> +	getrawmonotonic(&now);
> +	event.time.tv_sec = now.tv_sec;
> +	event.time.tv_usec = now.tv_nsec/1000;
>  	event.type = type;
>  	event.code = code;
>  	event.value = value;
> -- 
> 1.7.3.1

Good thing per se, but reporting time relative to boot instead of
using real time, for all input events, may cause regression on some
obscure systems. Perhaps it is possible to improve on the desired
monotonicity in most cases, without such a drastic change.

Thanks,
Henrik

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Daniel Kurtz]

Hi Henrik,

On Mon, Oct 3, 2011 at 5:06 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
>
> Hi Daniel,
>
> > Using wallclock time for event timestamps subjects inter-event timing to
> > ntp and other clock adjustments.  This complicates userspace drivers
> > that use these timestamps to calculate velocities, or while processing
> > state transitions.
> >
> > Instead, use the kernel monotonic clock for event timestamps, which is
> > at least guaranteed never to go backwards.
> >
> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> > ---
> >  drivers/input/evdev.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> > index 4cf2534..118f936 100644
> > --- a/drivers/input/evdev.c
> > +++ b/drivers/input/evdev.c
> > @@ -94,8 +94,11 @@ static void evdev_event(struct input_handle *handle,
> >       struct evdev *evdev = handle->private;
> >       struct evdev_client *client;
> >       struct input_event event;
> > +     struct timespec now;
> >
> > -     do_gettimeofday(&event.time);
> > +     getrawmonotonic(&now);
> > +     event.time.tv_sec = now.tv_sec;
> > +     event.time.tv_usec = now.tv_nsec/1000;
> >       event.type = type;
> >       event.code = code;
> >       event.value = value;
> > --
> > 1.7.3.1
>
> Good thing per se, but reporting time relative to boot instead of
> using real time, for all input events, may cause regression on some
> obscure systems. Perhaps it is possible to improve on the desired
> monotonicity in most cases, without such a drastic change.
>
> Thanks,
> Henrik

Thanks for responding!  This is the smallest possible patch that I
could think of that illustrates what I'd like to see.  What other
options do we have to achieve the same affect?

I understand your concern about breaking random drivers, and am hoping
that someon on this list could indicate whether this is a real concern
or not.  To get a better feeling for possible regressions, I checked
xf86-input-evdev & -synaptics, and neither uses the evdev timestamp in
their current incarnations.  Any idea what else might be a good place
to check?

One option is to make the evdev timestamp clock source a per-driver
configuration option (controllable from userspace?).  This sounds like
it is doable, but would be significantly more complicated.

Another option would be to timestamp with monotonicraw + boottime +
sleeptime.  This would be approximately wall clock time, but without
ntp and slew adjustments.  But, I fear this would just make the rare
driver issue less obvious, since it would only become obvious when the
two clock sources started drifting apart.

-Dan
--
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

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Henrik Rydberg]

> I understand your concern about breaking random drivers, and am hoping
> that someon on this list could indicate whether this is a real concern
> or not.  To get a better feeling for possible regressions, I checked
> xf86-input-evdev & -synaptics, and neither uses the evdev timestamp in
> their current incarnations.  Any idea what else might be a good place
> to check?

The input system is used for all sorts of events - switches, for
instance. The point is that it is nearly impossible to know if
something will break or not, hence the reluctance to modify interfaces.

> One option is to make the evdev timestamp clock source a per-driver
> configuration option (controllable from userspace?).  This sounds like
> it is doable, but would be significantly more complicated.
> 
> Another option would be to timestamp with monotonicraw + boottime +
> sleeptime.  This would be approximately wall clock time, but without
> ntp and slew adjustments.  But, I fear this would just make the rare
> driver issue less obvious, since it would only become obvious when the
> two clock sources started drifting apart.

I agree, the problem is not really solvable. Dmitry?

Thanks,
Henrik

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Chase Douglas]

On 10/05/2011 10:23 AM, Henrik Rydberg wrote:
>> I understand your concern about breaking random drivers, and am hoping
>> that someon on this list could indicate whether this is a real concern
>> or not.  To get a better feeling for possible regressions, I checked
>> xf86-input-evdev & -synaptics, and neither uses the evdev timestamp in
>> their current incarnations.  Any idea what else might be a good place
>> to check?
> 
> The input system is used for all sorts of events - switches, for
> instance. The point is that it is nearly impossible to know if
> something will break or not, hence the reluctance to modify interfaces.
> 
>> One option is to make the evdev timestamp clock source a per-driver
>> configuration option (controllable from userspace?).  This sounds like
>> it is doable, but would be significantly more complicated.
>>
>> Another option would be to timestamp with monotonicraw + boottime +
>> sleeptime.  This would be approximately wall clock time, but without
>> ntp and slew adjustments.  But, I fear this would just make the rare
>> driver issue less obvious, since it would only become obvious when the
>> two clock sources started drifting apart.
> 
> I agree, the problem is not really solvable. Dmitry?

We could put it into the -next tree early on in the cycle, and then it
will be in -next for a cycle and in Linus' tree for the real dev cycle.
By that time we would hope any issues would have emerged.

I'm not sure if that is a responsible approach. I agree that the change
would be good, but how sure would we be that nothing would break based
only on testing in development trees?

My personal thoughts are that I doubt it would cause issues. Based on
that gut feel, I would say that this approach is reasonable. However,
I'm just one voice in all this :).

-- Chase

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Dmitry Torokhov]

On Wed, Oct 05, 2011 at 03:35:11PM +0100, Chase Douglas wrote:
> On 10/05/2011 10:23 AM, Henrik Rydberg wrote:
> >> I understand your concern about breaking random drivers, and am hoping
> >> that someon on this list could indicate whether this is a real concern
> >> or not.  To get a better feeling for possible regressions, I checked
> >> xf86-input-evdev & -synaptics, and neither uses the evdev timestamp in
> >> their current incarnations.  Any idea what else might be a good place
> >> to check?
> > 
> > The input system is used for all sorts of events - switches, for
> > instance. The point is that it is nearly impossible to know if
> > something will break or not, hence the reluctance to modify interfaces.
> > 
> >> One option is to make the evdev timestamp clock source a per-driver
> >> configuration option (controllable from userspace?).  This sounds like
> >> it is doable, but would be significantly more complicated.
> >>
> >> Another option would be to timestamp with monotonicraw + boottime +
> >> sleeptime.  This would be approximately wall clock time, but without
> >> ntp and slew adjustments.  But, I fear this would just make the rare
> >> driver issue less obvious, since it would only become obvious when the
> >> two clock sources started drifting apart.
> > 
> > I agree, the problem is not really solvable. Dmitry?
> 
> We could put it into the -next tree early on in the cycle, and then it
> will be in -next for a cycle and in Linus' tree for the real dev cycle.
> By that time we would hope any issues would have emerged.

No, I do not think so - as we already descovered users of that field (if
they are exist) are pretty obscure and I doubt that they actively track
development kernels.

> 
> I'm not sure if that is a responsible approach. I agree that the change
> would be good, but how sure would we be that nothing would break based
> only on testing in development trees?
> 
> My personal thoughts are that I doubt it would cause issues. Based on
> that gut feel, I would say that this approach is reasonable. However,
> I'm just one voice in all this :).

I can see key loggers wanting to see real time of captured events
instead of monotonic time...

If we really need this I think we'll have to go per-file descriptor time
source selection and ioctl way. However can we get the use case
explained again? Why is wall clock jumps so much in middle of the
motion? Can userspace detect negative time jumps and simply abort
gestures in such cases?

Thanks.

-- 
Dmitry

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Daniel Kurtz]

On Thu, Oct 6, 2011 at 11:42 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Oct 05, 2011 at 03:35:11PM +0100, Chase Douglas wrote:
>> On 10/05/2011 10:23 AM, Henrik Rydberg wrote:
>> >> I understand your concern about breaking random drivers, and am hoping
>> >> that someon on this list could indicate whether this is a real concern
>> >> or not.  To get a better feeling for possible regressions, I checked
>> >> xf86-input-evdev & -synaptics, and neither uses the evdev timestamp in
>> >> their current incarnations.  Any idea what else might be a good place
>> >> to check?
>> >
>> > The input system is used for all sorts of events - switches, for
>> > instance. The point is that it is nearly impossible to know if
>> > something will break or not, hence the reluctance to modify interfaces.
>> >
>> >> One option is to make the evdev timestamp clock source a per-driver
>> >> configuration option (controllable from userspace?).  This sounds like
>> >> it is doable, but would be significantly more complicated.
>> >>
>> >> Another option would be to timestamp with monotonicraw + boottime +
>> >> sleeptime.  This would be approximately wall clock time, but without
>> >> ntp and slew adjustments.  But, I fear this would just make the rare
>> >> driver issue less obvious, since it would only become obvious when the
>> >> two clock sources started drifting apart.
>> >
>> > I agree, the problem is not really solvable. Dmitry?
>>
>> We could put it into the -next tree early on in the cycle, and then it
>> will be in -next for a cycle and in Linus' tree for the real dev cycle.
>> By that time we would hope any issues would have emerged.
>
> No, I do not think so - as we already descovered users of that field (if
> they are exist) are pretty obscure and I doubt that they actively track
> development kernels.
>
>>
>> I'm not sure if that is a responsible approach. I agree that the change
>> would be good, but how sure would we be that nothing would break based
>> only on testing in development trees?
>>
>> My personal thoughts are that I doubt it would cause issues. Based on
>> that gut feel, I would say that this approach is reasonable. However,
>> I'm just one voice in all this :).
>
> I can see key loggers wanting to see real time of captured events
> instead of monotonic time...

Such a userspace key logger or driver that uses the timestamp field
today is probably comparing it to userspace time queried by
gettimeofday().  To work with a monotonic timestamp, they could use
clock_gettime(CLOCK_MONOTONIC, &ts) instead.

> If we really need this I think we'll have to go per-file descriptor time
> source selection and ioctl way. However can we get the use case
> explained again? Why is wall clock jumps so much in middle of the
> motion? Can userspace detect negative time jumps and simply abort
> gestures in such cases?

Userspace can detect 'time going backwards' easily enough, but
detecting 'time slowing down' or 'time speeding up' is harder.

> Thanks.
>
> --
> Dmitry

Userspace (touch/mouse) input drivers that want to do ballistics
calculations would benefit from a good, accurate, monotonic timestamp
as low down in the stack as possible to reduce jitter.  In particular,
the timestamp deltas are used to determine contact velocity, and can also
be used to better understand and compensate for response latency and jitter
throughout the entire stack.

Another approach could be:
  Treat the "evdev-layer-assigned" timestamp as purely informational -
it is applied per-event anyway, which isn't really what userspace
needs.  Instead, have touch kernel drivers use another "timestamp"
axis/valuator (ABS_TIMESTAMP?), based off of a monotonic time source,
(preferably get_monotonic_boottime()) taken somewhere early in irq processing.

   My concern about this approach was that it would make drivers
significantly more chatty.  evdev currently squashes events, or even
entire reports (I don't know the proper term for collection of events
terminated by EV_SYN), if the values don't change.  The timestamp axis
would always change, hence it would always be sent, even if no other
axes change.  Of course, we can mitigate this with special-case
handling of the ABS_TIMESTAMP in the evdev layer.

Thoughts?

- Daniel

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Dmitry Torokhov]

On Thu, Oct 06, 2011 at 02:25:39PM +0800, Daniel Kurtz wrote:
> On Thu, Oct 6, 2011 at 11:42 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Wed, Oct 05, 2011 at 03:35:11PM +0100, Chase Douglas wrote:
> >> On 10/05/2011 10:23 AM, Henrik Rydberg wrote:
> >> >> I understand your concern about breaking random drivers, and am hoping
> >> >> that someon on this list could indicate whether this is a real concern
> >> >> or not.  To get a better feeling for possible regressions, I checked
> >> >> xf86-input-evdev & -synaptics, and neither uses the evdev timestamp in
> >> >> their current incarnations.  Any idea what else might be a good place
> >> >> to check?
> >> >
> >> > The input system is used for all sorts of events - switches, for
> >> > instance. The point is that it is nearly impossible to know if
> >> > something will break or not, hence the reluctance to modify interfaces.
> >> >
> >> >> One option is to make the evdev timestamp clock source a per-driver
> >> >> configuration option (controllable from userspace?).  This sounds like
> >> >> it is doable, but would be significantly more complicated.
> >> >>
> >> >> Another option would be to timestamp with monotonicraw + boottime +
> >> >> sleeptime.  This would be approximately wall clock time, but without
> >> >> ntp and slew adjustments.  But, I fear this would just make the rare
> >> >> driver issue less obvious, since it would only become obvious when the
> >> >> two clock sources started drifting apart.
> >> >
> >> > I agree, the problem is not really solvable. Dmitry?
> >>
> >> We could put it into the -next tree early on in the cycle, and then it
> >> will be in -next for a cycle and in Linus' tree for the real dev cycle.
> >> By that time we would hope any issues would have emerged.
> >
> > No, I do not think so - as we already descovered users of that field (if
> > they are exist) are pretty obscure and I doubt that they actively track
> > development kernels.
> >
> >>
> >> I'm not sure if that is a responsible approach. I agree that the change
> >> would be good, but how sure would we be that nothing would break based
> >> only on testing in development trees?
> >>
> >> My personal thoughts are that I doubt it would cause issues. Based on
> >> that gut feel, I would say that this approach is reasonable. However,
> >> I'm just one voice in all this :).
> >
> > I can see key loggers wanting to see real time of captured events
> > instead of monotonic time...
> 
> Such a userspace key logger or driver that uses the timestamp field
> today is probably comparing it to userspace time queried by
> gettimeofday().  To work with a monotonic timestamp, they could use
> clock_gettime(CLOCK_MONOTONIC, &ts) instead.

They sure can adopt. Still, this constitutes ABI breakage and thus is
not allowed.

> 
> > If we really need this I think we'll have to go per-file descriptor time
> > source selection and ioctl way. However can we get the use case
> > explained again? Why is wall clock jumps so much in middle of the
> > motion? Can userspace detect negative time jumps and simply abort
> > gestures in such cases?
> 
> Userspace can detect 'time going backwards' easily enough, but
> detecting 'time slowing down' or 'time speeding up' is harder.

Realistically how much of slowdown/speeding up are we talking about?

> 
> > Thanks.
> >
> > --
> > Dmitry
> 
> Userspace (touch/mouse) input drivers that want to do ballistics
> calculations would benefit from a good, accurate, monotonic timestamp
> as low down in the stack as possible to reduce jitter.  In particular,
> the timestamp deltas are used to determine contact velocity, and can also
> be used to better understand and compensate for response latency and jitter
> throughout the entire stack.
> 
> Another approach could be:
>   Treat the "evdev-layer-assigned" timestamp as purely informational -
> it is applied per-event anyway, which isn't really what userspace
> needs.  Instead, have touch kernel drivers use another "timestamp"
> axis/valuator (ABS_TIMESTAMP?), based off of a monotonic time source,
> (preferably get_monotonic_boottime()) taken somewhere early in irq processing.

That means that we should go through all the drivers and adjust...

> 
>    My concern about this approach was that it would make drivers
> significantly more chatty.  evdev currently squashes events, or even
> entire reports (I don't know the proper term for collection of events
> terminated by EV_SYN), 

I call it event packet.

> if the values don't change.  The timestamp axis
> would always change, hence it would always be sent, even if no other
> axes change.  Of course, we can mitigate this with special-case
> handling of the ABS_TIMESTAMP in the evdev layer.
> 

I'd prefer if we go this route then:

1. Have input core emit this event. Then we could decide if we should
suppress it when suppressing entire packet.
2. Make it MSC_ event.
3. Turn it on and off via ioctl since not all users are interested in
this facility.

OTOH if we do ioctl why don't we simply allow usres select monotnic vs
wall time in event structure.

Still, why does your clock fluctuate so much that it matters?

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

Re: [PATCH] Input: evdev - use monotonic clock for event timestamps

[Henrik Rydberg]

> > if the values don't change.  The timestamp axis
> > would always change, hence it would always be sent, even if no other
> > axes change.  Of course, we can mitigate this with special-case
> > handling of the ABS_TIMESTAMP in the evdev layer.
> > 
> 
> I'd prefer if we go this route then:
> 
> 1. Have input core emit this event. Then we could decide if we should
> suppress it when suppressing entire packet.

If we go this route, we could also make use of the time stamps of the
hardware, when existent.

> 2. Make it MSC_ event.
> 3. Turn it on and off via ioctl since not all users are interested in
> this facility.

Perhaps it is time for the per-file-descriptor event filters.

> OTOH if we do ioctl why don't we simply allow usres select monotnic vs
> wall time in event structure.
> 
> Still, why does your clock fluctuate so much that it matters?

Yes, some actual experimental timing data would be most interesting.

Thanks,
Henrik