All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] introduce new evdev interface type
@ 2015-11-27 10:00 WEN Pingbo
  2015-11-27 10:00 ` [PATCH 1/3] input: evdev: introduce new evdev interface WEN Pingbo
                   ` (3 more replies)
  0 siblings, 4 replies; 27+ messages in thread
From: WEN Pingbo @ 2015-11-27 10:00 UTC (permalink / raw)
  To: y2038, dmitry.torokhov, aksgarg1989, arnd
  Cc: linux-input, linux-kernel, linux-api, WEN Pingbo

To solve the y2038 problem in input_event, I had some attempts before [1],
and this is the second one.

We can force userspace to use monotonic time in event timestamp, so the
'struct timeval' is enough to keep y2038-safe, as Arnd suggested. But we
can not find a way to make kernel compatible with old binaries, which use
realtime, and there are still some devices, which depend on realtime.

So I get a idea to add a new evdev interface, which is y2038 safe. And
userspace can switch between the old and new interface via ioctl.

The patch series add three evdev interface type:

- EV_IF_LEGACY
  send event by input_event. This is the default option, keep kernel
  backward compatible.

- EV_IF_RAW
  send event by input_value, which doesn't contain a timestamp. If
  userspace don't need the event timestamp, EV_IF_RAW is the best choice.

- EV_IF_COMPOSITE
  send event by input_composite_event. This is a new structure, which
  append a nanosecond timestamp after input_value. Since the input_value
  and s64 are the same size, so you can treat it as two input_value.

Actually, evdev is not interesting in event timestamp, all it should do is
to keep event sequences, and evdev already satisfy this. So in the kernel,
the event should only store in input_value, input_event and
input_composite_event is used by userspace only.

I also wrote a evtest tool [2], to validate those patches. The tool create
a evdev device via uinput, and inject event through evdev or uinput, with 
different interface type.

I have run this test in my Dragonboard 410c, by this command:

$ evtest -l -r -c -n 400000

No problem found, all events are received normally.

[1]: previous patches - https://www.spinics.net/lists/y2038/msg00959.html
[2]: evtest source code - https://github.com/wengpingbo/evtest

WEN Pingbo (3):
  input: evdev: introduce new evdev interface
  input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE
  uinput: convert input_event to input_value

 drivers/input/evdev.c        | 117 ++++++++++++++++------------------
 drivers/input/input-compat.c | 148 ++++++++++++++++++++++++++++---------------
 drivers/input/input-compat.h |  48 ++++++++++----
 drivers/input/misc/uinput.c  |  23 ++++---
 include/linux/input.h        |  12 ----
 include/linux/uinput.h       |   2 +-
 include/uapi/linux/input.h   |  27 ++++++++
 7 files changed, 225 insertions(+), 152 deletions(-)

-- 
1.9.1


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

* [PATCH 1/3] input: evdev: introduce new evdev interface
  2015-11-27 10:00 [PATCH 0/3] introduce new evdev interface type WEN Pingbo
@ 2015-11-27 10:00 ` WEN Pingbo
  2015-11-27 10:37     ` kbuild test robot
  2015-11-27 10:00 ` [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE WEN Pingbo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 27+ messages in thread
From: WEN Pingbo @ 2015-11-27 10:00 UTC (permalink / raw)
  To: y2038, dmitry.torokhov, aksgarg1989, arnd
  Cc: linux-input, linux-kernel, linux-api, WEN Pingbo

The y2038 problem in 'struct input_event' is complained too much. And
after some discussion with other people, I found it's impossible to
solve this in a simple way, and keep backward compatible at the same
time, so we need some new y2038-safe interface here.

This patch add two new evdev interface type - EV_IF_RAW and
EV_IF_COMPOSITE. And leaving the old interface as EV_IF_LEGACY for
compatibility. Userspace can switch between those interface seamlessly
via ioctl, which will be introduced in another patch.

And since evdev doesn't really interest in event timestamp, the patch
has also converted input_event to input_value in evdev entirely, and
move all time-related operations to input_event_to/from_user().

Signed-off-by: WEN Pingbo <pingbo.wen@linaro.org>
---
 drivers/input/evdev.c        |  78 ++++++++---------------
 drivers/input/input-compat.c | 148 ++++++++++++++++++++++++++++---------------
 drivers/input/input-compat.h |  48 ++++++++++----
 include/linux/input.h        |  12 ----
 include/uapi/linux/input.h   |  17 +++++
 5 files changed, 176 insertions(+), 127 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index e9ae3d5..170681b 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -28,13 +28,6 @@
 #include <linux/cdev.h>
 #include "input-compat.h"
 
-enum evdev_clock_type {
-	EV_CLK_REAL = 0,
-	EV_CLK_MONO,
-	EV_CLK_BOOT,
-	EV_CLK_MAX
-};
-
 struct evdev {
 	int open;
 	struct input_handle handle;
@@ -57,10 +50,11 @@ struct evdev_client {
 	struct evdev *evdev;
 	struct list_head node;
 	unsigned int clk_type;
+	unsigned int if_type;
 	bool revoked;
 	unsigned long *evmasks[EV_CNT];
 	unsigned int bufsize;
-	struct input_event buffer[];
+	struct input_value buffer[];
 };
 
 static size_t evdev_get_mask_cnt(unsigned int type)
@@ -113,7 +107,7 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
 	unsigned int i, head, num;
 	unsigned int mask = client->bufsize - 1;
 	bool is_report;
-	struct input_event *ev;
+	struct input_value *ev;
 
 	BUG_ON(type == EV_SYN);
 
@@ -135,7 +129,6 @@ static void __evdev_flush_queue(struct evdev_client *client, unsigned int type)
 			continue;
 		} else if (head != i) {
 			/* move entry to fill the gap */
-			client->buffer[head].time = ev->time;
 			client->buffer[head].type = ev->type;
 			client->buffer[head].code = ev->code;
 			client->buffer[head].value = ev->value;
@@ -155,16 +148,8 @@ 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 input_value ev;
 
-	time = client->clk_type == EV_CLK_REAL ?
-			ktime_get_real() :
-			client->clk_type == EV_CLK_MONO ?
-				ktime_get() :
-				ktime_get_boottime();
-
-	ev.time = ktime_to_timeval(time);
 	ev.type = EV_SYN;
 	ev.code = SYN_DROPPED;
 	ev.value = 0;
@@ -229,7 +214,7 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
 }
 
 static void __pass_event(struct evdev_client *client,
-			 const struct input_event *event)
+			 const struct input_value *event)
 {
 	client->buffer[client->head++] = *event;
 	client->head &= client->bufsize - 1;
@@ -241,7 +226,6 @@ static void __pass_event(struct evdev_client *client,
 		 */
 		client->tail = (client->head - 2) & (client->bufsize - 1);
 
-		client->buffer[client->tail].time = event->time;
 		client->buffer[client->tail].type = EV_SYN;
 		client->buffer[client->tail].code = SYN_DROPPED;
 		client->buffer[client->tail].value = 0;
@@ -256,19 +240,15 @@ 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)
+			const struct input_value *vals, unsigned int count)
 {
 	struct evdev *evdev = client->evdev;
 	const struct input_value *v;
-	struct input_event event;
 	bool wakeup = false;
 
 	if (client->revoked)
 		return;
 
-	event.time = ktime_to_timeval(ev_time[client->clk_type]);
-
 	/* Interrupts are disabled, just acquire the lock. */
 	spin_lock(&client->buffer_lock);
 
@@ -284,10 +264,7 @@ static void evdev_pass_values(struct evdev_client *client,
 			wakeup = true;
 		}
 
-		event.type = v->type;
-		event.code = v->code;
-		event.value = v->value;
-		__pass_event(client, &event);
+		__pass_event(client, v);
 	}
 
 	spin_unlock(&client->buffer_lock);
@@ -304,22 +281,16 @@ static void evdev_events(struct input_handle *handle,
 {
 	struct evdev *evdev = handle->private;
 	struct evdev_client *client;
-	ktime_t 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);
 
 	rcu_read_lock();
 
 	client = rcu_dereference(evdev->grab);
 
 	if (client)
-		evdev_pass_values(client, vals, count, ev_time);
+		evdev_pass_values(client, vals, count);
 	else
 		list_for_each_entry_rcu(client, &evdev->client_list, node)
-			evdev_pass_values(client, vals, count, ev_time);
+			evdev_pass_values(client, vals, count);
 
 	rcu_read_unlock();
 }
@@ -498,7 +469,7 @@ static int evdev_open(struct inode *inode, struct file *file)
 	struct evdev *evdev = container_of(inode->i_cdev, struct evdev, cdev);
 	unsigned int bufsize = evdev_compute_buffer_size(evdev->handle.dev);
 	unsigned int size = sizeof(struct evdev_client) +
-					bufsize * sizeof(struct input_event);
+					bufsize * sizeof(struct input_value);
 	struct evdev_client *client;
 	int error;
 
@@ -533,10 +504,10 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
 {
 	struct evdev_client *client = file->private_data;
 	struct evdev *evdev = client->evdev;
-	struct input_event event;
+	struct input_value event;
 	int retval = 0;
 
-	if (count != 0 && count < input_event_size())
+	if (count != 0 && count < input_event_size(client->if_type))
 		return -EINVAL;
 
 	retval = mutex_lock_interruptible(&evdev->mutex);
@@ -548,13 +519,19 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
 		goto out;
 	}
 
-	while (retval + input_event_size() <= count) {
+	while (retval + input_event_size(client->if_type) <= count) {
 
-		if (input_event_from_user(buffer + retval, &event)) {
+		if (input_event_from_user(buffer + retval, &event,
+					client->if_type)) {
 			retval = -EFAULT;
 			goto out;
 		}
-		retval += input_event_size();
+
+		/*
+		 * We aren't interested in timestamp from userspace,
+		 * skip it if in composite interface
+		 */
+		retval += input_event_size(client->if_type);
 
 		input_inject_event(&evdev->handle,
 				   event.type, event.code, event.value);
@@ -566,7 +543,7 @@ static ssize_t evdev_write(struct file *file, const char __user *buffer,
 }
 
 static int evdev_fetch_next_event(struct evdev_client *client,
-				  struct input_event *event)
+				  struct input_value *event)
 {
 	int have_event;
 
@@ -588,11 +565,11 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
 {
 	struct evdev_client *client = file->private_data;
 	struct evdev *evdev = client->evdev;
-	struct input_event event;
+	struct input_value event;
 	size_t read = 0;
 	int error;
 
-	if (count != 0 && count < input_event_size())
+	if (count != 0 && count < input_event_size(client->if_type))
 		return -EINVAL;
 
 	for (;;) {
@@ -610,13 +587,14 @@ static ssize_t evdev_read(struct file *file, char __user *buffer,
 		if (count == 0)
 			break;
 
-		while (read + input_event_size() <= count &&
+		while (read + input_event_size(client->if_type) <= count &&
 		       evdev_fetch_next_event(client, &event)) {
 
-			if (input_event_to_user(buffer + read, &event))
+			if (input_event_to_user(buffer + read, &event,
+					client->clk_type, client->if_type))
 				return -EFAULT;
 
-			read += input_event_size();
+			read += input_event_size(client->if_type);
 		}
 
 		if (read)
diff --git a/drivers/input/input-compat.c b/drivers/input/input-compat.c
index 64ca711..09162c6 100644
--- a/drivers/input/input-compat.c
+++ b/drivers/input/input-compat.c
@@ -12,56 +12,118 @@
 #include <asm/uaccess.h>
 #include "input-compat.h"
 
-#ifdef CONFIG_COMPAT
+static ktime_t input_get_time(int clk_type)
+{
+	switch (clk_type) {
+	case EV_CLK_MONO:
+		return ktime_get();
+	case EV_CLK_BOOT:
+		return ktime_get_boottime();
+	case EV_CLK_REAL:
+	default:
+		return ktime_get_real();
+	}
+}
 
 int input_event_from_user(const char __user *buffer,
-			  struct input_event *event)
+			  struct input_value *event, int if_type)
 {
-	if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
-		struct input_event_compat compat_event;
-
-		if (copy_from_user(&compat_event, buffer,
-				   sizeof(struct input_event_compat)))
-			return -EFAULT;
-
-		event->time.tv_sec = compat_event.time.tv_sec;
-		event->time.tv_usec = compat_event.time.tv_usec;
-		event->type = compat_event.type;
-		event->code = compat_event.code;
-		event->value = compat_event.value;
-
-	} else {
-		if (copy_from_user(event, buffer, sizeof(struct input_event)))
+	if (if_type == EV_IF_LEGACY) {
+#ifdef CONFIG_COMPAT
+		if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
+			struct input_event_compat compat_event;
+
+			if (copy_from_user(&compat_event, buffer,
+					   sizeof(struct input_event_compat)))
+				return -EFAULT;
+
+			event->type = compat_event.type;
+			event->code = compat_event.code;
+			event->value = compat_event.value;
+		} else {
+#endif
+			struct input_event ev;
+
+			if (copy_from_user(&ev, buffer,
+						sizeof(struct input_event)))
+				return -EFAULT;
+
+			/* drop timestamp from userspace */
+			event->type = ev.type;
+			event->code = ev.code;
+			event->value = ev.value;
+#ifdef CONFIG_COMPAT
+		}
+#endif
+	} else if (if_type == EV_IF_RAW || if_type == EV_IF_COMPOSITE) {
+		if (copy_from_user(event, buffer, sizeof(struct input_value)))
 			return -EFAULT;
-	}
+	} else
+		return -EINVAL;
 
 	return 0;
 }
 
-int input_event_to_user(char __user *buffer,
-			const struct input_event *event)
+int input_event_to_user(char __user *buffer, const struct input_value *event,
+			int clk_type, int if_type)
 {
-	if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
-		struct input_event_compat compat_event;
-
-		compat_event.time.tv_sec = event->time.tv_sec;
-		compat_event.time.tv_usec = event->time.tv_usec;
-		compat_event.type = event->type;
-		compat_event.code = event->code;
-		compat_event.value = event->value;
+	if (if_type == EV_IF_LEGACY) {
+		struct timeval timestamp = ktime_to_timeval(
+				input_get_time(clk_type));
 
-		if (copy_to_user(buffer, &compat_event,
-				 sizeof(struct input_event_compat)))
+#ifdef CONFIG_COMPAT
+		if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) {
+			struct input_event_compat compat_event;
+
+			compat_event.time.tv_sec = timestamp.tv_sec;
+			compat_event.time.tv_usec = timestamp.tv_usec;
+			compat_event.type = event->type;
+			compat_event.code = event->code;
+			compat_event.value = event->value;
+
+			if (copy_to_user(buffer, &compat_event,
+					 sizeof(struct input_event_compat)))
+				return -EFAULT;
+		} else {
+#endif
+			struct input_event ev;
+
+			ev.time = timestamp;
+			ev.type = event->type;
+			ev.code = event->code;
+			ev.value = event->value;
+
+			if (copy_to_user(buffer, &ev,
+					sizeof(struct input_event)))
+				return -EFAULT;
+#ifdef CONFIG_COMPAT
+		}
+#endif
+	} else if (if_type == EV_IF_RAW || if_type == EV_IF_COMPOSITE) {
+		if (copy_to_user(buffer, event, sizeof(struct input_value)))
 			return -EFAULT;
 
-	} else {
-		if (copy_to_user(buffer, event, sizeof(struct input_event)))
-			return -EFAULT;
-	}
+		if (if_type != EV_IF_RAW) {
+			/*
+			 * composite interface, send timestamp event
+			 *
+			 * s64 and input_value are the same size, use s64
+			 * directly here.
+			 */
+			s64 time = ktime_to_ns(input_get_time(clk_type));
+
+			if (copy_to_user(buffer + sizeof(struct input_value),
+						&time, sizeof(s64)))
+				return -EFAULT;
+		}
+	} else
+		return -EINVAL;
 
 	return 0;
 }
 
+#ifdef CONFIG_COMPAT
+
 int input_ff_effect_from_user(const char __user *buffer, size_t size,
 			      struct ff_effect *effect)
 {
@@ -99,24 +161,6 @@ int input_ff_effect_from_user(const char __user *buffer, size_t size,
 
 #else
 
-int input_event_from_user(const char __user *buffer,
-			 struct input_event *event)
-{
-	if (copy_from_user(event, buffer, sizeof(struct input_event)))
-		return -EFAULT;
-
-	return 0;
-}
-
-int input_event_to_user(char __user *buffer,
-			const struct input_event *event)
-{
-	if (copy_to_user(buffer, event, sizeof(struct input_event)))
-		return -EFAULT;
-
-	return 0;
-}
-
 int input_ff_effect_from_user(const char __user *buffer, size_t size,
 			      struct ff_effect *effect)
 {
diff --git a/drivers/input/input-compat.h b/drivers/input/input-compat.h
index 148f66f..4ee8095 100644
--- a/drivers/input/input-compat.h
+++ b/drivers/input/input-compat.h
@@ -65,26 +65,48 @@ struct ff_effect_compat {
 	} u;
 };
 
-static inline size_t input_event_size(void)
-{
-	return (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME) ?
-		sizeof(struct input_event_compat) : sizeof(struct input_event);
-}
+#endif /* CONFIG_COMPAT */
 
-#else
+enum event_clock_type {
+	EV_CLK_REAL = 0,
+	EV_CLK_MONO,
+	EV_CLK_BOOT,
+	EV_CLK_MAX
+};
+
+enum event_if_type {
+	EV_IF_LEGACY = 0,
+	EV_IF_RAW,
+	EV_IF_COMPOSITE,
+	EV_IF_MAX
+};
 
-static inline size_t input_event_size(void)
+static inline size_t input_event_size(int if_type)
 {
-	return sizeof(struct input_event);
+	switch (if_type) {
+	case EV_IF_LEGACY:
+#ifdef CONFIG_COMPAT
+		if (INPUT_COMPAT_TEST && !COMPAT_USE_64BIT_TIME)
+			return sizeof(struct input_event_compat);
+#endif
+		return sizeof(struct input_event);
+	case EV_IF_RAW:
+		return sizeof(struct input_value);
+	case EV_IF_COMPOSITE:
+		return sizeof(struct input_composite_event);
+	default:
+		return 0;
+	}
 }
 
-#endif /* CONFIG_COMPAT */
-
 int input_event_from_user(const char __user *buffer,
-			 struct input_event *event);
+			  struct input_value *event, int if_type);
+
+int input_event_to_user(char __user *buffer, const struct input_value *event,
+			int clk_type, int if_type);
 
-int input_event_to_user(char __user *buffer,
-			const struct input_event *event);
+#define input_value_to_user(buffer, event, if_type)	\
+	input_event_to_user(buffer, event, 0, if_type)
 
 int input_ff_effect_from_user(const char __user *buffer, size_t size,
 			      struct ff_effect *effect);
diff --git a/include/linux/input.h b/include/linux/input.h
index 1e96769..9f9d551 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -25,18 +25,6 @@
 #include <linux/mod_devicetable.h>
 
 /**
- * struct input_value - input value representation
- * @type: type of value (EV_KEY, EV_ABS, etc)
- * @code: the value code
- * @value: the value
- */
-struct input_value {
-	__u16 type;
-	__u16 code;
-	__s32 value;
-};
-
-/**
  * struct input_dev - represents an input device
  * @name: name of the device
  * @phys: physical path to the device in the system hierarchy
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 2758687..79b35ff 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -29,6 +29,23 @@ struct input_event {
 	__s32 value;
 };
 
+/**
+ * struct input_value - input value representation
+ * @type: type of value (EV_KEY, EV_ABS, etc)
+ * @code: the value code
+ * @value: the value
+ */
+struct input_value {
+	__u16 type;
+	__u16 code;
+	__s32 value;
+};
+
+struct input_composite_event {
+	struct input_value v;
+	__s64 time;
+};
+
 /*
  * Protocol version.
  */
-- 
1.9.1


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

* [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE
  2015-11-27 10:00 [PATCH 0/3] introduce new evdev interface type WEN Pingbo
  2015-11-27 10:00 ` [PATCH 1/3] input: evdev: introduce new evdev interface WEN Pingbo
@ 2015-11-27 10:00 ` WEN Pingbo
  2015-11-27 16:59     ` Arnd Bergmann
  2015-11-27 10:00 ` [PATCH 3/3] uinput: convert input_event to input_value WEN Pingbo
  2015-11-27 16:58   ` Arnd Bergmann
  3 siblings, 1 reply; 27+ messages in thread
From: WEN Pingbo @ 2015-11-27 10:00 UTC (permalink / raw)
  To: y2038, dmitry.torokhov, aksgarg1989, arnd
  Cc: linux-input, linux-kernel, linux-api, WEN Pingbo

This patch depends on 'introduce new evdev interface'.

Userspace cat set / get evdev interface type via the two ioctl
commands. And default interface type is EV_IF_LEGACY, so the old binary
will work normal with new kernel. Maybe we should change this default
option to encourage people to move to new interface.

And since all events are stored as input_value in evdev, there are no
need to flush evdev_client's buffer if we change clk_type and if_type.

Signed-off-by: WEN Pingbo <pingbo.wen@linaro.org>
---
 drivers/input/evdev.c      | 39 +++++++++++++++++++++++++++------------
 include/uapi/linux/input.h | 10 ++++++++++
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index 170681b..090576b 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -175,7 +175,6 @@ static void evdev_queue_syn_dropped(struct evdev_client *client)
 
 static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
 {
-	unsigned long flags;
 	unsigned int clk_type;
 
 	switch (clkid) {
@@ -193,21 +192,29 @@ static int evdev_set_clk_type(struct evdev_client *client, unsigned int clkid)
 		return -EINVAL;
 	}
 
-	if (client->clk_type != clk_type) {
+	if (client->clk_type != clk_type)
 		client->clk_type = clk_type;
 
-		/*
-		 * Flush pending events and queue SYN_DROPPED event,
-		 * but only if the queue is not empty.
-		 */
-		spin_lock_irqsave(&client->buffer_lock, flags);
+	return 0;
+}
 
-		if (client->head != client->tail) {
-			client->packet_head = client->head = client->tail;
-			__evdev_queue_syn_dropped(client);
-		}
+static int evdev_set_if_type(struct evdev_client *client, unsigned int if_type)
+{
+	if (client->if_type == if_type)
+		return 0;
 
-		spin_unlock_irqrestore(&client->buffer_lock, flags);
+	switch (if_type) {
+	case EVDEV_LEGACY:
+		client->if_type = EV_IF_LEGACY;
+		break;
+	case EVDEV_RAW:
+		client->if_type = EV_IF_RAW;
+		break;
+	case EVDEV_COMPOSITE:
+		client->if_type = EV_IF_COMPOSITE;
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	return 0;
@@ -1046,6 +1053,7 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 	int __user *ip = (int __user *)p;
 	unsigned int i, t, u, v;
 	unsigned int size;
+	int if_type;
 	int error;
 
 	/* First we check for fixed-length commands */
@@ -1144,6 +1152,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 
 	case EVIOCSKEYCODE_V2:
 		return evdev_handle_set_keycode_v2(dev, p);
+	case EVIOCSIFTYPE:
+		if (get_user(if_type, ip))
+			return -EFAULT;
+
+		return evdev_set_if_type(client, if_type);
+	case EVIOCGIFTYPE:
+		return put_user(client->if_type, ip);
 	}
 
 	size = _IOC_SIZE(cmd);
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 79b35ff..9ae5243 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -234,6 +234,16 @@ struct input_mask {
 
 #define EVIOCSCLOCKID		_IOW('E', 0xa0, int)			/* Set clockid to be used for timestamps */
 
+#define EVIOCSIFTYPE		_IOW('E', 0xa1, int)			/* Set if_type */
+#define EVIOCGIFTYPE		_IOR('E', 0xa2, int)			/* Get if_type */
+
+/*
+ * evdev interface type
+ */
+#define EVDEV_LEGACY			0x00
+#define EVDEV_RAW			0x01
+#define EVDEV_COMPOSITE			0x02
+
 /*
  * IDs.
  */
-- 
1.9.1


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

* [PATCH 3/3] uinput: convert input_event to input_value
  2015-11-27 10:00 [PATCH 0/3] introduce new evdev interface type WEN Pingbo
  2015-11-27 10:00 ` [PATCH 1/3] input: evdev: introduce new evdev interface WEN Pingbo
  2015-11-27 10:00 ` [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE WEN Pingbo
@ 2015-11-27 10:00 ` WEN Pingbo
  2015-11-27 16:58   ` Arnd Bergmann
  3 siblings, 0 replies; 27+ messages in thread
From: WEN Pingbo @ 2015-11-27 10:00 UTC (permalink / raw)
  To: y2038, dmitry.torokhov, aksgarg1989, arnd
  Cc: linux-input, linux-kernel, linux-api, WEN Pingbo

This patch depends on 'add new evdev interface'.

We should not use input_event in kernel any more. Convert it in
uinput, and adapt to new api changes.

Signed-off-by: WEN Pingbo <pingbo.wen@linaro.org>
---
 drivers/input/misc/uinput.c | 23 +++++++++++------------
 include/linux/uinput.h      |  2 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 5adbced..d84c6d4 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -50,7 +50,6 @@ static int uinput_dev_event(struct input_dev *dev,
 	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);
 	udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
 
 	wake_up_interruptible(&udev->waitq);
@@ -436,24 +435,24 @@ static int uinput_setup_device(struct uinput_device *udev,
 static ssize_t uinput_inject_events(struct uinput_device *udev,
 				    const char __user *buffer, size_t count)
 {
-	struct input_event ev;
+	struct input_value ev;
 	size_t bytes = 0;
 
-	if (count != 0 && count < input_event_size())
+	if (count != 0 && count < input_event_size(EV_IF_LEGACY))
 		return -EINVAL;
 
-	while (bytes + input_event_size() <= count) {
+	while (bytes + input_event_size(EV_IF_LEGACY) <= count) {
 		/*
 		 * Note that even if some events were fetched successfully
 		 * we are still going to return EFAULT instead of partial
 		 * count to let userspace know that it got it's buffers
 		 * all wrong.
 		 */
-		if (input_event_from_user(buffer + bytes, &ev))
+		if (input_event_from_user(buffer + bytes, &ev, EV_IF_LEGACY))
 			return -EFAULT;
 
 		input_event(udev->dev, ev.type, ev.code, ev.value);
-		bytes += input_event_size();
+		bytes += input_event_size(EV_IF_LEGACY);
 	}
 
 	return bytes;
@@ -482,7 +481,7 @@ static ssize_t uinput_write(struct file *file, const char __user *buffer,
 }
 
 static bool uinput_fetch_next_event(struct uinput_device *udev,
-				    struct input_event *event)
+				    struct input_value *event)
 {
 	bool have_event;
 
@@ -502,16 +501,16 @@ static bool uinput_fetch_next_event(struct uinput_device *udev,
 static ssize_t uinput_events_to_user(struct uinput_device *udev,
 				     char __user *buffer, size_t count)
 {
-	struct input_event event;
+	struct input_value event;
 	size_t read = 0;
 
-	while (read + input_event_size() <= count &&
+	while (read + input_event_size(EV_IF_LEGACY) <= count &&
 	       uinput_fetch_next_event(udev, &event)) {
 
-		if (input_event_to_user(buffer + read, &event))
+		if (input_value_to_user(buffer + read, &event, EV_IF_LEGACY))
 			return -EFAULT;
 
-		read += input_event_size();
+		read += input_event_size(EV_IF_LEGACY);
 	}
 
 	return read;
@@ -523,7 +522,7 @@ static ssize_t uinput_read(struct file *file, char __user *buffer,
 	struct uinput_device *udev = file->private_data;
 	ssize_t retval;
 
-	if (count != 0 && count < input_event_size())
+	if (count != 0 && count < input_event_size(EV_IF_LEGACY))
 		return -EINVAL;
 
 	do {
diff --git a/include/linux/uinput.h b/include/linux/uinput.h
index 0994c0d..bdb6fca 100644
--- a/include/linux/uinput.h
+++ b/include/linux/uinput.h
@@ -66,7 +66,7 @@ struct uinput_device {
 	unsigned char		ready;
 	unsigned char		head;
 	unsigned char		tail;
-	struct input_event	buff[UINPUT_BUFFER_SIZE];
+	struct input_value	buff[UINPUT_BUFFER_SIZE];
 	unsigned int		ff_effects_max;
 
 	struct uinput_request	*requests[UINPUT_NUM_REQUESTS];
-- 
1.9.1


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

* Re: [PATCH 1/3] input: evdev: introduce new evdev interface
@ 2015-11-27 10:37     ` kbuild test robot
  0 siblings, 0 replies; 27+ messages in thread
From: kbuild test robot @ 2015-11-27 10:37 UTC (permalink / raw)
  To: WEN Pingbo
  Cc: kbuild-all, y2038, dmitry.torokhov, aksgarg1989, arnd,
	linux-input, linux-kernel, linux-api, WEN Pingbo

[-- Attachment #1: Type: text/plain, Size: 10560 bytes --]

Hi WEN,

[auto build test ERROR on: input/next]
[also build test ERROR on: v4.4-rc2 next-20151127]

url:    https://github.com/0day-ci/linux/commits/WEN-Pingbo/introduce-new-evdev-interface-type/20151127-180438
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s0-201547 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/WEN-Pingbo/introduce-new-evdev-interface-type/20151127-180438 HEAD fc81990de5842e76f794f755e095e4c5e55f8caa builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   drivers/input/misc/uinput.c: In function 'uinput_inject_events':
>> drivers/input/misc/uinput.c:442:28: error: too few arguments to function 'input_event_size'
     if (count != 0 && count < input_event_size())
                               ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
   drivers/input/misc/uinput.c:445:17: error: too few arguments to function 'input_event_size'
     while (bytes + input_event_size() <= count) {
                    ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
>> drivers/input/misc/uinput.c:452:45: warning: passing argument 2 of 'input_event_from_user' from incompatible pointer type [-Wincompatible-pointer-types]
      if (input_event_from_user(buffer + bytes, &ev))
                                                ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:102:5: note: expected 'struct input_value *' but argument is of type 'struct input_event *'
    int input_event_from_user(const char __user *buffer,
        ^
>> drivers/input/misc/uinput.c:452:7: error: too few arguments to function 'input_event_from_user'
      if (input_event_from_user(buffer + bytes, &ev))
          ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:102:5: note: declared here
    int input_event_from_user(const char __user *buffer,
        ^
   drivers/input/misc/uinput.c:456:12: error: too few arguments to function 'input_event_size'
      bytes += input_event_size();
               ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
   drivers/input/misc/uinput.c: In function 'uinput_events_to_user':
   drivers/input/misc/uinput.c:508:16: error: too few arguments to function 'input_event_size'
     while (read + input_event_size() <= count &&
                   ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
>> drivers/input/misc/uinput.c:511:42: warning: passing argument 2 of 'input_event_to_user' from incompatible pointer type [-Wincompatible-pointer-types]
      if (input_event_to_user(buffer + read, &event))
                                             ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:105:5: note: expected 'const struct input_value *' but argument is of type 'struct input_event *'
    int input_event_to_user(char __user *buffer, const struct input_value *event,
        ^
>> drivers/input/misc/uinput.c:511:7: error: too few arguments to function 'input_event_to_user'
      if (input_event_to_user(buffer + read, &event))
          ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:105:5: note: declared here
    int input_event_to_user(char __user *buffer, const struct input_value *event,
        ^
   drivers/input/misc/uinput.c:514:11: error: too few arguments to function 'input_event_size'
      read += input_event_size();
              ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
   drivers/input/misc/uinput.c: In function 'uinput_read':
   drivers/input/misc/uinput.c:526:28: error: too few arguments to function 'input_event_size'
     if (count != 0 && count < input_event_size())
                               ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^

vim +/input_event_size +442 drivers/input/misc/uinput.c

cbf05413 Ryan Mallon     2013-09-18  436  static ssize_t uinput_inject_events(struct uinput_device *udev,
54ce165e Dmitry Torokhov 2012-07-29  437  				    const char __user *buffer, size_t count)
^1da177e Linus Torvalds  2005-04-16  438  {
^1da177e Linus Torvalds  2005-04-16  439  	struct input_event ev;
cbf05413 Ryan Mallon     2013-09-18  440  	size_t bytes = 0;
^1da177e Linus Torvalds  2005-04-16  441  
cbf05413 Ryan Mallon     2013-09-18 @442  	if (count != 0 && count < input_event_size())
29506415 Dmitry Torokhov 2005-11-20  443  		return -EINVAL;
29506415 Dmitry Torokhov 2005-11-20  444  
cbf05413 Ryan Mallon     2013-09-18 @445  	while (bytes + input_event_size() <= count) {
cbf05413 Ryan Mallon     2013-09-18  446  		/*
cbf05413 Ryan Mallon     2013-09-18  447  		 * Note that even if some events were fetched successfully
cbf05413 Ryan Mallon     2013-09-18  448  		 * we are still going to return EFAULT instead of partial
cbf05413 Ryan Mallon     2013-09-18  449  		 * count to let userspace know that it got it's buffers
cbf05413 Ryan Mallon     2013-09-18  450  		 * all wrong.
cbf05413 Ryan Mallon     2013-09-18  451  		 */
cbf05413 Ryan Mallon     2013-09-18 @452  		if (input_event_from_user(buffer + bytes, &ev))
^1da177e Linus Torvalds  2005-04-16  453  			return -EFAULT;
29506415 Dmitry Torokhov 2005-11-20  454  
^1da177e Linus Torvalds  2005-04-16  455  		input_event(udev->dev, ev.type, ev.code, ev.value);
cbf05413 Ryan Mallon     2013-09-18  456  		bytes += input_event_size();
cbf05413 Ryan Mallon     2013-09-18  457  	}
^1da177e Linus Torvalds  2005-04-16  458  
cbf05413 Ryan Mallon     2013-09-18  459  	return bytes;
29506415 Dmitry Torokhov 2005-11-20  460  }
29506415 Dmitry Torokhov 2005-11-20  461  
54ce165e Dmitry Torokhov 2012-07-29  462  static ssize_t uinput_write(struct file *file, const char __user *buffer,
54ce165e Dmitry Torokhov 2012-07-29  463  			    size_t count, loff_t *ppos)
29506415 Dmitry Torokhov 2005-11-20  464  {
29506415 Dmitry Torokhov 2005-11-20  465  	struct uinput_device *udev = file->private_data;
29506415 Dmitry Torokhov 2005-11-20  466  	int retval;
29506415 Dmitry Torokhov 2005-11-20  467  
22ae19c6 Dmitry Torokhov 2012-07-29  468  	if (count == 0)
22ae19c6 Dmitry Torokhov 2012-07-29  469  		return 0;
22ae19c6 Dmitry Torokhov 2012-07-29  470  
221979aa Dmitry Torokhov 2006-02-19  471  	retval = mutex_lock_interruptible(&udev->mutex);
29506415 Dmitry Torokhov 2005-11-20  472  	if (retval)
29506415 Dmitry Torokhov 2005-11-20  473  		return retval;
29506415 Dmitry Torokhov 2005-11-20  474  
29506415 Dmitry Torokhov 2005-11-20  475  	retval = udev->state == UIST_CREATED ?
cbf05413 Ryan Mallon     2013-09-18  476  			uinput_inject_events(udev, buffer, count) :
29506415 Dmitry Torokhov 2005-11-20  477  			uinput_setup_device(udev, buffer, count);
29506415 Dmitry Torokhov 2005-11-20  478  
221979aa Dmitry Torokhov 2006-02-19  479  	mutex_unlock(&udev->mutex);
29506415 Dmitry Torokhov 2005-11-20  480  
29506415 Dmitry Torokhov 2005-11-20  481  	return retval;
^1da177e Linus Torvalds  2005-04-16  482  }
^1da177e Linus Torvalds  2005-04-16  483  
929d1af5 Dmitry Torokhov 2012-07-29  484  static bool uinput_fetch_next_event(struct uinput_device *udev,
929d1af5 Dmitry Torokhov 2012-07-29  485  				    struct input_event *event)
929d1af5 Dmitry Torokhov 2012-07-29  486  {
929d1af5 Dmitry Torokhov 2012-07-29  487  	bool have_event;
929d1af5 Dmitry Torokhov 2012-07-29  488  
929d1af5 Dmitry Torokhov 2012-07-29  489  	spin_lock_irq(&udev->dev->event_lock);
929d1af5 Dmitry Torokhov 2012-07-29  490  
929d1af5 Dmitry Torokhov 2012-07-29  491  	have_event = udev->head != udev->tail;
929d1af5 Dmitry Torokhov 2012-07-29  492  	if (have_event) {
929d1af5 Dmitry Torokhov 2012-07-29  493  		*event = udev->buff[udev->tail];
929d1af5 Dmitry Torokhov 2012-07-29  494  		udev->tail = (udev->tail + 1) % UINPUT_BUFFER_SIZE;
929d1af5 Dmitry Torokhov 2012-07-29  495  	}
929d1af5 Dmitry Torokhov 2012-07-29  496  
929d1af5 Dmitry Torokhov 2012-07-29  497  	spin_unlock_irq(&udev->dev->event_lock);
929d1af5 Dmitry Torokhov 2012-07-29  498  
929d1af5 Dmitry Torokhov 2012-07-29  499  	return have_event;
929d1af5 Dmitry Torokhov 2012-07-29  500  }
929d1af5 Dmitry Torokhov 2012-07-29  501  
22ae19c6 Dmitry Torokhov 2012-07-29  502  static ssize_t uinput_events_to_user(struct uinput_device *udev,
22ae19c6 Dmitry Torokhov 2012-07-29  503  				     char __user *buffer, size_t count)
^1da177e Linus Torvalds  2005-04-16  504  {
929d1af5 Dmitry Torokhov 2012-07-29  505  	struct input_event event;
22ae19c6 Dmitry Torokhov 2012-07-29  506  	size_t read = 0;
^1da177e Linus Torvalds  2005-04-16  507  
22ae19c6 Dmitry Torokhov 2012-07-29 @508  	while (read + input_event_size() <= count &&
22ae19c6 Dmitry Torokhov 2012-07-29  509  	       uinput_fetch_next_event(udev, &event)) {
f40033ac David Herrmann  2012-07-29  510  
00ce756c Dmitry Torokhov 2012-07-29 @511  		if (input_event_to_user(buffer + read, &event))
00ce756c Dmitry Torokhov 2012-07-29  512  			return -EFAULT;
^1da177e Linus Torvalds  2005-04-16  513  
22ae19c6 Dmitry Torokhov 2012-07-29  514  		read += input_event_size();

:::::: The code at line 442 was first introduced by commit
:::::: cbf0541374e2fcfdfdcaf8365c957a137eb9feea Input: uinput - support injecting multiple events in one write() call

:::::: TO: Ryan Mallon <rmallon@gmail.com>
:::::: CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 20054 bytes --]

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

* Re: [PATCH 1/3] input: evdev: introduce new evdev interface
@ 2015-11-27 10:37     ` kbuild test robot
  0 siblings, 0 replies; 27+ messages in thread
From: kbuild test robot @ 2015-11-27 10:37 UTC (permalink / raw)
  Cc: kbuild-all-JC7UmRfGjtg, y2038-oiO2AC+8nQse6UdIy6AN3A,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w, arnd-r2nGTMty4D4,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, WEN Pingbo

[-- Attachment #1: Type: text/plain, Size: 10620 bytes --]

Hi WEN,

[auto build test ERROR on: input/next]
[also build test ERROR on: v4.4-rc2 next-20151127]

url:    https://github.com/0day-ci/linux/commits/WEN-Pingbo/introduce-new-evdev-interface-type/20151127-180438
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s0-201547 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/WEN-Pingbo/introduce-new-evdev-interface-type/20151127-180438 HEAD fc81990de5842e76f794f755e095e4c5e55f8caa builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   drivers/input/misc/uinput.c: In function 'uinput_inject_events':
>> drivers/input/misc/uinput.c:442:28: error: too few arguments to function 'input_event_size'
     if (count != 0 && count < input_event_size())
                               ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
   drivers/input/misc/uinput.c:445:17: error: too few arguments to function 'input_event_size'
     while (bytes + input_event_size() <= count) {
                    ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
>> drivers/input/misc/uinput.c:452:45: warning: passing argument 2 of 'input_event_from_user' from incompatible pointer type [-Wincompatible-pointer-types]
      if (input_event_from_user(buffer + bytes, &ev))
                                                ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:102:5: note: expected 'struct input_value *' but argument is of type 'struct input_event *'
    int input_event_from_user(const char __user *buffer,
        ^
>> drivers/input/misc/uinput.c:452:7: error: too few arguments to function 'input_event_from_user'
      if (input_event_from_user(buffer + bytes, &ev))
          ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:102:5: note: declared here
    int input_event_from_user(const char __user *buffer,
        ^
   drivers/input/misc/uinput.c:456:12: error: too few arguments to function 'input_event_size'
      bytes += input_event_size();
               ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
   drivers/input/misc/uinput.c: In function 'uinput_events_to_user':
   drivers/input/misc/uinput.c:508:16: error: too few arguments to function 'input_event_size'
     while (read + input_event_size() <= count &&
                   ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
>> drivers/input/misc/uinput.c:511:42: warning: passing argument 2 of 'input_event_to_user' from incompatible pointer type [-Wincompatible-pointer-types]
      if (input_event_to_user(buffer + read, &event))
                                             ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:105:5: note: expected 'const struct input_value *' but argument is of type 'struct input_event *'
    int input_event_to_user(char __user *buffer, const struct input_value *event,
        ^
>> drivers/input/misc/uinput.c:511:7: error: too few arguments to function 'input_event_to_user'
      if (input_event_to_user(buffer + read, &event))
          ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:105:5: note: declared here
    int input_event_to_user(char __user *buffer, const struct input_value *event,
        ^
   drivers/input/misc/uinput.c:514:11: error: too few arguments to function 'input_event_size'
      read += input_event_size();
              ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^
   drivers/input/misc/uinput.c: In function 'uinput_read':
   drivers/input/misc/uinput.c:526:28: error: too few arguments to function 'input_event_size'
     if (count != 0 && count < input_event_size())
                               ^
   In file included from drivers/input/misc/uinput.c:43:0:
   drivers/input/misc/../input-compat.h:84:22: note: declared here
    static inline size_t input_event_size(int if_type)
                         ^

vim +/input_event_size +442 drivers/input/misc/uinput.c

cbf05413 Ryan Mallon     2013-09-18  436  static ssize_t uinput_inject_events(struct uinput_device *udev,
54ce165e Dmitry Torokhov 2012-07-29  437  				    const char __user *buffer, size_t count)
^1da177e Linus Torvalds  2005-04-16  438  {
^1da177e Linus Torvalds  2005-04-16  439  	struct input_event ev;
cbf05413 Ryan Mallon     2013-09-18  440  	size_t bytes = 0;
^1da177e Linus Torvalds  2005-04-16  441  
cbf05413 Ryan Mallon     2013-09-18 @442  	if (count != 0 && count < input_event_size())
29506415 Dmitry Torokhov 2005-11-20  443  		return -EINVAL;
29506415 Dmitry Torokhov 2005-11-20  444  
cbf05413 Ryan Mallon     2013-09-18 @445  	while (bytes + input_event_size() <= count) {
cbf05413 Ryan Mallon     2013-09-18  446  		/*
cbf05413 Ryan Mallon     2013-09-18  447  		 * Note that even if some events were fetched successfully
cbf05413 Ryan Mallon     2013-09-18  448  		 * we are still going to return EFAULT instead of partial
cbf05413 Ryan Mallon     2013-09-18  449  		 * count to let userspace know that it got it's buffers
cbf05413 Ryan Mallon     2013-09-18  450  		 * all wrong.
cbf05413 Ryan Mallon     2013-09-18  451  		 */
cbf05413 Ryan Mallon     2013-09-18 @452  		if (input_event_from_user(buffer + bytes, &ev))
^1da177e Linus Torvalds  2005-04-16  453  			return -EFAULT;
29506415 Dmitry Torokhov 2005-11-20  454  
^1da177e Linus Torvalds  2005-04-16  455  		input_event(udev->dev, ev.type, ev.code, ev.value);
cbf05413 Ryan Mallon     2013-09-18  456  		bytes += input_event_size();
cbf05413 Ryan Mallon     2013-09-18  457  	}
^1da177e Linus Torvalds  2005-04-16  458  
cbf05413 Ryan Mallon     2013-09-18  459  	return bytes;
29506415 Dmitry Torokhov 2005-11-20  460  }
29506415 Dmitry Torokhov 2005-11-20  461  
54ce165e Dmitry Torokhov 2012-07-29  462  static ssize_t uinput_write(struct file *file, const char __user *buffer,
54ce165e Dmitry Torokhov 2012-07-29  463  			    size_t count, loff_t *ppos)
29506415 Dmitry Torokhov 2005-11-20  464  {
29506415 Dmitry Torokhov 2005-11-20  465  	struct uinput_device *udev = file->private_data;
29506415 Dmitry Torokhov 2005-11-20  466  	int retval;
29506415 Dmitry Torokhov 2005-11-20  467  
22ae19c6 Dmitry Torokhov 2012-07-29  468  	if (count == 0)
22ae19c6 Dmitry Torokhov 2012-07-29  469  		return 0;
22ae19c6 Dmitry Torokhov 2012-07-29  470  
221979aa Dmitry Torokhov 2006-02-19  471  	retval = mutex_lock_interruptible(&udev->mutex);
29506415 Dmitry Torokhov 2005-11-20  472  	if (retval)
29506415 Dmitry Torokhov 2005-11-20  473  		return retval;
29506415 Dmitry Torokhov 2005-11-20  474  
29506415 Dmitry Torokhov 2005-11-20  475  	retval = udev->state == UIST_CREATED ?
cbf05413 Ryan Mallon     2013-09-18  476  			uinput_inject_events(udev, buffer, count) :
29506415 Dmitry Torokhov 2005-11-20  477  			uinput_setup_device(udev, buffer, count);
29506415 Dmitry Torokhov 2005-11-20  478  
221979aa Dmitry Torokhov 2006-02-19  479  	mutex_unlock(&udev->mutex);
29506415 Dmitry Torokhov 2005-11-20  480  
29506415 Dmitry Torokhov 2005-11-20  481  	return retval;
^1da177e Linus Torvalds  2005-04-16  482  }
^1da177e Linus Torvalds  2005-04-16  483  
929d1af5 Dmitry Torokhov 2012-07-29  484  static bool uinput_fetch_next_event(struct uinput_device *udev,
929d1af5 Dmitry Torokhov 2012-07-29  485  				    struct input_event *event)
929d1af5 Dmitry Torokhov 2012-07-29  486  {
929d1af5 Dmitry Torokhov 2012-07-29  487  	bool have_event;
929d1af5 Dmitry Torokhov 2012-07-29  488  
929d1af5 Dmitry Torokhov 2012-07-29  489  	spin_lock_irq(&udev->dev->event_lock);
929d1af5 Dmitry Torokhov 2012-07-29  490  
929d1af5 Dmitry Torokhov 2012-07-29  491  	have_event = udev->head != udev->tail;
929d1af5 Dmitry Torokhov 2012-07-29  492  	if (have_event) {
929d1af5 Dmitry Torokhov 2012-07-29  493  		*event = udev->buff[udev->tail];
929d1af5 Dmitry Torokhov 2012-07-29  494  		udev->tail = (udev->tail + 1) % UINPUT_BUFFER_SIZE;
929d1af5 Dmitry Torokhov 2012-07-29  495  	}
929d1af5 Dmitry Torokhov 2012-07-29  496  
929d1af5 Dmitry Torokhov 2012-07-29  497  	spin_unlock_irq(&udev->dev->event_lock);
929d1af5 Dmitry Torokhov 2012-07-29  498  
929d1af5 Dmitry Torokhov 2012-07-29  499  	return have_event;
929d1af5 Dmitry Torokhov 2012-07-29  500  }
929d1af5 Dmitry Torokhov 2012-07-29  501  
22ae19c6 Dmitry Torokhov 2012-07-29  502  static ssize_t uinput_events_to_user(struct uinput_device *udev,
22ae19c6 Dmitry Torokhov 2012-07-29  503  				     char __user *buffer, size_t count)
^1da177e Linus Torvalds  2005-04-16  504  {
929d1af5 Dmitry Torokhov 2012-07-29  505  	struct input_event event;
22ae19c6 Dmitry Torokhov 2012-07-29  506  	size_t read = 0;
^1da177e Linus Torvalds  2005-04-16  507  
22ae19c6 Dmitry Torokhov 2012-07-29 @508  	while (read + input_event_size() <= count &&
22ae19c6 Dmitry Torokhov 2012-07-29  509  	       uinput_fetch_next_event(udev, &event)) {
f40033ac David Herrmann  2012-07-29  510  
00ce756c Dmitry Torokhov 2012-07-29 @511  		if (input_event_to_user(buffer + read, &event))
00ce756c Dmitry Torokhov 2012-07-29  512  			return -EFAULT;
^1da177e Linus Torvalds  2005-04-16  513  
22ae19c6 Dmitry Torokhov 2012-07-29  514  		read += input_event_size();

:::::: The code at line 442 was first introduced by commit
:::::: cbf0541374e2fcfdfdcaf8365c957a137eb9feea Input: uinput - support injecting multiple events in one write() call

:::::: TO: Ryan Mallon <rmallon-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
:::::: CC: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 20054 bytes --]

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-11-27 16:58   ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-11-27 16:58 UTC (permalink / raw)
  To: WEN Pingbo
  Cc: y2038, dmitry.torokhov, aksgarg1989, linux-input, linux-kernel,
	linux-api

On Friday 27 November 2015 18:00:29 WEN Pingbo wrote:
> To solve the y2038 problem in input_event, I had some attempts before [1],
> and this is the second one.
> 
> We can force userspace to use monotonic time in event timestamp, so the
> 'struct timeval' is enough to keep y2038-safe, as Arnd suggested. But we
> can not find a way to make kernel compatible with old binaries, which use
> realtime, and there are still some devices, which depend on realtime.
> 
> So I get a idea to add a new evdev interface, which is y2038 safe. And
> userspace can switch between the old and new interface via ioctl.
> 
> The patch series add three evdev interface type:
> 
> - EV_IF_LEGACY
>   send event by input_event. This is the default option, keep kernel
>   backward compatible.

The problem I see with this approach is that it still breaks any
legacy source code that is compiled with a new libc that uses 64-bit
time_t. If we are requiring source code changes for building users
of input devices with a new libc, we can easily get them to handle
the overflow (they normally only care about the microsecond portion
anyway, so it doesn't matter in most cases), or to use monotonic time.

Did I miss something here?

	Arnd

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-11-27 16:58   ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-11-27 16:58 UTC (permalink / raw)
  To: WEN Pingbo
  Cc: y2038-cunTk1MwBs8s++Sfvej+rw,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

On Friday 27 November 2015 18:00:29 WEN Pingbo wrote:
> To solve the y2038 problem in input_event, I had some attempts before [1],
> and this is the second one.
> 
> We can force userspace to use monotonic time in event timestamp, so the
> 'struct timeval' is enough to keep y2038-safe, as Arnd suggested. But we
> can not find a way to make kernel compatible with old binaries, which use
> realtime, and there are still some devices, which depend on realtime.
> 
> So I get a idea to add a new evdev interface, which is y2038 safe. And
> userspace can switch between the old and new interface via ioctl.
> 
> The patch series add three evdev interface type:
> 
> - EV_IF_LEGACY
>   send event by input_event. This is the default option, keep kernel
>   backward compatible.

The problem I see with this approach is that it still breaks any
legacy source code that is compiled with a new libc that uses 64-bit
time_t. If we are requiring source code changes for building users
of input devices with a new libc, we can easily get them to handle
the overflow (they normally only care about the microsecond portion
anyway, so it doesn't matter in most cases), or to use monotonic time.

Did I miss something here?

	Arnd

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

* Re: [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE
  2015-11-27 10:00 ` [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE WEN Pingbo
@ 2015-11-27 16:59     ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-11-27 16:59 UTC (permalink / raw)
  To: WEN Pingbo
  Cc: y2038, dmitry.torokhov, aksgarg1989, linux-input, linux-kernel,
	linux-api

On Friday 27 November 2015 18:00:31 WEN Pingbo wrote:
> This patch depends on 'introduce new evdev interface'.
> 
> Userspace cat set / get evdev interface type via the two ioctl
> commands. And default interface type is EV_IF_LEGACY, so the old binary
> will work normal with new kernel. Maybe we should change this default
> option to encourage people to move to new interface.
> 
> And since all events are stored as input_value in evdev, there are no
> need to flush evdev_client's buffer if we change clk_type and if_type.

I would split out the change to evdev_set_clk_type into a separate patch.

> +	case EVIOCSIFTYPE:
> +		if (get_user(if_type, ip))
> +			return -EFAULT;
> +
> +		return evdev_set_if_type(client, if_type);
> +	case EVIOCGIFTYPE:
> +		return put_user(client->if_type, ip);
>  	}

This look asymmetric: EVIOCSIFTYPE uses a EVDEV_* constant, while
EVIOCGIFTYPE returns a EV_IF_* constant. Should those just
be the same constants anyway?

	Arnd

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

* Re: [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE
@ 2015-11-27 16:59     ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-11-27 16:59 UTC (permalink / raw)
  To: WEN Pingbo
  Cc: y2038, linux-api, dmitry.torokhov, linux-kernel, aksgarg1989,
	linux-input

On Friday 27 November 2015 18:00:31 WEN Pingbo wrote:
> This patch depends on 'introduce new evdev interface'.
> 
> Userspace cat set / get evdev interface type via the two ioctl
> commands. And default interface type is EV_IF_LEGACY, so the old binary
> will work normal with new kernel. Maybe we should change this default
> option to encourage people to move to new interface.
> 
> And since all events are stored as input_value in evdev, there are no
> need to flush evdev_client's buffer if we change clk_type and if_type.

I would split out the change to evdev_set_clk_type into a separate patch.

> +	case EVIOCSIFTYPE:
> +		if (get_user(if_type, ip))
> +			return -EFAULT;
> +
> +		return evdev_set_if_type(client, if_type);
> +	case EVIOCGIFTYPE:
> +		return put_user(client->if_type, ip);
>  	}

This look asymmetric: EVIOCSIFTYPE uses a EVDEV_* constant, while
EVIOCGIFTYPE returns a EV_IF_* constant. Should those just
be the same constants anyway?

	Arnd
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

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

* Re: [PATCH 0/3] introduce new evdev interface type
  2015-11-27 16:58   ` Arnd Bergmann
@ 2015-11-29  9:13     ` Pingbo Wen
  -1 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-11-29  9:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, dmitry.torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api

Hi, Arnd

> 在 2015年11月28日,00:58,Arnd Bergmann <arnd@arndb.de> 写道:
> 
> On Friday 27 November 2015 18:00:29 WEN Pingbo wrote:
>> To solve the y2038 problem in input_event, I had some attempts before [1],
>> and this is the second one.
>> 
>> We can force userspace to use monotonic time in event timestamp, so the
>> 'struct timeval' is enough to keep y2038-safe, as Arnd suggested. But we
>> can not find a way to make kernel compatible with old binaries, which use
>> realtime, and there are still some devices, which depend on realtime.
>> 
>> So I get a idea to add a new evdev interface, which is y2038 safe. And
>> userspace can switch between the old and new interface via ioctl.
>> 
>> The patch series add three evdev interface type:
>> 
>> - EV_IF_LEGACY
>>  send event by input_event. This is the default option, keep kernel
>>  backward compatible.
> 
> The problem I see with this approach is that it still breaks any
> legacy source code that is compiled with a new libc that uses 64-bit
> time_t. If we are requiring source code changes for building users
> of input devices with a new libc, we can easily get them to handle
> the overflow (they normally only care about the microsecond portion
> anyway, so it doesn't matter in most cases), or to use monotonic time.

I don’t think so.

Actually, from the view of userspace, EV_IF_LEGACY interface is work
exactly the same as old evdev. We didn’t change anything in input_event
and input_event_compat. And the problem you said will still be there,
even without those patches.

Pingbo

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-11-29  9:13     ` Pingbo Wen
  0 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-11-29  9:13 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, dmitry.torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api

Hi, Arnd

> 在 2015年11月28日,00:58,Arnd Bergmann <arnd@arndb.de> 写道:
> 
> On Friday 27 November 2015 18:00:29 WEN Pingbo wrote:
>> To solve the y2038 problem in input_event, I had some attempts before [1],
>> and this is the second one.
>> 
>> We can force userspace to use monotonic time in event timestamp, so the
>> 'struct timeval' is enough to keep y2038-safe, as Arnd suggested. But we
>> can not find a way to make kernel compatible with old binaries, which use
>> realtime, and there are still some devices, which depend on realtime.
>> 
>> So I get a idea to add a new evdev interface, which is y2038 safe. And
>> userspace can switch between the old and new interface via ioctl.
>> 
>> The patch series add three evdev interface type:
>> 
>> - EV_IF_LEGACY
>>  send event by input_event. This is the default option, keep kernel
>>  backward compatible.
> 
> The problem I see with this approach is that it still breaks any
> legacy source code that is compiled with a new libc that uses 64-bit
> time_t. If we are requiring source code changes for building users
> of input devices with a new libc, we can easily get them to handle
> the overflow (they normally only care about the microsecond portion
> anyway, so it doesn't matter in most cases), or to use monotonic time.

I don’t think so.

Actually, from the view of userspace, EV_IF_LEGACY interface is work
exactly the same as old evdev. We didn’t change anything in input_event
and input_event_compat. And the problem you said will still be there,
even without those patches.

Pingbo--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE
  2015-11-27 16:59     ` Arnd Bergmann
@ 2015-11-29  9:19       ` Pingbo Wen
  -1 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-11-29  9:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, dmitry.torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api


> 在 2015年11月28日,00:59,Arnd Bergmann <arnd@arndb.de> 写道:
> 
> On Friday 27 November 2015 18:00:31 WEN Pingbo wrote:
>> This patch depends on 'introduce new evdev interface'.
>> 
>> Userspace cat set / get evdev interface type via the two ioctl
>> commands. And default interface type is EV_IF_LEGACY, so the old binary
>> will work normal with new kernel. Maybe we should change this default
>> option to encourage people to move to new interface.
>> 
>> And since all events are stored as input_value in evdev, there are no
>> need to flush evdev_client's buffer if we change clk_type and if_type.
> 
> I would split out the change to evdev_set_clk_type into a separate patch.

Agreed.

> 
>> +	case EVIOCSIFTYPE:
>> +		if (get_user(if_type, ip))
>> +			return -EFAULT;
>> +
>> +		return evdev_set_if_type(client, if_type);
>> +	case EVIOCGIFTYPE:
>> +		return put_user(client->if_type, ip);
>> 	}
> 
> This look asymmetric: EVIOCSIFTYPE uses a EVDEV_* constant, while
> EVIOCGIFTYPE returns a EV_IF_* constant. Should those just
> be the same constants anyway?

Yes, thanks for pointing it out. I need add evdev_get_if_type() here.

Pingbo


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

* Re: [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE
@ 2015-11-29  9:19       ` Pingbo Wen
  0 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-11-29  9:19 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038-cunTk1MwBs8s++Sfvej+rw,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA


> 在 2015年11月28日,00:59,Arnd Bergmann <arnd@arndb.de> 写道:
> 
> On Friday 27 November 2015 18:00:31 WEN Pingbo wrote:
>> This patch depends on 'introduce new evdev interface'.
>> 
>> Userspace cat set / get evdev interface type via the two ioctl
>> commands. And default interface type is EV_IF_LEGACY, so the old binary
>> will work normal with new kernel. Maybe we should change this default
>> option to encourage people to move to new interface.
>> 
>> And since all events are stored as input_value in evdev, there are no
>> need to flush evdev_client's buffer if we change clk_type and if_type.
> 
> I would split out the change to evdev_set_clk_type into a separate patch.

Agreed.

> 
>> +	case EVIOCSIFTYPE:
>> +		if (get_user(if_type, ip))
>> +			return -EFAULT;
>> +
>> +		return evdev_set_if_type(client, if_type);
>> +	case EVIOCGIFTYPE:
>> +		return put_user(client->if_type, ip);
>> 	}
> 
> This look asymmetric: EVIOCSIFTYPE uses a EVDEV_* constant, while
> EVIOCGIFTYPE returns a EV_IF_* constant. Should those just
> be the same constants anyway?

Yes, thanks for pointing it out. I need add evdev_get_if_type() here.

Pingbo

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

* Re: [PATCH 0/3] introduce new evdev interface type
  2015-11-29  9:13     ` Pingbo Wen
  (?)
@ 2015-11-30 15:13     ` Arnd Bergmann
  2015-12-01  8:34         ` Pingbo Wen
  2015-12-01  8:34         ` Pingbo Wen
  -1 siblings, 2 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-11-30 15:13 UTC (permalink / raw)
  To: Pingbo Wen
  Cc: y2038, dmitry.torokhov, aksgarg1989, linux-input, linux-kernel,
	linux-api

On Sunday 29 November 2015 17:13:50 Pingbo Wen wrote:
> > 在 2015年11月28日,00:58,Arnd Bergmann <arnd@arndb.de> 写道:
> > 
> > On Friday 27 November 2015 18:00:29 WEN Pingbo wrote:
> >> To solve the y2038 problem in input_event, I had some attempts before [1],
> >> and this is the second one.
> >> 
> >> We can force userspace to use monotonic time in event timestamp, so the
> >> 'struct timeval' is enough to keep y2038-safe, as Arnd suggested. But we
> >> can not find a way to make kernel compatible with old binaries, which use
> >> realtime, and there are still some devices, which depend on realtime.
> >> 
> >> So I get a idea to add a new evdev interface, which is y2038 safe. And
> >> userspace can switch between the old and new interface via ioctl.
> >> 
> >> The patch series add three evdev interface type:
> >> 
> >> - EV_IF_LEGACY
> >>  send event by input_event. This is the default option, keep kernel
> >>  backward compatible.
> > 
> > The problem I see with this approach is that it still breaks any
> > legacy source code that is compiled with a new libc that uses 64-bit
> > time_t. If we are requiring source code changes for building users
> > of input devices with a new libc, we can easily get them to handle
> > the overflow (they normally only care about the microsecond portion
> > anyway, so it doesn't matter in most cases), or to use monotonic time.
> 
> I don’t think so.
> 
> Actually, from the view of userspace, EV_IF_LEGACY interface is work
> exactly the same as old evdev. We didn’t change anything in input_event
> and input_event_compat. And the problem you said will still be there,
> even without those patches.

I think we're still talking past one another. I thought we had established
that

1. the current interface is broken when time_t changes in user space
2. we can fix it by redefining struct input_event in a way that
   is independent of time_t
3. once both user space and kernel are using the same ABI independent
   of time_t, we can look improving the timestamps so they don't
   overflow
4. the monotonic timestamp interface already avoids the overflow, so
   it would be sufficient as a solution for 3.

Where did I lose you here? Did you find any other facts that I
was missing? I don't know whether the two new event structures make
the interface better in some other way, but it seems mostly unrelated
to either of the two problems we already have with time_t (the
ABI mismatch, and the use of non-monotonic timestamps).

	Arnd

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

* Re: [PATCH 0/3] introduce new evdev interface type
  2015-11-30 15:13     ` Arnd Bergmann
@ 2015-12-01  8:34         ` Pingbo Wen
  2015-12-01  8:34         ` Pingbo Wen
  1 sibling, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-12-01  8:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, Dmitry Torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api

Hi, Arnd

>>>> 
>>>> The patch series add three evdev interface type:
>>>> 
>>>> - EV_IF_LEGACY
>>>> send event by input_event. This is the default option, keep kernel
>>>> backward compatible.
>>> 
>>> The problem I see with this approach is that it still breaks any
>>> legacy source code that is compiled with a new libc that uses 64-bit
>>> time_t. If we are requiring source code changes for building users
>>> of input devices with a new libc, we can easily get them to handle
>>> the overflow (they normally only care about the microsecond portion
>>> anyway, so it doesn't matter in most cases), or to use monotonic time.
>> 
>> I don’t think so.
>> 
>> Actually, from the view of userspace, EV_IF_LEGACY interface is work
>> exactly the same as old evdev. We didn’t change anything in input_event
>> and input_event_compat. And the problem you said will still be there,
>> even without those patches.
> 
> I think we're still talking past one another. I thought we had established
> that
> 
> 1. the current interface is broken when time_t changes in user space

What kinds of changes in time_t? Extending it to 64-bits in both kernel
and userspace? Ok, I get confused here, if there are some sample codes
or use-cases here, it will help me a lot.

> 2. we can fix it by redefining struct input_event in a way that
>   is independent of time_t
> 3. once both user space and kernel are using the same ABI independent
>   of time_t, we can look improving the timestamps so they don't
>   overflow
> 4. the monotonic timestamp interface already avoids the overflow, so
>   it would be sufficient as a solution for 3.
> 
> Where did I lose you here? Did you find any other facts that I
> was missing? I don't know whether the two new event structures make
> the interface better in some other way, but it seems mostly unrelated
> to either of the two problems we already have with time_t (the
> ABI mismatch, and the use of non-monotonic timestamps).

It seems we are mismatch here.

Actually input_composite_event has the similar structure with input_event,
but with a nicer definition, which can take both monotonic and non-monotonic
timestamps safely.

What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
interface. If userspace try to adapt to new libc and kernel, it should move
to new interface. The userspace can do a lazy update, keep the code untouched,
but suffer the y2038 problem by itself.

We can force kernel using monotonic time in EV_IF_LEGACY interface, and
making input_event independent from time_t(after evdev has converted to
input_value, it’s easy to do that), but that also imply userspace
must change their code to fit this change. If changing userspace code is
a mandatory option, why not to force them do a complete conversion?

Pingbo

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

* Re: [PATCH 0/3] introduce new evdev interface type
  2015-11-30 15:13     ` Arnd Bergmann
@ 2015-12-01  8:34         ` Pingbo Wen
  2015-12-01  8:34         ` Pingbo Wen
  1 sibling, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-12-01  8:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, Dmitry Torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api

Hi, Arnd

>>>> 
>>>> The patch series add three evdev interface type:
>>>> 
>>>> - EV_IF_LEGACY
>>>> send event by input_event. This is the default option, keep kernel
>>>> backward compatible.
>>> 
>>> The problem I see with this approach is that it still breaks any
>>> legacy source code that is compiled with a new libc that uses 64-bit
>>> time_t. If we are requiring source code changes for building users
>>> of input devices with a new libc, we can easily get them to handle
>>> the overflow (they normally only care about the microsecond portion
>>> anyway, so it doesn't matter in most cases), or to use monotonic time.
>> 
>> I don’t think so.
>> 
>> Actually, from the view of userspace, EV_IF_LEGACY interface is work
>> exactly the same as old evdev. We didn’t change anything in input_event
>> and input_event_compat. And the problem you said will still be there,
>> even without those patches.
> 
> I think we're still talking past one another. I thought we had established
> that
> 
> 1. the current interface is broken when time_t changes in user space

What kinds of changes in time_t? Extending it to 64-bits in both kernel
and userspace? Ok, I get confused here, if there are some sample codes
or use-cases here, it will help me a lot.

> 2. we can fix it by redefining struct input_event in a way that
>   is independent of time_t
> 3. once both user space and kernel are using the same ABI independent
>   of time_t, we can look improving the timestamps so they don't
>   overflow
> 4. the monotonic timestamp interface already avoids the overflow, so
>   it would be sufficient as a solution for 3.
> 
> Where did I lose you here? Did you find any other facts that I
> was missing? I don't know whether the two new event structures make
> the interface better in some other way, but it seems mostly unrelated
> to either of the two problems we already have with time_t (the
> ABI mismatch, and the use of non-monotonic timestamps).

It seems we are mismatch here.

Actually input_composite_event has the similar structure with input_event,
but with a nicer definition, which can take both monotonic and non-monotonic
timestamps safely.

What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
interface. If userspace try to adapt to new libc and kernel, it should move
to new interface. The userspace can do a lazy update, keep the code untouched,
but suffer the y2038 problem by itself.

We can force kernel using monotonic time in EV_IF_LEGACY interface, and
making input_event independent from time_t(after evdev has converted to
input_value, it’s easy to do that), but that also imply userspace
must change their code to fit this change. If changing userspace code is
a mandatory option, why not to force them do a complete conversion?

Pingbo

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-12-01  8:34         ` Pingbo Wen
  0 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-12-01  8:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, Dmitry Torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api

Hi, Arnd

>>>> 
>>>> The patch series add three evdev interface type:
>>>> 
>>>> - EV_IF_LEGACY
>>>> send event by input_event. This is the default option, keep kernel
>>>> backward compatible.
>>> 
>>> The problem I see with this approach is that it still breaks any
>>> legacy source code that is compiled with a new libc that uses 64-bit
>>> time_t. If we are requiring source code changes for building users
>>> of input devices with a new libc, we can easily get them to handle
>>> the overflow (they normally only care about the microsecond portion
>>> anyway, so it doesn't matter in most cases), or to use monotonic time.
>> 
>> I don’t think so.
>> 
>> Actually, from the view of userspace, EV_IF_LEGACY interface is work
>> exactly the same as old evdev. We didn’t change anything in input_event
>> and input_event_compat. And the problem you said will still be there,
>> even without those patches.
> 
> I think we're still talking past one another. I thought we had established
> that
> 
> 1. the current interface is broken when time_t changes in user space

What kinds of changes in time_t? Extending it to 64-bits in both kernel
and userspace? Ok, I get confused here, if there are some sample codes
or use-cases here, it will help me a lot.

> 2. we can fix it by redefining struct input_event in a way that
>   is independent of time_t
> 3. once both user space and kernel are using the same ABI independent
>   of time_t, we can look improving the timestamps so they don't
>   overflow
> 4. the monotonic timestamp interface already avoids the overflow, so
>   it would be sufficient as a solution for 3.
> 
> Where did I lose you here? Did you find any other facts that I
> was missing? I don't know whether the two new event structures make
> the interface better in some other way, but it seems mostly unrelated
> to either of the two problems we already have with time_t (the
> ABI mismatch, and the use of non-monotonic timestamps).

It seems we are mismatch here.

Actually input_composite_event has the similar structure with input_event,
but with a nicer definition, which can take both monotonic and non-monotonic
timestamps safely.

What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
interface. If userspace try to adapt to new libc and kernel, it should move
to new interface. The userspace can do a lazy update, keep the code untouched,
but suffer the y2038 problem by itself.

We can force kernel using monotonic time in EV_IF_LEGACY interface, and
making input_event independent from time_t(after evdev has converted to
input_value, it’s easy to do that), but that also imply userspace
must change their code to fit this change. If changing userspace code is
a mandatory option, why not to force them do a complete conversion?

Pingbo--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-12-01  8:34         ` Pingbo Wen
  0 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-12-01  8:34 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038-cunTk1MwBs8s++Sfvej+rw, Dmitry Torokhov,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

Hi, Arnd

>>>> 
>>>> The patch series add three evdev interface type:
>>>> 
>>>> - EV_IF_LEGACY
>>>> send event by input_event. This is the default option, keep kernel
>>>> backward compatible.
>>> 
>>> The problem I see with this approach is that it still breaks any
>>> legacy source code that is compiled with a new libc that uses 64-bit
>>> time_t. If we are requiring source code changes for building users
>>> of input devices with a new libc, we can easily get them to handle
>>> the overflow (they normally only care about the microsecond portion
>>> anyway, so it doesn't matter in most cases), or to use monotonic time.
>> 
>> I don’t think so.
>> 
>> Actually, from the view of userspace, EV_IF_LEGACY interface is work
>> exactly the same as old evdev. We didn’t change anything in input_event
>> and input_event_compat. And the problem you said will still be there,
>> even without those patches.
> 
> I think we're still talking past one another. I thought we had established
> that
> 
> 1. the current interface is broken when time_t changes in user space

What kinds of changes in time_t? Extending it to 64-bits in both kernel
and userspace? Ok, I get confused here, if there are some sample codes
or use-cases here, it will help me a lot.

> 2. we can fix it by redefining struct input_event in a way that
>   is independent of time_t
> 3. once both user space and kernel are using the same ABI independent
>   of time_t, we can look improving the timestamps so they don't
>   overflow
> 4. the monotonic timestamp interface already avoids the overflow, so
>   it would be sufficient as a solution for 3.
> 
> Where did I lose you here? Did you find any other facts that I
> was missing? I don't know whether the two new event structures make
> the interface better in some other way, but it seems mostly unrelated
> to either of the two problems we already have with time_t (the
> ABI mismatch, and the use of non-monotonic timestamps).

It seems we are mismatch here.

Actually input_composite_event has the similar structure with input_event,
but with a nicer definition, which can take both monotonic and non-monotonic
timestamps safely.

What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
interface. If userspace try to adapt to new libc and kernel, it should move
to new interface. The userspace can do a lazy update, keep the code untouched,
but suffer the y2038 problem by itself.

We can force kernel using monotonic time in EV_IF_LEGACY interface, and
making input_event independent from time_t(after evdev has converted to
input_value, it’s easy to do that), but that also imply userspace
must change their code to fit this change. If changing userspace code is
a mandatory option, why not to force them do a complete conversion?

Pingbo--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-12-01 10:47           ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-12-01 10:47 UTC (permalink / raw)
  To: Pingbo Wen
  Cc: y2038, Dmitry Torokhov, aksgarg1989, linux-input, linux-kernel,
	linux-api

On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
> Hi, Arnd
> 
> >>>> 
> >>>> The patch series add three evdev interface type:
> >>>> 
> >>>> - EV_IF_LEGACY
> >>>> send event by input_event. This is the default option, keep kernel
> >>>> backward compatible.
> >>> 
> >>> The problem I see with this approach is that it still breaks any
> >>> legacy source code that is compiled with a new libc that uses 64-bit
> >>> time_t. If we are requiring source code changes for building users
> >>> of input devices with a new libc, we can easily get them to handle
> >>> the overflow (they normally only care about the microsecond portion
> >>> anyway, so it doesn't matter in most cases), or to use monotonic time.
> >> 
> >> I don’t think so.
> >> 
> >> Actually, from the view of userspace, EV_IF_LEGACY interface is work
> >> exactly the same as old evdev. We didn’t change anything in input_event
> >> and input_event_compat. And the problem you said will still be there,
> >> even without those patches.
> > 
> > I think we're still talking past one another. I thought we had established
> > that
> > 
> > 1. the current interface is broken when time_t changes in user space
> 
> What kinds of changes in time_t? Extending it to 64-bits in both kernel
> and userspace? Ok, I get confused here, if there are some sample codes
> or use-cases here, it will help me a lot.

We don't change time_t in the kernel, we just try to replace it
with time64_t, or ktime_t where appropriate.

What I meant is the problem when glibc defines their time_t to
be 'long long' so that user space can be built that runs after
2038. This changes the timeval and timespec definitions, so
a process that tries to use 'struct input_event' has a different
layout compared to what the kernel has.

I though that we had already discussed that multiple times.

> > 2. we can fix it by redefining struct input_event in a way that
> >   is independent of time_t
> > 3. once both user space and kernel are using the same ABI independent
> >   of time_t, we can look improving the timestamps so they don't
> >   overflow
> > 4. the monotonic timestamp interface already avoids the overflow, so
> >   it would be sufficient as a solution for 3.
> > 
> > Where did I lose you here? Did you find any other facts that I
> > was missing? I don't know whether the two new event structures make
> > the interface better in some other way, but it seems mostly unrelated
> > to either of the two problems we already have with time_t (the
> > ABI mismatch, and the use of non-monotonic timestamps).
> 
> It seems we are mismatch here.
> 
> Actually input_composite_event has the similar structure with input_event,
> but with a nicer definition, which can take both monotonic and non-monotonic
> timestamps safely.
> 
> What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
> interface. If userspace try to adapt to new libc and kernel, it should move
> to new interface. The userspace can do a lazy update, keep the code untouched,
> but suffer the y2038 problem by itself.

Forcing the move to the new API is very ugly, because you can't do it
in a way that works on old kernels as well, unless you then try to support
both APIs at runtime.

Just requiring user space to switch to monotonic time is easily done,
as it can likely work either way.

> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
> making input_event independent from time_t(after evdev has converted to
> input_value, it’s easy to do that), but that also imply userspace
> must change their code to fit this change. If changing userspace code is
> a mandatory option, why not to force them do a complete conversion?

Most user space programs won't care, as they don't even look at the tv_sec
portion, and the goal is to avoid having to change them.

There is still an open question to how exactly we want to get user space
to change.

We could do some compile-time trick by having a union in struct input_event
and mark the existing timeval part as deprecated, so we flag any use of the
32-bit tv_sec member, such as:

struct input_event {
#if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
        struct timeval time;
#else
	struct {
		union {
			__u32 tv_sec __attribute__((deprecated));
			__u32 tv_sec_monotonic;
		};
		__s32 tv_usec;
	} time;
#endif
        __u16 type;
        __u16 code;
        __s32 value;
};

Another option is to do a runtime change, and always set the time field
to zero when the kernel is built without support for 32-bit time_t
and user space has not yet called the ioctl to change to monotonic time.

We can also do a combination of the two.

	Arnd

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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-12-01 10:47           ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-12-01 10:47 UTC (permalink / raw)
  To: Pingbo Wen
  Cc: y2038-cunTk1MwBs8s++Sfvej+rw, Dmitry Torokhov,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA

On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
> Hi, Arnd
> 
> >>>> 
> >>>> The patch series add three evdev interface type:
> >>>> 
> >>>> - EV_IF_LEGACY
> >>>> send event by input_event. This is the default option, keep kernel
> >>>> backward compatible.
> >>> 
> >>> The problem I see with this approach is that it still breaks any
> >>> legacy source code that is compiled with a new libc that uses 64-bit
> >>> time_t. If we are requiring source code changes for building users
> >>> of input devices with a new libc, we can easily get them to handle
> >>> the overflow (they normally only care about the microsecond portion
> >>> anyway, so it doesn't matter in most cases), or to use monotonic time.
> >> 
> >> I don’t think so.
> >> 
> >> Actually, from the view of userspace, EV_IF_LEGACY interface is work
> >> exactly the same as old evdev. We didn’t change anything in input_event
> >> and input_event_compat. And the problem you said will still be there,
> >> even without those patches.
> > 
> > I think we're still talking past one another. I thought we had established
> > that
> > 
> > 1. the current interface is broken when time_t changes in user space
> 
> What kinds of changes in time_t? Extending it to 64-bits in both kernel
> and userspace? Ok, I get confused here, if there are some sample codes
> or use-cases here, it will help me a lot.

We don't change time_t in the kernel, we just try to replace it
with time64_t, or ktime_t where appropriate.

What I meant is the problem when glibc defines their time_t to
be 'long long' so that user space can be built that runs after
2038. This changes the timeval and timespec definitions, so
a process that tries to use 'struct input_event' has a different
layout compared to what the kernel has.

I though that we had already discussed that multiple times.

> > 2. we can fix it by redefining struct input_event in a way that
> >   is independent of time_t
> > 3. once both user space and kernel are using the same ABI independent
> >   of time_t, we can look improving the timestamps so they don't
> >   overflow
> > 4. the monotonic timestamp interface already avoids the overflow, so
> >   it would be sufficient as a solution for 3.
> > 
> > Where did I lose you here? Did you find any other facts that I
> > was missing? I don't know whether the two new event structures make
> > the interface better in some other way, but it seems mostly unrelated
> > to either of the two problems we already have with time_t (the
> > ABI mismatch, and the use of non-monotonic timestamps).
> 
> It seems we are mismatch here.
> 
> Actually input_composite_event has the similar structure with input_event,
> but with a nicer definition, which can take both monotonic and non-monotonic
> timestamps safely.
> 
> What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
> interface. If userspace try to adapt to new libc and kernel, it should move
> to new interface. The userspace can do a lazy update, keep the code untouched,
> but suffer the y2038 problem by itself.

Forcing the move to the new API is very ugly, because you can't do it
in a way that works on old kernels as well, unless you then try to support
both APIs at runtime.

Just requiring user space to switch to monotonic time is easily done,
as it can likely work either way.

> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
> making input_event independent from time_t(after evdev has converted to
> input_value, it’s easy to do that), but that also imply userspace
> must change their code to fit this change. If changing userspace code is
> a mandatory option, why not to force them do a complete conversion?

Most user space programs won't care, as they don't even look at the tv_sec
portion, and the goal is to avoid having to change them.

There is still an open question to how exactly we want to get user space
to change.

We could do some compile-time trick by having a union in struct input_event
and mark the existing timeval part as deprecated, so we flag any use of the
32-bit tv_sec member, such as:

struct input_event {
#if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
        struct timeval time;
#else
	struct {
		union {
			__u32 tv_sec __attribute__((deprecated));
			__u32 tv_sec_monotonic;
		};
		__s32 tv_usec;
	} time;
#endif
        __u16 type;
        __u16 code;
        __s32 value;
};

Another option is to do a runtime change, and always set the time field
to zero when the kernel is built without support for 32-bit time_t
and user space has not yet called the ioctl to change to monotonic time.

We can also do a combination of the two.

	Arnd

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

* Re: [PATCH 0/3] introduce new evdev interface type
  2015-12-01 10:47           ` Arnd Bergmann
@ 2015-12-03 12:49             ` Pingbo Wen
  -1 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-12-03 12:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Pingbo Wen, y2038, Dmitry Torokhov, aksgarg1989, linux-input,
	linux-kernel, linux-api


> 在 2015年12月1日,18:47,Arnd Bergmann <arnd@arndb.de> 写道:
> 
> On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
>> 
>> 
>> What kinds of changes in time_t? Extending it to 64-bits in both kernel
>> and userspace? Ok, I get confused here, if there are some sample codes
>> or use-cases here, it will help me a lot.
> 
> We don't change time_t in the kernel, we just try to replace it
> with time64_t, or ktime_t where appropriate.
> 
> What I meant is the problem when glibc defines their time_t to
> be 'long long' so that user space can be built that runs after
> 2038. This changes the timeval and timespec definitions, so
> a process that tries to use 'struct input_event' has a different
> layout compared to what the kernel has.

Ok, I didn’t reach this point. Thanks to clarify.

>> 
>> It seems we are mismatch here.
>> 
>> Actually input_composite_event has the similar structure with input_event,
>> but with a nicer definition, which can take both monotonic and non-monotonic
>> timestamps safely.
>> 
>> What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
>> interface. If userspace try to adapt to new libc and kernel, it should move
>> to new interface. The userspace can do a lazy update, keep the code untouched,
>> but suffer the y2038 problem by itself.
> 
> Forcing the move to the new API is very ugly, because you can't do it
> in a way that works on old kernels as well, unless you then try to support
> both APIs at runtime.
> 
> Just requiring user space to switch to monotonic time is easily done,
> as it can likely work either way.

It seems I’m more aggressive:) But the legacy binary can still live 30 years longer
without any changes.

> 
>> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
>> making input_event independent from time_t(after evdev has converted to
>> input_value, it’s easy to do that), but that also imply userspace
>> must change their code to fit this change. If changing userspace code is
>> a mandatory option, why not to force them do a complete conversion?
> 
> Most user space programs won't care, as they don't even look at the tv_sec
> portion, and the goal is to avoid having to change them.
> 
> There is still an open question to how exactly we want to get user space
> to change.
> 
> We could do some compile-time trick by having a union in struct input_event
> and mark the existing timeval part as deprecated, so we flag any use of the
> 32-bit tv_sec member, such as:
> 
> struct input_event {
> #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
>        struct timeval time;

> #else
> 	struct {
> 		union {
> 			__u32 tv_sec __attribute__((deprecated));
> 			__u32 tv_sec_monotonic;
> 		};
> 		__s32 tv_usec;
> 	} time;
> #endif
>        __u16 type;
>        __u16 code;
>        __s32 value;
> };

I have one question here, if userspace use this structure, all helper functions
of timeval will not work. And userspace need to write extra helper function for
this fake timeval. This just create an another urgly time structure.

And this method also forces most of old binaries to compile with new libc, adjust
their codes with new fake time structure.

Besides, I get an idea to combine your structure with input_composite_event:

union {
	struct {
		__s32 tv_usec;
		__s32 tv_sec;
	};
	__s64 time;
} time;

I prefer to use a single s64 timestamp, if our goal is to remove timeval from kernel.

Pingbo


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

* Re: [PATCH 0/3] introduce new evdev interface type
@ 2015-12-03 12:49             ` Pingbo Wen
  0 siblings, 0 replies; 27+ messages in thread
From: Pingbo Wen @ 2015-12-03 12:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: y2038, linux-api, Dmitry Torokhov, linux-kernel, aksgarg1989,
	linux-input, Pingbo Wen


> 在 2015年12月1日,18:47,Arnd Bergmann <arnd@arndb.de> 写道:
> 
> On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
>> 
>> 
>> What kinds of changes in time_t? Extending it to 64-bits in both kernel
>> and userspace? Ok, I get confused here, if there are some sample codes
>> or use-cases here, it will help me a lot.
> 
> We don't change time_t in the kernel, we just try to replace it
> with time64_t, or ktime_t where appropriate.
> 
> What I meant is the problem when glibc defines their time_t to
> be 'long long' so that user space can be built that runs after
> 2038. This changes the timeval and timespec definitions, so
> a process that tries to use 'struct input_event' has a different
> layout compared to what the kernel has.

Ok, I didn’t reach this point. Thanks to clarify.

>> 
>> It seems we are mismatch here.
>> 
>> Actually input_composite_event has the similar structure with input_event,
>> but with a nicer definition, which can take both monotonic and non-monotonic
>> timestamps safely.
>> 
>> What I assumed here, is that leaving EV_IF_LEGACY as a untouched, deprecated
>> interface. If userspace try to adapt to new libc and kernel, it should move
>> to new interface. The userspace can do a lazy update, keep the code untouched,
>> but suffer the y2038 problem by itself.
> 
> Forcing the move to the new API is very ugly, because you can't do it
> in a way that works on old kernels as well, unless you then try to support
> both APIs at runtime.
> 
> Just requiring user space to switch to monotonic time is easily done,
> as it can likely work either way.

It seems I’m more aggressive:) But the legacy binary can still live 30 years longer
without any changes.

> 
>> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
>> making input_event independent from time_t(after evdev has converted to
>> input_value, it’s easy to do that), but that also imply userspace
>> must change their code to fit this change. If changing userspace code is
>> a mandatory option, why not to force them do a complete conversion?
> 
> Most user space programs won't care, as they don't even look at the tv_sec
> portion, and the goal is to avoid having to change them.
> 
> There is still an open question to how exactly we want to get user space
> to change.
> 
> We could do some compile-time trick by having a union in struct input_event
> and mark the existing timeval part as deprecated, so we flag any use of the
> 32-bit tv_sec member, such as:
> 
> struct input_event {
> #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
>        struct timeval time;

> #else
> 	struct {
> 		union {
> 			__u32 tv_sec __attribute__((deprecated));
> 			__u32 tv_sec_monotonic;
> 		};
> 		__s32 tv_usec;
> 	} time;
> #endif
>        __u16 type;
>        __u16 code;
>        __s32 value;
> };

I have one question here, if userspace use this structure, all helper functions
of timeval will not work. And userspace need to write extra helper function for
this fake timeval. This just create an another urgly time structure.

And this method also forces most of old binaries to compile with new libc, adjust
their codes with new fake time structure.

Besides, I get an idea to combine your structure with input_composite_event:

union {
	struct {
		__s32 tv_usec;
		__s32 tv_sec;
	};
	__s64 time;
} time;

I prefer to use a single s64 timestamp, if our goal is to remove timeval from kernel.

Pingbo

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

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

* Re: [Y2038] [PATCH 0/3] introduce new evdev interface type
@ 2015-12-03 12:54               ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-12-03 12:54 UTC (permalink / raw)
  To: y2038
  Cc: Pingbo Wen, linux-api, Dmitry Torokhov, linux-kernel,
	aksgarg1989, linux-input

On Thursday 03 December 2015 20:49:06 Pingbo Wen wrote:
> > 在 2015年12月1日,18:47,Arnd Bergmann <arnd@arndb.de> 写道:
> > On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
> >> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
> >> making input_event independent from time_t(after evdev has converted to
> >> input_value, it’s easy to do that), but that also imply userspace
> >> must change their code to fit this change. If changing userspace code is
> >> a mandatory option, why not to force them do a complete conversion?
> > 
> > Most user space programs won't care, as they don't even look at the tv_sec
> > portion, and the goal is to avoid having to change them.
> > 
> > There is still an open question to how exactly we want to get user space
> > to change.
> > 
> > We could do some compile-time trick by having a union in struct input_event
> > and mark the existing timeval part as deprecated, so we flag any use of the
> > 32-bit tv_sec member, such as:
> > 
> > struct input_event {
> > #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
> >        struct timeval time;
> 
> > #else
> > 	struct {
> > 		union {
> > 			__u32 tv_sec __attribute__((deprecated));
> > 			__u32 tv_sec_monotonic;
> > 		};
> > 		__s32 tv_usec;
> > 	} time;
> > #endif
> >        __u16 type;
> >        __u16 code;
> >        __s32 value;
> > };
> 
> I have one question here, if userspace use this structure, all helper functions
> of timeval will not work. And userspace need to write extra helper function for
> this fake timeval. This just create an another urgly time structure.

Correct, this is a useful side-effect of the change: any user space access to
the event->time member that assumes it's a timeval will cause a compile-time
warning or error (depending on the access), which helps us identify the
broken code and fix it to use monotonic times as well as access the right
struct members.

> And this method also forces most of old binaries to compile with new libc, adjust
> their codes with new fake time structure.
> 
> Besides, I get an idea to combine your structure with input_composite_event:
> 
> union {
> 	struct {
> 		__s32 tv_usec;
> 		__s32 tv_sec;
> 	};
> 	__s64 time;
> } time;
> 
> I prefer to use a single s64 timestamp, if our goal is to remove timeval from kernel.

We can't really remove this instance of timeval anyway, so adding an __s64 member
here is not all that helpful. We should use __s64 nanoseconds for new interfaces
like this, but I see no reason to change the one we have here.

	Arnd

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

* Re: [Y2038] [PATCH 0/3] introduce new evdev interface type
@ 2015-12-03 12:54               ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-12-03 12:54 UTC (permalink / raw)
  To: y2038-cunTk1MwBs8s++Sfvej+rw
  Cc: Pingbo Wen, linux-api-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA

On Thursday 03 December 2015 20:49:06 Pingbo Wen wrote:
> > 在 2015年12月1日,18:47,Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> 写道:
> > On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
> >> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
> >> making input_event independent from time_t(after evdev has converted to
> >> input_value, it’s easy to do that), but that also imply userspace
> >> must change their code to fit this change. If changing userspace code is
> >> a mandatory option, why not to force them do a complete conversion?
> > 
> > Most user space programs won't care, as they don't even look at the tv_sec
> > portion, and the goal is to avoid having to change them.
> > 
> > There is still an open question to how exactly we want to get user space
> > to change.
> > 
> > We could do some compile-time trick by having a union in struct input_event
> > and mark the existing timeval part as deprecated, so we flag any use of the
> > 32-bit tv_sec member, such as:
> > 
> > struct input_event {
> > #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
> >        struct timeval time;
> 
> > #else
> > 	struct {
> > 		union {
> > 			__u32 tv_sec __attribute__((deprecated));
> > 			__u32 tv_sec_monotonic;
> > 		};
> > 		__s32 tv_usec;
> > 	} time;
> > #endif
> >        __u16 type;
> >        __u16 code;
> >        __s32 value;
> > };
> 
> I have one question here, if userspace use this structure, all helper functions
> of timeval will not work. And userspace need to write extra helper function for
> this fake timeval. This just create an another urgly time structure.

Correct, this is a useful side-effect of the change: any user space access to
the event->time member that assumes it's a timeval will cause a compile-time
warning or error (depending on the access), which helps us identify the
broken code and fix it to use monotonic times as well as access the right
struct members.

> And this method also forces most of old binaries to compile with new libc, adjust
> their codes with new fake time structure.
> 
> Besides, I get an idea to combine your structure with input_composite_event:
> 
> union {
> 	struct {
> 		__s32 tv_usec;
> 		__s32 tv_sec;
> 	};
> 	__s64 time;
> } time;
> 
> I prefer to use a single s64 timestamp, if our goal is to remove timeval from kernel.

We can't really remove this instance of timeval anyway, so adding an __s64 member
here is not all that helpful. We should use __s64 nanoseconds for new interfaces
like this, but I see no reason to change the one we have here.

	Arnd

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

* Re: [Y2038] [PATCH 0/3] introduce new evdev interface type
  2015-12-03 12:54               ` Arnd Bergmann
@ 2015-12-03 12:56                 ` Arnd Bergmann
  -1 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-12-03 12:56 UTC (permalink / raw)
  To: y2038
  Cc: Pingbo Wen, linux-api, Dmitry Torokhov, linux-kernel,
	aksgarg1989, linux-input

On Thursday 03 December 2015 13:54:47 Arnd Bergmann wrote:
> > > struct input_event {
> > > #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
> > >        struct timeval time;
> > 
> > > #else
> > >     struct {
> > >             union {
> > >                     __u32 tv_sec __attribute__((deprecated));
> > >                     __u32 tv_sec_monotonic;
> > >             };
> > >             __s32 tv_usec;
> > >     } time;
> > > #endif
> > >        __u16 type;
> > >        __u16 code;
> > >        __s32 value;
> > > };
> > 
> > I have one question here, if userspace use this structure, all helper functions
> > of timeval will not work. And userspace need to write extra helper function for
> > this fake timeval. This just create an another urgly time structure.
> 
> Correct, this is a useful side-effect of the change: any user space access to
> the event->time member that assumes it's a timeval will cause a compile-time
> warning or error (depending on the access), which helps us identify the
> broken code and fix it to use monotonic times as well as access the right
> struct members.
> 

To clarify, the code also intentionally only changes the types when
we are compiling with a new 32-bit libc: everything that builds today
will continue to build and work without warnings, unless it gets
recompiled with 64-bit time_t and needs to be fixed.

	Arnd

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

* Re: [Y2038] [PATCH 0/3] introduce new evdev interface type
@ 2015-12-03 12:56                 ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2015-12-03 12:56 UTC (permalink / raw)
  To: y2038-cunTk1MwBs8s++Sfvej+rw
  Cc: Pingbo Wen, linux-api-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	aksgarg1989-Re5JQEeQqe8AvxtiuMwx3w,
	linux-input-u79uwXL29TY76Z2rM5mHXA

On Thursday 03 December 2015 13:54:47 Arnd Bergmann wrote:
> > > struct input_event {
> > > #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
> > >        struct timeval time;
> > 
> > > #else
> > >     struct {
> > >             union {
> > >                     __u32 tv_sec __attribute__((deprecated));
> > >                     __u32 tv_sec_monotonic;
> > >             };
> > >             __s32 tv_usec;
> > >     } time;
> > > #endif
> > >        __u16 type;
> > >        __u16 code;
> > >        __s32 value;
> > > };
> > 
> > I have one question here, if userspace use this structure, all helper functions
> > of timeval will not work. And userspace need to write extra helper function for
> > this fake timeval. This just create an another urgly time structure.
> 
> Correct, this is a useful side-effect of the change: any user space access to
> the event->time member that assumes it's a timeval will cause a compile-time
> warning or error (depending on the access), which helps us identify the
> broken code and fix it to use monotonic times as well as access the right
> struct members.
> 

To clarify, the code also intentionally only changes the types when
we are compiling with a new 32-bit libc: everything that builds today
will continue to build and work without warnings, unless it gets
recompiled with 64-bit time_t and needs to be fixed.

	Arnd

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

end of thread, other threads:[~2015-12-03 12:56 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-27 10:00 [PATCH 0/3] introduce new evdev interface type WEN Pingbo
2015-11-27 10:00 ` [PATCH 1/3] input: evdev: introduce new evdev interface WEN Pingbo
2015-11-27 10:37   ` kbuild test robot
2015-11-27 10:37     ` kbuild test robot
2015-11-27 10:00 ` [PATCH 2/3] input: evdev: add new ioctl EVIOCSIFTYPE / EVIOCGIFTYPE WEN Pingbo
2015-11-27 16:59   ` Arnd Bergmann
2015-11-27 16:59     ` Arnd Bergmann
2015-11-29  9:19     ` Pingbo Wen
2015-11-29  9:19       ` Pingbo Wen
2015-11-27 10:00 ` [PATCH 3/3] uinput: convert input_event to input_value WEN Pingbo
2015-11-27 16:58 ` [PATCH 0/3] introduce new evdev interface type Arnd Bergmann
2015-11-27 16:58   ` Arnd Bergmann
2015-11-29  9:13   ` Pingbo Wen
2015-11-29  9:13     ` Pingbo Wen
2015-11-30 15:13     ` Arnd Bergmann
2015-12-01  8:34       ` Pingbo Wen
2015-12-01  8:34         ` Pingbo Wen
2015-12-01 10:47         ` Arnd Bergmann
2015-12-01 10:47           ` Arnd Bergmann
2015-12-03 12:49           ` Pingbo Wen
2015-12-03 12:49             ` Pingbo Wen
2015-12-03 12:54             ` [Y2038] " Arnd Bergmann
2015-12-03 12:54               ` Arnd Bergmann
2015-12-03 12:56               ` Arnd Bergmann
2015-12-03 12:56                 ` Arnd Bergmann
2015-12-01  8:34       ` Pingbo Wen
2015-12-01  8:34         ` Pingbo Wen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.