[PATCH 0/7] qedf: Misc fixes for the driver.

[PATCH 0/7] qedf: Misc fixes for the driver.

[Javed Hasan]

This series has misc bug fixes and code enhancements.

Kindly apply this series to scsi-queue at your earliest convenience.

Thanks,
~Javed

Saurav Kashyap (7):
  qedf: Check for port type and role before processing an event.
  qedf: check the validity of rjt frame before processing.
  qedf: Do not kill timeout work for original IO on RRQ completion.
  qedf: Send cleanup even for RRQ on timeout.
  qedf: Initiate cleanup for ELS commands as well.
  qedf: Don't process ELS completion if its flushed or cleaned up.
  qedf: Fix race between els completion and flushing els request.

 drivers/scsi/qedf/qedf_els.c  | 32 ++++++++++++++++++++++++++++----
 drivers/scsi/qedf/qedf_io.c   | 11 ++++++++++-
 drivers/scsi/qedf/qedf_main.c | 11 +++++++++++
 3 files changed, 49 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[PATCH 1/7] qedf: Check for port type and role before processing an event.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

 The rport lock get initialized during offload. If the no FCP or
 non target rport got logout then this rport will be uninitiazed.
 Kasan was complaining because of it.
=========
[   14.384434] the code is fine but needs lockdep annotation.
[   14.384482] turning off the locking correctness validator.
========

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Javed Hasan <jhasan@marvell.com>
---
 drivers/scsi/qedf/qedf_main.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 47fc14f..86b9479 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -1536,6 +1536,17 @@ static void qedf_rport_event_handler(struct fc_lport *lport,
 		if (port_id == FC_FID_DIR_SERV)
 			break;
 
+		if (rdata->spp_type != FC_TYPE_FCP) {
+			QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
+			    "No action since spp type isn't FCP\n");
+			break;
+		}
+		if (!(rdata->ids.roles & FC_RPORT_ROLE_FCP_TARGET)) {
+			QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
+			    "Not FCP target so no action\n");
+			break;
+		}
+
 		if (!rport) {
 			QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC,
 			    "port_id=%x - rport notcreated Yet!!\n", port_id);
-- 
1.8.3.1

[PATCH 2/7] qedf: check the validity of rjt frame before processing.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

This is reported by Klockwork.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Change-Id: I22a440fc4834bca858570dff8a3e41933121f831
---
 drivers/scsi/qedf/qedf_els.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/qedf/qedf_els.c b/drivers/scsi/qedf/qedf_els.c
index 542ba94..ab4b1a9 100644
--- a/drivers/scsi/qedf/qedf_els.c
+++ b/drivers/scsi/qedf/qedf_els.c
@@ -883,6 +883,11 @@ static void qedf_rec_compl(struct qedf_els_cb_arg *cb_arg)
 	opcode = fc_frame_payload_op(fp);
 	if (opcode == ELS_LS_RJT) {
 		rjt = fc_frame_payload_get(fp, sizeof(*rjt));
+		if (!rjt) {
+			QEDF_ERR(&qedf->dbg_ctx, "payload get failed");
+			goto out_free_frame;
+		}
+
 		QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS,
 		    "Received LS_RJT for REC: er_reason=0x%x, "
 		    "er_explan=0x%x.\n", rjt->er_reason, rjt->er_explan);
-- 
1.8.3.1

[PATCH 3/7] qedf: Do not kill timeout work for original IO on RRQ completion.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

 -The timer is already cancelled when abort get completed, hence
  no need of cancelling it again.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Javed Hasan <jhasan@marvell.com>
---
 drivers/scsi/qedf/qedf_els.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/scsi/qedf/qedf_els.c b/drivers/scsi/qedf/qedf_els.c
index ab4b1a9..edd6702 100644
--- a/drivers/scsi/qedf/qedf_els.c
+++ b/drivers/scsi/qedf/qedf_els.c
@@ -185,10 +185,6 @@ static void qedf_rrq_compl(struct qedf_els_cb_arg *cb_arg)
 		goto out_free;
 	}
 
-	if (rrq_req->event != QEDF_IOREQ_EV_ELS_TMO &&
-	    rrq_req->event != QEDF_IOREQ_EV_ELS_ERR_DETECT)
-		cancel_delayed_work_sync(&orig_io_req->timeout_work);
-
 	refcount = kref_read(&orig_io_req->refcount);
 	QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS, "rrq_compl: orig io = %p,"
 		   " orig xid = 0x%x, rrq_xid = 0x%x, refcount=%d\n",
-- 
1.8.3.1

[PATCH 4/7] qedf: Send cleanup even for RRQ on timeout.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

 -Send cleanup even for RRQ on timeout.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Javed Hasan <jhasan@marvell.com>
---
 drivers/scsi/qedf/qedf_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index acd9774..26d11cc 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -85,13 +85,13 @@ static void qedf_cmd_timeout(struct work_struct *work)
 		 */
 		QEDF_ERR(&(qedf->dbg_ctx), "ELS timeout, xid=0x%x.\n",
 			  io_req->xid);
+		qedf_initiate_cleanup(io_req, true);
 		io_req->event = QEDF_IOREQ_EV_ELS_TMO;
 		/* Call callback function to complete command */
 		if (io_req->cb_func && io_req->cb_arg) {
 			io_req->cb_func(io_req->cb_arg);
 			io_req->cb_arg = NULL;
 		}
-		qedf_initiate_cleanup(io_req, true);
 		kref_put(&io_req->refcount, qedf_release_cmd);
 		break;
 	case QEDF_SEQ_CLEANUP:
-- 
1.8.3.1

[PATCH 5/7] qedf: Initiate cleanup for ELS commands as well.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

 Initiate cleanup for ELS commands as well.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Javed Hasan <jhasan@marvell.com>
---
 drivers/scsi/qedf/qedf_io.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index 26d11cc..7969f3a 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -1704,8 +1704,10 @@ void qedf_flush_active_ios(struct qedf_rport *fcport, int lun)
 				    io_req, io_req->xid);
 				continue;
 			}
+			qedf_initiate_cleanup(io_req, false);
 			flush_cnt++;
 			qedf_flush_els_req(qedf, io_req);
+
 			/*
 			 * Release the kref and go back to the top of the
 			 * loop.
@@ -2169,6 +2171,10 @@ int qedf_initiate_cleanup(struct qedf_ioreq *io_req,
 		return SUCCESS;
 	}
 
+	if (io_req->cmd_type == QEDF_ELS) {
+		goto process_els;
+	}
+
 	if (!test_bit(QEDF_CMD_OUTSTANDING, &io_req->flags) ||
 	    test_and_set_bit(QEDF_CMD_IN_CLEANUP, &io_req->flags)) {
 		QEDF_ERR(&(qedf->dbg_ctx), "io_req xid=0x%x already in "
@@ -2178,6 +2184,7 @@ int qedf_initiate_cleanup(struct qedf_ioreq *io_req,
 	}
 	set_bit(QEDF_CMD_IN_CLEANUP, &io_req->flags);
 
+process_els:
 	/* Ensure room on SQ */
 	if (!atomic_read(&fcport->free_sqes)) {
 		QEDF_ERR(&(qedf->dbg_ctx), "No SQ entries available\n");
-- 
1.8.3.1

[PATCH 6/7] qedf: Don't process ELS completion if its flushed or cleaned up.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

  - Don't process ELS completion if its flushed or cleaned up.

Signed-off-by: Saurav Kashyap <skashyap@marvell.com>
Signed-off-by: Javed Hasan <jhasan@marvell.com>
---
 drivers/scsi/qedf/qedf_els.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/qedf/qedf_els.c b/drivers/scsi/qedf/qedf_els.c
index edd6702..e2e80ea 100644
--- a/drivers/scsi/qedf/qedf_els.c
+++ b/drivers/scsi/qedf/qedf_els.c
@@ -147,6 +147,15 @@ void qedf_process_els_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
 	QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS, "Entered with xid = 0x%x"
 		   " cmd_type = %d.\n", els_req->xid, els_req->cmd_type);
 
+	if ((els_req->event == QEDF_IOREQ_EV_ELS_FLUSH)
+		|| (els_req->event == QEDF_IOREQ_EV_CLEANUP_SUCCESS)
+		|| (els_req->event == QEDF_IOREQ_EV_CLEANUP_FAILED)) {
+		QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_IO,
+			"ELS completion xid=0x%x after flush event=0x%x",
+			els_req->xid, els_req->event);
+		return;
+	}
+
 	clear_bit(QEDF_CMD_OUTSTANDING, &els_req->flags);
 
 	/* Kill the ELS timer */
-- 
1.8.3.1

[PATCH 7/7] qedf: Fix race between els completion and flushing els request.

[Javed Hasan]

From: Saurav Kashyap <skashyap@marvell.com>

   Fix race between els completion and flushing els
    request.

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

diff --git a/drivers/scsi/qedf/qedf_els.c b/drivers/scsi/qedf/qedf_els.c
index e2e80ea..6cb8c9b 100644
--- a/drivers/scsi/qedf/qedf_els.c
+++ b/drivers/scsi/qedf/qedf_els.c
@@ -143,6 +143,7 @@ void qedf_process_els_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
 	struct qedf_ioreq *els_req)
 {
 	struct fcoe_cqe_midpath_info *mp_info;
+	struct qedf_rport *fcport;
 
 	QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_ELS, "Entered with xid = 0x%x"
 		   " cmd_type = %d.\n", els_req->xid, els_req->cmd_type);
@@ -156,6 +157,19 @@ void qedf_process_els_compl(struct qedf_ctx *qedf, struct fcoe_cqe *cqe,
 		return;
 	}
 
+	fcport = els_req->fcport;
+
+	/* When flush is active,
+	 * let the cmds be completed from the cleanup context
+	 */
+	if (test_bit(QEDF_RPORT_IN_TARGET_RESET, &fcport->flags) ||
+		test_bit(QEDF_RPORT_IN_LUN_RESET, &fcport->flags)) {
+		QEDF_INFO(&qedf->dbg_ctx, QEDF_LOG_IO,
+			"Dropping ELS completion xid=0x%x as fcport is flushing",
+			els_req->xid);
+		return;
+	}
+
 	clear_bit(QEDF_CMD_OUTSTANDING, &els_req->flags);
 
 	/* Kill the ELS timer */
diff --git a/drivers/scsi/qedf/qedf_io.c b/drivers/scsi/qedf/qedf_io.c
index 7969f3a..86c8afb 100644
--- a/drivers/scsi/qedf/qedf_io.c
+++ b/drivers/scsi/qedf/qedf_io.c
@@ -1562,6 +1562,8 @@ static void qedf_flush_els_req(struct qedf_ctx *qedf,
 	 */
 	els_req->event = QEDF_IOREQ_EV_ELS_FLUSH;
 
+	clear_bit(QEDF_CMD_OUTSTANDING, &els_req->flags);
+
 	/* Cancel the timer */
 	cancel_delayed_work_sync(&els_req->timeout_work);
 
-- 
1.8.3.1

Re: [PATCH 0/7] qedf: Misc fixes for the driver.

[Martin K. Petersen]

On Fri, 7 Aug 2020 04:06:49 -0700, Javed Hasan wrote:

> This series has misc bug fixes and code enhancements.
> 
> Kindly apply this series to scsi-queue at your earliest convenience.
> 
> Thanks,
> ~Javed
> 
> [...]

Applied to 5.10/scsi-queue, thanks!

[1/7] scsi: qedf: Check for port type and role before processing an event
      https://git.kernel.org/mkp/scsi/c/04fedba774e8
[2/7] scsi: qedf: Check the validity of rjt frame before processing
      https://git.kernel.org/mkp/scsi/c/df89b0e5bfad
[3/7] scsi: qedf: Do not kill timeout work for original I/O on RRQ completion
      https://git.kernel.org/mkp/scsi/c/76693f4f1e2d
[4/7] scsi: qedf: Send cleanup even for RRQ on timeout
      https://git.kernel.org/mkp/scsi/c/8ea8f3eae8e5
[5/7] scsi: qedf: Initiate cleanup for ELS commands as well
      https://git.kernel.org/mkp/scsi/c/170ce800ed5c
[6/7] scsi: qedf: Don't process ELS completion if event is flushed or cleaned up
      https://git.kernel.org/mkp/scsi/c/49cf23e02935
[7/7] scsi: qedf: Fix race between ELS completion and flushing ELS request
      https://git.kernel.org/mkp/scsi/c/9acf1f771914

-- 
Martin K. Petersen	Oracle Linux Engineering