On Tuesday 27 October 2015 01:11:07 Amitoj Kaur Chawla wrote:

32 bit systems using 'struct timeval' will break in the year 2038, so we modify the code appropriately.

This patch replaces the use of struct timeval and do_gettimeofday() with 64 bit ktime_get_real_seconds().

Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com

It looks like you are doing two different things here, as you replace two unrelated structures (struct bfa_diag_dport_result_s and stats_reset_time)

---

drivers/scsi/bfa/bfa_svc.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c index 625225f..a880a7e 100644 --- a/drivers/scsi/bfa/bfa_svc.c +++ b/drivers/scsi/bfa/bfa_svc.c @@ -15,6 +15,8 @@

• General Public License for more details.

*/ +#include <linux/ktime.h>

#include "bfad_drv.h" #include "bfad_im.h" #include "bfa_plog.h" @@ -306,13 +308,8 @@ plkd_validate_logrec(struct bfa_plog_rec_s *pl_rec) 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;

• return ktime_get_real_seconds();

}

This part looks good, but you can go further and remove the function completely.

static void @@ -3092,7 +3089,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; @@ -3105,8 +3101,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_real_seconds(); fcport->stats_dma_ready = BFA_FALSE;

The change here is probably ok, but you are missing the fact that stats_reset_time is too short and needs to be changed to a time64_t in the header, as part of the same patch. As mentioned above, this patch should be separate from the bfa_get_log_time change.

/* @@ -3358,10 +3353,6 @@ __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);
  

• list_for_each_safe(qe, qen, &fcport->stats_pending_q) { bfa_q_deq(&fcport->stats_pending_q, &qe); cb = (struct bfa_cb_pending_q_s *)qe;

@@ -3375,7 +3366,8 @@ __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;
  

• 			ktime_get_real_seconds() - 
  

• 				fcport->stats_reset_time;
  	}
  }
  bfa_cb_queue_status(fcport->bfa, &cb->hcb_qe,
  

Here you have a slight change in behavior: you now call ktime_get_real_seconds() once per loop iteration, rather than getting the time once at the start. If there is a good reason for this change, please explain that reason in the patch description, otherwise leave it alone.

I also note that the stats_reset_time variable is only used to compute elapsed seconds, so it should be converted to monotonic time in the process, to avoid suffering from gettimeofday() or leap second changes.

Arnd