Re: [Y2038] [PATCH v2 01/24] Input: input_event: fix struct padding on sparc64

2019-12-13 Thread Dmitry Torokhov
On Fri, Dec 13, 2019 at 09:49:10PM +0100, Arnd Bergmann wrote:
> Going through all uses of timeval, I noticed that we screwed up
> input_event in the previous attempts to fix it:
> 
> The time fields now match between kernel and user space, but
> all following fields are in the wrong place.
> 
> Add the required padding that is implied by the glibc timeval
> definition to fix the layout, and use a struct initializer
> to avoid leaking kernel stack data.
> 
> Cc: sparcli...@vger.kernel.org
> Cc: "David S. Miller" 
> Fixes: 141e5dcaa735 ("Input: input_event - fix the CONFIG_SPARC64 mixup")
> Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
> Signed-off-by: Arnd Bergmann 

Applied, thank you.

> ---
>  drivers/input/evdev.c   | 14 +++---
>  drivers/input/misc/uinput.c | 14 +-
>  include/uapi/linux/input.h  |  1 +
>  3 files changed, 17 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index d7dd6fcf2db0..f918fca9ada3 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -224,13 +224,13 @@ static void __pass_event(struct evdev_client *client,
>*/
>   client->tail = (client->head - 2) & (client->bufsize - 1);
>  
> - client->buffer[client->tail].input_event_sec =
> - event->input_event_sec;
> - client->buffer[client->tail].input_event_usec =
> - event->input_event_usec;
> - client->buffer[client->tail].type = EV_SYN;
> - client->buffer[client->tail].code = SYN_DROPPED;
> - client->buffer[client->tail].value = 0;
> + client->buffer[client->tail] = (struct input_event) {
> + .input_event_sec = event->input_event_sec,
> + .input_event_usec = event->input_event_usec,
> + .type = EV_SYN,
> + .code = SYN_DROPPED,
> + .value = 0,
> + };
>  
>   client->packet_head = client->tail;
>   }
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index fd253781be71..2dabbe47d43e 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -74,12 +74,16 @@ static int uinput_dev_event(struct input_dev *dev,
>   struct uinput_device*udev = input_get_drvdata(dev);
>   struct timespec64   ts;
>  
> - udev->buff[udev->head].type = type;
> - udev->buff[udev->head].code = code;
> - udev->buff[udev->head].value = value;
>   ktime_get_ts64();
> - udev->buff[udev->head].input_event_sec = ts.tv_sec;
> - udev->buff[udev->head].input_event_usec = ts.tv_nsec / NSEC_PER_USEC;
> +
> + udev->buff[udev->head] = (struct input_event) {
> + .input_event_sec = ts.tv_sec,
> + .input_event_usec = ts.tv_nsec / NSEC_PER_USEC,
> + .type = type,
> + .code = code,
> + .value = value,
> + };
> +
>   udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
>  
>   wake_up_interruptible(>waitq);
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index f056b2a00d5c..9a61c28ed3ae 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -34,6 +34,7 @@ struct input_event {
>   __kernel_ulong_t __sec;
>  #if defined(__sparc__) && defined(__arch64__)
>   unsigned int __usec;
> + unsigned int __pad;
>  #else
>   __kernel_ulong_t __usec;
>  #endif
> -- 
> 2.20.0
> 

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH 8/8] Input: input_event: fix struct padding on sparc64

2019-11-11 Thread Dmitry Torokhov
Hi Arnd,

On Fri, Nov 08, 2019 at 09:34:31PM +0100, Arnd Bergmann wrote:
> Going through all uses of timeval, I noticed that we screwed up
> input_event in the previous attempts to fix it:
> 
> The time fields now match between kernel and user space, but
> all following fields are in the wrong place.
> 
> Add the required padding that is implied by the glibc timeval
> definition to fix the layout, and add explicit initialization
> to avoid leaking kernel stack data.
> 
> Cc: sparcli...@vger.kernel.org
> Cc: "David S. Miller" 
> Cc: sta...@vger.kernel.org
> Fixes: 141e5dcaa735 ("Input: input_event - fix the CONFIG_SPARC64 mixup")
> Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/input/evdev.c   | 3 +++
>  drivers/input/misc/uinput.c | 3 +++
>  include/uapi/linux/input.h  | 1 +
>  3 files changed, 7 insertions(+)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index d7dd6fcf2db0..24a90793caf0 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -228,6 +228,9 @@ static void __pass_event(struct evdev_client *client,
>   event->input_event_sec;
>   client->buffer[client->tail].input_event_usec =
>   event->input_event_usec;
> +#ifdef CONFIG_SPARC64
> + client->buffer[client->tail].__pad = 0;
> +#endif
>   client->buffer[client->tail].type = EV_SYN;
>   client->buffer[client->tail].code = SYN_DROPPED;
>   client->buffer[client->tail].value = 0;

I do not like ifdefs here, do you think we could write:

client->buffer[client->tail] = (struct input_event) {
.input_event_sec = event->input_event_sec,
.input_event_usec = event->input_event_usec,
.type = EV_SYN,
.code = SYN_DROPPED,
};

to ensure all padded fields are initialized? This is not hot path as we
do not expect queue to overfill too often.

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH] input: Fix the CONFIG_SPARC64 mixup

2019-01-24 Thread Dmitry Torokhov
On Sun, Jan 20, 2019 at 05:49:14PM -0800, Deepa Dinamani wrote:
> Arnd Bergmann pointed out that CONFIG_* cannot be used
> in a uapi header. Override with an equivalent conditional.
> 
> Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
> Fixes: 152194fe9c3f ("Input: extend usable life of event timestamps to 2106 
> on 32 bit systems")
> Signed-off-by: Deepa Dinamani 

Applied, thank you.

> ---
>  include/uapi/linux/input.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index ffab958bc512..f056b2a00d5c 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -32,7 +32,7 @@ struct input_event {
>  #define input_event_usec time.tv_usec
>  #else
>   __kernel_ulong_t __sec;
> -#ifdef CONFIG_SPARC64
> +#if defined(__sparc__) && defined(__arch64__)
>   unsigned int __usec;
>  #else
>   __kernel_ulong_t __usec;
> -- 
> 2.17.1
> 

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH] input_event: Provide override for sparc64

2019-01-15 Thread Dmitry Torokhov
On Tue, Jan 15, 2019 at 1:29 PM David Miller  wrote:
>
> From: Arnd Bergmann 
> Date: Tue, 15 Jan 2019 22:19:27 +0100
>
> > The correct check appears to be
> >
> > #if defined(__sparc__) && defined(__arch64__)
>
> That is correct.

OK. Deepa, could you please send me a fixup as I already pushed out
the original patch?

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH] input_event: Provide override for sparc64

2019-01-13 Thread Dmitry Torokhov
On Sat, Dec 29, 2018 at 10:35:14AM -0800, Deepa Dinamani wrote:
> The usec part of the timeval is defined as
> __kernel_suseconds_t  tv_usec; /* microseconds */
> 
> Arnd noticed that sparc64 is the only architecture
> that defines __kernel_suseconds_t as int rather than long.
> 
> This breaks the current y2038 fix for kernel as we only
> access and define the timeval struct for non-kernel use cases.
> But, this was hidden by an another typo in the use of __KERNEL__
> qualifier.
> 
> Fix the typo, and provide an override for sparc64.
> 
> Fixes: 152194fe9c3f ("Input: extend usable life of event timestamps to 2106 
> on 32 bit systems")
> Reported-by: Arnd Bergmann 
> Signed-off-by: Deepa Dinamani 

Applied, thank you.

> ---
>  include/uapi/linux/input.h | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index fb78f6f500f3..ffab958bc512 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -26,13 +26,17 @@
>   */
>  
>  struct input_event {
> -#if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) && 
> !defined(__KERNEL)
> +#if (__BITS_PER_LONG != 32 || !defined(__USE_TIME_BITS64)) && 
> !defined(__KERNEL__)
>   struct timeval time;
>  #define input_event_sec time.tv_sec
>  #define input_event_usec time.tv_usec
>  #else
>   __kernel_ulong_t __sec;
> +#ifdef CONFIG_SPARC64
> + unsigned int __usec;
> +#else
>   __kernel_ulong_t __usec;
> +#endif
>  #define input_event_sec  __sec
>  #define input_event_usec __usec
>  #endif
> -- 
> 2.17.1
> 

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH] Update struct input_event

2018-01-30 Thread Dmitry Torokhov
On Tue, Jan 30, 2018 at 05:45:55PM -0800, Deepa Dinamani wrote:
> On Tue, Jan 30, 2018 at 9:50 AM, Martin Kepplinger  wrote:
> > Am 16.01.2018 01:16 schrieb Deepa Dinamani:
> >>
> >> The struct input_event is not y2038 safe.
> >> Update the struct according to the kernel patch:
> >> https://lkml.org/lkml/2018/1/6/324
> >>
> >
> > For me, this patch doesn't even apply -.- Could be my fault, but what are
> > you creating
> > this against?
> 
> These patches were based on
> 
> mtdev commit 5f9caa26b81155feede6ff71c9b14fa0e8980fbd
> mtdev-matching.c: declare global variables static
> evemu commit 8cde0770ac4e45a93d4bcb0710c44b3ffe547a6f tools:
> s/evtest/evemu/ in the evemu-describe man page
> libevdev commit 022b2bc3b03320131966a465c464f989fa91905e include: sync
> with kernel 4.13
> 
> 
> > Other than that, I would really not pull in changes that aren't even in
> > Linus' tree yet.
> > We'd have a lot of unnecessary work here if we track an experimental input
> > tree
> > before it's ever released in Linux.
> 
> The patch is in linux-next:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=152194fe9c3f03232b9c0d0264793a7fa4af82f8
> .
> Adding Dmitry to clarify when he plans to merge this.

It will be merged in this merge window that just opened.

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v6 1/1] input: Deprecate real timestamps beyond year 2106

2018-01-08 Thread Dmitry Torokhov
Hi Deepa,

On Sat, Jan 06, 2018 at 04:19:15PM -0800, Deepa Dinamani wrote:
> struct timeval is not y2038 safe.
> All usage of timeval in the kernel will be replaced by
> y2038 safe structures.
> The change is also necessary as glibc is introducing support
> for 32 bit applications to use 64 bit time_t. Without this
> change, many applications would incorrectly interpret values
> in the struct input_event.
> More details about glibc at
> https://sourceware.org/glibc/wiki/Y2038ProofnessDesign .
> 
> struct input_event maintains time for each input event.
> Real time timestamps are not ideal for input as this
> time can go backwards as noted in the patch a80b83b7b8
> by John Stultz. Hence, having the input_event.time fields
> only big enough for monotonic and boot times are
> sufficient.

I am happy with the patch, but have some concerns about changelog. The
change does not really prevent anyone from using CLOCK_REALTIME past
2106, especially on 64 bit arches. We are simply extending range of
reported seconds in input event by converting from signed to unsigned
value.

> 
> The change leaves the representation of struct input_event as is
> on 64 bit architectures. But uses 2 unsigned long values on 32 bit
> machines to support real timestamps until year 2106.
> This intentionally breaks the ABI on 32 bit architectures and
> compat handling on 64 bit architectures.
> This is as per maintainer's preference to introduce compile time errors
> rather than run into runtime incompatibilities.

We are breaking API, not ABI though. The ABI for existing programs is
exactly the same, it is only when system starts using the newer glibc
supporting extended time the compilation will break.

> The change requires any 32 bit userspace utilities reading or writing
> from event nodes to update their reading format to match the new
> input_event. The changes to the popular libraries will be posted once
> we agree on the kernel change.

I propose we have the following changelog:


Input: extend usable life of event timestamps to 2106 on 32 bit systems

The input events use struct timeval to store event time, unfortunately
this structure is not y2038 safe and is being replaced in kernel with
y2038 safe structures.

Because of ABI concerns we can not change the size or the layout of
structure input_event, so we opt to re-interpreting the 'seconds' part
of timestamp as an unsigned value, effectively doubling the range of
values, to year 2106.

Newer glibc that has support for 32 bit applications to use 64 bit
time_t supplies __USE_TIME_BITS64 define [1], that we can use to present
the userspace with updated input_event layout. The updated layout will
cause the compile time breakage, alerting applications and distributions
maintainers to the issue. Existing 32 binaries will continue working
without any changes until 2038.

Ultimately userspace applications should switch to using monotonic or
boot time clocks, as realtime clock is not very well suited for input
event timestamps as it can go backwards (see a80b83b7b8 "Input: evdev -
add CLOCK_BOOTTIME support" by by John Stultz). With monotonic clock the
practical range of reported times will always fit into the pair of 32
bit values, as we do not expect any system to stay up for a hundred
years without a single reboot.

[1] https://sourceware.org/glibc/wiki/Y2038ProofnessDesign


Please let me know if you disagee with the above.

Thanks!

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v5 2/3] input: evdev: Replace timeval with timespec64

2018-01-08 Thread Dmitry Torokhov
On Sat, Jan 06, 2018 at 01:43:34PM -0800, Deepa Dinamani wrote:
> On Tue, Jan 2, 2018 at 7:35 AM, Arnd Bergmann <a...@arndb.de> wrote:
> > On Tue, Jan 2, 2018 at 7:46 AM, Dmitry Torokhov
> > <dmitry.torok...@gmail.com> wrote:
> >> On Sun, Dec 17, 2017 at 09:18:43PM -0800, Deepa Dinamani wrote:
> >>> @@ -304,12 +314,11 @@ static void evdev_events(struct input_handle 
> >>> *handle,
> >>>  {
> >>>   struct evdev *evdev = handle->private;
> >>>   struct evdev_client *client;
> >>> - ktime_t ev_time[EV_CLK_MAX];
> >>> + struct timespec64 ev_time[EV_CLK_MAX];
> >>>
> >>> - ev_time[EV_CLK_MONO] = ktime_get();
> >>> - ev_time[EV_CLK_REAL] = ktime_mono_to_real(ev_time[EV_CLK_MONO]);
> >>> - ev_time[EV_CLK_BOOT] = ktime_mono_to_any(ev_time[EV_CLK_MONO],
> >>> -  TK_OFFS_BOOT);
> >>> + ktime_get_ts64(_time[EV_CLK_MONO]);
> >>> + ktime_get_real_ts64(_time[EV_CLK_REAL]);
> >>> + get_monotonic_boottime64(_time[EV_CLK_BOOT]);
> >>
> >> This may result in different ev_time[] members holding different times,
> >> whereas the original code would take one time sample and convert it to
> >> different clocks.
> >
> > Is this important? On each client we only return one of the two
> > times, and I would guess that you cannot rely on a correlation
> > between timestamps on different devices, since the boot and real
> > offsets can change over time.
> 
> Right. I didn't think this was an issue either.
> 
> >> Also, why can't we keep using ktime_t internally? It is y2038 safe,
> >> right?
> >
> > Correct, but there may also be a performance difference if we get
> > a lot of events, not sure if that matters.
> >
> >> I think you should drop this patch and adjust the 3rd one to
> >> massage the input event timestamp patch to do ktime->timespec64->input
> >> timestamp conversion.
> >
> > The change in __evdev_queue_syn_dropped still seems useful to me
> > as ktime_get_*ts64() is a bit more efficient than ktime_get*() followed by
> > a slow ktime_to_timespec64() or ktime_to_timeval().
> >
> > For evdev_events(), doing a single ktime_get() followed by a
> > ktime_to_timespec64/ktime_to_timeval can be faster than three
> > ktime_get_*ts64 (depending on the hardware clock source), or
> > it can be slower depending on the CPU and the clocksource
> > hardware. Again, no idea if this matters at the usual rate of
> > input events.
> >
> > I guess dropping the evdev_events() change and replacing it with a
> > ktime_to_timespec64 change in evdev_pass_values()
> > would be fine here, it should keep the current performance
> > behavior and get rid of the timeval.
> 
> I was trying to use timespec64 everywhere so that we would not have
> conversions back and forth at the input layer.
> I dropped the ktime_t conversions for now and merged this patch with
> the next one as requested.
> 
> Let me know if you would like to keep the changes Arnd preferred above
> for __evdev_queue_syn_dropped(). I can submit a separate patch if this
> is preferred.

__evdev_queue_syn_dropped() is extremely cold path (hopefully, if it is
not we have much bigger problems) so I'd leave it as is.

Thanks!

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v5 2/3] input: evdev: Replace timeval with timespec64

2018-01-01 Thread Dmitry Torokhov
Hi Deepa,

On Sun, Dec 17, 2017 at 09:18:43PM -0800, Deepa Dinamani wrote:
> struct timeval is not y2038 safe.
> 
> All references to timeval in the kernel will be replaced
> by y2038 safe structures.
> Replace all references to timeval with y2038 safe
> struct timespec64 here.
> 
> struct input_event will be changed in a different patch.
> 
> Signed-off-by: Deepa Dinamani 
> Reviewed-by: Arnd Bergmann 
> Acked-by: Peter Hutterer 
> ---
>  drivers/input/evdev.c | 37 +++--
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
> index 0193dd4f0452..3171c4882d80 100644
> --- a/drivers/input/evdev.c
> +++ b/drivers/input/evdev.c
> @@ -156,15 +156,22 @@ static void __evdev_flush_queue(struct evdev_client 
> *client, unsigned int type)
>  static void __evdev_queue_syn_dropped(struct evdev_client *client)
>  {
>   struct input_event ev;
> - ktime_t time;
> + struct timespec64 ts;
>  
> - time = client->clk_type == EV_CLK_REAL ?
> - ktime_get_real() :
> - client->clk_type == EV_CLK_MONO ?
> - ktime_get() :
> - ktime_get_boottime();
> + switch (client->clk_type) {
> + case EV_CLK_REAL:
> + ktime_get_real_ts64();
> + break;
> + case EV_CLK_MONO:
> + ktime_get_ts64();
> + break;
> + case EV_CLK_BOOT:
> + get_monotonic_boottime64();
> + break;
> + }
>  
> - ev.time = ktime_to_timeval(time);
> + ev.time.tv_sec = ts.tv_sec;
> + ev.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>   ev.type = EV_SYN;
>   ev.code = SYN_DROPPED;
>   ev.value = 0;
> @@ -257,17 +264,20 @@ static void __pass_event(struct evdev_client *client,
>  
>  static void evdev_pass_values(struct evdev_client *client,
>   const struct input_value *vals, unsigned int count,
> - ktime_t *ev_time)
> + struct timespec64 *ev_time)
>  {
>   struct evdev *evdev = client->evdev;
>   const struct input_value *v;
>   struct input_event event;
> + struct timespec64 ts;
>   bool wakeup = false;
>  
>   if (client->revoked)
>   return;
>  
> - event.time = ktime_to_timeval(ev_time[client->clk_type]);
> + ts = ev_time[client->clk_type];
> + event.time.tv_sec = ts.tv_sec;
> + event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>  
>   /* Interrupts are disabled, just acquire the lock. */
>   spin_lock(>buffer_lock);
> @@ -304,12 +314,11 @@ static void evdev_events(struct input_handle *handle,
>  {
>   struct evdev *evdev = handle->private;
>   struct evdev_client *client;
> - ktime_t ev_time[EV_CLK_MAX];
> + struct timespec64 ev_time[EV_CLK_MAX];
>  
> - ev_time[EV_CLK_MONO] = ktime_get();
> - ev_time[EV_CLK_REAL] = ktime_mono_to_real(ev_time[EV_CLK_MONO]);
> - ev_time[EV_CLK_BOOT] = ktime_mono_to_any(ev_time[EV_CLK_MONO],
> -  TK_OFFS_BOOT);
> + ktime_get_ts64(_time[EV_CLK_MONO]);
> + ktime_get_real_ts64(_time[EV_CLK_REAL]);
> + get_monotonic_boottime64(_time[EV_CLK_BOOT]);

This may result in different ev_time[] members holding different times,
whereas the original code would take one time sample and convert it to
different clocks.

Also, why can't we keep using ktime_t internally? It is y2038 safe,
right? I think you should drop this patch and adjust the 3rd one to
massage the input event timestamp patch to do ktime->timespec64->input
timestamp conversion.

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v5 1/3] uinput: Use monotonic times for uinput timestamps.

2018-01-01 Thread Dmitry Torokhov
On Sun, Dec 17, 2017 at 09:18:42PM -0800, Deepa Dinamani wrote:
> struct timeval which is part of struct input_event to
> maintain the event times is not y2038 safe.
> 
> Real time timestamps are also not ideal for input_event
> as this time can go backwards as noted in the patch
> a80b83b7b8 by John Stultz.
> 
> The patch switches the timestamps to use monotonic time
> from realtime time. This is assuming no one is using
> absolute times from these timestamps.
> 
> The structure to maintain input events will be changed
> in a different patch.
> 
> Signed-off-by: Deepa Dinamani 
> Acked-by: Peter Hutterer 

Applied, thank you.

> ---
>  drivers/input/misc/uinput.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 91df0df15e68..9251765645d1 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -84,11 +84,14 @@ static int uinput_dev_event(struct input_dev *dev,
>   unsigned int type, unsigned int code, int value)
>  {
>   struct uinput_device*udev = input_get_drvdata(dev);
> + struct timespec64   ts;
>  
>   udev->buff[udev->head].type = type;
>   udev->buff[udev->head].code = code;
>   udev->buff[udev->head].value = value;
> - do_gettimeofday(>buff[udev->head].time);
> + ktime_get_ts64();
> + udev->buff[udev->head].time.tv_sec = ts.tv_sec;
> + udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC;
>   udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE;
>  
>   wake_up_interruptible(>waitq);
> -- 
> 2.14.1
> 

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v3 1/4] uinput: Add ioctl for using monotonic/ boot times

2017-12-04 Thread Dmitry Torokhov
On Mon, Dec 04, 2017 at 01:38:01PM -0800, Deepa Dinamani wrote:
> On Mon, Dec 4, 2017 at 1:18 PM, Arnd Bergmann  wrote:
> > On Mon, Dec 4, 2017 at 9:36 PM, Deepa Dinamani  
> > wrote:
> >> On Mon, Dec 4, 2017 at 6:21 AM, Arnd Bergmann  wrote:
> >>> On Mon, Dec 4, 2017 at 1:55 AM, Deepa Dinamani  
> >>> wrote:
>  struct timeval which is part of struct input_event to
>  maintain the event times is not y2038 safe.
> 
>  Real time timestamps are also not ideal for input_event
>  as this time can go backwards as noted in the patch
>  a80b83b7b8 by John Stultz.
> 
>  Arnd Bergmann suggested deprecating real time and using
>  monotonic or other timers for all input_event times as a
>  solution to both the above problems.
> 
>  Add a new ioctl to let the user dictate the kind of time
>  to be used for input events. This is similar to the evdev
>  implementation of the feature. Realtime is still the
>  default time. This is to maintain backward compatibility.
> 
>  The structure to maintain input events will be changed
>  in a different patch.
> >>>
> >>> Based on Peter's comment from when you first posted this,
> >>> https://patchwork.kernel.org/patch/9381209/, I tried to follow
> >>> the code path again, to see if we can come up with a way
> >>> to avoid introducing a new ioctl.
> >>>
> >>> There is one idea I had now: The two events we
> >>> get (upload and erase) are both triggered from evdev,
> >>> which gets called from user space through the EVIOCSFF
> >>> and EVIOCRMFF ioctls. This device already sets the
> >>> clock domain. Would it make sense to send the event
> >>> to the uinput owner using the same clock domain that
> >>> was set by the evdev owner, or are these two separate
> >>> by definition?
> >>
> >> uinput and evdev are two separate drivers. One is to write events to a
> >> virtual device and the other is to read from any input device.
> >> I considered both of these separate as the two events.
> >> Let me know if you guys prefer something else.
> >
> > Ok
> >
> >> We could also do away with this patch and just say we extend time till
> >> 2106 as we change struct input_event if we are okay with using
> >> realtime only for uinput.
> >
> > Another option might be to use monotonic times unconditionally
> > in uinput. The DRM drivers actually did this conversion successfully:
> > they changed the timestamps from real-time to monotonic-time and
> > added a module parameter to revert back to the old behavior. In
> > the end (after a few years) it turned out that nothing relied on real
> > time anyway, so I sent a patch to kill off the option.
> >
> > It seems rather likely that this is in the same category: the user
> > space reading the events either doesn't access the timestamps
> > at all, or is only interested in relative times, not absolute ones.
> >
> > The one program that I found that reads from /dev/uinput
> > is this one: http://svn.navi.cx/misc/trunk/inputpipe/src/
> > Here, all input_events get relayed between two machines,
> > so an input device on one can be used on the other across
> > a socket. The input_event data that we get from uinput gets
> > either interpreted (ignoring the timestamp) or pushed into
> > the evdev interface on the other machine, which then ignores
> > the timestamp in the kernel and creates a new stamp instead.
> 
> Right. I considered using just monotonic times before.
> I decided against it as John did not change the default times to
> monotonic times in a80b83b7b8.
> But, if nobody really cares about the actual timestamps and only
> relative timestamps matter, this might be a better option.

The timestamps in the kernel->userspace path in uinput are only for
force feedback control messages; userspace is not supposed to analyze
them but simply execute the instructions as they come in. I think we
should switch to the monotonic time and see if someone screams at us.
Then we can either add an option or see if there are other means of
resolving the issue.

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-28 Thread Dmitry Torokhov
On Fri, Oct 28, 2016 at 2:47 PM, Arnd Bergmann  wrote:
> On Friday, October 28, 2016 2:39:35 PM CEST Deepa Dinamani wrote:
>> >> >> @@ -55,24 +60,24 @@ struct ff_effect_compat {
>> >> >>
>> >> >>  static inline size_t input_event_size(void)
>> >> >>  {
>> >> >> -   return (in_compat_syscall() && !COMPAT_USE_64BIT_TIME) ?
>> >> >> -   sizeof(struct input_event_compat) : sizeof(struct 
>> >> >> input_event);
>> >> >> +   return in_compat_syscall() ? sizeof(struct 
>> >> >> raw_input_event_compat) :
>> >> >> +sizeof(struct raw_input_event);
>> >> >>  }
>> >> >
>> >> > I think the COMPAT_USE_64BIT_TIME check has to stay here,
>> >> > it's needed for x32 mode on x86-64.
>> >>
>> >> There is no time_t anymore in the raw_input_event structure.
>> >> The struct uses __kernel_ulong_t type.
>> >> This should take care of x32 support.
>> >
>> > I don't think it does.
>> >
>> >> From this cover letter:
>> >> https://www.spinics.net/lists/linux-arch/msg16356.html
>> >>
>> >> I see that that the __kernel types were introduced to address the ABI
>> >> issues for x32.
>> >
>> > This is a variation of the problem we are trying to solve for
>> > the other architectures in your patch set:
>> >
>> > On x32, the kernel uses produces a structure with the 64-bit
>> > layout, using __u64 tv_sec, to match the current user space
>> > that has 64-bit __kernel_ulong_t and 64-bit time_t, but
>> > in_compat_syscall() also returns 'true' here, as this is
>> > mostly a 32-bit ABI (time_t being one of the exceptions).
>>
>> Yes, I missed this.
>>
>> in_compat_syscall() is true for x32, this would mean we end up here
>> even if it is a x32 syscall.
>> But, wouldn't it be better to use in_x32_syscall() here since there is
>> no timeval any more?
>
> We have to distinguish four cases on x86:
>
> - native 32-bit, input_event with 32-bit time_t
> - compat 32-bit, input_event_compat with 32-bit time_t
> - native 64-bit, input_event with 64-bit time_t
> - compat x32, input_event with 64-bit time_t
>
> The first three can happen on other architectures too,
> the last one is x86 specific. There are probably other ways
> to express the condition above, but I can't think of one
> that is better than the one we have today.

Can we detect if given task is compat x32, like we do for compat
64/32? Or entire userspace has to be x32?

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH v2 3/4] input: Deprecate real timestamps beyond year 2106

2016-10-26 Thread Dmitry Torokhov
Hi Deepa,

On Mon, Oct 17, 2016 at 08:27:32PM -0700, Deepa Dinamani wrote:
> struct timeval is not y2038 safe.
> All usage of timeval in the kernel will be replaced by
> y2038 safe structures.
> 
> struct input_event maintains time for each input event.
> Real time timestamps are not ideal for input as this
> time can go backwards as noted in the patch a80b83b7b8
> by John Stultz. Hence, having the input_event.time fields
> only big enough for monotonic and boot times are
> sufficient.
> 
> Leave the original input_event as is. This is to maintain
> backward compatibility with existing userspace interfaces
> that use input_event.
> Introduce a new replacement struct raw_input_event.
> This replaces timeval with struct input_timeval. This structure
> maintains time in __kernel_ulong_t or compat_ulong_t to allow
> for architectures to override types as in the case of x32.
> 
> The change requires any userspace utilities reading or writing
> from event nodes to update their reading format to match
> raw_input_event. The changes to the popular libraries will be
> posted along with the kernel changes.
> The driver version is also updated to reflect the change in
> event format.

If users are forced to update to adapt to the new event format, should
we consider more radical changes? For example, does it make sense to
send timestamp on every event? Maybe we should only send it once per
event packet (between EV_SYN/SYN_REPORT)? What granularity do we need?
Is there anything else in current protocol that we'd like to change?

Thanks.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038


Re: [Y2038] [PATCH V2] hp_sdc_rtc: fixed y2038 problem in proc_show

2015-10-18 Thread Dmitry Torokhov
On Wed, Sep 16, 2015 at 03:55:37PM +0200, Arnd Bergmann wrote:
> On Wednesday 16 September 2015 21:45:38 WEN Pingbo wrote:
> > hp_sdc_rtc_proc_show() use timeval to store the time, which will
> > overflowed in 2038.
> > 
> > This patch fixes this problem by replacing timeval with timespec64.
> > hp_sdc_rtc_proc_show() only output string, so that userspace will work
> > normally if we apply this patch.
> > 
> > Not all timer in i8042 have y2038 risk(handshake, match timer, etc),
> > Replacements in those timer are just for consistency.
> > 
> > Version 2 Updates:
> > - compiled in m68k gcc cross compiler(4.6.3), no extra warnings
> > - placed s64 type cast in tv.tv_sec, making sure it work properly in
> > both 32bit and 64bit platform.
> > 
> > Signed-off-by: WEN Pingbo 
> > Cc: Y2038 
> > 
> 
> Looks very good,
> 
> Reviewed-by: Arnd Bergmann 

Applied, thank you.

-- 
Dmitry
___
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038