All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer
@ 2016-03-18 18:22 Satish Kharat
  2016-03-18 18:22 ` [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset Satish Kharat
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Satish Kharat @ 2016-03-18 18:22 UTC (permalink / raw)
  To: satishkh, linux-scsi; +Cc: Sesidhar Baddela

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 <satishkh@cisco.com>
Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
Reviewed-by: Ewan D. Milne <emilne@redhat.com>
---
 * v1
 - Moved CMD_ABTS_STATUS assignment to else when not FCPIO_SUCCESS

 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 PFX			DRV_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


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

* [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset
  2016-03-18 18:22 [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Satish Kharat
@ 2016-03-18 18:22 ` Satish Kharat
  2016-03-29 14:06   ` Ewan D. Milne
  2016-03-18 18:22 ` [PATCH v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup Satish Kharat
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Satish Kharat @ 2016-03-18 18:22 UTC (permalink / raw)
  To: satishkh, linux-scsi; +Cc: Sesidhar Baddela

In case of LUN reset, the device reset command is issued with one of
the I/Os that has timed out on that LUN. The change is to also return
this I/O with error status set to DID_RESET. In case when the reset
is issued using the sg_reset tool (from sg3_utils) it is a new command
and new_sc is set to 1.
Fnic driver version changed from 1.6.0.19 to 1.6.0.20

Signed-off-by: Satish Kharat <satishkh@cisco.com>
Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
---
 * v1
 - new_sc is set to 1 in cases like lunreset from sg_reset
---
 drivers/scsi/fnic/fnic.h      |  2 +-
 drivers/scsi/fnic/fnic_scsi.c | 38 ++++++++++++++++++++++++++++----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 52a53f8..1023eae 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.19"
+#define DRV_VERSION		"1.6.0.20"
 #define PFX			DRV_NAME ": "
 #define DFX                     DRV_NAME "%d: "
 
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index b732fa3..026b93d 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -2042,7 +2042,9 @@ lr_io_req_end:
  * successfully aborted, 1 otherwise
  */
 static int fnic_clean_pending_aborts(struct fnic *fnic,
-				     struct scsi_cmnd *lr_sc)
+				     struct scsi_cmnd *lr_sc,
+					 bool new_sc)
+
 {
 	int tag, abt_tag;
 	struct fnic_io_req *io_req;
@@ -2060,10 +2062,10 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
 		spin_lock_irqsave(io_lock, flags);
 		sc = scsi_host_find_tag(fnic->lport->host, tag);
 		/*
-		 * ignore this lun reset cmd or cmds that do not belong to
-		 * this lun
+		 * ignore this lun reset cmd if issued using new SC
+		 * or cmds that do not belong to this lun
 		 */
-		if (!sc || sc == lr_sc || sc->device != lun_dev) {
+		if (!sc || ((sc == lr_sc) && new_sc) || sc->device != lun_dev) {
 			spin_unlock_irqrestore(io_lock, flags);
 			continue;
 		}
@@ -2169,11 +2171,27 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
 			goto clean_pending_aborts_end;
 		}
 		CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE;
-		CMD_SP(sc) = NULL;
+
+		/* original sc used for lr is handled by dev reset code */
+		if (sc != lr_sc)
+			CMD_SP(sc) = NULL;
 		spin_unlock_irqrestore(io_lock, flags);
 
-		fnic_release_ioreq_buf(fnic, io_req, sc);
-		mempool_free(io_req, fnic->io_req_pool);
+		/* original sc used for lr is handled by dev reset code */
+		if (sc != lr_sc) {
+			fnic_release_ioreq_buf(fnic, io_req, sc);
+			mempool_free(io_req, fnic->io_req_pool);
+		}
+
+		/*
+		 * Any IO is returned during reset, it needs to call scsi_done
+		 * to return the scsi_cmnd to upper layer.
+		 */
+		if (sc->scsi_done) {
+			/* Set result to let upper SCSI layer retry */
+			sc->result = DID_RESET << 16;
+			sc->scsi_done(sc);
+		}
 	}
 
 	schedule_timeout(msecs_to_jiffies(2 * fnic->config.ed_tov));
@@ -2267,6 +2285,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
 	int tag = 0;
 	DECLARE_COMPLETION_ONSTACK(tm_done);
 	int tag_gen_flag = 0;   /*to track tags allocated by fnic driver*/
+	bool new_sc = 0;
 
 	/* Wait for rport to unblock */
 	fc_block_scsi_eh(sc);
@@ -2312,13 +2331,12 @@ int fnic_device_reset(struct scsi_cmnd *sc)
 		 * fix the way the EH ioctls work for real, but until
 		 * that happens we fail these explicit requests here.
 		 */
-		if (shost_use_blk_mq(sc->device->host))
-			goto fnic_device_reset_end;
 
 		tag = fnic_scsi_host_start_tag(fnic, sc);
 		if (unlikely(tag == SCSI_NO_TAG))
 			goto fnic_device_reset_end;
 		tag_gen_flag = 1;
+		new_sc=1;
 	}
 	io_lock = fnic_io_lock_hash(fnic, sc);
 	spin_lock_irqsave(io_lock, flags);
@@ -2453,7 +2471,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
 	 * the lun reset cmd. If all cmds get cleaned, the lun reset
 	 * succeeds
 	 */
-	if (fnic_clean_pending_aborts(fnic, sc)) {
+	if (fnic_clean_pending_aborts(fnic, sc, new_sc)) {
 		spin_lock_irqsave(io_lock, flags);
 		io_req = (struct fnic_io_req *)CMD_SP(sc);
 		FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
-- 
2.4.3


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

* [PATCH v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup.
  2016-03-18 18:22 [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Satish Kharat
  2016-03-18 18:22 ` [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset Satish Kharat
@ 2016-03-18 18:22 ` Satish Kharat
  2016-03-29 14:06   ` Ewan D. Milne
  2016-03-29 14:06 ` [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Ewan D. Milne
  2016-03-30  0:46 ` Martin K. Petersen
  3 siblings, 1 reply; 7+ messages in thread
From: Satish Kharat @ 2016-03-18 18:22 UTC (permalink / raw)
  To: satishkh, linux-scsi; +Cc: Sesidhar Baddela

When issuing I/O we check if rport is online through libfc
rport_lookup() function which needs to be protected by mutex lock
that cannot acquired in I/O context. The change is to use midlayer
remote port’s dd_data which is preserved until its devloss timeout
and no protection is required.
The the scsi_cmnd error code is expected to be in the left 16 bits
of the result field. Changed to correct this.
Fnic driver version changed from 1.6.0.20 to 1.6.0.21

Signed-off-by: Satish Kharat <satishkh@cisco.com>
Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
---
 * v1
 - DID_NO_CONNECT left shifted 16 bits before assigning to sc->result
---
 drivers/scsi/fnic/fnic.h      |  2 +-
 drivers/scsi/fnic/fnic_scsi.c | 20 +++++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 1023eae..9ddc920 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.20"
+#define DRV_VERSION		"1.6.0.21"
 #define PFX			DRV_NAME ": "
 #define DFX                     DRV_NAME "%d: "
 
diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 026b93d..0e874a5 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -439,7 +439,6 @@ static int fnic_queuecommand_lck(struct scsi_cmnd *sc, void (*done)(struct scsi_
 	int sg_count = 0;
 	unsigned long flags = 0;
 	unsigned long ptr;
-	struct fc_rport_priv *rdata;
 	spinlock_t *io_lock = NULL;
 	int io_lock_acquired = 0;
 
@@ -455,14 +454,17 @@ static int fnic_queuecommand_lck(struct scsi_cmnd *sc, void (*done)(struct scsi_
 		return 0;
 	}
 
-	rdata = lp->tt.rport_lookup(lp, rport->port_id);
-	if (!rdata || (rdata->rp_state == RPORT_ST_DELETE)) {
-		FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
-			"returning IO as rport is removed\n");
-		atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
-		sc->result = DID_NO_CONNECT;
-		done(sc);
-		return 0;
+	if (rport) {
+		struct fc_rport_libfc_priv *rp = rport->dd_data;
+
+		if (!rp || rp->rp_state != RPORT_ST_READY) {
+			FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
+				"returning DID_NO_CONNECT for IO as rport is removed\n");
+			atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
+			sc->result = DID_NO_CONNECT<<16;
+			done(sc);
+			return 0;
+		}
 	}
 
 	if (lp->state != LPORT_ST_READY || !(lp->link_up))
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer
  2016-03-18 18:22 [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Satish Kharat
  2016-03-18 18:22 ` [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset Satish Kharat
  2016-03-18 18:22 ` [PATCH v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup Satish Kharat
@ 2016-03-29 14:06 ` Ewan D. Milne
  2016-03-30  0:46 ` Martin K. Petersen
  3 siblings, 0 replies; 7+ messages in thread
From: Ewan D. Milne @ 2016-03-29 14:06 UTC (permalink / raw)
  To: Satish Kharat; +Cc: linux-scsi, Sesidhar Baddela

On Fri, 2016-03-18 at 11:22 -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.
> 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 <satishkh@cisco.com>
> Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
> Reviewed-by: Ewan D. Milne <emilne@redhat.com>
> ---
>  * v1
>  - Moved CMD_ABTS_STATUS assignment to else when not FCPIO_SUCCESS
> 
>  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 PFX			DRV_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,

For v1:

Reviewed-by: Ewan D. Milne <emilne@redhat.com>


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

* Re: [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset
  2016-03-18 18:22 ` [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset Satish Kharat
@ 2016-03-29 14:06   ` Ewan D. Milne
  0 siblings, 0 replies; 7+ messages in thread
From: Ewan D. Milne @ 2016-03-29 14:06 UTC (permalink / raw)
  To: Satish Kharat; +Cc: linux-scsi, Sesidhar Baddela

On Fri, 2016-03-18 at 11:22 -0700, Satish Kharat wrote:
> In case of LUN reset, the device reset command is issued with one of
> the I/Os that has timed out on that LUN. The change is to also return
> this I/O with error status set to DID_RESET. In case when the reset
> is issued using the sg_reset tool (from sg3_utils) it is a new command
> and new_sc is set to 1.
> Fnic driver version changed from 1.6.0.19 to 1.6.0.20
> 
> Signed-off-by: Satish Kharat <satishkh@cisco.com>
> Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
> ---
>  * v1
>  - new_sc is set to 1 in cases like lunreset from sg_reset
> ---
>  drivers/scsi/fnic/fnic.h      |  2 +-
>  drivers/scsi/fnic/fnic_scsi.c | 38 ++++++++++++++++++++++++++++----------
>  2 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index 52a53f8..1023eae 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.19"
> +#define DRV_VERSION		"1.6.0.20"
>  #define PFX			DRV_NAME ": "
>  #define DFX                     DRV_NAME "%d: "
>  
> diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
> index b732fa3..026b93d 100644
> --- a/drivers/scsi/fnic/fnic_scsi.c
> +++ b/drivers/scsi/fnic/fnic_scsi.c
> @@ -2042,7 +2042,9 @@ lr_io_req_end:
>   * successfully aborted, 1 otherwise
>   */
>  static int fnic_clean_pending_aborts(struct fnic *fnic,
> -				     struct scsi_cmnd *lr_sc)
> +				     struct scsi_cmnd *lr_sc,
> +					 bool new_sc)
> +
>  {
>  	int tag, abt_tag;
>  	struct fnic_io_req *io_req;
> @@ -2060,10 +2062,10 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
>  		spin_lock_irqsave(io_lock, flags);
>  		sc = scsi_host_find_tag(fnic->lport->host, tag);
>  		/*
> -		 * ignore this lun reset cmd or cmds that do not belong to
> -		 * this lun
> +		 * ignore this lun reset cmd if issued using new SC
> +		 * or cmds that do not belong to this lun
>  		 */
> -		if (!sc || sc == lr_sc || sc->device != lun_dev) {
> +		if (!sc || ((sc == lr_sc) && new_sc) || sc->device != lun_dev) {
>  			spin_unlock_irqrestore(io_lock, flags);
>  			continue;
>  		}
> @@ -2169,11 +2171,27 @@ static int fnic_clean_pending_aborts(struct fnic *fnic,
>  			goto clean_pending_aborts_end;
>  		}
>  		CMD_STATE(sc) = FNIC_IOREQ_ABTS_COMPLETE;
> -		CMD_SP(sc) = NULL;
> +
> +		/* original sc used for lr is handled by dev reset code */
> +		if (sc != lr_sc)
> +			CMD_SP(sc) = NULL;
>  		spin_unlock_irqrestore(io_lock, flags);
>  
> -		fnic_release_ioreq_buf(fnic, io_req, sc);
> -		mempool_free(io_req, fnic->io_req_pool);
> +		/* original sc used for lr is handled by dev reset code */
> +		if (sc != lr_sc) {
> +			fnic_release_ioreq_buf(fnic, io_req, sc);
> +			mempool_free(io_req, fnic->io_req_pool);
> +		}
> +
> +		/*
> +		 * Any IO is returned during reset, it needs to call scsi_done
> +		 * to return the scsi_cmnd to upper layer.
> +		 */
> +		if (sc->scsi_done) {
> +			/* Set result to let upper SCSI layer retry */
> +			sc->result = DID_RESET << 16;
> +			sc->scsi_done(sc);
> +		}
>  	}
>  
>  	schedule_timeout(msecs_to_jiffies(2 * fnic->config.ed_tov));
> @@ -2267,6 +2285,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
>  	int tag = 0;
>  	DECLARE_COMPLETION_ONSTACK(tm_done);
>  	int tag_gen_flag = 0;   /*to track tags allocated by fnic driver*/
> +	bool new_sc = 0;
>  
>  	/* Wait for rport to unblock */
>  	fc_block_scsi_eh(sc);
> @@ -2312,13 +2331,12 @@ int fnic_device_reset(struct scsi_cmnd *sc)
>  		 * fix the way the EH ioctls work for real, but until
>  		 * that happens we fail these explicit requests here.
>  		 */
> -		if (shost_use_blk_mq(sc->device->host))
> -			goto fnic_device_reset_end;
>  
>  		tag = fnic_scsi_host_start_tag(fnic, sc);
>  		if (unlikely(tag == SCSI_NO_TAG))
>  			goto fnic_device_reset_end;
>  		tag_gen_flag = 1;
> +		new_sc=1;
>  	}
>  	io_lock = fnic_io_lock_hash(fnic, sc);
>  	spin_lock_irqsave(io_lock, flags);
> @@ -2453,7 +2471,7 @@ int fnic_device_reset(struct scsi_cmnd *sc)
>  	 * the lun reset cmd. If all cmds get cleaned, the lun reset
>  	 * succeeds
>  	 */
> -	if (fnic_clean_pending_aborts(fnic, sc)) {
> +	if (fnic_clean_pending_aborts(fnic, sc, new_sc)) {
>  		spin_lock_irqsave(io_lock, flags);
>  		io_req = (struct fnic_io_req *)CMD_SP(sc);
>  		FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,

For v1:

Reviewed-by: Ewan D. Milne <emilne@redhat.com>



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

* Re: [PATCH v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup.
  2016-03-18 18:22 ` [PATCH v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup Satish Kharat
@ 2016-03-29 14:06   ` Ewan D. Milne
  0 siblings, 0 replies; 7+ messages in thread
From: Ewan D. Milne @ 2016-03-29 14:06 UTC (permalink / raw)
  To: Satish Kharat; +Cc: linux-scsi, Sesidhar Baddela

On Fri, 2016-03-18 at 11:22 -0700, Satish Kharat wrote:
> When issuing I/O we check if rport is online through libfc
> rport_lookup() function which needs to be protected by mutex lock
> that cannot acquired in I/O context. The change is to use midlayer
> remote port’s dd_data which is preserved until its devloss timeout
> and no protection is required.
> The the scsi_cmnd error code is expected to be in the left 16 bits
> of the result field. Changed to correct this.
> Fnic driver version changed from 1.6.0.20 to 1.6.0.21
> 
> Signed-off-by: Satish Kharat <satishkh@cisco.com>
> Signed-off-by: Sesidhar Baddela <sebaddel@cisco.com>
> ---
>  * v1
>  - DID_NO_CONNECT left shifted 16 bits before assigning to sc->result
> ---
>  drivers/scsi/fnic/fnic.h      |  2 +-
>  drivers/scsi/fnic/fnic_scsi.c | 20 +++++++++++---------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index 1023eae..9ddc920 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.20"
> +#define DRV_VERSION		"1.6.0.21"
>  #define PFX			DRV_NAME ": "
>  #define DFX                     DRV_NAME "%d: "
>  
> diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
> index 026b93d..0e874a5 100644
> --- a/drivers/scsi/fnic/fnic_scsi.c
> +++ b/drivers/scsi/fnic/fnic_scsi.c
> @@ -439,7 +439,6 @@ static int fnic_queuecommand_lck(struct scsi_cmnd *sc, void (*done)(struct scsi_
>  	int sg_count = 0;
>  	unsigned long flags = 0;
>  	unsigned long ptr;
> -	struct fc_rport_priv *rdata;
>  	spinlock_t *io_lock = NULL;
>  	int io_lock_acquired = 0;
>  
> @@ -455,14 +454,17 @@ static int fnic_queuecommand_lck(struct scsi_cmnd *sc, void (*done)(struct scsi_
>  		return 0;
>  	}
>  
> -	rdata = lp->tt.rport_lookup(lp, rport->port_id);
> -	if (!rdata || (rdata->rp_state == RPORT_ST_DELETE)) {
> -		FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
> -			"returning IO as rport is removed\n");
> -		atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
> -		sc->result = DID_NO_CONNECT;
> -		done(sc);
> -		return 0;
> +	if (rport) {
> +		struct fc_rport_libfc_priv *rp = rport->dd_data;
> +
> +		if (!rp || rp->rp_state != RPORT_ST_READY) {
> +			FNIC_SCSI_DBG(KERN_DEBUG, fnic->lport->host,
> +				"returning DID_NO_CONNECT for IO as rport is removed\n");
> +			atomic64_inc(&fnic_stats->misc_stats.rport_not_ready);
> +			sc->result = DID_NO_CONNECT<<16;
> +			done(sc);
> +			return 0;
> +		}
>  	}
>  
>  	if (lp->state != LPORT_ST_READY || !(lp->link_up))

For v1:

Reviewed-by: Ewan D. Milne <emilne@redhat.com>



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer
  2016-03-18 18:22 [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Satish Kharat
                   ` (2 preceding siblings ...)
  2016-03-29 14:06 ` [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Ewan D. Milne
@ 2016-03-30  0:46 ` Martin K. Petersen
  3 siblings, 0 replies; 7+ messages in thread
From: Martin K. Petersen @ 2016-03-30  0:46 UTC (permalink / raw)
  To: Satish Kharat; +Cc: linux-scsi, Sesidhar Baddela, Ewan Milne

>>>>> "Satish" == Satish Kharat <satishkh@cisco.com> writes:

Satish,

Satish> If an I/O times out and an abort issued by host, if the abort is
Satish> successful we need to set scsi status as DID_ABORT. Or else the
Satish> mid-layer error handler which looks for this error code, will
Satish> offline the device. Also if the original I/O is not found in
Satish> fnic firmware, we will consider the abort as successful.  The
Satish> start_time assignment is moved because of the new goto.  Fnic
Satish> driver version changed from 1.6.0.17a to 1.6.0.19, version
Satish> 1.6.0.18 has been skipped

I applied patches 1-3 to 4.7/scsi-queue.

In the future, please make sure you add an "fnic:" prefix to the patch
description so it's clear in the commit log which driver is being
modified.

Also, always run checkpatch before submitting. I had to fix up a few
warnings.

Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

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

end of thread, other threads:[~2016-03-30  0:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-18 18:22 [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Satish Kharat
2016-03-18 18:22 ` [PATCH v1 2/3] Cleanup the I/O pending with fw and has timed out and is used to issue LUN reset Satish Kharat
2016-03-29 14:06   ` Ewan D. Milne
2016-03-18 18:22 ` [PATCH v1 3/3] Using rport->dd_data to check rport online instead of rport_lookup Satish Kharat
2016-03-29 14:06   ` Ewan D. Milne
2016-03-29 14:06 ` [PATCH v1 1/3] Fix to cleanup aborted IO to avoid device being offlined by mid-layer Ewan D. Milne
2016-03-30  0:46 ` 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.