Re: [Y2038] [PATCH v2 01/24] Input: input_event: fix struct padding on sparc64
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(&ts); > - 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(&udev->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
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
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
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
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
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
On Mon, Jan 08, 2018 at 04:07:22PM -0800, Deepa Dinamani wrote: > On Mon, Jan 8, 2018 at 1:51 PM, Dmitry Torokhov > wrote: > > 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. > > I was interpreting working incorrectly on 32 bit architectures, but > working correctly on 64 bit architectures as a failure of the feature > to use realtime clock at all. > But, you are correct that the patch does not actively do anything to > stop people from using realtime clock. > > >> > >> 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. > > I was interpreting not being able to use negative timestamps on 32 bit > machines as breaking the ABI. > > >> 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. > > I'm fine with this commit text. Let me know if you would like me to update > this. No, unless somebody yells I'll change it on my side. 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
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
On Sat, Jan 06, 2018 at 01:43:34PM -0800, Deepa Dinamani wrote: > On Tue, Jan 2, 2018 at 7:35 AM, Arnd Bergmann wrote: > > On Tue, Jan 2, 2018 at 7:46 AM, Dmitry Torokhov > > 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(&ev_time[EV_CLK_MONO]); > >>> + ktime_get_real_ts64(&ev_time[EV_CLK_REAL]); > >>> + get_monotonic_boottime64(&ev_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
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(&ts); > + break; > + case EV_CLK_MONO: > + ktime_get_ts64(&ts); > + break; > + case EV_CLK_BOOT: > + get_monotonic_boottime64(&ts); > + break; > + } > > - ev.time = ktime_to_timeval(time); > + ev.time.tv_sec = ts.tv_sec; > + ev.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC; > ev.type = EV_SYN; > ev.code = SYN_DROPPED; > ev.value = 0; > @@ -257,17 +264,20 @@ static void __pass_event(struct evdev_client *client, > > static void evdev_pass_values(struct evdev_client *client, > const struct input_value *vals, unsigned int count, > - ktime_t *ev_time) > + struct timespec64 *ev_time) > { > struct evdev *evdev = client->evdev; > const struct input_value *v; > struct input_event event; > + struct timespec64 ts; > bool wakeup = false; > > if (client->revoked) > return; > > - event.time = ktime_to_timeval(ev_time[client->clk_type]); > + ts = ev_time[client->clk_type]; > + event.time.tv_sec = ts.tv_sec; > + event.time.tv_usec = ts.tv_nsec / NSEC_PER_USEC; > > /* Interrupts are disabled, just acquire the lock. */ > spin_lock(&client->buffer_lock); > @@ -304,12 +314,11 @@ static void evdev_events(struct input_handle *handle, > { > struct evdev *evdev = handle->private; > struct evdev_client *client; > - ktime_t ev_time[EV_CLK_MAX]; > + struct timespec64 ev_time[EV_CLK_MAX]; > > - ev_time[EV_CLK_MONO] = ktime_get(); > - ev_time[EV_CLK_REAL] = ktime_mono_to_real(ev_time[EV_CLK_MONO]); > - ev_time[EV_CLK_BOOT] = ktime_mono_to_any(ev_time[EV_CLK_MONO], > - TK_OFFS_BOOT); > + ktime_get_ts64(&ev_time[EV_CLK_MONO]); > + ktime_get_real_ts64(&ev_time[EV_CLK_REAL]); > + get_monotonic_boottime64(&ev_time[EV_CLK_BOOT]); 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.
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(&udev->buff[udev->head].time); > + ktime_get_ts64(&ts); > + udev->buff[udev->head].time.tv_sec = ts.tv_sec; > + udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC; > udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE; > > wake_up_interruptible(&udev->waitq); > -- > 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
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
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
On Thu, Oct 27, 2016 at 03:25:43PM -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? > > I did see the thread with Pingbo's patches where you had a similar comment. > > I see my series as decoupling the kernel input event format from the > userspace format. > The formats also are really the same still. > Could this be considered the first step towards changing the protocol? I really do not see the point. I think we agree that the current protocol is not working past 2038 and it does not seem we can fix it transparently for the user. So we need to define new protocol and let kernel and clients negotiate which one is used. I am not concerned about in-kernel representation much as it does not get stored anywhere so we can adjust it as needed without too much effort. > > The protocol changes might need new interfaces to be defined between > libraries. > And, could end up being a substantial change. > Would a step by step approach make sense? It would depend largely on the scope. 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
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 0/3] fix y2038 problem in input_event
Hi Wen, On Mon, Nov 02, 2015 at 09:35:36PM +0800, WEN Pingbo wrote: > Before this, I have discussed this problem with Arnd. And Arnd have > an idea that by converting timeval to long / long in input_event, so that > input_event structure size will be unchanged, and timeval structure will > removed entirely. But we also need to avoid using CLOCK_REALTIME in > userland, to keep the new input_event structure y2038 safe. > > The input_event will only support monotonic time in Arnd's idea. And > we still need to add wall time support for old 32-bit binary. > > Those patches try to keep original input capacity, and resolve y2038 > problem in input_event radically. > > struct input_event is only used between kernel and userspace > communication (except uinput). So that we can replace input_event > with input_event64 in kernel entirely, and add a conversion in > input_event_from/to_user() to keep compatible with old 32-bits binary. > > userland can switch to input_event64, which is y2038 safe, via ioctl. If we are forcing userspace to change the protocol I'd rather explore whether we need to transmit the timestamp in each and every event. I would much rather drop it and instead introduce new event code for timestamp (we already have MSC_TIMESTAMP for hardware-generated timestamps, maybe we can introduce new ones for kernel-generated timestamps). 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
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