diff mbox series

[v3,2/7] qedf: Fix for the deviations from the SAM-4 spec.

Message ID 20200403120957.2431-3-skashyap@marvell.com (mailing list archive)
State Superseded
Headers show
Series qed/qedf: Firmware recovery, bw update and misc fixes. | expand

Commit Message

Saurav Kashyap April 3, 2020, 12:09 p.m. UTC 
From: Javed Hasan <jhasan@marvell.com>

- Upper limit for retry delay(QEDF_RETRY_DELAY_MAX)
  increased from 20 sec to 1 min.
- Log an event/message indicating throttling of I/O
  for the target and include scope and retry delay
  time returned by the target and the driver enforced delay.
- Synchronizing the update of the fcport->retry_delay_timestamp
  between qedf_queuecommand() and qedf_scsi_completion().

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Javed Hasan <jhasan@marvell.com>
---
 drivers/scsi/qedf/qedf.h    |  2 +-
 drivers/scsi/qedf/qedf_io.c | 47 +++++++++++++++++++++++++++++++++++----------
 2 files changed, 38 insertions(+), 11 deletions(-)

Comments

Martin K. Petersen April 14, 2020, 1:22 a.m. UTC | #1 
Saurav,

This should be 3 patches since there are 3 different functional
changes.

> - Upper limit for retry delay(QEDF_RETRY_DELAY_MAX)
>   increased from 20 sec to 1 min.

> - Log an event/message indicating throttling of I/O
>   for the target and include scope and retry delay
>   time returned by the target and the driver enforced delay.

> - Synchronizing the update of the fcport->retry_delay_timestamp
>   between qedf_queuecommand() and qedf_scsi_completion().

"Synchronize".

Please describe why this needs to be synchronized.
Saurav Kashyap April 14, 2020, 4:20 a.m. UTC | #2 
Hi Martin,

> -----Original Message-----
> From: Martin K. Petersen <martin.petersen@oracle.com>
> Sent: Tuesday, April 14, 2020 6:53 AM
> To: Saurav Kashyap <skashyap@marvell.com>
> Cc: martin.petersen@oracle.com; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@marvell.com>; linux-scsi@vger.kernel.org; Javed Hasan
> <jhasan@marvell.com>; netdev@vger.kernel.org
> Subject: [EXT] Re: [PATCH v3 2/7] qedf: Fix for the deviations from the SAM-4
> spec.
> 
> External Email
> 
> ----------------------------------------------------------------------
> 
> Saurav,
> 
> This should be 3 patches since there are 3 different functional
> changes.
> 
> > - Upper limit for retry delay(QEDF_RETRY_DELAY_MAX)
> >   increased from 20 sec to 1 min.
> 
> > - Log an event/message indicating throttling of I/O
> >   for the target and include scope and retry delay
> >   time returned by the target and the driver enforced delay.
> 
> > - Synchronizing the update of the fcport->retry_delay_timestamp
> >   between qedf_queuecommand() and qedf_scsi_completion().
> 
> "Synchronize".
> 
> Please describe why this needs to be synchronized.

<SK> Sure, Shall I need to submit full patch set again?

Thanks,
~Saurav
> 
> --
> Martin K. Petersen	Oracle Linux Engineering
diff mbox series

Patch

diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
index 042ebf6..aaa2ac9 100644
--- a/drivers/scsi/qedf/qedf.h
+++ b/drivers/scsi/qedf/qedf.h
@@ -470,7 +470,7 @@  static inline void qedf_stop_all_io(struct qedf_ctx *qedf)
 extern uint qedf_io_tracing;
 extern uint qedf_stop_io_on_error;
 extern uint qedf_link_down_tmo;
-#define QEDF_RETRY_DELAY_MAX		20 /* 2 seconds */
+#define QEDF_RETRY_DELAY_MAX		600 /* 60 seconds */
 extern bool qedf_retry_delay;
 extern uint qedf_debug;
 
diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index e749a2d..f0f455e 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -1021,14 +1021,18 @@  int qedf_post_io_req(struct qedf_rport *fcport, struct qedf_ioreq *io_req)
 	atomic_inc(&fcport->ios_to_queue);
 
 	if (fcport->retry_delay_timestamp) {
+		/* Take fcport->rport_lock for resetting the delay_timestamp */
+		spin_lock_irqsave(&fcport->rport_lock, flags);
 		if (time_after(jiffies, fcport->retry_delay_timestamp)) {
 			fcport->retry_delay_timestamp = 0;
 		} else {
+			spin_unlock_irqrestore(&fcport->rport_lock, flags);
 			/* If retry_delay timer is active, flow off the ML */
 			rc = SCSI_MLQUEUE_TARGET_BUSY;
 			atomic_dec(&fcport->ios_to_queue);
 			goto exit_qcmd;
 		}
+		spin_unlock_irqrestore(&fcport->rport_lock, flags);
 	}
 
 	io_req = qedf_alloc_cmd(fcport, QEDF_SCSI_CMD);
@@ -1134,6 +1138,8 @@  void qedf_scsi_completion(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
 	int refcount;
 	u16 scope, qualifier = 0;
 	u8 fw_residual_flag = 0;
+	unsigned long flags = 0;
+	u16 chk_scope = 0;
 
 	if (!io_req)
 		return;
@@ -1267,16 +1273,8 @@  void qedf_scsi_completion(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
 				/* Lower 14 bits */
 				qualifier = fcp_rsp->retry_delay_timer & 0x3FFF;
 
-				if (qedf_retry_delay &&
-				    scope > 0 && qualifier > 0 &&
-				    qualifier <= 0x3FEF) {
-					/* Check we don't go over the max */
-					if (qualifier > QEDF_RETRY_DELAY_MAX)
-						qualifier =
-						    QEDF_RETRY_DELAY_MAX;
-					fcport->retry_delay_timestamp =
-					    jiffies + (qualifier * HZ / 10);
-				}
+				if (qedf_retry_delay)
+					chk_scope = 1;
 				/* Record stats */
 				if (io_req->cdb_status ==
 				    SAM_STAT_TASK_SET_FULL)
@@ -1287,6 +1285,35 @@  void qedf_scsi_completion(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
 		}
 		if (io_req->fcp_resid)
 			scsi_set_resid(sc_cmd, io_req->fcp_resid);
+
+		if (chk_scope == 1)
+			if ((scope == 1 || scope == 2) &&
+			    (qualifier > 0 && qualifier <= 0x3FEF)) {
+				/* Check we don't go over the max */
+				if (qualifier > QEDF_RETRY_DELAY_MAX) {
+					qualifier = QEDF_RETRY_DELAY_MAX;
+					QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_IO,
+						  "qualifier = %d\n",
+						  (fcp_rsp->retry_delay_timer &
+						  0x3FFF));
+				}
+				QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_IO,
+					  "Scope = %d and qualifier = %d",
+					  scope, qualifier);
+				/*  Take fcport->rport_lock to
+				 *  update the retry_delay_timestamp
+				 */
+				spin_lock_irqsave(&fcport->rport_lock, flags);
+				fcport->retry_delay_timestamp =
+					jiffies + (qualifier * HZ / 10);
+				spin_unlock_irqrestore(&fcport->rport_lock,
+						       flags);
+
+			} else {
+				QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_IO,
+					  "combination of scope = %d and qualifier = %d is not handled in qedf.\n",
+					  scope, qualifier);
+			}
 		break;
 	default:
 		QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_IO, "fcp_status=%d.\n",