diff mbox

[1/4] uinput: Add ioctl for using monotonic/ boot times

Message ID 1473775805-2242-2-git-send-email-deepa.kernel@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Deepa Dinamani Sept. 13, 2016, 2:10 p.m. UTC
struct timeval which is part of struct input_event to
maintain the event times is not y2038 safe.

Real time timestamps are also not ideal for input_event
as this time can go backwards as noted in the patch
a80b83b7b8 by John Stultz.

Arnd Bergmann suggested deprecating real time and using
monotonic or other timers for all input_event times as a
solution to both the above problems.

Add a new ioctl to let the user dictate the kind of time
to be used for input events. This is similar to the evdev
implementation of the feature. Realtime is still the
default time. This is to maintain backward compatibility.

The structure to maintain input events will be changed
in a different patch.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 drivers/input/misc/uinput.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/uinput.h      |  1 +
 include/uapi/linux/uinput.h |  3 +++
 3 files changed, 61 insertions(+), 1 deletion(-)

Comments

Arnd Bergmann Sept. 13, 2016, 3:07 p.m. UTC | #1
On Tuesday, September 13, 2016 7:10:02 AM CEST Deepa Dinamani wrote:
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -46,11 +46,28 @@ static int uinput_dev_event(struct input_dev *dev,
>                             unsigned int type, unsigned int code, int value)
>  {
>         struct uinput_device    *udev = input_get_drvdata(dev);
> +       struct timespec64       ts;
> +       ktime_t kt;
>  
>         udev->buff[udev->head].type = type;
>         udev->buff[udev->head].code = code;
>         udev->buff[udev->head].value = value;
> -       do_gettimeofday(&udev->buff[udev->head].time);
> +
> +       kt = ktime_get();
> +       switch (udev->clk_type) {
> +       case CLOCK_REALTIME:
> +               ts = ktime_to_timespec64(ktime_mono_to_real(kt));
> +               break;
> +       case CLOCK_MONOTONIC:
> +               ts = ktime_to_timespec64(kt);
> +               break;
> +       case CLOCK_BOOTTIME:
> +               ts = ktime_to_timespec64(ktime_mono_to_any(kt, TK_OFFS_BOOT));
> +               break;
> +       }
> +
> +       udev->buff[udev->head].time.tv_sec = ts.tv_sec;
> +       udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
> 

This is a bit inefficient. I think it's better to use
ktime_get_real_ts64(), ktime_get_ts64() and get_monotonic_boottime64().

I see that we do the same thing in evdev_events(), so maybe that isn't
overly critical though. Or we might want to change both.

The last one of these also uses the slow ktime_to_timespec64(), but
that is something we can improve later in the common code.

	Arnd
--
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
Deepa Dinamani Sept. 15, 2016, 5:11 p.m. UTC | #2
On Tue, Sep 13, 2016 at 8:07 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, September 13, 2016 7:10:02 AM CEST Deepa Dinamani wrote:
>> --- a/drivers/input/misc/uinput.c
>> +++ b/drivers/input/misc/uinput.c
>> @@ -46,11 +46,28 @@ static int uinput_dev_event(struct input_dev *dev,
>>                             unsigned int type, unsigned int code, int value)
>>  {
>>         struct uinput_device    *udev = input_get_drvdata(dev);
>> +       struct timespec64       ts;
>> +       ktime_t kt;
>>
>>         udev->buff[udev->head].type = type;
>>         udev->buff[udev->head].code = code;
>>         udev->buff[udev->head].value = value;
>> -       do_gettimeofday(&udev->buff[udev->head].time);
>> +
>> +       kt = ktime_get();
>> +       switch (udev->clk_type) {
>> +       case CLOCK_REALTIME:
>> +               ts = ktime_to_timespec64(ktime_mono_to_real(kt));
>> +               break;
>> +       case CLOCK_MONOTONIC:
>> +               ts = ktime_to_timespec64(kt);
>> +               break;
>> +       case CLOCK_BOOTTIME:
>> +               ts = ktime_to_timespec64(ktime_mono_to_any(kt, TK_OFFS_BOOT));
>> +               break;
>> +       }
>> +
>> +       udev->buff[udev->head].time.tv_sec = ts.tv_sec;
>> +       udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>>
>
> This is a bit inefficient. I think it's better to use
> ktime_get_real_ts64(), ktime_get_ts64() and get_monotonic_boottime64().

Right. The way I have is using sequence counters twice, once for ktime_get()
and then again for offset.

> I see that we do the same thing in evdev_events(), so maybe that isn't
> overly critical though. Or we might want to change both.
>
> The last one of these also uses the slow ktime_to_timespec64(), but
> that is something we can improve later in the common code.

Yes, I tried to keep it same as evdev here.
But, evdev itself has a couple of different ways of doing this:

For instance, __evdev_queue_syn_dropped() uses

time = client->clk_type == EV_CLK_REAL ?
            ktime_get_real() :
            client->clk_type == EV_CLK_MONO ?
                ktime_get() :
                ktime_get_boottime();

I can change all this to look the same.

I will wait until I hear if this approach is ok for input events.
I will post an update to series after.

Thanks,
Deepa
--
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
diff mbox

Patch

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 92595b9..ae22927 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -46,11 +46,28 @@  static int uinput_dev_event(struct input_dev *dev,
 			    unsigned int type, unsigned int code, int value)
 {
 	struct uinput_device	*udev = input_get_drvdata(dev);
+	struct timespec64	ts;
+	ktime_t kt;
 
 	udev->buff[udev->head].type = type;
 	udev->buff[udev->head].code = code;
 	udev->buff[udev->head].value = value;
-	do_gettimeofday(&udev->buff[udev->head].time);
+
+	kt = ktime_get();
+	switch (udev->clk_type) {
+	case CLOCK_REALTIME:
+		ts = ktime_to_timespec64(ktime_mono_to_real(kt));
+		break;
+	case CLOCK_MONOTONIC:
+		ts = ktime_to_timespec64(kt);
+		break;
+	case CLOCK_BOOTTIME:
+		ts = ktime_to_timespec64(ktime_mono_to_any(kt, TK_OFFS_BOOT));
+		break;
+	}
+
+	udev->buff[udev->head].time.tv_sec = ts.tv_sec;
+	udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 	udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
 
 	wake_up_interruptible(&udev->waitq);
@@ -295,6 +312,7 @@  static int uinput_create_device(struct uinput_device *udev)
 	if (error)
 		goto fail2;
 
+	udev->clk_type = CLOCK_REALTIME;
 	udev->state = UIST_CREATED;
 
 	return 0;
@@ -304,6 +322,38 @@  static int uinput_create_device(struct uinput_device *udev)
 	return error;
 }
 
+static int uinput_set_clk_type(struct uinput_device *udev, unsigned int clkid)
+{
+	unsigned int clk_type;
+
+	if (udev->state != UIST_CREATED)
+		return -EINVAL;
+
+	switch (clkid) {
+	/* Realtime clock is only valid until year 2038.*/
+	case CLOCK_REALTIME:
+		clk_type = CLOCK_REALTIME;
+		break;
+	case CLOCK_MONOTONIC:
+		clk_type = CLOCK_MONOTONIC;
+		break;
+	case CLOCK_BOOTTIME:
+		clk_type = CLOCK_BOOTTIME;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (udev->clk_type != clk_type) {
+		udev->clk_type = clk_type;
+
+		/* Flush pending events */
+		uinput_flush_requests(udev);
+	}
+
+	return 0;
+}
+
 static int uinput_open(struct inode *inode, struct file *file)
 {
 	struct uinput_device *newdev;
@@ -787,6 +837,7 @@  static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 	char			*phys;
 	const char		*name;
 	unsigned int		size;
+	int			clock_id;
 
 	retval = mutex_lock_interruptible(&udev->mutex);
 	if (retval)
@@ -817,6 +868,11 @@  static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
 			retval = uinput_dev_setup(udev, p);
 			goto out;
 
+		case UI_SET_CLOCKID:
+			if (copy_from_user(&clock_id, p, sizeof(unsigned int)))
+				return -EFAULT;
+			return uinput_set_clk_type(udev, clock_id);
+
 		/* UI_ABS_SETUP is handled in the variable size ioctls */
 
 		case UI_SET_EVBIT:
diff --git a/include/linux/uinput.h b/include/linux/uinput.h
index 75de43d..6527fb7 100644
--- a/include/linux/uinput.h
+++ b/include/linux/uinput.h
@@ -72,6 +72,7 @@  struct uinput_device {
 	unsigned char		head;
 	unsigned char		tail;
 	struct input_event	buff[UINPUT_BUFFER_SIZE];
+	int			clk_type;
 	unsigned int		ff_effects_max;
 
 	struct uinput_request	*requests[UINPUT_NUM_REQUESTS];
diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h
index dc652e2..d9494ae 100644
--- a/include/uapi/linux/uinput.h
+++ b/include/uapi/linux/uinput.h
@@ -133,6 +133,9 @@  struct uinput_abs_setup {
  */
 #define UI_ABS_SETUP _IOW(UINPUT_IOCTL_BASE, 4, struct uinput_abs_setup)
 
+/* Set clockid to be used for timestamps */
+#define UI_SET_CLOCKID _IOW(UINPUT_IOCTL_BASE, 5, int)
+
 #define UI_SET_EVBIT		_IOW(UINPUT_IOCTL_BASE, 100, int)
 #define UI_SET_KEYBIT		_IOW(UINPUT_IOCTL_BASE, 101, int)
 #define UI_SET_RELBIT		_IOW(UINPUT_IOCTL_BASE, 102, int)