Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1502674 > unrolled thread

[PATCH v2 0/4] Make input drivers y2038 safe

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] Make input drivers y2038 safe Deepa Dinamani <deepa.kernel@gmail.com> - 2016-10-18 05:30 +0200
    [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times Deepa Dinamani <deepa.kernel@gmail.com> - 2016-10-18 05:30 +0200
      Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times Peter Hutterer <peter.hutterer@who-t.net> - 2016-10-27 03:50 +0200
        Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times Deepa Dinamani <deepa.kernel@gmail.com> - 2016-10-27 22:40 +0200
          Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times Peter Hutterer <peter.hutterer@who-t.net> - 2016-10-28 06:40 +0200
    [PATCH v2 2/4] input: evdev: Replace timeval with timespec64 Deepa Dinamani <deepa.kernel@gmail.com> - 2016-10-18 05:30 +0200
      Re: [PATCH v2 2/4] input: evdev: Replace timeval with timespec64 Peter Hutterer <peter.hutterer@who-t.net> - 2016-10-27 04:00 +0200
        Re: [Y2038] [PATCH v2 2/4] input: evdev: Replace timeval with timespec64 Arnd Bergmann <arnd@arndb.de> - 2016-10-27 16:40 +0200

#1502674 — [PATCH v2 0/4] Make input drivers y2038 safe

Reposting the series as I did not hear comments from the
maintainers.

The series is aimed at making input events y2038 safe.
It extends the lifetime of the realtime timestamps in the
events to year 2106.
The plan is to deprecate realtime timestamps anyway as they
are not appropriate for these timestamps as noted in the patch
a80b83b7b8 by John Stultz.

The series is a result of many discussions with Arnd Bergmann.

The design updates the format of the input events read/ written
to the device nodes. This structure and the uapi/input.h
header file are copied across many userspace libraries.
These libraries not only use this struct input_event locally,
but also expose interfaces that contain input_event. To maintain
backward compatibility with all these interfaces, kernel
updates the format of the input events at the dev node to
struct raw_input_event. Kernel also maintains the old struct
input_event and provides apis to convert between the old and new
input event types.

The userspace library changes to libevdev, libuinput and mtdev
will be posted to the respective mailing groups for review.
Once there is a consensus on the design, all the changes dependent
on the kernel change will be merged at the same time.

Changes from v1:
* Updated changes according to review comments.
* Posted userspace library changes that go along with the series.

Deepa Dinamani (4):
  uinput: Add ioctl for using monotonic/ boot times
  input: evdev: Replace timeval with timespec64
  input: Deprecate real timestamps beyond year 2106
  input: serio: Replace timeval by timespec64

 drivers/input/evdev.c            | 57 ++++++++++++++++++++----------------
 drivers/input/input-compat.c     | 29 ++++++++++---------
 drivers/input/input-compat.h     | 19 +++++++-----
 drivers/input/misc/uinput.c      | 62 +++++++++++++++++++++++++++++++++++++---
 drivers/input/serio/hil_mlc.c    | 37 ++++++++++++------------
 drivers/input/serio/hp_sdc.c     | 17 +++++------
 drivers/input/serio/hp_sdc_mlc.c | 10 +++----
 include/linux/hil_mlc.h          |  6 ++--
 include/linux/hp_sdc.h           |  6 ++--
 include/linux/uinput.h           |  3 +-
 include/uapi/linux/input.h       | 47 ++++++++++++++++++++++++++++++
 include/uapi/linux/uinput.h      |  3 ++
 12 files changed, 208 insertions(+), 88 deletions(-)

-- 
2.7.4

[toc] | [next] | [standalone]

#1502675 — [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times

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 | 56 ++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/uinput.h      |  1 +
 include/uapi/linux/uinput.h |  3 +++
 3 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 92595b9..3d75c5a 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -46,11 +46,26 @@ 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;
 
 	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);
+
+	switch (udev->clk_type) {
+	case CLOCK_REALTIME:
+		ktime_get_real_ts64(&ts);
+		break;
+	case CLOCK_MONOTONIC:
+		ktime_get_ts64(&ts);
+		break;
+	case CLOCK_BOOTTIME:
+		get_monotonic_boottime64(&ts);
+		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 +310,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 +320,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 +835,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 +866,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)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1510015 — Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times

On Mon, Oct 17, 2016 at 08:27:30PM -0700, Deepa Dinamani wrote:
> 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 | 56 ++++++++++++++++++++++++++++++++++++++++++++-
>  include/linux/uinput.h      |  1 +
>  include/uapi/linux/uinput.h |  3 +++
>  3 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 92595b9..3d75c5a 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -46,11 +46,26 @@ 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;
>  
>  	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);
> +
> +	switch (udev->clk_type) {
> +	case CLOCK_REALTIME:
> +		ktime_get_real_ts64(&ts);
> +		break;
> +	case CLOCK_MONOTONIC:
> +		ktime_get_ts64(&ts);
> +		break;
> +	case CLOCK_BOOTTIME:
> +		get_monotonic_boottime64(&ts);
> +		break;
> +	}

hmm, I'm a bit confused here. This is an in-kernel bit only (passing the
time through uinput events has no effect). So why do we need an ioctl here?
it's an in-kernel decision only anyway and the time in the events sent to
the evdev client should be dictated by what that client sets for the clock
type, right?

Cheers,
   Peter

> +
> +	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 +310,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 +320,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 +835,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 +866,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)
> -- 
> 2.7.4
> 
> --
> 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
> 

[toc] | [prev] | [next] | [standalone]

#1510638 — Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times

> hmm, I'm a bit confused here. This is an in-kernel bit only (passing the
> time through uinput events has no effect). So why do we need an ioctl here?
> it's an in-kernel decision only anyway and the time in the events sent to
> the evdev client should be dictated by what that client sets for the clock
> type, right?

This is for input events queued by the uinput driver for the virtual
input device.
This can be read through uinput_read() fops.
I don't think anybody is doing a read on uinput nodes, so another
option(Arnd and I considered this) could be not supporting reads on
these nodes at all.

This is not related to evdev events in the kernel.
Currently, this timestamp could be the same format as the evdev
timestamps or not.

-Deepa

[toc] | [prev] | [next] | [standalone]

#1510849 — Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times

On Thu, Oct 27, 2016 at 01:39:30PM -0700, Deepa Dinamani wrote:
> > hmm, I'm a bit confused here. This is an in-kernel bit only (passing the
> > time through uinput events has no effect). So why do we need an ioctl here?
> > it's an in-kernel decision only anyway and the time in the events sent to
> > the evdev client should be dictated by what that client sets for the clock
> > type, right?
> 
> This is for input events queued by the uinput driver for the virtual
> input device.

oh, right. I thought this was in the path for uinput_write(). sorry about
that.

> This can be read through uinput_read() fops.
> I don't think anybody is doing a read on uinput nodes, so another
> option(Arnd and I considered this) could be not supporting reads on
> these nodes at all.
> 
> This is not related to evdev events in the kernel.
> Currently, this timestamp could be the same format as the evdev
> timestamps or not.

I can say I've never done the read from the uinput device, never even
occured to me. quick skim of the code looks like this only matters for
force_feedback stuff. can't really comment on that too much.

Cheers,
   Peter

[toc] | [prev] | [next] | [standalone]

#1502676 — [PATCH v2 2/4] input: evdev: Replace timeval with timespec64

struct timeval is not y2038 safe.

All references to timeval in the kernel will be replaced
by y2038 safe structures.
Replace all references to timeval with y2038 safe
struct timespec64 here.

struct input_event will be changed in a different patch.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/input/evdev.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e9ae3d5..b4e3171 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -156,15 +156,22 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
 static void __evdev_queue_syn_dropped(struct evdev_client *client)
 {
 	struct input_event ev;
-	ktime_t time;
+	struct timespec64 ts;
 
-	time = client->clk_type == EV_CLK_REAL ?
-			ktime_get_real() :
-			client->clk_type == EV_CLK_MONO ?
-				ktime_get() :
-				ktime_get_boottime();
+	switch (client->clk_type) {
+	case EV_CLK_REAL:
+		ktime_get_real_ts64(&ts);
+		break;
+	case EV_CLK_MONO:
+		ktime_get_ts64(&ts);
+		break;
+	case EV_CLK_BOOT:
+		get_monotonic_boottime64(&ts);
+		break;
+	}
 
-	ev.time = ktime_to_timeval(time);
+	ev.time.tv_sec = ts.tv_sec;
+	ev.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 	ev.type = EV_SYN;
 	ev.code = SYN_DROPPED;
 	ev.value = 0;
@@ -257,17 +264,20 @@ static void __pass_event(struct evdev_client *client,
 
 static void evdev_pass_values(struct evdev_client *client,
 			const struct input_value *vals, unsigned int count,
-			ktime_t *ev_time)
+			struct timespec64 *ev_time)
 {
 	struct evdev *evdev = client->evdev;
 	const struct input_value *v;
 	struct input_event event;
+	struct timespec64 ts;
 	bool wakeup = false;
 
 	if (client->revoked)
 		return;
 
-	event.time = ktime_to_timeval(ev_time[client->clk_type]);
+	ts = ev_time[client->clk_type];
+	event.time.tv_sec = ts.tv_sec;
+	event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
 
 	/* Interrupts are disabled, just acquire the lock. */
 	spin_lock(&client->buffer_lock);
@@ -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]);
 
 	rcu_read_lock();
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1510020 — Re: [PATCH v2 2/4] input: evdev: Replace timeval with timespec64

On Mon, Oct 17, 2016 at 08:27:31PM -0700, Deepa Dinamani wrote:
> struct timeval is not y2038 safe.
> 
> All references to timeval in the kernel will be replaced
> by y2038 safe structures.
> Replace all references to timeval with y2038 safe
> struct timespec64 here.
> 
> struct input_event will be changed in a different patch.
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/input/evdev.c | 37 +++++++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index e9ae3d5..b4e3171 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -156,15 +156,22 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
>  static void __evdev_queue_syn_dropped(struct evdev_client *client)
>  {
>  	struct input_event ev;
> -	ktime_t time;
> +	struct timespec64 ts;
>  
> -	time = client->clk_type == EV_CLK_REAL ?
> -			ktime_get_real() :
> -			client->clk_type == EV_CLK_MONO ?
> -				ktime_get() :
> -				ktime_get_boottime();
> +	switch (client->clk_type) {
> +	case EV_CLK_REAL:
> +		ktime_get_real_ts64(&ts);
> +		break;
> +	case EV_CLK_MONO:
> +		ktime_get_ts64(&ts);
> +		break;
> +	case EV_CLK_BOOT:
> +		get_monotonic_boottime64(&ts);
> +		break;
> +	}
>  
> -	ev.time = ktime_to_timeval(time);
> +	ev.time.tv_sec = ts.tv_sec;
> +	ev.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  	ev.type = EV_SYN;
>  	ev.code = SYN_DROPPED;
>  	ev.value = 0;
> @@ -257,17 +264,20 @@ static void __pass_event(struct evdev_client *client,
>  
>  static void evdev_pass_values(struct evdev_client *client,
>  			const struct input_value *vals, unsigned int count,
> -			ktime_t *ev_time)
> +			struct timespec64 *ev_time)
>  {
>  	struct evdev *evdev = client->evdev;
>  	const struct input_value *v;
>  	struct input_event event;
> +	struct timespec64 ts;
>  	bool wakeup = false;
>  
>  	if (client->revoked)
>  		return;
>  
> -	event.time = ktime_to_timeval(ev_time[client->clk_type]);
> +	ts = ev_time[client->clk_type];
> +	event.time.tv_sec = ts.tv_sec;
> +	event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;

you have ktime_get_* helpers below but you don't have one for timespec64 to
struct timeval? That seems like a bug waitig to happen.

Cheers,
   Peter

>  
>  	/* Interrupts are disabled, just acquire the lock. */
>  	spin_lock(&client->buffer_lock);
> @@ -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]);
>  
>  	rcu_read_lock();
>  
> -- 
> 2.7.4
> 
> --
> 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
> 

[toc] | [prev] | [next] | [standalone]

#1510302 — Re: [Y2038] [PATCH v2 2/4] input: evdev: Replace timeval with timespec64

On Thursday, October 27, 2016 11:34:33 AM CEST Peter Hutterer wrote:
> > @@ -257,17 +264,20 @@ static void __pass_event(struct evdev_client *client,
> >  
> >  static void evdev_pass_values(struct evdev_client *client,
> >                       const struct input_value *vals, unsigned int count,
> > -                     ktime_t *ev_time)
> > +                     struct timespec64 *ev_time)
> >  {
> >       struct evdev *evdev = client->evdev;
> >       const struct input_value *v;
> >       struct input_event event;
> > +     struct timespec64 ts;
> >       bool wakeup = false;
> >  
> >       if (client->revoked)
> >               return;
> >  
> > -     event.time = ktime_to_timeval(ev_time[client->clk_type]);
> > +     ts = ev_time[client->clk_type];
> > +     event.time.tv_sec = ts.tv_sec;
> > +     event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
> 
> you have ktime_get_* helpers below but you don't have one for timespec64 to
> struct timeval? That seems like a bug waitig to happen.

This is intentional to a certain degree: we don't have a timeval64
because any conversion to a new interface should prefer timespec64
or 64-bit nanoseconds, and we try to remove timeval (along with
timespec) from everywhere in the kernel because basically all uses
are problematic for y2038.

Note that after patch 3, event->time is no longer a 'timeval'
either, so even if we had a conversion function, we could
no longer use it here.

	Arnd

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web