RE: [PATCH -next] scsi: fnic: Remove set but not used variable 'vdev'
Looks good to me.
Acked-by: Satish Kharat
Satish Kharat
TECHNICAL LEADER.ENGINEERING
satis...@cisco.com
Tel: +1 408 526 7504
Cisco Systems, Inc.
3800 Zanker Road
SAN JOSE
95134
United States
cisco.com
Think before you print.
This email may contain confidential and privileged material for the sole use of
the intended recipient. Any review, use, distribution or disclosure by others
is strictly prohibited. If you are not the intended recipient (or authorized to
receive for the recipient), please contact the sender by reply email and delete
all copies of this message.
http://www.cisco.com/c/en/us/about/legal/terms-sale-software-license-agreement/company-registration-information.html
-Original Message-
From: YueHaibing
Sent: Thursday, January 24, 2019 6:00 PM
To: Satish Kharat (satishkh) ; Sesidhar Baddela (sebaddel)
; Karan Tilak Kumar (kartilak) ;
j...@linux.ibm.com; martin.peter...@oracle.com
Cc: linux-ker...@vger.kernel.org; linux-scsi@vger.kernel.org; YueHaibing
Subject: [PATCH -next] scsi: fnic: Remove set but not used variable 'vdev'
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/scsi/fnic/vnic_wq.c: In function 'vnic_wq_alloc_bufs':
drivers/scsi/fnic/vnic_wq.c:50:19: warning:
variable 'vdev' set but not used [-Wunused-but-set-variable]
drivers/scsi/fnic/vnic_rq.c: In function 'vnic_rq_alloc_bufs':
drivers/scsi/fnic/vnic_rq.c:30:19: warning:
variable 'vdev' set but not used [-Wunused-but-set-variable]
It never used since introduction
Signed-off-by: YueHaibing
---
drivers/scsi/fnic/vnic_rq.c | 3 ---
drivers/scsi/fnic/vnic_wq.c | 3 ---
2 files changed, 6 deletions(-)
diff --git a/drivers/scsi/fnic/vnic_rq.c b/drivers/scsi/fnic/vnic_rq.c index
74d2f2c..6a35b1b 100644
--- a/drivers/scsi/fnic/vnic_rq.c
+++ b/drivers/scsi/fnic/vnic_rq.c
@@ -27,12 +27,9 @@
static int vnic_rq_alloc_bufs(struct vnic_rq *rq) {
struct vnic_rq_buf *buf;
- struct vnic_dev *vdev;
unsigned int i, j, count = rq->ring.desc_count;
unsigned int blks = VNIC_RQ_BUF_BLKS_NEEDED(count);
- vdev = rq->vdev;
-
for (i = 0; i < blks; i++) {
rq->bufs[i] = kzalloc(VNIC_RQ_BUF_BLK_SZ, GFP_ATOMIC);
if (!rq->bufs[i]) {
diff --git a/drivers/scsi/fnic/vnic_wq.c b/drivers/scsi/fnic/vnic_wq.c index
74cfc56..015af2c 100644
--- a/drivers/scsi/fnic/vnic_wq.c
+++ b/drivers/scsi/fnic/vnic_wq.c
@@ -47,12 +47,9 @@ int vnic_wq_alloc_ring(struct vnic_dev *vdev, struct vnic_wq
*wq, static int vnic_wq_alloc_bufs(struct vnic_wq *wq) {
struct vnic_wq_buf *buf;
- struct vnic_dev *vdev;
unsigned int i, j, count = wq->ring.desc_count;
unsigned int blks = VNIC_WQ_BUF_BLKS_NEEDED(count);
- vdev = wq->vdev;
-
for (i = 0; i < blks; i++) {
wq->bufs[i] = kzalloc(VNIC_WQ_BUF_BLK_SZ, GFP_ATOMIC);
if (!wq->bufs[i]) {
--
2.7.0
RE: [PATCH 3/7] scsi: fnic: no need to check return value of debugfs_create functions
Looks good to me.
Acked-by: Satish Kharat
Satish Kharat
TECHNICAL LEADER.ENGINEERING
satis...@cisco.com
-Original Message-
From: Greg Kroah-Hartman
Sent: Tuesday, January 22, 2019 7:09 AM
To: James Bottomley ; Martin Petersen
Cc: linux-ker...@vger.kernel.org; linux-scsi@vger.kernel.org; Greg
Kroah-Hartman ; Satish Kharat (satishkh)
; Sesidhar Baddela (sebaddel) ; Karan
Tilak Kumar (kartilak)
Subject: [PATCH 3/7] scsi: fnic: no need to check return value of
debugfs_create functions
When calling debugfs functions, there is no need to ever check the return
value. The function can work or not, but the code logic should never do
something different based on this.
Cc: Satish Kharat
Cc: Sesidhar Baddela
Cc: Karan Tilak Kumar
Cc: "James E.J. Bottomley"
Cc: "Martin K. Petersen"
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman
---
drivers/scsi/fnic/fnic_debugfs.c | 88 ++--
drivers/scsi/fnic/fnic_main.c| 7 +--
drivers/scsi/fnic/fnic_stats.h | 2 +-
drivers/scsi/fnic/fnic_trace.c | 17 ++
drivers/scsi/fnic/fnic_trace.h | 4 +-
5 files changed, 10 insertions(+), 108 deletions(-)
diff --git a/drivers/scsi/fnic/fnic_debugfs.c b/drivers/scsi/fnic/fnic_debugfs.c
index 139fffa3658a..21991c99db7c 100644
--- a/drivers/scsi/fnic/fnic_debugfs.c
+++ b/drivers/scsi/fnic/fnic_debugfs.c
@@ -54,23 +54,9 @@ int fnic_debugfs_init(void) {
int rc = -1;
fnic_trace_debugfs_root = debugfs_create_dir("fnic", NULL);
- if (!fnic_trace_debugfs_root) {
- printk(KERN_DEBUG "Cannot create debugfs root\n");
- return rc;
- }
-
- if (!fnic_trace_debugfs_root) {
- printk(KERN_DEBUG
- "fnic root directory doesn't exist in debugfs\n");
- return rc;
- }
fnic_stats_debugfs_root = debugfs_create_dir("statistics",
fnic_trace_debugfs_root);
- if (!fnic_stats_debugfs_root) {
- printk(KERN_DEBUG "Cannot create Statistics directory\n");
- return rc;
- }
/* Allocate memory to structure */
fc_trc_flag = (struct fc_trace_flag_type *) @@ -356,39 +342,19 @@
static const struct file_operations fnic_trace_debugfs_fops = {
* it will also create file trace_enable to control enable/disable of
* trace logging into trace buffer.
*/
-int fnic_trace_debugfs_init(void)
+void fnic_trace_debugfs_init(void)
{
- int rc = -1;
- if (!fnic_trace_debugfs_root) {
- printk(KERN_DEBUG
- "FNIC Debugfs root directory doesn't exist\n");
- return rc;
- }
fnic_trace_enable = debugfs_create_file("tracing_enable",
S_IFREG|S_IRUGO|S_IWUSR,
fnic_trace_debugfs_root,
&(fc_trc_flag->fnic_trace),
&fnic_trace_ctrl_fops);
- if (!fnic_trace_enable) {
- printk(KERN_DEBUG
- "Cannot create trace_enable file under debugfs\n");
- return rc;
- }
-
fnic_trace_debugfs_file = debugfs_create_file("trace",
S_IFREG|S_IRUGO|S_IWUSR,
fnic_trace_debugfs_root,
&(fc_trc_flag->fnic_trace),
&fnic_trace_debugfs_fops);
-
- if (!fnic_trace_debugfs_file) {
- printk(KERN_DEBUG
- "Cannot create trace file under debugfs\n");
- return rc;
- }
- rc = 0;
- return rc;
}
/*
@@ -419,37 +385,20 @@ void fnic_trace_debugfs_terminate(void)
* trace logging into trace buffer.
*/
-int fnic_fc_trace_debugfs_init(void)
+void fnic_fc_trace_debugfs_init(void)
{
- int rc = -1;
-
- if (!fnic_trace_debugfs_root) {
- pr_err("fnic:Debugfs root directory doesn't exist\n");
- return rc;
- }
-
fnic_fc_trace_enable = debugfs_create_file("fc_trace_enable",
S_IFREG|S_IRUGO|S_IWUSR,
fnic_trace_debugfs_root,
&(fc_trc_flag->fc_trace),
&fnic_trace_ctrl_fops);
- if (!fnic_fc_trace_enable) {
- pr_err("fnic: Failed create fc_trace_enable file\n");
- return rc;
- }
-
fnic_fc_trace_clear = debugfs_create_file("fc_trace_clear",
S_IFREG|S_IRUGO|S_IWUSR,
RE: [PATCH] [RESEND] scsi: fnic: use 64-bit timestamps
Looks good to me.
Acked-by: Satish Kharat
-Original Message-
From: Arnd Bergmann [mailto:a...@arndb.de]
Sent: Wednesday, January 17, 2018 7:17 AM
To: Satish Kharat (satishkh) ; Sesidhar Baddela (sebaddel)
; Karan Tilak Kumar (kartilak) ; James
E.J. Bottomley ; Martin K. Petersen
Cc: Arnd Bergmann ; Vasyl Gomonovych ;
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
---
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->stats_timestamps.last_reset_time);
+ ktime_get_real_ts64(&stats->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(&fnic_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, &val);
+ jiffies_to_timespec64(tbp->timestamp.low, &val);
} else {
sprint_symbol(str, tbp->fnaddr.val);
- jiffies_to_timespec(tbp->timestamp.val, &val);
+ jiffies_to_timespec64(tbp->timestamp.val, &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, &val);
+ jiffies_to_timespec64(tbp->timestamp.low, &val);
} else {
sprint_symbol(str, tbp->fnaddr.val);
-
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) Cc: linux-scsi@vger.kernel.org; Sesidhar Baddela (sebaddel) Subject: Re: [PATCH 1/1] fnic: Adding Check Condition counter to misc fnicstats >>>>> "Satish" == Satish Kharat 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
---
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(
&term_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(&abts_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(&fnic_stats->io_stats.active_ios);
if (atomic64_read(&fnic->io_cmpl_skip))
atomic64_dec(&fnic->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
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,
> version 1.6.0.18 has been ski
