2015-11-09 19:34:30

by Alison Schofield

[permalink] [raw]
Subject: [PATCH v3] scsi: pmcraid: replace struct timeval with ktime_get_real_seconds()

Replace the use of struct timeval and do_gettimeofday() with
64 bit ktime_get_real_seconds. Prevents 32-bit type overflow
in year 2038 on 32-bit systems.

Driver was using the seconds portion of struct timeval (.tv_secs)
to pass a millseconds timestamp to the firmware. This change maintains
that same behavior using ktime_get_real_seconds.

The structure used to pass the timestamp to firmware is 48 bits and
works fine as long as the top 16 bits are zero and they will be zero
for a long time..ie. thousands of years.

Alternative Change: Add sub second granularity to timestamp

As noted above, the driver only used the seconds portion of timeval,
ignores the microseconds portion, and by multiplying by 1000 effectively
does a <<10 and always writes zero into timestamp[0].

The alternative change would pass all the bits to the firmware:

struct timespec64 ts;

ktime_get_real_ts64(&ts);
timestamp = ts.tv_sec * MSEC_PER_SEC + ts.tv_nsec / NSEC_PER_MSEC;

MAINTAINER: Please request alternate change if preferred.

Signed-off-by: Alison Schofield <[email protected]>
---
Change in v3:
* move alternative change proposal above the '---' to save
Changes in v2:
* add pmcraid to subject line
* update commit msg w additional detail
* add alternative change proposal per reviewer feedback

drivers/scsi/pmcraid.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index ed31d8c..3f64275 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -45,6 +45,7 @@
#include <asm/processor.h>
#include <linux/libata.h>
#include <linux/mutex.h>
+#include <linux/ktime.h>
#include <scsi/scsi.h>
#include <scsi/scsi_host.h>
#include <scsi/scsi_device.h>
@@ -5563,11 +5564,9 @@ static void pmcraid_set_timestamp(struct pmcraid_cmd *cmd)
__be32 time_stamp_len = cpu_to_be32(PMCRAID_TIMESTAMP_LEN);
struct pmcraid_ioadl_desc *ioadl = ioarcb->add_data.u.ioadl;

- struct timeval tv;
__le64 timestamp;

- do_gettimeofday(&tv);
- timestamp = tv.tv_sec * 1000;
+ timestamp = ktime_get_real_seconds() * 1000;

pinstance->timestamp_data->timestamp[0] = (__u8)(timestamp);
pinstance->timestamp_data->timestamp[1] = (__u8)((timestamp) >> 8);
--
2.1.4


2015-11-09 20:33:43

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [Y2038] [PATCH v3] scsi: pmcraid: replace struct timeval with ktime_get_real_seconds()

On Monday 09 November 2015 11:34:20 Alison Schofield wrote:
> Replace the use of struct timeval and do_gettimeofday() with
> 64 bit ktime_get_real_seconds. Prevents 32-bit type overflow
> in year 2038 on 32-bit systems.
>
> Driver was using the seconds portion of struct timeval (.tv_secs)
> to pass a millseconds timestamp to the firmware. This change maintains
> that same behavior using ktime_get_real_seconds.
>
> The structure used to pass the timestamp to firmware is 48 bits and
> works fine as long as the top 16 bits are zero and they will be zero
> for a long time..ie. thousands of years.
>
> Alternative Change: Add sub second granularity to timestamp
>
> As noted above, the driver only used the seconds portion of timeval,
> ignores the microseconds portion, and by multiplying by 1000 effectively
> does a <<10 and always writes zero into timestamp[0].
>
> The alternative change would pass all the bits to the firmware:
>
> struct timespec64 ts;
>
> ktime_get_real_ts64(&ts);
> timestamp = ts.tv_sec * MSEC_PER_SEC + ts.tv_nsec / NSEC_PER_MSEC;
>
> MAINTAINER: Please request alternate change if preferred.
>
> Signed-off-by: Alison Schofield <[email protected]>
>

Reviewed-by: Arnd Bergmann <[email protected]>

just the last sentence of the changelog should probably go below the
"---" line as it is irrelevant in the git history if the patch gets
applied. James can probably remove that line manually when he applies
it, just remember this if you have to send a version 4 for another
reason.

Arnd

2015-11-12 01:55:09

by Martin K. Petersen

[permalink] [raw]
Subject: Re: [PATCH v3] scsi: pmcraid: replace struct timeval with ktime_get_real_seconds()

>>>>> "Alison" == Alison Schofield <[email protected]> writes:

Alison> Replace the use of struct timeval and do_gettimeofday() with 64
Alison> bit ktime_get_real_seconds. Prevents 32-bit type overflow in
Alison> year 2038 on 32-bit systems.

Applied.

--
Martin K. Petersen Oracle Linux Engineering