RE: [PATCH] [RESEND] scsi: fnic: use 64-bit timestamps
Looks good to me. Acked-by: Satish Kharat <satis...@cisco.com> -Original Message- From: Arnd Bergmann [mailto:a...@arndb.de] Sent: Wednesday, January 17, 2018 7:17 AM To: Satish Kharat (satishkh) <satis...@cisco.com>; Sesidhar Baddela (sebaddel) <sebad...@cisco.com>; Karan Tilak Kumar (kartilak) <karti...@cisco.com>; James E.J. Bottomley <j...@linux.vnet.ibm.com>; Martin K. Petersen <martin.peter...@oracle.com> Cc: Arnd Bergmann <a...@arndb.de>; Vasyl Gomonovych <gomonov...@gmail.com>; linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org Subject: [PATCH] [RESEND] scsi: fnic: use 64-bit timestamps struct timespec is deprecated since it overflows in 2038 on 32-bit architectures, so we should use timespec64 consistently. I'm slightly adapting the format strings here, to make sure we print the nanoseconds with the correct number of leading zeroes. Signed-off-by: Arnd Bergmann <a...@arndb.de> --- Originally submitted in November 2017. Martin asked Satish to review my patch, but I never heard back after that, so I'm trying again now. --- drivers/scsi/fnic/fnic_debugfs.c | 2 +- drivers/scsi/fnic/fnic_stats.h | 4 +-- drivers/scsi/fnic/fnic_trace.c | 58 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c index 9858484dd126..6d3e1cb4fea6 100644 --- a/drivers/scsi/fnic/fnic_debugfs.c +++ b/drivers/scsi/fnic/fnic_debugfs.c @@ -614,7 +614,7 @@ static ssize_t fnic_reset_stats_write(struct file *file, sizeof(struct io_path_stats) - sizeof(u64)); memset(fw_stats_p+1, 0, sizeof(struct fw_stats) - sizeof(u64)); - getnstimeofday(>stats_timestamps.last_reset_time); + ktime_get_real_ts64(>stats_timestamps.last_reset_time); } (*ppos)++; diff --git a/drivers/scsi/fnic/fnic_stats.h b/drivers/scsi/fnic/fnic_stats.h index e007feedbf72..9daa6ada6fa0 100644 --- a/drivers/scsi/fnic/fnic_stats.h +++ b/drivers/scsi/fnic/fnic_stats.h @@ -18,8 +18,8 @@ #define _FNIC_STATS_H_ struct stats_timestamps { - struct timespec last_reset_time; - struct timespec last_read_time; + struct timespec64 last_reset_time; + struct timespec64 last_read_time; }; struct io_path_stats { diff --git a/drivers/scsi/fnic/fnic_trace.c b/drivers/scsi/fnic/fnic_trace.c index 4826f596cb31..abddde11982b 100644 --- a/drivers/scsi/fnic/fnic_trace.c +++ b/drivers/scsi/fnic/fnic_trace.c @@ -111,7 +111,7 @@ int fnic_get_trace_data(fnic_dbgfs_t *fnic_dbgfs_prt) int len = 0; unsigned long flags; char str[KSYM_SYMBOL_LEN]; - struct timespec val; + struct timespec64 val; fnic_trace_data_t *tbp; spin_lock_irqsave(_trace_lock, flags); @@ -129,10 +129,10 @@ int fnic_get_trace_data(fnic_dbgfs_t *fnic_dbgfs_prt) /* Convert function pointer to function name */ if (sizeof(unsigned long) < 8) { sprint_symbol(str, tbp->fnaddr.low); - jiffies_to_timespec(tbp->timestamp.low, ); + jiffies_to_timespec64(tbp->timestamp.low, ); } else { sprint_symbol(str, tbp->fnaddr.val); - jiffies_to_timespec(tbp->timestamp.val, ); + jiffies_to_timespec64(tbp->timestamp.val, ); } /* * Dump trace buffer entry to memory file @@ -140,8 +140,8 @@ int fnic_get_trace_data(fnic_dbgfs_t *fnic_dbgfs_prt) */ len += snprintf(fnic_dbgfs_prt->buffer + len, (trace_max_pages * PAGE_SIZE * 3) - len, - "%16lu.%16lu %-50s %8x %8x %16llx %16llx " - "%16llx %16llx %16llx\n", val.tv_sec, + "%16llu.%09lu %-50s %8x %8x %16llx %16llx " + "%16llx %16llx %16llx\n", (u64)val.tv_sec, val.tv_nsec, str, tbp->host_no, tbp->tag, tbp->data[0], tbp->data[1], tbp->data[2], tbp->data[3], tbp->data[4]); @@ -171,10 +171,10 @@ int fnic_get_trace_data(fnic_dbgfs_t *fnic_dbgfs_prt) /* Convert function pointer to function name */ if (sizeof(unsigned long) < 8) { sprint_symbol(str, tbp->fnaddr.low); - jiffies_to_timespec(tbp->timestamp.low, ); +
RE: [PATCH 1/1] fnic: Adding Check Condition counter to misc fnicstats
Hi Martin, Apologies for the delay. I was not able to verify this because of another fnic issue blocking this test. Just now submitted a fix for that 'fnic issue' (in the patch => [PATCH 1/1] fnic: bug fix for fip.fip_subcode in fnic_fcoe_send_vlan_req) Did some quick verification and basic IO test, this patch looks good. Thanks, Satish Kharat -Original Message- From: Martin K. Petersen [mailto:martin.peter...@oracle.com] Sent: Wednesday, March 01, 2017 7:04 PM To: Satish Kharat (satishkh) <satis...@cisco.com> Cc: linux-scsi@vger.kernel.org; Sesidhar Baddela (sebaddel) <sebad...@cisco.com> Subject: Re: [PATCH 1/1] fnic: Adding Check Condition counter to misc fnicstats >>>>> "Satish" == Satish Kharat <satis...@cisco.com> writes: Satish, Satish> Just a simple counter of number of check conditions encountered Satish> on that host. Please test and review the following: https://patchwork.kernel.org/patch/9549777/ Thank you! -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH 1/5] Fix to cleanup aborted IO to avoid device being offlined by mid-layer
Thanks for the review comments. I incorporated the comments, below is the updated patch: If an I/O times out and an abort issued by host, if the abort is successful we need to set scsi status as DID_ABORT. Or else the mid-layer error handler which looks for this error code, will offline the device. Also if the original I/O is not found in fnic firmware, we will consider the abort as successful. The start_time assignment is moved because of the new goto. Fnic driver version changed from 1.6.0.17a to 1.6.0.19, version 1.6.0.18 has been skipped Signed-off-by: Satish Kharat <satis...@cisco.com> --- drivers/scsi/fnic/fnic.h | 2 +- drivers/scsi/fnic/fnic_scsi.c | 35 +-- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h index ce129e5..52a53f8 100644 --- a/drivers/scsi/fnic/fnic.h +++ b/drivers/scsi/fnic/fnic.h @@ -39,7 +39,7 @@ #define DRV_NAME "fnic" #define DRV_DESCRIPTION"Cisco FCoE HBA Driver" -#define DRV_VERSION"1.6.0.17a" +#define DRV_VERSION"1.6.0.19" #define PFXDRV_NAME ": " #define DFX DRV_NAME "%d: " diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c index 266b909..b732fa3 100644 --- a/drivers/scsi/fnic/fnic_scsi.c +++ b/drivers/scsi/fnic/fnic_scsi.c @@ -1092,6 +1092,11 @@ static void fnic_fcpio_itmf_cmpl_handler(struct fnic *fnic, atomic64_inc( _stats->terminate_fw_timeouts); break; + case FCPIO_ITMF_REJECTED: + FNIC_SCSI_DBG(KERN_INFO, fnic->lport->host, + "abort reject recd. id %d\n", + (int)(id & FNIC_TAG_MASK)); + break; case FCPIO_IO_NOT_FOUND: if (CMD_FLAGS(sc) & FNIC_IO_ABTS_ISSUED) atomic64_inc(_stats->abort_io_not_found); @@ -1112,9 +1117,15 @@ static void fnic_fcpio_itmf_cmpl_handler(struct fnic *fnic, spin_unlock_irqrestore(io_lock, flags); return; } - CMD_ABTS_STATUS(sc) = hdr_status; + CMD_FLAGS(sc) |= FNIC_IO_ABT_TERM_DONE; + /* If the status is IO not found consider it as success */ + if (hdr_status == FCPIO_IO_NOT_FOUND) + CMD_ABTS_STATUS(sc) = FCPIO_SUCCESS; + else + CMD_ABTS_STATUS(sc) = hdr_status; + atomic64_dec(_stats->io_stats.active_ios); if (atomic64_read(>io_cmpl_skip)) atomic64_dec(>io_cmpl_skip); @@ -1927,21 +1938,33 @@ int fnic_abort_cmd(struct scsi_cmnd *sc) CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE; + start_time = io_req->start_time; /* * firmware completed the abort, check the status, -* free the io_req irrespective of failure or success +* free the io_req if successful. If abort fails, +* Device reset will clean the I/O. */ - if (CMD_ABTS_STATUS(sc) != FCPIO_SUCCESS) + if (CMD_ABTS_STATUS(sc) == FCPIO_SUCCESS) { + CMD_SP(sc) = NULL; + } + else + { ret = FAILED; - - CMD_SP(sc) = NULL; + spin_unlock_irqrestore(io_lock, flags); + goto fnic_abort_cmd_end; + } spin_unlock_irqrestore(io_lock, flags); - start_time = io_req->start_time; fnic_release_ioreq_buf(fnic, io_req, sc); mempool_free(io_req, fnic->io_req_pool); + if (sc->scsi_done) { + /* Call SCSI completion function to complete the IO */ + sc->result = (DID_ABORT << 16); + sc->scsi_done(sc); + } + fnic_abort_cmd_end: FNIC_TRACE(fnic_abort_cmd, sc->device->host->host_no, sc->request->tag, sc, -- 2.4.3 From: Ewan Milne <emi...@redhat.com> Sent: Monday, March 14, 2016 12:59 PM To: Satish Kharat (satishkh) Cc: linux-scsi@vger.kernel.org Subject: Re: [PATCH 1/5] Fix to cleanup aborted IO to avoid device being offlined by mid-layer On Mon, 2016-03-14 at 11:14 -0700, Satish Kharat wrote: > If an I/O times out and an abort issued by host, if the abort is > successful we need to set scsi status as DID_ABORT. Or else the > mid-layer error handler which looks for this error code, will > offline the device. Also if the original I/O is not found in fnic > firmware, we will consider the abort as successful. > Fnic driver version changed from 1.6.0.17a to 1.6.0.19, > v