Re: [RFC][PATCH] Input: Use monotonic time for event time stamps.

On Wed, 2011-12-21 at 10:11 +0800, Daniel Kurtz wrote:

On Wed, Dec 21, 2011 at 9:41 AM, Wanlong Gao [off-list ref] wrote:

quoted

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

quoted

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