Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934392AbcJ0BpU (ORCPT ); Wed, 26 Oct 2016 21:45:20 -0400 Received: from leo.clearchain.com ([199.73.29.74]:50936 "EHLO mail.clearchain.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932465AbcJ0BpS (ORCPT ); Wed, 26 Oct 2016 21:45:18 -0400 X-Greylist: delayed 635 seconds by postgrey-1.27 at vger.kernel.org; Wed, 26 Oct 2016 21:45:18 EDT Date: Thu, 27 Oct 2016 11:45:09 +1000 From: Peter Hutterer To: Deepa Dinamani Cc: Dmitry Torokhov , linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, arnd@arndb.de, y2038@lists.linaro.org Subject: Re: [PATCH v2 1/4] uinput: Add ioctl for using monotonic/ boot times Message-ID: <20161027014509.GB14832@jelly> References: <1476761253-13450-1-git-send-email-deepa.kernel@gmail.com> <1476761253-13450-2-git-send-email-deepa.kernel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1476761253-13450-2-git-send-email-deepa.kernel@gmail.com> User-Agent: Mutt/1.7.1 (2016-10-04) X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.4.3 (mail.clearchain.com [127.0.0.1]); Thu, 27 Oct 2016 12:20:56 +1030 (CST) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5449 Lines: 170 On Mon, Oct 17, 2016 at 08:27:30PM -0700, Deepa Dinamani wrote: > struct timeval which is part of struct input_event to > maintain the event times is not y2038 safe. > > Real time timestamps are also not ideal for input_event > as this time can go backwards as noted in the patch > a80b83b7b8 by John Stultz. > > Arnd Bergmann suggested deprecating real time and using > monotonic or other timers for all input_event times as a > solution to both the above problems. > > Add a new ioctl to let the user dictate the kind of time > to be used for input events. This is similar to the evdev > implementation of the feature. Realtime is still the > default time. This is to maintain backward compatibility. > > The structure to maintain input events will be changed > in a different patch. > > Signed-off-by: Deepa Dinamani > --- > drivers/input/misc/uinput.c | 56 ++++++++++++++++++++++++++++++++++++++++++++- > include/linux/uinput.h | 1 + > include/uapi/linux/uinput.h | 3 +++ > 3 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c > index 92595b9..3d75c5a 100644 > --- a/drivers/input/misc/uinput.c > +++ b/drivers/input/misc/uinput.c > @@ -46,11 +46,26 @@ static int uinput_dev_event(struct input_dev *dev, > unsigned int type, unsigned int code, int value) > { > struct uinput_device *udev = input_get_drvdata(dev); > + struct timespec64 ts; > > udev->buff[udev->head].type = type; > udev->buff[udev->head].code = code; > udev->buff[udev->head].value = value; > - do_gettimeofday(&udev->buff[udev->head].time); > + > + switch (udev->clk_type) { > + case CLOCK_REALTIME: > + ktime_get_real_ts64(&ts); > + break; > + case CLOCK_MONOTONIC: > + ktime_get_ts64(&ts); > + break; > + case CLOCK_BOOTTIME: > + get_monotonic_boottime64(&ts); > + break; > + } hmm, I'm a bit confused here. This is an in-kernel bit only (passing the time through uinput events has no effect). So why do we need an ioctl here? it's an in-kernel decision only anyway and the time in the events sent to the evdev client should be dictated by what that client sets for the clock type, right? Cheers, Peter > + > + udev->buff[udev->head].time.tv_sec = ts.tv_sec; > + udev->buff[udev->head].time.tv_usec = ts.tv_nsec / NSEC_PER_USEC; > udev->head = (udev->head + 1) % UINPUT_BUFFER_SIZE; > > wake_up_interruptible(&udev->waitq); > @@ -295,6 +310,7 @@ static int uinput_create_device(struct uinput_device *udev) > if (error) > goto fail2; > > + udev->clk_type = CLOCK_REALTIME; > udev->state = UIST_CREATED; > > return 0; > @@ -304,6 +320,38 @@ static int uinput_create_device(struct uinput_device *udev) > return error; > } > > +static int uinput_set_clk_type(struct uinput_device *udev, unsigned int clkid) > +{ > + unsigned int clk_type; > + > + if (udev->state != UIST_CREATED) > + return -EINVAL; > + > + switch (clkid) { > + /* Realtime clock is only valid until year 2038.*/ > + case CLOCK_REALTIME: > + clk_type = CLOCK_REALTIME; > + break; > + case CLOCK_MONOTONIC: > + clk_type = CLOCK_MONOTONIC; > + break; > + case CLOCK_BOOTTIME: > + clk_type = CLOCK_BOOTTIME; > + break; > + default: > + return -EINVAL; > + } > + > + if (udev->clk_type != clk_type) { > + udev->clk_type = clk_type; > + > + /* Flush pending events */ > + uinput_flush_requests(udev); > + } > + > + return 0; > +} > + > static int uinput_open(struct inode *inode, struct file *file) > { > struct uinput_device *newdev; > @@ -787,6 +835,7 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > char *phys; > const char *name; > unsigned int size; > + int clock_id; > > retval = mutex_lock_interruptible(&udev->mutex); > if (retval) > @@ -817,6 +866,11 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd, > retval = uinput_dev_setup(udev, p); > goto out; > > + case UI_SET_CLOCKID: > + if (copy_from_user(&clock_id, p, sizeof(unsigned int))) > + return -EFAULT; > + return uinput_set_clk_type(udev, clock_id); > + > /* UI_ABS_SETUP is handled in the variable size ioctls */ > > case UI_SET_EVBIT: > diff --git a/include/linux/uinput.h b/include/linux/uinput.h > index 75de43d..6527fb7 100644 > --- a/include/linux/uinput.h > +++ b/include/linux/uinput.h > @@ -72,6 +72,7 @@ struct uinput_device { > unsigned char head; > unsigned char tail; > struct input_event buff[UINPUT_BUFFER_SIZE]; > + int clk_type; > unsigned int ff_effects_max; > > struct uinput_request *requests[UINPUT_NUM_REQUESTS]; > diff --git a/include/uapi/linux/uinput.h b/include/uapi/linux/uinput.h > index dc652e2..d9494ae 100644 > --- a/include/uapi/linux/uinput.h > +++ b/include/uapi/linux/uinput.h > @@ -133,6 +133,9 @@ struct uinput_abs_setup { > */ > #define UI_ABS_SETUP _IOW(UINPUT_IOCTL_BASE, 4, struct uinput_abs_setup) > > +/* Set clockid to be used for timestamps */ > +#define UI_SET_CLOCKID _IOW(UINPUT_IOCTL_BASE, 5, int) > + > #define UI_SET_EVBIT _IOW(UINPUT_IOCTL_BASE, 100, int) > #define UI_SET_KEYBIT _IOW(UINPUT_IOCTL_BASE, 101, int) > #define UI_SET_RELBIT _IOW(UINPUT_IOCTL_BASE, 102, int) > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-input" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >