struct timeval' uses 32-bits for its seconds field and will overflow in
the year 2038 and beyond. This patch replaces the usage of 'struct timeval'
in mon_get_timestamp() with timespec64 which uses a 64-bit seconds field
and is y2038-safe. mon_get_timestamp() truncates the timestamp at 4096 seconds,
so the correctness of the code is not affected. This patch is part of a larger
attempt to remove instances of struct timeval and other 32-bit timekeeping
(time_t, struct timespec) from the kernel.
Signed-off-by: Tina Ruchandani <ruchandani.tina(a)gmail.com>
Suggested-by: Arnd Bergmann <arnd(a)arndb.de>
---
Changes in v2:
- Switch to monotonic time since we only care about time elapsed.
---
drivers/usb/mon/mon_text.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/mon/mon_text.c b/drivers/usb/mon/mon_text.c
index ad40825..98e4f63 100644
--- a/drivers/usb/mon/mon_text.c
+++ b/drivers/usb/mon/mon_text.c
@@ -9,6 +9,7 @@
#include <linux/usb.h>
#include <linux/slab.h>
#include <linux/time.h>
+#include <linux/ktime.h>
#include <linux/export.h>
#include <linux/mutex.h>
#include <linux/debugfs.h>
@@ -176,12 +177,12 @@ static inline char mon_text_get_data(struct mon_event_text *ep, struct urb *urb,
static inline unsigned int mon_get_timestamp(void)
{
- struct timeval tval;
+ struct timespec64 now;
unsigned int stamp;
- do_gettimeofday(&tval);
- stamp = tval.tv_sec & 0xFFF; /* 2^32 = 4294967296. Limit to 4096s. */
- stamp = stamp * 1000000 + tval.tv_usec;
+ ktime_get_ts64(&now);
+ stamp = now.tv_sec & 0xFFF; /* 2^32 = 4294967296. Limit to 4096s. */
+ stamp = stamp * USEC_PER_SEC + now.tv_nsec / NSEC_PER_USEC;
return stamp;
}
--
2.6.0.rc2.230.g3dd15c0
This patchset removes the use of struct timeval because struct timeval
will break in 32 bit systems in the year 2038.
Amitoj Kaur Chawla(2):
scsi: bfa: bfa_svc: Use ktime_get_real_seconds()
scsi: bfa: bfa_svc: Remove use of struct timeval
drivers/scsi/bfa/bfa_svc.c | 30 ++++++++----------------------
drivers/scsi/bfa/bfa_svc.h | 2 +-
2 files changed, 9 insertions(+), 23 deletions(-)
--
1.9.1
On Thursday 05 November 2015 14:34:48 Stefan Richter wrote:
> On Oct 22 Arnd Bergmann wrote:
> > On Thursday 22 October 2015 04:05:00 Amitoj Kaur Chawla wrote:
> > > 32 bit systems using 'struct timeval' will break in the year 2038, so
> > > we replace the code appropriately. However, this driver is not broken
> > > in 2038 since we are using only the microseconds portion of the
> > > current time.
> > >
> > > This patch replaces timeval with timespec64.
> > >
> > > Signed-off-by: Amitoj Kaur Chawla <amitoj1606(a)gmail.com>
> >
> > Reviewed-by: Arnd Bergmann <arnd(a)arndb.de>
> >
> > (adding the y2038 mailing list as well)
>
> Committed to linux1394.git.
> Arnd, do you have special y2038 plans that make it desirable to have this
> merged before v4.4? Otherwise I would submit it for v4.5-rc1.
4.5-rc1 is fine.
Arnd
Hi Arnd,
We're redesigning the timestamp handling in the video4linux subsystem moving away
from struct timeval to a single timestamp in ns (what ktime_get_ns() gives us).
But I was wondering: ktime_get_ns() gives a s64, so should we use s64 as well as
the timestamp type we'll eventually be returning to the user, or should we use u64?
The current patch series we made uses a u64, but I am now beginning to doubt that
decision.
Regards,
Hans
This patchset removes the use of struct timeval because struct timeval
will break in 32 bit systems in the year 2038.
Changes in v2:
-Removed unnecessary if condition in second patch
-Removed u32 to u64 conversion since monotonic time doesn't
require the change.
Amitoj Kaur Chawla (2):
scsi: bfa: bfa_svc: Use ktime_get_real_seconds()
scsi: bfa: bfa_svc: Remove use of 'struct timeval'
drivers/scsi/bfa/bfa_svc.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)
--
1.9.1
timeval is deprecated and not y2038 safe. Its size also changes according
to 32 bit/ 64 bit compilation. Replace it with 32 and 64 bit versions of
its individual fields, giving two ioctls with different code values.
The two ioctls are necessary to maintain the 32 bit and 64 bit userspace
compatibility with a 64/32 bit kernel.
Change unsigned to __u32 types for a definitive userspace interface.
This is in accordance with the psABI that the unsigned type is always
32 bits.
Also use motonic timer instead of real time to ensure positive delta
values.
Refactor usbtest_ioctl for readability to isolate the handling of the
testing timing measurement.
The official testusb userspace tool can be changed in a separate patch
to reflect the __u32 changes as well. It can use the usbtest_param_32
struct, since 32 bit seconds is long enough for test durations.
Signed-off-by: Deepa Dinamani <deepa.kernel(a)gmail.com>
---
drivers/usb/misc/usbtest.c | 229 +++++++++++++++++++++++++++++----------------
1 file changed, 147 insertions(+), 82 deletions(-)
diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 637f3f7..31f5f49 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -22,18 +22,42 @@ static void complicated_callback(struct urb *urb);
/*-------------------------------------------------------------------------*/
/* FIXME make these public somewhere; usbdevfs.h? */
-struct usbtest_param {
+
+/* Parameter for usbtest driver. */
+struct usbtest_param_32 {
/* inputs */
- unsigned test_num; /* 0..(TEST_CASES-1) */
- unsigned iterations;
- unsigned length;
- unsigned vary;
- unsigned sglen;
+ __u32 test_num; /* 0..(TEST_CASES-1) */
+ __u32 iterations;
+ __u32 length;
+ __u32 vary;
+ __u32 sglen;
/* outputs */
- struct timeval duration;
+ __s32 duration_sec;
+ __s32 duration_usec;
};
-#define USBTEST_REQUEST _IOWR('U', 100, struct usbtest_param)
+
+/*
+ * Compat parameter to the usbtest driver.
+ * This supports older user space binaries compiled with 64 bit compiler.
+ */
+struct usbtest_param_64 {
+ /* inputs */
+ __u32 test_num; /* 0..(TEST_CASES-1) */
+ __u32 iterations;
+ __u32 length;
+ __u32 vary;
+ __u32 sglen;
+
+ /* outputs */
+ __s64 duration_sec;
+ __s64 duration_usec;
+};
+
+/* IOCTL interface to the driver. */
+#define USBTEST_REQUEST_32 _IOWR('U', 100, struct usbtest_param_32)
+/* COMPAT IOCTL interface to the driver. */
+#define USBTEST_REQUEST_64 _IOWR('U', 100, struct usbtest_param_64)
/*-------------------------------------------------------------------------*/
@@ -1030,7 +1054,7 @@ struct ctrl_ctx {
unsigned pending;
int status;
struct urb **urb;
- struct usbtest_param *param;
+ struct usbtest_param_32 *param;
int last;
};
@@ -1155,7 +1179,7 @@ error:
}
static int
-test_ctrl_queue(struct usbtest_dev *dev, struct usbtest_param *param)
+test_ctrl_queue(struct usbtest_dev *dev, struct usbtest_param_32 *param)
{
struct usb_device *udev = testdev_to_usbdev(dev);
struct urb **urb;
@@ -1930,7 +1954,7 @@ static struct urb *iso_alloc_urb(
}
static int
-test_queue(struct usbtest_dev *dev, struct usbtest_param *param,
+test_queue(struct usbtest_dev *dev, struct usbtest_param_32 *param,
int pipe, struct usb_endpoint_descriptor *desc, unsigned offset)
{
struct transfer_context context;
@@ -2049,81 +2073,20 @@ static int test_unaligned_bulk(
return retval;
}
-/*-------------------------------------------------------------------------*/
-
-/* We only have this one interface to user space, through usbfs.
- * User mode code can scan usbfs to find N different devices (maybe on
- * different busses) to use when testing, and allocate one thread per
- * test. So discovery is simplified, and we have no device naming issues.
- *
- * Don't use these only as stress/load tests. Use them along with with
- * other USB bus activity: plugging, unplugging, mousing, mp3 playback,
- * video capture, and so on. Run different tests at different times, in
- * different sequences. Nothing here should interact with other devices,
- * except indirectly by consuming USB bandwidth and CPU resources for test
- * threads and request completion. But the only way to know that for sure
- * is to test when HC queues are in use by many devices.
- *
- * WARNING: Because usbfs grabs udev->dev.sem before calling this ioctl(),
- * it locks out usbcore in certain code paths. Notably, if you disconnect
- * the device-under-test, hub_wq will wait block forever waiting for the
- * ioctl to complete ... so that usb_disconnect() can abort the pending
- * urbs and then call usbtest_disconnect(). To abort a test, you're best
- * off just killing the userspace task and waiting for it to exit.
- */
-
+/* Run tests. */
static int
-usbtest_ioctl(struct usb_interface *intf, unsigned int code, void *buf)
+usbtest_do_ioctl(struct usb_interface *intf, struct usbtest_param_32 *param)
{
struct usbtest_dev *dev = usb_get_intfdata(intf);
struct usb_device *udev = testdev_to_usbdev(dev);
- struct usbtest_param *param = buf;
- int retval = -EOPNOTSUPP;
struct urb *urb;
struct scatterlist *sg;
struct usb_sg_request req;
- struct timeval start;
unsigned i;
-
- /* FIXME USBDEVFS_CONNECTINFO doesn't say how fast the device is. */
-
- pattern = mod_pattern;
-
- if (code != USBTEST_REQUEST)
- return -EOPNOTSUPP;
+ int retval = -EOPNOTSUPP;
if (param->iterations <= 0)
return -EINVAL;
-
- if (param->sglen > MAX_SGLEN)
- return -EINVAL;
-
- if (mutex_lock_interruptible(&dev->lock))
- return -ERESTARTSYS;
-
- /* FIXME: What if a system sleep starts while a test is running? */
-
- /* some devices, like ez-usb default devices, need a non-default
- * altsetting to have any active endpoints. some tests change
- * altsettings; force a default so most tests don't need to check.
- */
- if (dev->info->alt >= 0) {
- int res;
-
- if (intf->altsetting->desc.bInterfaceNumber) {
- mutex_unlock(&dev->lock);
- return -ENODEV;
- }
- res = set_altsetting(dev, dev->info->alt);
- if (res) {
- dev_err(&intf->dev,
- "set altsetting to %d failed, %d\n",
- dev->info->alt, res);
- mutex_unlock(&dev->lock);
- return res;
- }
- }
-
/*
* Just a bunch of test cases that every HCD is expected to handle.
*
@@ -2133,7 +2096,6 @@ usbtest_ioctl(struct usb_interface *intf, unsigned int code, void *buf)
* FIXME add more tests! cancel requests, verify the data, control
* queueing, concurrent read+write threads, and so on.
*/
- do_gettimeofday(&start);
switch (param->test_num) {
case 0:
@@ -2548,13 +2510,116 @@ usbtest_ioctl(struct usb_interface *intf, unsigned int code, void *buf)
dev->in_pipe, NULL, 0);
break;
}
- do_gettimeofday(¶m->duration);
- param->duration.tv_sec -= start.tv_sec;
- param->duration.tv_usec -= start.tv_usec;
- if (param->duration.tv_usec < 0) {
- param->duration.tv_usec += 1000 * 1000;
- param->duration.tv_sec -= 1;
+ return retval;
+}
+
+/*-------------------------------------------------------------------------*/
+
+/* We only have this one interface to user space, through usbfs.
+ * User mode code can scan usbfs to find N different devices (maybe on
+ * different busses) to use when testing, and allocate one thread per
+ * test. So discovery is simplified, and we have no device naming issues.
+ *
+ * Don't use these only as stress/load tests. Use them along with with
+ * other USB bus activity: plugging, unplugging, mousing, mp3 playback,
+ * video capture, and so on. Run different tests at different times, in
+ * different sequences. Nothing here should interact with other devices,
+ * except indirectly by consuming USB bandwidth and CPU resources for test
+ * threads and request completion. But the only way to know that for sure
+ * is to test when HC queues are in use by many devices.
+ *
+ * WARNING: Because usbfs grabs udev->dev.sem before calling this ioctl(),
+ * it locks out usbcore in certain code paths. Notably, if you disconnect
+ * the device-under-test, hub_wq will wait block forever waiting for the
+ * ioctl to complete ... so that usb_disconnect() can abort the pending
+ * urbs and then call usbtest_disconnect(). To abort a test, you're best
+ * off just killing the userspace task and waiting for it to exit.
+ */
+
+static int
+usbtest_ioctl(struct usb_interface *intf, unsigned int code, void *buf)
+{
+
+ struct usbtest_dev *dev = usb_get_intfdata(intf);
+ struct usbtest_param_64 *param_64 = buf;
+ struct usbtest_param_32 temp;
+ struct usbtest_param_32 *param_32 = buf;
+ struct timespec64 start;
+ struct timespec64 end;
+ struct timespec64 duration;
+ int retval = -EOPNOTSUPP;
+
+ /* FIXME USBDEVFS_CONNECTINFO doesn't say how fast the device is. */
+
+ pattern = mod_pattern;
+
+ if (mutex_lock_interruptible(&dev->lock))
+ return -ERESTARTSYS;
+
+ /* FIXME: What if a system sleep starts while a test is running? */
+
+ /* some devices, like ez-usb default devices, need a non-default
+ * altsetting to have any active endpoints. some tests change
+ * altsettings; force a default so most tests don't need to check.
+ */
+ if (dev->info->alt >= 0) {
+ if (intf->altsetting->desc.bInterfaceNumber) {
+ retval = -ENODEV;
+ goto free_mutex;
+ }
+ retval = set_altsetting(dev, dev->info->alt);
+ if (retval) {
+ dev_err(&intf->dev,
+ "set altsetting to %d failed, %d\n",
+ dev->info->alt, retval);
+ goto free_mutex;
+ }
+ }
+
+ switch (code) {
+ case USBTEST_REQUEST_64:
+ temp.test_num = param_64->test_num;
+ temp.iterations = param_64->iterations;
+ temp.length = param_64->length;
+ temp.sglen = param_64->sglen;
+ temp.vary = param_64->vary;
+ param_32 = &temp;
+ break;
+
+ case USBTEST_REQUEST_32:
+ break;
+
+ default:
+ retval = -EOPNOTSUPP;
+ goto free_mutex;
+ }
+
+ ktime_get_ts64(&start);
+
+ retval = usbtest_do_ioctl(intf, param_32);
+ if (retval)
+ goto free_mutex;
+
+ ktime_get_ts64(&end);
+
+ duration = timespec64_sub(end, start);
+
+ temp.duration_sec = duration.tv_sec;
+ temp.duration_usec = duration.tv_nsec/NSEC_PER_USEC;
+
+ switch (code) {
+ case USBTEST_REQUEST_32:
+ param_32->duration_sec = temp.duration_sec;
+ param_32->duration_usec = temp.duration_usec;
+ break;
+
+ case USBTEST_REQUEST_64:
+ param_64->duration_sec = temp.duration_sec;
+ param_64->duration_usec = temp.duration_usec;
+ break;
}
+
+free_mutex:
mutex_unlock(&dev->lock);
return retval;
}
--
1.9.1