All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] scsi: bfa: do_gettimeofday removal
@ 2017-11-10 15:37 ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

The bfa driver is one of the main users of do_gettimeofday(), a function
that I'm trying to remove as part of the y2038 cleanup.

The timestamps are all uses in slightly different ways, so this has turned
into a rather longish series for doing something that should be simple.

The last patch in the series ("scsi: bfa: use 64-bit times in
bfa_aen_entry_s ABI") is one that needs to be reviewed very carefully,
and it can be skipped if the maintainers prefer to leave the 32-bit ABI
unchanged, the rest are hopefully fairly straightforward.

      Arnd

Arnd Bergmann (7):
  scsi: bfa: use ktime_get_real_ts64 for firmware timestamp
  scsi: bfa: use proper time accessor for stats_reset_time
  scsi: bfa: improve bfa_ioc_send_enable/disable data
  scsi: bfa: document overflow of io_profile_start_time
  scsi: bfa: replace bfa_get_log_time() with ktime_get_real_seconds()
  scsi: bfa: try to sanitize vendor netlink events
  scsi: bfa: use 64-bit times in bfa_aen_entry_s ABI

 drivers/scsi/bfa/bfa_cs.h       |  6 +++---
 drivers/scsi/bfa/bfa_defs_svc.h |  3 ++-
 drivers/scsi/bfa/bfa_fcpim.c    |  3 ++-
 drivers/scsi/bfa/bfa_fcpim.h    |  4 ++--
 drivers/scsi/bfa/bfa_ioc.c      |  8 ++++---
 drivers/scsi/bfa/bfa_port.c     | 15 +++----------
 drivers/scsi/bfa/bfa_port.h     |  2 +-
 drivers/scsi/bfa/bfa_svc.c      | 47 ++++++++++++-----------------------------
 drivers/scsi/bfa/bfa_svc.h      |  2 +-
 drivers/scsi/bfa/bfad_bsg.c     |  4 +---
 drivers/scsi/bfa/bfad_im.h      | 32 +++++++++++++++++++---------
 11 files changed, 56 insertions(+), 70 deletions(-)

-- 
2.9.0

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/7] scsi: bfa: do_gettimeofday removal
@ 2017-11-10 15:37 ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: hare, Arnd Bergmann, linux-scsi, y2038, linux-kernel, jthumshirn, hch

The bfa driver is one of the main users of do_gettimeofday(), a function
that I'm trying to remove as part of the y2038 cleanup.

The timestamps are all uses in slightly different ways, so this has turned
into a rather longish series for doing something that should be simple.

The last patch in the series ("scsi: bfa: use 64-bit times in
bfa_aen_entry_s ABI") is one that needs to be reviewed very carefully,
and it can be skipped if the maintainers prefer to leave the 32-bit ABI
unchanged, the rest are hopefully fairly straightforward.

      Arnd

Arnd Bergmann (7):
  scsi: bfa: use ktime_get_real_ts64 for firmware timestamp
  scsi: bfa: use proper time accessor for stats_reset_time
  scsi: bfa: improve bfa_ioc_send_enable/disable data
  scsi: bfa: document overflow of io_profile_start_time
  scsi: bfa: replace bfa_get_log_time() with ktime_get_real_seconds()
  scsi: bfa: try to sanitize vendor netlink events
  scsi: bfa: use 64-bit times in bfa_aen_entry_s ABI

 drivers/scsi/bfa/bfa_cs.h       |  6 +++---
 drivers/scsi/bfa/bfa_defs_svc.h |  3 ++-
 drivers/scsi/bfa/bfa_fcpim.c    |  3 ++-
 drivers/scsi/bfa/bfa_fcpim.h    |  4 ++--
 drivers/scsi/bfa/bfa_ioc.c      |  8 ++++---
 drivers/scsi/bfa/bfa_port.c     | 15 +++----------
 drivers/scsi/bfa/bfa_port.h     |  2 +-
 drivers/scsi/bfa/bfa_svc.c      | 47 ++++++++++++-----------------------------
 drivers/scsi/bfa/bfa_svc.h      |  2 +-
 drivers/scsi/bfa/bfad_bsg.c     |  4 +---
 drivers/scsi/bfa/bfad_im.h      | 32 +++++++++++++++++++---------
 11 files changed, 56 insertions(+), 70 deletions(-)

-- 
2.9.0

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] scsi: bfa: use ktime_get_real_ts64 for firmware timestamp
  2017-11-10 15:37 ` Arnd Bergmann
@ 2017-11-10 15:37   ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

BFA_TRC_TS() calculates a 32-bit microsecond timestamp using the
deprecated do_gettimeofday() function. This overflows roughly every
71 minutes, so it's obviously not used as an absolute time stamp,
but it seems wrong to use a time base for it that will jump
during settimeofday() calls, leap seconds, or the y2038 overflow.

This converts it to ktime_get_ts64(), which has none of those
problems but is not synchronized to wall-clock time.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_cs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_cs.h b/drivers/scsi/bfa/bfa_cs.h
index df6760ca0911..9685efc59b16 100644
--- a/drivers/scsi/bfa/bfa_cs.h
+++ b/drivers/scsi/bfa/bfa_cs.h
@@ -35,10 +35,10 @@
 
 #define BFA_TRC_TS(_trcm)                               \
 	({                                              \
-		struct timeval tv;                      \
+		struct timespec64 ts;                   \
 							\
-		do_gettimeofday(&tv);                   \
-		(tv.tv_sec*1000000+tv.tv_usec);         \
+		ktime_get_ts64(&ts);                    \
+		(ts.tv_sec*1000000+ts.tv_nsec / 1000);  \
 	})
 
 #ifndef BFA_TRC_TS
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/7] scsi: bfa: use ktime_get_real_ts64 for firmware timestamp
@ 2017-11-10 15:37   ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: hare, Arnd Bergmann, linux-scsi, y2038, linux-kernel, jthumshirn, hch

BFA_TRC_TS() calculates a 32-bit microsecond timestamp using the
deprecated do_gettimeofday() function. This overflows roughly every
71 minutes, so it's obviously not used as an absolute time stamp,
but it seems wrong to use a time base for it that will jump
during settimeofday() calls, leap seconds, or the y2038 overflow.

This converts it to ktime_get_ts64(), which has none of those
problems but is not synchronized to wall-clock time.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_cs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_cs.h b/drivers/scsi/bfa/bfa_cs.h
index df6760ca0911..9685efc59b16 100644
--- a/drivers/scsi/bfa/bfa_cs.h
+++ b/drivers/scsi/bfa/bfa_cs.h
@@ -35,10 +35,10 @@
 
 #define BFA_TRC_TS(_trcm)                               \
 	({                                              \
-		struct timeval tv;                      \
+		struct timespec64 ts;                   \
 							\
-		do_gettimeofday(&tv);                   \
-		(tv.tv_sec*1000000+tv.tv_usec);         \
+		ktime_get_ts64(&ts);                    \
+		(ts.tv_sec*1000000+ts.tv_nsec / 1000);  \
 	})
 
 #ifndef BFA_TRC_TS
-- 
2.9.0

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] scsi: bfa: use proper time accessor for stats_reset_time
  2017-11-10 15:37 ` Arnd Bergmann
  (?)
  (?)
@ 2017-11-10 15:37 ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

We use the deprecated do_gettimeofday() function to read the
current time when resetting the statistics in both bfa_port
and bfa_svc. This works fine because overflow is handled
correctly, but we want to get rid of do_gettimeofday() and
using a non-monotonic time suffers from concurrent
settimeofday calls and other problems.

This uses the ktime_get_seconds() function instead, which
does what we need here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_port.c | 15 +++------------
 drivers/scsi/bfa/bfa_port.h |  2 +-
 drivers/scsi/bfa/bfa_svc.c  | 15 ++++-----------
 drivers/scsi/bfa/bfa_svc.h  |  2 +-
 4 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_port.c b/drivers/scsi/bfa/bfa_port.c
index da1721e0d167..079bc77f4102 100644
--- a/drivers/scsi/bfa/bfa_port.c
+++ b/drivers/scsi/bfa/bfa_port.c
@@ -96,14 +96,11 @@ bfa_port_get_stats_isr(struct bfa_port_s *port, bfa_status_t status)
 	port->stats_busy = BFA_FALSE;
 
 	if (status == BFA_STATUS_OK) {
-		struct timeval tv;
-
 		memcpy(port->stats, port->stats_dma.kva,
 		       sizeof(union bfa_port_stats_u));
 		bfa_port_stats_swap(port, port->stats);
 
-		do_gettimeofday(&tv);
-		port->stats->fc.secs_reset = tv.tv_sec - port->stats_reset_time;
+		port->stats->fc.secs_reset = ktime_get_seconds() - port->stats_reset_time;
 	}
 
 	if (port->stats_cbfn) {
@@ -124,16 +121,13 @@ bfa_port_get_stats_isr(struct bfa_port_s *port, bfa_status_t status)
 static void
 bfa_port_clear_stats_isr(struct bfa_port_s *port, bfa_status_t status)
 {
-	struct timeval tv;
-
 	port->stats_status = status;
 	port->stats_busy   = BFA_FALSE;
 
 	/*
 	* re-initialize time stamp for stats reset
 	*/
-	do_gettimeofday(&tv);
-	port->stats_reset_time = tv.tv_sec;
+	port->stats_reset_time = ktime_get_seconds();
 
 	if (port->stats_cbfn) {
 		port->stats_cbfn(port->stats_cbarg, status);
@@ -471,8 +465,6 @@ void
 bfa_port_attach(struct bfa_port_s *port, struct bfa_ioc_s *ioc,
 		 void *dev, struct bfa_trc_mod_s *trcmod)
 {
-	struct timeval tv;
-
 	WARN_ON(!port);
 
 	port->dev    = dev;
@@ -494,8 +486,7 @@ bfa_port_attach(struct bfa_port_s *port, struct bfa_ioc_s *ioc,
 	/*
 	 * initialize time stamp for stats reset
 	 */
-	do_gettimeofday(&tv);
-	port->stats_reset_time = tv.tv_sec;
+	port->stats_reset_time = ktime_get_seconds();
 
 	bfa_trc(port, 0);
 }
diff --git a/drivers/scsi/bfa/bfa_port.h b/drivers/scsi/bfa/bfa_port.h
index 26dc1bf14c85..0c3b200243ca 100644
--- a/drivers/scsi/bfa/bfa_port.h
+++ b/drivers/scsi/bfa/bfa_port.h
@@ -36,7 +36,7 @@ struct bfa_port_s {
 	bfa_port_stats_cbfn_t		stats_cbfn;
 	void				*stats_cbarg;
 	bfa_status_t			stats_status;
-	u32			stats_reset_time;
+	time64_t			stats_reset_time;
 	union bfa_port_stats_u		*stats;
 	struct bfa_dma_s		stats_dma;
 	bfa_boolean_t			endis_pending;
diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index e640223bab3c..dd7d1e6bc2d8 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -3047,7 +3047,6 @@ bfa_fcport_attach(struct bfa_s *bfa, void *bfad, struct bfa_iocfc_cfg_s *cfg,
 	struct bfa_fcport_s *fcport = BFA_FCPORT_MOD(bfa);
 	struct bfa_port_cfg_s *port_cfg = &fcport->cfg;
 	struct bfa_fcport_ln_s *ln = &fcport->ln;
-	struct timeval tv;
 
 	fcport->bfa = bfa;
 	ln->fcport = fcport;
@@ -3060,8 +3059,7 @@ bfa_fcport_attach(struct bfa_s *bfa, void *bfad, struct bfa_iocfc_cfg_s *cfg,
 	/*
 	 * initialize time stamp for stats reset
 	 */
-	do_gettimeofday(&tv);
-	fcport->stats_reset_time = tv.tv_sec;
+	fcport->stats_reset_time = ktime_get_seconds();
 	fcport->stats_dma_ready = BFA_FALSE;
 
 	/*
@@ -3295,9 +3293,7 @@ __bfa_cb_fcport_stats_get(void *cbarg, bfa_boolean_t complete)
 	union bfa_fcport_stats_u *ret;
 
 	if (complete) {
-		struct timeval tv;
-		if (fcport->stats_status == BFA_STATUS_OK)
-			do_gettimeofday(&tv);
+		time64_t time = ktime_get_seconds();
 
 		list_for_each_safe(qe, qen, &fcport->stats_pending_q) {
 			bfa_q_deq(&fcport->stats_pending_q, &qe);
@@ -3312,7 +3308,7 @@ __bfa_cb_fcport_stats_get(void *cbarg, bfa_boolean_t complete)
 					bfa_fcport_fcoe_stats_swap(&ret->fcoe,
 							&fcport->stats->fcoe);
 					ret->fcoe.secs_reset =
-					tv.tv_sec - fcport->stats_reset_time;
+						time - fcport->stats_reset_time;
 				}
 			}
 			bfa_cb_queue_status(fcport->bfa, &cb->hcb_qe,
@@ -3373,13 +3369,10 @@ __bfa_cb_fcport_stats_clr(void *cbarg, bfa_boolean_t complete)
 	struct list_head *qe, *qen;
 
 	if (complete) {
-		struct timeval tv;
-
 		/*
 		 * re-initialize time stamp for stats reset
 		 */
-		do_gettimeofday(&tv);
-		fcport->stats_reset_time = tv.tv_sec;
+		fcport->stats_reset_time = ktime_get_seconds();
 		list_for_each_safe(qe, qen, &fcport->statsclr_pending_q) {
 			bfa_q_deq(&fcport->statsclr_pending_q, &qe);
 			cb = (struct bfa_cb_pending_q_s *)qe;
diff --git a/drivers/scsi/bfa/bfa_svc.h b/drivers/scsi/bfa/bfa_svc.h
index ea2278bc78a8..7e8fb6231d49 100644
--- a/drivers/scsi/bfa/bfa_svc.h
+++ b/drivers/scsi/bfa/bfa_svc.h
@@ -505,7 +505,7 @@ struct bfa_fcport_s {
 	struct list_head	stats_pending_q;
 	struct list_head	statsclr_pending_q;
 	bfa_boolean_t		stats_qfull;
-	u32		stats_reset_time; /*  stats reset time stamp */
+	time64_t		stats_reset_time; /*  stats reset time stamp */
 	bfa_boolean_t		diag_busy; /*  diag busy status */
 	bfa_boolean_t		beacon; /*  port beacon status */
 	bfa_boolean_t		link_e2e_beacon; /*  link beacon status */
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] scsi: bfa: improve bfa_ioc_send_enable/disable data
  2017-11-10 15:37 ` Arnd Bergmann
                   ` (2 preceding siblings ...)
  (?)
@ 2017-11-10 15:37 ` Arnd Bergmann
  2017-11-13 14:08   ` [Y2038] " Ben Hutchings
  -1 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

In bfa_ioc_send_enable, we use the deprecated do_gettimeofday() function
to read the current time. This is not a problem, since the firmware
interface is already limited to 32-bit timestamps, but it's better
to use ktime_get_seconds() and document what the limitation is.

I noticed that I did the same change in commit a5af83925363 ("bna:
avoid writing uninitialized data into hw registers") for the ethernet
driver. That commit also changed the "disable" funtion to initialize
the data we pass to the firmware properly, so I'm doing the same
thing here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_ioc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 256f4afaccf9..117332537763 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -1809,13 +1809,12 @@ static void
 bfa_ioc_send_enable(struct bfa_ioc_s *ioc)
 {
 	struct bfi_ioc_ctrl_req_s enable_req;
-	struct timeval tv;
 
 	bfi_h2i_set(enable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_ENABLE_REQ,
 		    bfa_ioc_portid(ioc));
 	enable_req.clscode = cpu_to_be16(ioc->clscode);
-	do_gettimeofday(&tv);
-	enable_req.tv_sec = be32_to_cpu(tv.tv_sec);
+	/* unsigned 32-bit time_t overflow in y2106 */
+	enable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());
 	bfa_ioc_mbox_send(ioc, &enable_req, sizeof(struct bfi_ioc_ctrl_req_s));
 }
 
@@ -1826,6 +1825,9 @@ bfa_ioc_send_disable(struct bfa_ioc_s *ioc)
 
 	bfi_h2i_set(disable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_DISABLE_REQ,
 		    bfa_ioc_portid(ioc));
+	disable_req.clscode = cpu_to_be16(ioc->clscode);
+	/* unsigned 32-bit time_t overflow in y2106 */
+	disable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());
 	bfa_ioc_mbox_send(ioc, &disable_req, sizeof(struct bfi_ioc_ctrl_req_s));
 }
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] scsi: bfa: document overflow of io_profile_start_time
  2017-11-10 15:37 ` Arnd Bergmann
                   ` (3 preceding siblings ...)
  (?)
@ 2017-11-10 15:37 ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

io_profile_start_time() gets read using do_gettimeofday() and passed
down as a 32-bit value through multiple functions. This will overflow
in y2038 or y2106, depending on whether it gets interpreted as
unsigned in the end.

This changes do_gettimeofday() to ktime_get_real_seconds() and pushes
the point at which it overflows to where we actually assign it to
the bfa_fcpim_del_itn_stats_s structure, with an appropriate
comment.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_fcpim.c | 3 ++-
 drivers/scsi/bfa/bfa_fcpim.h | 4 ++--
 drivers/scsi/bfa/bfad_bsg.c  | 4 +---
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_fcpim.c b/drivers/scsi/bfa/bfa_fcpim.c
index 5f53b3276234..2c85f5b1f9c1 100644
--- a/drivers/scsi/bfa/bfa_fcpim.c
+++ b/drivers/scsi/bfa/bfa_fcpim.c
@@ -468,7 +468,7 @@ bfa_ioim_profile_start(struct bfa_ioim_s *ioim)
 }
 
 bfa_status_t
-bfa_fcpim_profile_on(struct bfa_s *bfa, u32 time)
+bfa_fcpim_profile_on(struct bfa_s *bfa, time64_t time)
 {
 	struct bfa_itnim_s *itnim;
 	struct bfa_fcpim_s *fcpim = BFA_FCPIM(bfa);
@@ -1478,6 +1478,7 @@ bfa_itnim_get_ioprofile(struct bfa_itnim_s *itnim,
 		return BFA_STATUS_IOPROFILE_OFF;
 
 	itnim->ioprofile.index = BFA_IOBUCKET_MAX;
+	/* unsigned 32-bit time_t overflow here in y2106 */
 	itnim->ioprofile.io_profile_start_time =
 				bfa_io_profile_start_time(itnim->bfa);
 	itnim->ioprofile.clock_res_mul = bfa_io_lat_clock_res_mul;
diff --git a/drivers/scsi/bfa/bfa_fcpim.h b/drivers/scsi/bfa/bfa_fcpim.h
index e93921dec347..ec8f863540ae 100644
--- a/drivers/scsi/bfa/bfa_fcpim.h
+++ b/drivers/scsi/bfa/bfa_fcpim.h
@@ -136,7 +136,7 @@ struct bfa_fcpim_s {
 	struct bfa_fcpim_del_itn_stats_s del_itn_stats;
 	bfa_boolean_t		ioredirect;
 	bfa_boolean_t		io_profile;
-	u32			io_profile_start_time;
+	time64_t		io_profile_start_time;
 	bfa_fcpim_profile_t     profile_comp;
 	bfa_fcpim_profile_t     profile_start;
 };
@@ -310,7 +310,7 @@ bfa_status_t bfa_fcpim_port_iostats(struct bfa_s *bfa,
 			struct bfa_itnim_iostats_s *stats, u8 lp_tag);
 void bfa_fcpim_add_stats(struct bfa_itnim_iostats_s *fcpim_stats,
 			struct bfa_itnim_iostats_s *itnim_stats);
-bfa_status_t bfa_fcpim_profile_on(struct bfa_s *bfa, u32 time);
+bfa_status_t bfa_fcpim_profile_on(struct bfa_s *bfa, time64_t time);
 bfa_status_t bfa_fcpim_profile_off(struct bfa_s *bfa);
 
 #define bfa_fcpim_ioredirect_enabled(__bfa)				\
diff --git a/drivers/scsi/bfa/bfad_bsg.c b/drivers/scsi/bfa/bfad_bsg.c
index 72ca2a2e08e2..01fc51a70356 100644
--- a/drivers/scsi/bfa/bfad_bsg.c
+++ b/drivers/scsi/bfa/bfad_bsg.c
@@ -2094,13 +2094,11 @@ bfad_iocmd_fcpim_cfg_profile(struct bfad_s *bfad, void *cmd, unsigned int v_cmd)
 {
 	struct bfa_bsg_fcpim_profile_s *iocmd =
 				(struct bfa_bsg_fcpim_profile_s *)cmd;
-	struct timeval  tv;
 	unsigned long	flags;
 
-	do_gettimeofday(&tv);
 	spin_lock_irqsave(&bfad->bfad_lock, flags);
 	if (v_cmd == IOCMD_FCPIM_PROFILE_ON)
-		iocmd->status = bfa_fcpim_profile_on(&bfad->bfa, tv.tv_sec);
+		iocmd->status = bfa_fcpim_profile_on(&bfad->bfa, ktime_get_real_seconds());
 	else if (v_cmd == IOCMD_FCPIM_PROFILE_OFF)
 		iocmd->status = bfa_fcpim_profile_off(&bfad->bfa);
 	spin_unlock_irqrestore(&bfad->bfad_lock, flags);
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] scsi: bfa: replace bfa_get_log_time() with ktime_get_real_seconds()
  2017-11-10 15:37 ` Arnd Bergmann
@ 2017-11-10 15:37   ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

The bfa_get_log_time() returns a 64-bit timestamp that does not
suffer from the y2038 overflow on 64-bit systems. However, on 32-bit
architectures the timestamp will jump from 0x000000007fffffff
to 0xffffffff80000000 in y2038 and produce wrong results.

The ktime_get_real_seconds() function does the same thing as
bfa_get_log_time() without that problem, so we can simply remove
the former use ktime_get_real_seconds() instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_svc.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index dd7d1e6bc2d8..9d20d2c92e8c 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -288,18 +288,6 @@ plkd_validate_logrec(struct bfa_plog_rec_s *pl_rec)
 	return 0;
 }
 
-static u64
-bfa_get_log_time(void)
-{
-	u64 system_time = 0;
-	struct timeval tv;
-	do_gettimeofday(&tv);
-
-	/* We are interested in seconds only. */
-	system_time = tv.tv_sec;
-	return system_time;
-}
-
 static void
 bfa_plog_add(struct bfa_plog_s *plog, struct bfa_plog_rec_s *pl_rec)
 {
@@ -320,7 +308,7 @@ bfa_plog_add(struct bfa_plog_s *plog, struct bfa_plog_rec_s *pl_rec)
 
 	memcpy(pl_recp, pl_rec, sizeof(struct bfa_plog_rec_s));
 
-	pl_recp->tv = bfa_get_log_time();
+	pl_recp->tv = ktime_get_real_seconds();
 	BFA_PL_LOG_REC_INCR(plog->tail);
 
 	if (plog->head == plog->tail)
@@ -6141,13 +6129,13 @@ bfa_fcdiag_lb_is_running(struct bfa_s *bfa)
 /*
  *	D-port
  */
-#define bfa_dport_result_start(__dport, __mode) do {			\
-		(__dport)->result.start_time = bfa_get_log_time();	\
-		(__dport)->result.status = DPORT_TEST_ST_INPRG;		\
-		(__dport)->result.mode = (__mode);			\
-		(__dport)->result.rp_pwwn = (__dport)->rp_pwwn;		\
-		(__dport)->result.rp_nwwn = (__dport)->rp_nwwn;		\
-		(__dport)->result.lpcnt = (__dport)->lpcnt;		\
+#define bfa_dport_result_start(__dport, __mode) do {				\
+		(__dport)->result.start_time = ktime_get_real_seconds();	\
+		(__dport)->result.status = DPORT_TEST_ST_INPRG;			\
+		(__dport)->result.mode = (__mode);				\
+		(__dport)->result.rp_pwwn = (__dport)->rp_pwwn;			\
+		(__dport)->result.rp_nwwn = (__dport)->rp_nwwn;			\
+		(__dport)->result.lpcnt = (__dport)->lpcnt;			\
 } while (0)
 
 static bfa_boolean_t bfa_dport_send_req(struct bfa_dport_s *dport,
@@ -6581,7 +6569,7 @@ bfa_dport_scn(struct bfa_dport_s *dport, struct bfi_diag_dport_scn_s *msg)
 
 	switch (dport->i2hmsg.scn.state) {
 	case BFI_DPORT_SCN_TESTCOMP:
-		dport->result.end_time = bfa_get_log_time();
+		dport->result.end_time = ktime_get_real_seconds();
 		bfa_trc(dport->bfa, dport->result.end_time);
 
 		dport->result.status = msg->info.testcomp.status;
@@ -6628,7 +6616,7 @@ bfa_dport_scn(struct bfa_dport_s *dport, struct bfi_diag_dport_scn_s *msg)
 	case BFI_DPORT_SCN_SUBTESTSTART:
 		subtesttype = msg->info.teststart.type;
 		dport->result.subtest[subtesttype].start_time =
-			bfa_get_log_time();
+			ktime_get_real_seconds();
 		dport->result.subtest[subtesttype].status =
 			DPORT_TEST_ST_INPRG;
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] scsi: bfa: replace bfa_get_log_time() with ktime_get_real_seconds()
@ 2017-11-10 15:37   ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: hare, Arnd Bergmann, linux-scsi, y2038, linux-kernel, jthumshirn, hch

The bfa_get_log_time() returns a 64-bit timestamp that does not
suffer from the y2038 overflow on 64-bit systems. However, on 32-bit
architectures the timestamp will jump from 0x000000007fffffff
to 0xffffffff80000000 in y2038 and produce wrong results.

The ktime_get_real_seconds() function does the same thing as
bfa_get_log_time() without that problem, so we can simply remove
the former use ktime_get_real_seconds() instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_svc.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c
index dd7d1e6bc2d8..9d20d2c92e8c 100644
--- a/drivers/scsi/bfa/bfa_svc.c
+++ b/drivers/scsi/bfa/bfa_svc.c
@@ -288,18 +288,6 @@ plkd_validate_logrec(struct bfa_plog_rec_s *pl_rec)
 	return 0;
 }
 
-static u64
-bfa_get_log_time(void)
-{
-	u64 system_time = 0;
-	struct timeval tv;
-	do_gettimeofday(&tv);
-
-	/* We are interested in seconds only. */
-	system_time = tv.tv_sec;
-	return system_time;
-}
-
 static void
 bfa_plog_add(struct bfa_plog_s *plog, struct bfa_plog_rec_s *pl_rec)
 {
@@ -320,7 +308,7 @@ bfa_plog_add(struct bfa_plog_s *plog, struct bfa_plog_rec_s *pl_rec)
 
 	memcpy(pl_recp, pl_rec, sizeof(struct bfa_plog_rec_s));
 
-	pl_recp->tv = bfa_get_log_time();
+	pl_recp->tv = ktime_get_real_seconds();
 	BFA_PL_LOG_REC_INCR(plog->tail);
 
 	if (plog->head == plog->tail)
@@ -6141,13 +6129,13 @@ bfa_fcdiag_lb_is_running(struct bfa_s *bfa)
 /*
  *	D-port
  */
-#define bfa_dport_result_start(__dport, __mode) do {			\
-		(__dport)->result.start_time = bfa_get_log_time();	\
-		(__dport)->result.status = DPORT_TEST_ST_INPRG;		\
-		(__dport)->result.mode = (__mode);			\
-		(__dport)->result.rp_pwwn = (__dport)->rp_pwwn;		\
-		(__dport)->result.rp_nwwn = (__dport)->rp_nwwn;		\
-		(__dport)->result.lpcnt = (__dport)->lpcnt;		\
+#define bfa_dport_result_start(__dport, __mode) do {				\
+		(__dport)->result.start_time = ktime_get_real_seconds();	\
+		(__dport)->result.status = DPORT_TEST_ST_INPRG;			\
+		(__dport)->result.mode = (__mode);				\
+		(__dport)->result.rp_pwwn = (__dport)->rp_pwwn;			\
+		(__dport)->result.rp_nwwn = (__dport)->rp_nwwn;			\
+		(__dport)->result.lpcnt = (__dport)->lpcnt;			\
 } while (0)
 
 static bfa_boolean_t bfa_dport_send_req(struct bfa_dport_s *dport,
@@ -6581,7 +6569,7 @@ bfa_dport_scn(struct bfa_dport_s *dport, struct bfi_diag_dport_scn_s *msg)
 
 	switch (dport->i2hmsg.scn.state) {
 	case BFI_DPORT_SCN_TESTCOMP:
-		dport->result.end_time = bfa_get_log_time();
+		dport->result.end_time = ktime_get_real_seconds();
 		bfa_trc(dport->bfa, dport->result.end_time);
 
 		dport->result.status = msg->info.testcomp.status;
@@ -6628,7 +6616,7 @@ bfa_dport_scn(struct bfa_dport_s *dport, struct bfi_diag_dport_scn_s *msg)
 	case BFI_DPORT_SCN_SUBTESTSTART:
 		subtesttype = msg->info.teststart.type;
 		dport->result.subtest[subtesttype].start_time =
-			bfa_get_log_time();
+			ktime_get_real_seconds();
 		dport->result.subtest[subtesttype].status =
 			DPORT_TEST_ST_INPRG;
 
-- 
2.9.0

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] scsi: bfa: try to sanitize vendor netlink events
  2017-11-10 15:37 ` Arnd Bergmann
@ 2017-11-10 15:37   ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

bfa_aen_entry_s is passed to user space in a netlink message, but
is defined using a 'struct timeval' and an 'enum' that are not only
different between architectures, but also between 32-bit user space and
64-bit kernels they may run on, as well as depending on the particular
C library that defines timeval.

This changes the in-kernel definition to no longer use the timeval
type directly but instead use two open-coded 'unsigned long' members.
This keeps the existing ABI, but making the variable unsigned also
helps make it work after y2038, until it overflows in 2106.

Since the macro becomes overly complex at this point, I'm changing
it to an inline function for readability.

I'm not changing the 32-bit user-space ABI at this point, to keep the
changes separate, I deally this would be defined using the same
binary layout for all architectures.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_defs_svc.h |  3 ++-
 drivers/scsi/bfa/bfad_im.h      | 32 ++++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_defs_svc.h b/drivers/scsi/bfa/bfa_defs_svc.h
index e81707f938cb..df1e874015c4 100644
--- a/drivers/scsi/bfa/bfa_defs_svc.h
+++ b/drivers/scsi/bfa/bfa_defs_svc.h
@@ -1455,7 +1455,8 @@ struct bfa_aen_entry_s {
 	enum bfa_aen_category   aen_category;
 	u32                     aen_type;
 	union bfa_aen_data_u    aen_data;
-	struct timeval          aen_tv;
+	unsigned long		aen_tv_sec;
+	unsigned long		aen_tv_usec;
 	u32                     seq_num;
 	u32                     bfad_num;
 };
diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
index c81ec2a77ef5..7f7616c52814 100644
--- a/drivers/scsi/bfa/bfad_im.h
+++ b/drivers/scsi/bfa/bfad_im.h
@@ -131,16 +131,28 @@ struct bfad_im_s {
 } while (0)
 
 /* post fc_host vendor event */
-#define bfad_im_post_vendor_event(_entry, _drv, _cnt, _cat, _evt) do {	      \
-	do_gettimeofday(&(_entry)->aen_tv);				      \
-	(_entry)->bfad_num = (_drv)->inst_no;				      \
-	(_entry)->seq_num = (_cnt);					      \
-	(_entry)->aen_category = (_cat);				      \
-	(_entry)->aen_type = (_evt);					      \
-	if ((_drv)->bfad_flags & BFAD_FC4_PROBE_DONE)			      \
-		queue_work((_drv)->im->drv_workq,			      \
-			   &(_drv)->im->aen_im_notify_work);		      \
-} while (0)
+static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
+					     struct bfad_s *drv, int cnt,
+					     enum bfa_aen_category cat,
+					     enum bfa_ioc_aen_event evt)
+{
+	struct timespec64 ts;
+
+	ktime_get_real_ts64(&ts);
+	/*
+	 * 'unsigned long aen_tv_sec' overflows in y2106 on 32-bit
+	 * architectures, or in 2038 if user space interprets it
+	 * as 'signed'.
+	 */
+	entry->aen_tv_sec = ts.tv_sec;
+	entry->aen_tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+	entry->bfad_num = drv->inst_no;
+	entry->seq_num = cnt;
+	entry->aen_category = cat;
+	entry->aen_type = evt;
+	if (drv->bfad_flags & BFAD_FC4_PROBE_DONE)
+		queue_work(drv->im->drv_workq, &drv->im->aen_im_notify_work);
+}
 
 struct Scsi_Host *bfad_scsi_host_alloc(struct bfad_im_port_s *im_port,
 				struct bfad_s *);
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] scsi: bfa: try to sanitize vendor netlink events
@ 2017-11-10 15:37   ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: hare, Arnd Bergmann, linux-scsi, y2038, linux-kernel, jthumshirn, hch

bfa_aen_entry_s is passed to user space in a netlink message, but
is defined using a 'struct timeval' and an 'enum' that are not only
different between architectures, but also between 32-bit user space and
64-bit kernels they may run on, as well as depending on the particular
C library that defines timeval.

This changes the in-kernel definition to no longer use the timeval
type directly but instead use two open-coded 'unsigned long' members.
This keeps the existing ABI, but making the variable unsigned also
helps make it work after y2038, until it overflows in 2106.

Since the macro becomes overly complex at this point, I'm changing
it to an inline function for readability.

I'm not changing the 32-bit user-space ABI at this point, to keep the
changes separate, I deally this would be defined using the same
binary layout for all architectures.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/bfa/bfa_defs_svc.h |  3 ++-
 drivers/scsi/bfa/bfad_im.h      | 32 ++++++++++++++++++++++----------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_defs_svc.h b/drivers/scsi/bfa/bfa_defs_svc.h
index e81707f938cb..df1e874015c4 100644
--- a/drivers/scsi/bfa/bfa_defs_svc.h
+++ b/drivers/scsi/bfa/bfa_defs_svc.h
@@ -1455,7 +1455,8 @@ struct bfa_aen_entry_s {
 	enum bfa_aen_category   aen_category;
 	u32                     aen_type;
 	union bfa_aen_data_u    aen_data;
-	struct timeval          aen_tv;
+	unsigned long		aen_tv_sec;
+	unsigned long		aen_tv_usec;
 	u32                     seq_num;
 	u32                     bfad_num;
 };
diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
index c81ec2a77ef5..7f7616c52814 100644
--- a/drivers/scsi/bfa/bfad_im.h
+++ b/drivers/scsi/bfa/bfad_im.h
@@ -131,16 +131,28 @@ struct bfad_im_s {
 } while (0)
 
 /* post fc_host vendor event */
-#define bfad_im_post_vendor_event(_entry, _drv, _cnt, _cat, _evt) do {	      \
-	do_gettimeofday(&(_entry)->aen_tv);				      \
-	(_entry)->bfad_num = (_drv)->inst_no;				      \
-	(_entry)->seq_num = (_cnt);					      \
-	(_entry)->aen_category = (_cat);				      \
-	(_entry)->aen_type = (_evt);					      \
-	if ((_drv)->bfad_flags & BFAD_FC4_PROBE_DONE)			      \
-		queue_work((_drv)->im->drv_workq,			      \
-			   &(_drv)->im->aen_im_notify_work);		      \
-} while (0)
+static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
+					     struct bfad_s *drv, int cnt,
+					     enum bfa_aen_category cat,
+					     enum bfa_ioc_aen_event evt)
+{
+	struct timespec64 ts;
+
+	ktime_get_real_ts64(&ts);
+	/*
+	 * 'unsigned long aen_tv_sec' overflows in y2106 on 32-bit
+	 * architectures, or in 2038 if user space interprets it
+	 * as 'signed'.
+	 */
+	entry->aen_tv_sec = ts.tv_sec;
+	entry->aen_tv_usec = ts.tv_nsec / NSEC_PER_USEC;
+	entry->bfad_num = drv->inst_no;
+	entry->seq_num = cnt;
+	entry->aen_category = cat;
+	entry->aen_type = evt;
+	if (drv->bfad_flags & BFAD_FC4_PROBE_DONE)
+		queue_work(drv->im->drv_workq, &drv->im->aen_im_notify_work);
+}
 
 struct Scsi_Host *bfad_scsi_host_alloc(struct bfad_im_port_s *im_port,
 				struct bfad_s *);
-- 
2.9.0

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] scsi: bfa: use 64-bit times in bfa_aen_entry_s ABI
  2017-11-10 15:37 ` Arnd Bergmann
                   ` (6 preceding siblings ...)
  (?)
@ 2017-11-10 15:37 ` Arnd Bergmann
  -1 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-10 15:37 UTC (permalink / raw)
  To: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen
  Cc: linux-scsi, linux-kernel, y2038, hch, hare, jthumshirn, Arnd Bergmann

bfa_aen_entry_s is passed through a netlink socket that can be read
by either 32-bit or 64-bit processes, but the data format is different
between the two on current implementations.

Originally, this was using a 'struct timeval', which also suffers from
getting redefined with a new libc implementation.

With this patch, the layout gets fixed to having two 64-bit members
for the time, making it the same on 32-bit kernels and 64-bit kernels
running either compat or native user space including x32.

Provided that the new header file gets used to recompile any 32-bit
application binaries, this will fix running those on a 64-bit kernel
(with or without this patch) e.g. in a container environment,
and it will make binaries work that will be built against a future
32-bit glibc that uses a 64-bit time_t, and avoid the y2038
overflow there.

However, this also breaks compatibility with any existing 32-bit
binary running on a native 32-bit kernel, those must be recompiled
against the new header, which in turn makes them incompatible with
older kernels unless the same change gets applied there.

Obviously this patch should only be applied when the benefits
outweigh the possible breakage. I'm posting it under the assumption
that there are no open-source tools using the netlink interface,
and that users of the binaries provided by qlogic for SLES10/11
and RHEL5/6 are not actually being used on new future systems
with 32-bit x86 kernels.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
---
 drivers/scsi/bfa/bfa_defs_svc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_defs_svc.h b/drivers/scsi/bfa/bfa_defs_svc.h
index df1e874015c4..3d0c96a5c873 100644
--- a/drivers/scsi/bfa/bfa_defs_svc.h
+++ b/drivers/scsi/bfa/bfa_defs_svc.h
@@ -1455,8 +1455,8 @@ struct bfa_aen_entry_s {
 	enum bfa_aen_category   aen_category;
 	u32                     aen_type;
 	union bfa_aen_data_u    aen_data;
-	unsigned long		aen_tv_sec;
-	unsigned long		aen_tv_usec;
+	u64			aen_tv_sec;
+	u64			aen_tv_usec;
 	u32                     seq_num;
 	u32                     bfad_num;
 };
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] scsi: bfa: do_gettimeofday removal
  2017-11-10 15:37 ` Arnd Bergmann
                   ` (7 preceding siblings ...)
  (?)
@ 2017-11-10 17:24 ` Christoph Hellwig
  2017-11-10 17:33   ` Gurumurthy, Anil
  -1 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2017-11-10 17:24 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, linux-kernel, y2038, hch, hare,
	jthumshirn

This stuff look ok - but I have a bigger question and that is if
bfa is still alive at all.  Everytime I look at it I have doubts if
it works at all, so I wonder if we need to keep it on life support.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* RE: [PATCH 0/7] scsi: bfa: do_gettimeofday removal
  2017-11-10 17:24 ` [PATCH 0/7] scsi: bfa: do_gettimeofday removal Christoph Hellwig
@ 2017-11-10 17:33   ` Gurumurthy, Anil
  0 siblings, 0 replies; 18+ messages in thread
From: Gurumurthy, Anil @ 2017-11-10 17:33 UTC (permalink / raw)
  To: Christoph Hellwig, Arnd Bergmann
  Cc: Kalluru, Sudarsana, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, linux-kernel, y2038, hare, jthumshirn

The bfa drivers are in deep maintenance mode for some time now. We have not had issues reported on them for a while, but it would be good to continue to have them.

The series looks ok to me too.

Thanks,
Anil

-----Original Message-----
From: Christoph Hellwig [mailto:hch@lst.de] 
Sent: 10 November 2017 22:55
To: Arnd Bergmann <arnd@arndb.de>
Cc: Gurumurthy, Anil <Anil.Gurumurthy@cavium.com>; Kalluru, Sudarsana <Sudarsana.Kalluru@cavium.com>; James E . J . Bottomley <jejb@linux.vnet.ibm.com>; Martin K . Petersen <martin.petersen@oracle.com>; linux-scsi@vger.kernel.org; linux-kernel@vger.kernel.org; y2038@lists.linaro.org; hch@lst.de; hare@suse.com; jthumshirn@suse.de
Subject: Re: [PATCH 0/7] scsi: bfa: do_gettimeofday removal

This stuff look ok - but I have a bigger question and that is if bfa is still alive at all.  Everytime I look at it I have doubts if it works at all, so I wonder if we need to keep it on life support.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Y2038] [PATCH 3/7] scsi: bfa: improve bfa_ioc_send_enable/disable data
  2017-11-10 15:37 ` [PATCH 3/7] scsi: bfa: improve bfa_ioc_send_enable/disable data Arnd Bergmann
@ 2017-11-13 14:08   ` Ben Hutchings
  2017-11-13 14:55     ` Arnd Bergmann
  0 siblings, 1 reply; 18+ messages in thread
From: Ben Hutchings @ 2017-11-13 14:08 UTC (permalink / raw)
  To: Arnd Bergmann, Anil Gurumurthy, Sudarsana Kalluru,
	James E . J . Bottomley, Martin K . Petersen
  Cc: hare, linux-scsi, y2038, linux-kernel, jthumshirn, hch

On Fri, 2017-11-10 at 16:37 +0100, Arnd Bergmann wrote:
> In bfa_ioc_send_enable, we use the deprecated do_gettimeofday() function
> to read the current time. This is not a problem, since the firmware
> interface is already limited to 32-bit timestamps, but it's better
> to use ktime_get_seconds() and document what the limitation is.
> 
> I noticed that I did the same change in commit a5af83925363 ("bna:
> avoid writing uninitialized data into hw registers") for the ethernet
> driver. That commit also changed the "disable" funtion to initialize
> the data we pass to the firmware properly, so I'm doing the same
> thing here.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/scsi/bfa/bfa_ioc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
> index 256f4afaccf9..117332537763 100644
> --- a/drivers/scsi/bfa/bfa_ioc.c
> +++ b/drivers/scsi/bfa/bfa_ioc.c
> @@ -1809,13 +1809,12 @@ static void
>  bfa_ioc_send_enable(struct bfa_ioc_s *ioc)
>  {
>  	struct bfi_ioc_ctrl_req_s enable_req;
> -	struct timeval tv;
>  
>  	bfi_h2i_set(enable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_ENABLE_REQ,
>  		    bfa_ioc_portid(ioc));
>  	enable_req.clscode = cpu_to_be16(ioc->clscode);
> -	do_gettimeofday(&tv);
> -	enable_req.tv_sec = be32_to_cpu(tv.tv_sec);
> +	/* unsigned 32-bit time_t overflow in y2106 */
> +	enable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());

The byte order conversion should also logically be cpu_to_be32(), not
be32_to_cpu().

Ben.

>  	bfa_ioc_mbox_send(ioc, &enable_req, sizeof(struct bfi_ioc_ctrl_req_s));
>  }
>  
> @@ -1826,6 +1825,9 @@ bfa_ioc_send_disable(struct bfa_ioc_s *ioc)
>  
>  	bfi_h2i_set(disable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_DISABLE_REQ,
>  		    bfa_ioc_portid(ioc));
> +	disable_req.clscode = cpu_to_be16(ioc->clscode);
> +	/* unsigned 32-bit time_t overflow in y2106 */
> +	disable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());
>  	bfa_ioc_mbox_send(ioc, &disable_req, sizeof(struct bfi_ioc_ctrl_req_s));
>  }
>  
-- 
Ben Hutchings
Software Developer, Codethink Ltd.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [Y2038] [PATCH 3/7] scsi: bfa: improve bfa_ioc_send_enable/disable data
  2017-11-13 14:08   ` [Y2038] " Ben Hutchings
@ 2017-11-13 14:55     ` Arnd Bergmann
  0 siblings, 0 replies; 18+ messages in thread
From: Arnd Bergmann @ 2017-11-13 14:55 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen, Hannes Reinecke, linux-scsi,
	y2038 Mailman List, Linux Kernel Mailing List,
	Johannes Thumshirn, Christoph Hellwig

On Mon, Nov 13, 2017 at 3:08 PM, Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
> On Fri, 2017-11-10 at 16:37 +0100, Arnd Bergmann wrote:

>>
>>       bfi_h2i_set(enable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_ENABLE_REQ,
>>                   bfa_ioc_portid(ioc));
>>       enable_req.clscode = cpu_to_be16(ioc->clscode);
>> -     do_gettimeofday(&tv);
>> -     enable_req.tv_sec = be32_to_cpu(tv.tv_sec);
>> +     /* unsigned 32-bit time_t overflow in y2106 */
>> +     enable_req.tv_sec = be32_to_cpu(ktime_get_real_seconds());
>
> The byte order conversion should also logically be cpu_to_be32(), not
> be32_to_cpu().
>

Right, I had not noticed that. I just tried fixing this, looking at what
sparse thinks of the file as a whole. I found it basically hopeless
to fix the endianess warnings in this driver, it seems completely
random here whether they are used correctly, in the opposite
direction as expected, or just flip data in place as in

attr->part[i].part_off = be32_to_cpu(f->part[i].part_off);

I'd rather not touch that part. IIRC I had at some point spent
a day trying to clean up the "warning: symbol
 'bfa_flash_sem_get' was not declared. Should it be static?"
sparse warnings for some driver, before giving up for
similar reasons.

       Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] scsi: bfa: do_gettimeofday removal
  2017-11-10 15:37 ` Arnd Bergmann
@ 2017-11-21  3:04   ` Martin K. Petersen
  -1 siblings, 0 replies; 18+ messages in thread
From: Martin K. Petersen @ 2017-11-21  3:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Anil Gurumurthy, Sudarsana Kalluru, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, linux-kernel, y2038, hch, hare,
	jthumshirn


Arnd,

> The bfa driver is one of the main users of do_gettimeofday(), a
> function that I'm trying to remove as part of the y2038 cleanup.
>
> The timestamps are all uses in slightly different ways, so this has
> turned into a rather longish series for doing something that should be
> simple.
>
> The last patch in the series ("scsi: bfa: use 64-bit times in
> bfa_aen_entry_s ABI") is one that needs to be reviewed very carefully,
> and it can be skipped if the maintainers prefer to leave the 32-bit
> ABI unchanged, the rest are hopefully fairly straightforward.

Applied to 4.16/scsi-queue, thanks!

Will drop #7 if something breaks.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] scsi: bfa: do_gettimeofday removal
@ 2017-11-21  3:04   ` Martin K. Petersen
  0 siblings, 0 replies; 18+ messages in thread
From: Martin K. Petersen @ 2017-11-21  3:04 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: hare, James E . J . Bottomley, Sudarsana Kalluru,
	Martin K . Petersen, y2038, jthumshirn, Anil Gurumurthy,
	linux-kernel, linux-scsi, hch


Arnd,

> The bfa driver is one of the main users of do_gettimeofday(), a
> function that I'm trying to remove as part of the y2038 cleanup.
>
> The timestamps are all uses in slightly different ways, so this has
> turned into a rather longish series for doing something that should be
> simple.
>
> The last patch in the series ("scsi: bfa: use 64-bit times in
> bfa_aen_entry_s ABI") is one that needs to be reviewed very carefully,
> and it can be skipped if the maintainers prefer to leave the 32-bit
> ABI unchanged, the rest are hopefully fairly straightforward.

Applied to 4.16/scsi-queue, thanks!

Will drop #7 if something breaks.

-- 
Martin K. Petersen	Oracle Linux Engineering
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-11-21  3:05 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-10 15:37 [PATCH 0/7] scsi: bfa: do_gettimeofday removal Arnd Bergmann
2017-11-10 15:37 ` Arnd Bergmann
2017-11-10 15:37 ` [PATCH 1/7] scsi: bfa: use ktime_get_real_ts64 for firmware timestamp Arnd Bergmann
2017-11-10 15:37   ` Arnd Bergmann
2017-11-10 15:37 ` [PATCH 2/7] scsi: bfa: use proper time accessor for stats_reset_time Arnd Bergmann
2017-11-10 15:37 ` [PATCH 3/7] scsi: bfa: improve bfa_ioc_send_enable/disable data Arnd Bergmann
2017-11-13 14:08   ` [Y2038] " Ben Hutchings
2017-11-13 14:55     ` Arnd Bergmann
2017-11-10 15:37 ` [PATCH 4/7] scsi: bfa: document overflow of io_profile_start_time Arnd Bergmann
2017-11-10 15:37 ` [PATCH 5/7] scsi: bfa: replace bfa_get_log_time() with ktime_get_real_seconds() Arnd Bergmann
2017-11-10 15:37   ` Arnd Bergmann
2017-11-10 15:37 ` [PATCH 6/7] scsi: bfa: try to sanitize vendor netlink events Arnd Bergmann
2017-11-10 15:37   ` Arnd Bergmann
2017-11-10 15:37 ` [PATCH 7/7] scsi: bfa: use 64-bit times in bfa_aen_entry_s ABI Arnd Bergmann
2017-11-10 17:24 ` [PATCH 0/7] scsi: bfa: do_gettimeofday removal Christoph Hellwig
2017-11-10 17:33   ` Gurumurthy, Anil
2017-11-21  3:04 ` Martin K. Petersen
2017-11-21  3:04   ` Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.