linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH] Input: Use monotonic time for event time stamps.
@ 2011-12-21  1:01 john stultz
  2011-12-21  1:41 ` Wanlong Gao
  0 siblings, 1 reply; 10+ messages in thread
From: john stultz @ 2011-12-21  1:01 UTC (permalink / raw)
  To: linux-input; +Cc: Dmitry Torokhov, Arve Hjønnevåg

Hi
	In reviewing the Android patch set, I noticed the following patch from
Arve which looks like it resolves a reasonable issue (events using
CLOCK_REALTIME timestamps, which can jump forwards or backwards via
settimeofday()).

I'm not very familiar with the evdev interface, so I'm not sure if
changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
with legacy apps. Even so, I wanted to send the patch out for review and
consideration as it seems like a reasonable fix.

thanks
-john



From 20950b728f120cd808965c1a20b024aa79706d8d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=3D=3FUTF-8=3Fq=3FArve=3D20Hj=3DC3=3DB8nnev=3DC3=3DA5g=3F=3D?= <arve@android.com>
Date: Fri, 2 Dec 2011 13:59:05 +0800
Subject: [PATCH] Input: Use monotonic time for event time stamps.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since wall time can jump backwards, it cannot be used to determine if one
event occured before another or for how long a key was pressed.

CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: linux-input@vger.kernel.org
Signed-off-by: Arve Hjønnevåg <arve@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.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..ec329b4 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 ts;
 
-	do_gettimeofday(&event.time);
+	ktime_get_ts(&ts);
+	event.time.tv_sec = ts.tv_sec;
+	event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 	event.type = type;
 	event.code = code;
 	event.value = value;
-- 
1.7.3.2.146.gca209



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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  1:01 [RFC][PATCH] Input: Use monotonic time for event time stamps john stultz
@ 2011-12-21  1:41 ` Wanlong Gao
  2011-12-21  2:11   ` Daniel Kurtz
  0 siblings, 1 reply; 10+ messages in thread
From: Wanlong Gao @ 2011-12-21  1:41 UTC (permalink / raw)
  To: john stultz; +Cc: linux-input, Dmitry Torokhov, Arve Hjønnevåg

On 12/21/2011 09:01 AM, john stultz wrote:

> Hi
> 	In reviewing the Android patch set, I noticed the following patch from
> Arve which looks like it resolves a reasonable issue (events using
> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
> settimeofday()).
> 
> I'm not very familiar with the evdev interface, so I'm not sure if
> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
> with legacy apps. Even so, I wanted to send the patch out for review and
> consideration as it seems like a reasonable fix.
> 
> thanks
> -john
> 
> 
> 
>>From 20950b728f120cd808965c1a20b024aa79706d8d Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=3D=3FUTF-8=3Fq=3FArve=3D20Hj=3DC3=3DB8nnev=3DC3=3DA5g=3F=3D?= <arve@android.com>
> Date: Fri, 2 Dec 2011 13:59:05 +0800
> Subject: [PATCH] Input: Use monotonic time for event time stamps.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Since wall time can jump backwards, it cannot be used to determine if one
> event occured before another or for how long a key was pressed.


seems reasonable.

> 
> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> CC: linux-input@vger.kernel.org
> Signed-off-by: Arve Hjønnevåg <arve@android.com>
> Signed-off-by: John Stultz <john.stultz@linaro.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..ec329b4 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 ts;
>  
> -	do_gettimeofday(&event.time);
> +	ktime_get_ts(&ts);
> +	event.time.tv_sec = ts.tv_sec;
> +	event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  	event.type = type;
>  	event.code = code;
>  	event.value = value;


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  1:41 ` Wanlong Gao
@ 2011-12-21  2:11   ` Daniel Kurtz
  2011-12-21  2:23     ` john stultz
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Daniel Kurtz @ 2011-12-21  2:11 UTC (permalink / raw)
  To: gaowanlong
  Cc: john stultz, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> On 12/21/2011 09:01 AM, john stultz wrote:
>
>> Hi
>>       In reviewing the Android patch set, I noticed the following patch from
>> Arve which looks like it resolves a reasonable issue (events using
>> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
>> settimeofday()).
>>
>> I'm not very familiar with the evdev interface, so I'm not sure if
>> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
>> with legacy apps. Even so, I wanted to send the patch out for review and
>> consideration as it seems like a reasonable fix.

Maybe you will have more luck this time.  You can read the previous
thread on this exact topic, here:
https://lkml.org/lkml/2011/10/3/37

The previous attempt got bogged down when people wanted more data on
use cases and how this patch would promote world peace.  I strongly
support the concept, but we found other ways to address our major
concern at the time, so didn't invest more effort to get that patch
accepted.

Just a question though, why ktime_get_ts() and not getrawmonotonic()?

Thanks!

>>
>> thanks
>> -john
>>
>>
>>
>>>From 20950b728f120cd808965c1a20b024aa79706d8d Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?=3D=3FUTF-8=3Fq=3FArve=3D20Hj=3DC3=3DB8nnev=3DC3=3DA5g=3F=3D?= <arve@android.com>
>> Date: Fri, 2 Dec 2011 13:59:05 +0800
>> Subject: [PATCH] Input: Use monotonic time for event time stamps.
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> Since wall time can jump backwards, it cannot be used to determine if one
>> event occured before another or for how long a key was pressed.
>
> seems reasonable.
>
>>
>> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> CC: linux-input@vger.kernel.org
>> Signed-off-by: Arve Hjønnevåg <arve@android.com>
>> Signed-off-by: John Stultz <john.stultz@linaro.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..ec329b4 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 ts;
>>
>> -     do_gettimeofday(&event.time);
>> +     ktime_get_ts(&ts);
>> +     event.time.tv_sec = ts.tv_sec;
>> +     event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>       event.type = type;
>>       event.code = code;
>>       event.value = value;
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  2:11   ` Daniel Kurtz
@ 2011-12-21  2:23     ` john stultz
  2011-12-21  3:34       ` Daniel Kurtz
  2011-12-21  2:29     ` john stultz
  2011-12-21  3:12     ` john stultz
  2 siblings, 1 reply; 10+ messages in thread
From: john stultz @ 2011-12-21  2:23 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
> On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> > On 12/21/2011 09:01 AM, john stultz wrote:
> >
> >> Hi
> >>       In reviewing the Android patch set, I noticed the following patch from
> >> Arve which looks like it resolves a reasonable issue (events using
> >> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
> >> settimeofday()).
> >>
> >> I'm not very familiar with the evdev interface, so I'm not sure if
> >> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
> >> with legacy apps. Even so, I wanted to send the patch out for review and
> >> consideration as it seems like a reasonable fix.
> 
> Maybe you will have more luck this time.  You can read the previous
> thread on this exact topic, here:
> https://lkml.org/lkml/2011/10/3/37

Interesting! Thanks for the link!

> The previous attempt got bogged down when people wanted more data on
> use cases and how this patch would promote world peace.  I strongly
> support the concept, but we found other ways to address our major
> concern at the time, so didn't invest more effort to get that patch
> accepted.
> 
> Just a question though, why ktime_get_ts() and not getrawmonotonic()?

So rawmonotonic isn't frequency corrected via NTP, while the monotonic
clock is. So if you're calculating intervals, you will get more accurate
times (where a second is a second) w/ ktime_get_ts().

The raw monotonic clock is really only useful for time correction
applications (being able to accurately measure how much of a correction
was applied), or when you really want a abstracted sense of hardware
cycles.

thanks
-john



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  2:11   ` Daniel Kurtz
  2011-12-21  2:23     ` john stultz
@ 2011-12-21  2:29     ` john stultz
  2011-12-21  3:23       ` Daniel Kurtz
  2011-12-21  3:12     ` john stultz
  2 siblings, 1 reply; 10+ messages in thread
From: john stultz @ 2011-12-21  2:29 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
> Maybe you will have more luck this time.  You can read the previous
> thread on this exact topic, here:
> https://lkml.org/lkml/2011/10/3/37
> 
> The previous attempt got bogged down when people wanted more data on
> use cases and how this patch would promote world peace.  I strongly
> support the concept, but we found other ways to address our major
> concern at the time, so didn't invest more effort to get that patch
> accepted.

Also, are you able to discuss the alternative way you addressed the
issue?

thanks
-john


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  2:11   ` Daniel Kurtz
  2011-12-21  2:23     ` john stultz
  2011-12-21  2:29     ` john stultz
@ 2011-12-21  3:12     ` john stultz
  2 siblings, 0 replies; 10+ messages in thread
From: john stultz @ 2011-12-21  3:12 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
> On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> > On 12/21/2011 09:01 AM, john stultz wrote:
> >
> >> Hi
> >>       In reviewing the Android patch set, I noticed the following patch from
> >> Arve which looks like it resolves a reasonable issue (events using
> >> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
> >> settimeofday()).
> >>
> >> I'm not very familiar with the evdev interface, so I'm not sure if
> >> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
> >> with legacy apps. Even so, I wanted to send the patch out for review and
> >> consideration as it seems like a reasonable fix.
> 
> Maybe you will have more luck this time.  You can read the previous
> thread on this exact topic, here:
> https://lkml.org/lkml/2011/10/3/37
> 
> The previous attempt got bogged down when people wanted more data on
> use cases and how this patch would promote world peace.  I strongly
> support the concept, but we found other ways to address our major
> concern at the time, so didn't invest more effort to get that patch
> accepted.

Would something more like the following be an acceptable solution? 

NOTE: This is completely untested and off the cuff (all I know is it
builds). Just wanted to post it here to get some feedback and I'll try
to clean it up and make sure it actually works tomorrow if folks like
it.

Arve: Do you think having an IOCTL switch to set the evdev to monotonic
time would be acceptable for Android? 

thanks
-john



As noted by Arve and others, since wall time can jump backwards, it
is difficult to use for input because one cannot determine if one
event occurred before another or for how long a key was pressed.

However, the timestamp field is part of the kernel ABI, and cannot
be changed without possibly breaking existing users.

This patch adds a new IOCTL that sets a flag in the evdev_client
struct that will switch the timestamps to CLOCK_MONOTONIC instead
of CLOCK_REALTIME. This allows users of the evdev to specify
which clock id they want the timestamps to use.

The default remains CLOCK_REALTIME.

CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: linux-input@vger.kernel.org
CC: Arve Hjønnevåg <arve@android.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/input/evdev.c |   21 ++++++++++++++++++++-
 include/linux/input.h |    2 ++
 2 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 4cf2534..9069fab 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -46,6 +46,7 @@ struct evdev_client {
 	struct fasync_struct *fasync;
 	struct evdev *evdev;
 	struct list_head node;
+	bool use_monotonic;
 	unsigned int bufsize;
 	struct input_event buffer[];
 };
@@ -94,8 +95,12 @@ static void evdev_event(struct input_handle *handle,
 	struct evdev *evdev = handle->private;
 	struct evdev_client *client;
 	struct input_event event;
+	ktime_t time_mono, time_real;
+	struct timespec ts;
+
+	time_mono = ktime_get();
+	time_real = ktime_sub(time_mono, ktime_get_monotonic_offset());
 
-	do_gettimeofday(&event.time);
 	event.type = type;
 	event.code = code;
 	event.value = value;
@@ -103,6 +108,14 @@ static void evdev_event(struct input_handle *handle,
 	rcu_read_lock();
 
 	client = rcu_dereference(evdev->grab);
+
+	if (client->use_monotonic)
+		ts = ktime_to_timespec(time_mono);
+	else
+		ts = ktime_to_timespec(time_real);
+	event.time.tv_sec = ts.tv_sec;
+	event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+
 	if (client)
 		evdev_pass_event(client, &event);
 	else
@@ -683,6 +696,12 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 		else
 			return evdev_ungrab(evdev, client);
 
+	case EVIOCMONTIME:
+		if (copy_from_user(&i, p, sizeof(unsigned int)))
+			return -EFAULT;
+		client->use_monotonic = i;
+		return 0;
+
 	case EVIOCGKEYCODE:
 		return evdev_handle_get_keycode(dev, p);
 
diff --git a/include/linux/input.h b/include/linux/input.h
index 3862e32..245bfcc 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -129,6 +129,8 @@ struct input_keymap_entry {
 
 #define EVIOCGRAB		_IOW('E', 0x90, int)			/* Grab/Release device */
 
+#define EVIOCMONTIME		_IOW('E', 0xA0, int)			/* Set CLOCK_MONOTONIC Timestamps */
+
 /*
  * Device properties and quirks
  */
-- 
1.7.3.2.146.gca209



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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  2:29     ` john stultz
@ 2011-12-21  3:23       ` Daniel Kurtz
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Kurtz @ 2011-12-21  3:23 UTC (permalink / raw)
  To: john stultz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, Dec 21, 2011 at 10:29 AM, john stultz <johnstul@us.ibm.com> wrote:
> On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
>> Maybe you will have more luck this time.  You can read the previous
>> thread on this exact topic, here:
>> https://lkml.org/lkml/2011/10/3/37
>>
>> The previous attempt got bogged down when people wanted more data on
>> use cases and how this patch would promote world peace.  I strongly
>> support the concept, but we found other ways to address our major
>> concern at the time, so didn't invest more effort to get that patch
>> accepted.
>
> Also, are you able to discuss the alternative way you addressed the
> issue?

By ignoring big / backwards time jumps in time in userspace, and just
ignoring smaller, forward adjustments.  Admittedly inelegant, but got
us past a crisis.

>
> thanks
> -john
>
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  2:23     ` john stultz
@ 2011-12-21  3:34       ` Daniel Kurtz
  2011-12-21  7:53         ` john stultz
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Kurtz @ 2011-12-21  3:34 UTC (permalink / raw)
  To: john stultz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, Dec 21, 2011 at 10:23 AM, john stultz <johnstul@us.ibm.com> wrote:
> On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
>> On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
>> > On 12/21/2011 09:01 AM, john stultz wrote:
>> >
>> >> Hi
>> >>       In reviewing the Android patch set, I noticed the following patch from
>> >> Arve which looks like it resolves a reasonable issue (events using
>> >> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
>> >> settimeofday()).
>> >>
>> >> I'm not very familiar with the evdev interface, so I'm not sure if
>> >> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
>> >> with legacy apps. Even so, I wanted to send the patch out for review and
>> >> consideration as it seems like a reasonable fix.
>>
>> Maybe you will have more luck this time.  You can read the previous
>> thread on this exact topic, here:
>> https://lkml.org/lkml/2011/10/3/37
>
> Interesting! Thanks for the link!
>
>> The previous attempt got bogged down when people wanted more data on
>> use cases and how this patch would promote world peace.  I strongly
>> support the concept, but we found other ways to address our major
>> concern at the time, so didn't invest more effort to get that patch
>> accepted.
>>
>> Just a question though, why ktime_get_ts() and not getrawmonotonic()?
>
> So rawmonotonic isn't frequency corrected via NTP, while the monotonic
> clock is. So if you're calculating intervals, you will get more accurate
> times (where a second is a second) w/ ktime_get_ts().
>
> The raw monotonic clock is really only useful for time correction
> applications (being able to accurately measure how much of a correction
> was applied), or when you really want a abstracted sense of hardware
> cycles.

And here is where it gets complicated.  I think it depends on what you
want from your timestamps.

Do you care that the actual timestamps are accurate (wrt wall clock =
MONOTONIC), or that inter-event timetamp spacing is consistent
(MONOTONIC_RAW)?

For our use case, we don't really care about the absolute timestamp
itself, just relative timestamp differences.  We wanted to make sure
that the inter-event times were always consistent, and didn't wobble
about in response to cock adjustments.  The affect of clock skew on
timestamp deltas is miniscule.

-Dan

>
> thanks
> -john
>
>
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  3:34       ` Daniel Kurtz
@ 2011-12-21  7:53         ` john stultz
  2011-12-21  8:54           ` Daniel Kurtz
  0 siblings, 1 reply; 10+ messages in thread
From: john stultz @ 2011-12-21  7:53 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, 2011-12-21 at 11:34 +0800, Daniel Kurtz wrote:
> On Wed, Dec 21, 2011 at 10:23 AM, john stultz <johnstul@us.ibm.com> wrote:
> > On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
> >> On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
> >> > On 12/21/2011 09:01 AM, john stultz wrote:
> >> >
> >> >> Hi
> >> >>       In reviewing the Android patch set, I noticed the following patch from
> >> >> Arve which looks like it resolves a reasonable issue (events using
> >> >> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
> >> >> settimeofday()).
> >> >>
> >> >> I'm not very familiar with the evdev interface, so I'm not sure if
> >> >> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
> >> >> with legacy apps. Even so, I wanted to send the patch out for review and
> >> >> consideration as it seems like a reasonable fix.
> >>
> >> Maybe you will have more luck this time.  You can read the previous
> >> thread on this exact topic, here:
> >> https://lkml.org/lkml/2011/10/3/37
> >
> > Interesting! Thanks for the link!
> >
> >> The previous attempt got bogged down when people wanted more data on
> >> use cases and how this patch would promote world peace.  I strongly
> >> support the concept, but we found other ways to address our major
> >> concern at the time, so didn't invest more effort to get that patch
> >> accepted.
> >>
> >> Just a question though, why ktime_get_ts() and not getrawmonotonic()?
> >
> > So rawmonotonic isn't frequency corrected via NTP, while the monotonic
> > clock is. So if you're calculating intervals, you will get more accurate
> > times (where a second is a second) w/ ktime_get_ts().
> >
> > The raw monotonic clock is really only useful for time correction
> > applications (being able to accurately measure how much of a correction
> > was applied), or when you really want a abstracted sense of hardware
> > cycles.
> 
> And here is where it gets complicated.  I think it depends on what you
> want from your timestamps.
> 
> Do you care that the actual timestamps are accurate (wrt wall clock =
> MONOTONIC), or that inter-event timetamp spacing is consistent
> (MONOTONIC_RAW)?

Although, hardware fluctuates as well (I've watched clock skew along
with AC cycles). So neither is surely to be totally consistent.

> For our use case, we don't really care about the absolute timestamp
> itself, just relative timestamp differences.  We wanted to make sure
> that the inter-event times were always consistent, and didn't wobble
> about in response to cock adjustments.  The affect of clock skew on
> timestamp deltas is miniscule.

Hrm. I'm curious. If clock skew is miniscule, any correction should be
as well. Have you seen actual issues with clock freq correction here
that necessitated using the raw-monotonic clock? Any details if so?

thanks
-john
 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.
  2011-12-21  7:53         ` john stultz
@ 2011-12-21  8:54           ` Daniel Kurtz
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Kurtz @ 2011-12-21  8:54 UTC (permalink / raw)
  To: john stultz
  Cc: gaowanlong, linux-input, Dmitry Torokhov,
	Arve Hjønnevåg

On Wed, Dec 21, 2011 at 3:53 PM, john stultz <johnstul@us.ibm.com> wrote:
> On Wed, 2011-12-21 at 11:34 +0800, Daniel Kurtz wrote:
>> On Wed, Dec 21, 2011 at 10:23 AM, john stultz <johnstul@us.ibm.com> wrote:
>> > On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:
>> >> On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao <gaowanlong@cn.fujitsu.com> wrote:
>> >> > On 12/21/2011 09:01 AM, john stultz wrote:
>> >> >
>> >> >> Hi
>> >> >>       In reviewing the Android patch set, I noticed the following patch from
>> >> >> Arve which looks like it resolves a reasonable issue (events using
>> >> >> CLOCK_REALTIME timestamps, which can jump forwards or backwards via
>> >> >> settimeofday()).
>> >> >>
>> >> >> I'm not very familiar with the evdev interface, so I'm not sure if
>> >> >> changing the timestamps to CLOCK_MONOTONIC could cause an ABI problem
>> >> >> with legacy apps. Even so, I wanted to send the patch out for review and
>> >> >> consideration as it seems like a reasonable fix.
>> >>
>> >> Maybe you will have more luck this time.  You can read the previous
>> >> thread on this exact topic, here:
>> >> https://lkml.org/lkml/2011/10/3/37
>> >
>> > Interesting! Thanks for the link!
>> >
>> >> The previous attempt got bogged down when people wanted more data on
>> >> use cases and how this patch would promote world peace.  I strongly
>> >> support the concept, but we found other ways to address our major
>> >> concern at the time, so didn't invest more effort to get that patch
>> >> accepted.
>> >>
>> >> Just a question though, why ktime_get_ts() and not getrawmonotonic()?
>> >
>> > So rawmonotonic isn't frequency corrected via NTP, while the monotonic
>> > clock is. So if you're calculating intervals, you will get more accurate
>> > times (where a second is a second) w/ ktime_get_ts().
>> >
>> > The raw monotonic clock is really only useful for time correction
>> > applications (being able to accurately measure how much of a correction
>> > was applied), or when you really want a abstracted sense of hardware
>> > cycles.
>>
>> And here is where it gets complicated.  I think it depends on what you
>> want from your timestamps.
>>
>> Do you care that the actual timestamps are accurate (wrt wall clock =
>> MONOTONIC), or that inter-event timetamp spacing is consistent
>> (MONOTONIC_RAW)?
>
> Although, hardware fluctuates as well (I've watched clock skew along
> with AC cycles). So neither is surely to be totally consistent.
>
>> For our use case, we don't really care about the absolute timestamp
>> itself, just relative timestamp differences.  We wanted to make sure
>> that the inter-event times were always consistent, and didn't wobble
>> about in response to cock adjustments.  The affect of clock skew on
>> timestamp deltas is miniscule.
>
> Hrm. I'm curious. If clock skew is miniscule, any correction should be
> as well. Have you seen actual issues with clock freq correction here
> that necessitated using the raw-monotonic clock? Any details if so?

Let's say a really bad clock is 1% slow.  It will be off (relative to
wallclock time) by 3 seconds every 5 minutes.  Such a clock is really
bad and will need frequent adjustments.
  * Those individual frequent clock adjustments, will probably happen
in chunks on the order of several milliseconds.  If such an adjustment
happens between two input events, the resulting timestamp delta error
is on the same order of magnitude as the timestamp delta itself, if
not orders of magnitude larger.
  * However, when looking at inter-event arrival times, the affect of
this huge clock skew is still only 1%.  So a 10ms event arrival time
will be off by 100 us.  Even for this horrible clock, this is really
very small.

Real clocks have much less skew than this, however, the ntp clock
adjustments would still be on the order of magnitude as the input
event inter-arrival times, just much less frequent.  But, the affect
on an individual inter-event time is proportional to the skew.

Sorry, I don't have real data to back this up, but this is my
understanding of how it will work.  Hopefully, I'm explaining it
clearly enough to make my point.

-Dan

> thanks
> -john
>
>
> --
> 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
--
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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-12-21  8:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-21  1:01 [RFC][PATCH] Input: Use monotonic time for event time stamps john stultz
2011-12-21  1:41 ` Wanlong Gao
2011-12-21  2:11   ` Daniel Kurtz
2011-12-21  2:23     ` john stultz
2011-12-21  3:34       ` Daniel Kurtz
2011-12-21  7:53         ` john stultz
2011-12-21  8:54           ` Daniel Kurtz
2011-12-21  2:29     ` john stultz
2011-12-21  3:23       ` Daniel Kurtz
2011-12-21  3:12     ` john stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).