32 bit systems using 'struct timeval' will break in the year 2038, so we modify the code appropriately.
This patch removes the function bfa_get_log_time() which is only used to get current time in seconds. bfa_get_log_time() is replaced with ktime_get_real_seconds() which returns a 64 bit value.
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com --- Changes in v2: -None Changes in v3: -None
drivers/scsi/bfa/bfa_svc.c | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c index 625225f..b3668e9 100644 --- a/drivers/scsi/bfa/bfa_svc.c +++ b/drivers/scsi/bfa/bfa_svc.c @@ -15,6 +15,7 @@ * General Public License for more details. */
+#include <linux/ktime.h> #include "bfad_drv.h" #include "bfad_im.h" #include "bfa_plog.h" @@ -303,18 +304,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) { @@ -335,7 +324,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) @@ -6278,7 +6267,7 @@ 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.start_time = ktime_get_real_seconds();\ (__dport)->result.status = DPORT_TEST_ST_INPRG; \ (__dport)->result.mode = (__mode); \ (__dport)->result.rp_pwwn = (__dport)->rp_pwwn; \ @@ -6717,7 +6706,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; @@ -6764,7 +6753,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;
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_seconds() since we only need to find elapsed seconds.
Since we need monotonic time only, stats_reset_time variable does not need to be changed from u32 to u64 type.
timestamp variable of type 'time64_t' is used to store monotonic time returned by ktime_get_seconds().
status_ok boolean variable has been introduced to remove gcc warning of uninitialised variable 'timestamp'.
Signed-off-by: Amitoj Kaur Chawla amitoj1606@gmail.com --- Changes in v2: -Removed unnecessary if condition -Removed u32 to u64 conversion since monotonic time doesn't require the change. Changes in v3: -Introduced boolean variable to remove warning -Changed timestamp variable type from u64 to time64_t
drivers/scsi/bfa/bfa_svc.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/bfa/bfa_svc.c b/drivers/scsi/bfa/bfa_svc.c index b3668e9..6521896 100644 --- a/drivers/scsi/bfa/bfa_svc.c +++ b/drivers/scsi/bfa/bfa_svc.c @@ -3081,7 +3081,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; @@ -3094,8 +3093,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;
/* @@ -3345,16 +3343,17 @@ __bfa_cb_fcport_stats_get(void *cbarg, bfa_boolean_t complete) struct bfa_cb_pending_q_s *cb; struct list_head *qe, *qen; union bfa_fcport_stats_u *ret; + bool status_ok = (fcport->stats_status == BFA_STATUS_OK);
if (complete) { - struct timeval tv; - if (fcport->stats_status == BFA_STATUS_OK) - do_gettimeofday(&tv); + time64_t timestamp; + if (status_ok) + timestamp = ktime_get_seconds();
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; - if (fcport->stats_status == BFA_STATUS_OK) { + if (status_ok) { ret = (union bfa_fcport_stats_u *)cb->data; /* Swap FC QoS or FCoE stats */ if (bfa_ioc_get_fcmode(&fcport->bfa->ioc)) @@ -3364,7 +3363,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; + timestamp - fcport->stats_reset_time; } } bfa_cb_queue_status(fcport->bfa, &cb->hcb_qe,
On Thursday 05 November 2015 23:10:54 Amitoj Kaur Chawla wrote:
32 bit systems using 'struct timeval' will break in the year 2038, so we modify the code appropriately.
Looks good, this is ready to get posted to the driver maintainers for further review, but it would be nice to have all your bfa patches together in one series when you do that. I don't remember which ones were already ready and if there are some that need more work.
Regarding the changelog text, what you have above is roughly ok, but it's always better to start out describing mostly what the problem is and only then saying how you solve it. Let me give an example how the sentences can be improved:
This patch replaces the use of struct timeval and do_gettimeofday() with 64 bit ktime_get_seconds() since we only need to find elapsed seconds.
!We only need to find the elapsed seconds rather than absolute time, !and we only care about full seconds, so it's better to use monotonic !times, so using ktime_get_seconds() also makes the code more efficient !and more robust against a concurrent settimeofday().
Since we need monotonic time only, stats_reset_time variable does not need to be changed from u32 to u64 type.
(this one is good)
timestamp variable of type 'time64_t' is used to store monotonic time returned by ktime_get_seconds().
(this one is redundant now, you are not changing the type)
status_ok boolean variable has been introduced to remove gcc warning of uninitialised variable 'timestamp'.
!After the conversion, we get a harmless compiler warning about the !possible use of an uninitialized warning. gcc is wrong here and !the code is actually ok. Introducing a temporary status_ok variable !to store the condition avoids the warning.
As I said, it's not important for a single patch, but as you are sending a lot of patches it's better to make the texts as useful as possible.
Arnd
On Fri, Nov 6, 2015 at 5:59 PM, Arnd Bergmann arnd@arndb.de wrote:
On Thursday 05 November 2015 23:10:54 Amitoj Kaur Chawla wrote:
32 bit systems using 'struct timeval' will break in the year 2038, so we modify the code appropriately.
Looks good, this is ready to get posted to the driver maintainers for further review, but it would be nice to have all your bfa patches together in one series when you do that. I don't remember which ones were already ready and if there are some that need more work.
I'll work on all of them and send them in a series when you give the go ahead for all.
As I said, it's not important for a single patch, but as you are sending a lot of patches it's better to make the texts as useful as possible.
Arnd
When I send the final version to the maintainers, I'll make these changes in the changelog text.
-
Amitoj Kaur Chawla -
Arnd Bergmann