32-bit systems using 'struct timeval' will break in the year 2038, in order to avoid that replace the code with more appropriate types. This patch replaces timeval with 64 bit ktime_t which is y2038 safe. Since st->timestamp is only interested in seconds, directly using time64_t here. Function ktime_get_seconds is used since it uses monotonic instead of real time and thus will not cause overflow.
Signed-off-by: Shraddha Barke shraddha.6596@gmail.com --- drivers/block/sx8.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c index 59c91d4..baadb77 100644 --- a/drivers/block/sx8.c +++ b/drivers/block/sx8.c @@ -23,7 +23,7 @@ #include <linux/workqueue.h> #include <linux/bitops.h> #include <linux/delay.h> -#include <linux/time.h> +#include <linux/ktime.h> #include <linux/hdreg.h> #include <linux/dma-mapping.h> #include <linux/completion.h> @@ -671,16 +671,15 @@ static int carm_send_special (struct carm_host *host, carm_sspc_t func) static unsigned int carm_fill_sync_time(struct carm_host *host, unsigned int idx, void *mem) { - struct timeval tv; struct carm_msg_sync_time *st = mem;
- do_gettimeofday(&tv); + time64_t tv = ktime_get_seconds();
memset(st, 0, sizeof(*st)); st->type = CARM_MSG_MISC; st->subtype = MISC_SET_TIME; st->handle = cpu_to_le32(TAG_ENCODE(idx)); - st->timestamp = cpu_to_le32(tv.tv_sec); + st->timestamp = cpu_to_le32(tv);
return sizeof(struct carm_msg_sync_time); }
On 12/22/2015 01:32 AM, Shraddha Barke wrote:
32-bit systems using 'struct timeval' will break in the year 2038, in order to avoid that replace the code with more appropriate types. This patch replaces timeval with 64 bit ktime_t which is y2038 safe. Since st->timestamp is only interested in seconds, directly using time64_t here. Function ktime_get_seconds is used since it uses monotonic instead of real time and thus will not cause overflow.
Added for 4.5, thanks.
On Tuesday 22 December 2015, Jens Axboe wrote:
On 12/22/2015 01:32 AM, Shraddha Barke wrote:
32-bit systems using 'struct timeval' will break in the year 2038, in order to avoid that replace the code with more appropriate types. This patch replaces timeval with 64 bit ktime_t which is y2038 safe. Since st->timestamp is only interested in seconds, directly using time64_t here. Function ktime_get_seconds is used since it uses monotonic instead of real time and thus will not cause overflow.
Added for 4.5, thanks.
I think the patch is wrong unfortunately: This time is sent to the adapter firmware, and presumably used for logging. If we use monotonic time instead of real time, we end up getting duplicate timestamps after a reboot, which always starts the montonic time at zero, so I think it has to be real time.
The data structure is defined as
struct carm_msg_sync_time { u8 type; u8 subtype; u16 reserved1; __le32 handle; u32 reserved2; __le32 timestamp; } __attribute__((packed));
which lets me guess that the 'reserved2' field might actually contain the upper 32 bits of the timestamp, but it might as well be something else, or not used by the firmware at all, and that's hard to tell without documentation.
If we use the lower unsigned 32-bit portion of real-time time64_t, the driver will survive until 2106 without an overflow here, which may be the safest alternative.
Arnd
On 12/22/2015 02:36 PM, Arnd Bergmann wrote:
On Tuesday 22 December 2015, Jens Axboe wrote:
On 12/22/2015 01:32 AM, Shraddha Barke wrote:
32-bit systems using 'struct timeval' will break in the year 2038, in order to avoid that replace the code with more appropriate types. This patch replaces timeval with 64 bit ktime_t which is y2038 safe. Since st->timestamp is only interested in seconds, directly using time64_t here. Function ktime_get_seconds is used since it uses monotonic instead of real time and thus will not cause overflow.
Added for 4.5, thanks.
I think the patch is wrong unfortunately: This time is sent to the adapter firmware, and presumably used for logging. If we use monotonic time instead of real time, we end up getting duplicate timestamps after a reboot, which always starts the montonic time at zero, so I think it has to be real time.
The data structure is defined as
struct carm_msg_sync_time { u8 type; u8 subtype; u16 reserved1; __le32 handle; u32 reserved2; __le32 timestamp; } __attribute__((packed));
which lets me guess that the 'reserved2' field might actually contain the upper 32 bits of the timestamp, but it might as well be something else, or not used by the firmware at all, and that's hard to tell without documentation.
If we use the lower unsigned 32-bit portion of real-time time64_t, the driver will survive until 2106 without an overflow here, which may be the safest alternative.
Good points, Arnd. Shraddha, care to attempt to fix this up?
On Tue, 22 Dec 2015, Jens Axboe wrote:
On 12/22/2015 02:36 PM, Arnd Bergmann wrote:
On Tuesday 22 December 2015, Jens Axboe wrote:
On 12/22/2015 01:32 AM, Shraddha Barke wrote:
32-bit systems using 'struct timeval' will break in the year 2038, in order to avoid that replace the code with more appropriate types. This patch replaces timeval with 64 bit ktime_t which is y2038 safe. Since st->timestamp is only interested in seconds, directly using time64_t here. Function ktime_get_seconds is used since it uses monotonic instead of real time and thus will not cause overflow.
Added for 4.5, thanks.
I think the patch is wrong unfortunately: This time is sent to the adapter firmware, and presumably used for logging. If we use monotonic time instead of real time, we end up getting duplicate timestamps after a reboot, which always starts the montonic time at zero, so I think it has to be real time.
The data structure is defined as
struct carm_msg_sync_time { u8 type; u8 subtype; u16 reserved1; __le32 handle; u32 reserved2; __le32 timestamp; } __attribute__((packed));
which lets me guess that the 'reserved2' field might actually contain the upper 32 bits of the timestamp, but it might as well be something else, or not used by the firmware at all, and that's hard to tell without documentation.
If we use the lower unsigned 32-bit portion of real-time time64_t, the driver will survive until 2106 without an overflow here, which may be the safest alternative.
Good points, Arnd. Shraddha, care to attempt to fix this up?
Yes. I'll send a v2
Shraddha
-- Jens Axboe
-
Arnd Bergmann -
Jens Axboe -
Shraddha Barke