diff mbox series

[04/10] qla2xxx: Fix command flush during TMF

Message ID 20230712090535.34894-5-njavali@marvell.com
State Superseded
Headers show
Series qla2xxx driver bug fixes | expand

Commit Message

Nilesh Javali July 12, 2023, 9:05 a.m. UTC
From: Quinn Tran <qutran@marvell.com>

For each TMF request, driver iterate through each qpair and flush
commands associated to the TMF. At the end of the qpair flush, a
Marker is used to complete the flush transaction. This process
was repeated for each qpair. The multiple flush and marker for this
TMF request seems to cause confusion for FW.

Instead, 1 flush is send to FW. Driver would wait for FW to go through
all the IO's on each qpair to be read then return. Driver then close out
the transaction with a Marker.

Cc: stable@vger.kernel.org
Fixes: d90171dd0da5 ("scsi: qla2xxx: Multi-que support for TMF")
Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_init.c | 74 +++++++++++++++++----------------
 drivers/scsi/qla2xxx/qla_iocb.c |  1 +
 drivers/scsi/qla2xxx/qla_os.c   |  9 ++--
 3 files changed, 45 insertions(+), 39 deletions(-)

Comments

Himanshu Madhani July 13, 2023, 7:24 p.m. UTC | #1
> On Jul 12, 2023, at 2:05 AM, Nilesh Javali <njavali@marvell.com> wrote:
> 
> From: Quinn Tran <qutran@marvell.com>
> 
> For each TMF request, driver iterate through each qpair and flush
> commands associated to the TMF. At the end of the qpair flush, a
> Marker is used to complete the flush transaction. This process
> was repeated for each qpair. The multiple flush and marker for this
> TMF request seems to cause confusion for FW.
> 
> Instead, 1 flush is send to FW. Driver would wait for FW to go through
> all the IO's on each qpair to be read then return. Driver then close out
> the transaction with a Marker.
> 
> Cc: stable@vger.kernel.org
> Fixes: d90171dd0da5 ("scsi: qla2xxx: Multi-que support for TMF")
> Signed-off-by: Quinn Tran <qutran@marvell.com>
> Signed-off-by: Nilesh Javali <njavali@marvell.com>
> ---
> drivers/scsi/qla2xxx/qla_init.c | 74 +++++++++++++++++----------------
> drivers/scsi/qla2xxx/qla_iocb.c |  1 +
> drivers/scsi/qla2xxx/qla_os.c   |  9 ++--
> 3 files changed, 45 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
> index 5ec6f01ca635..3b32e65d6260 100644
> --- a/drivers/scsi/qla2xxx/qla_init.c
> +++ b/drivers/scsi/qla2xxx/qla_init.c
> @@ -2002,12 +2002,11 @@ qla2x00_tmf_iocb_timeout(void *data)
> int rc, h;
> unsigned long flags;
> 
> - if (sp->type == SRB_MARKER) {
> - complete(&tmf->u.tmf.comp);
> - return;
> - }
> + if (sp->type == SRB_MARKER)
> + rc = QLA_FUNCTION_FAILED;
> + else
> + rc = qla24xx_async_abort_cmd(sp, false);
> 
> - rc = qla24xx_async_abort_cmd(sp, false);
> if (rc) {
> spin_lock_irqsave(sp->qpair->qp_lock_ptr, flags);
> for (h = 1; h < sp->qpair->req->num_outstanding_cmds; h++) {
> @@ -2129,6 +2128,17 @@ static void qla2x00_tmf_sp_done(srb_t *sp, int res)
> complete(&tmf->u.tmf.comp);
> }
> 
> +static int qla_tmf_wait(struct tmf_arg *arg)
> +{
> + /* there are only 2 types of error handling that reaches here, lun or target reset */
> + if (arg->flags & (TCF_LUN_RESET | TCF_ABORT_TASK_SET | TCF_CLEAR_TASK_SET))
> + return qla2x00_eh_wait_for_pending_commands(arg->vha,
> +    arg->fcport->d_id.b24, arg->lun, WAIT_LUN);
> + else
> + return qla2x00_eh_wait_for_pending_commands(arg->vha,
> +    arg->fcport->d_id.b24, arg->lun, WAIT_TARGET);
> +}
> +
> static int
> __qla2x00_async_tm_cmd(struct tmf_arg *arg)
> {
> @@ -2136,8 +2146,9 @@ __qla2x00_async_tm_cmd(struct tmf_arg *arg)
> struct srb_iocb *tm_iocb;
> srb_t *sp;
> int rval = QLA_FUNCTION_FAILED;
> -
> fc_port_t *fcport = arg->fcport;
> + u32 chip_gen, login_gen;
> + u64 jif;
> 
> if (TMF_NOT_READY(arg->fcport)) {
> ql_dbg(ql_dbg_taskm, vha, 0x8032,
> @@ -2182,8 +2193,27 @@ __qla2x00_async_tm_cmd(struct tmf_arg *arg)
>    "TM IOCB failed (%x).\n", rval);
> }
> 
> - if (!test_bit(UNLOADING, &vha->dpc_flags) && !IS_QLAFX00(vha->hw))
> - rval = qla26xx_marker(arg);
> + if (!test_bit(UNLOADING, &vha->dpc_flags) && !IS_QLAFX00(vha->hw)) {
> + chip_gen = vha->hw->chip_reset;
> + login_gen = fcport->login_gen;
> +
> + jif = jiffies;
> + if (qla_tmf_wait(arg)) {
> + ql_log(ql_log_info, vha, 0x803e,
> +       "Waited %u ms Nexus=%ld:%06x:%llu.\n",
> +       jiffies_to_msecs(jiffies - jif), vha->host_no,
> +       fcport->d_id.b24, arg->lun);
> + }
> +
> + if (chip_gen == vha->hw->chip_reset && login_gen == fcport->login_gen) {
> + rval = qla26xx_marker(arg);
> + } else {
> + ql_log(ql_log_info, vha, 0x803e,
> +       "Skip Marker due to disruption. Nexus=%ld:%06x:%llu.\n",
> +       vha->host_no, fcport->d_id.b24, arg->lun);
> + rval = QLA_FUNCTION_FAILED;
> + }
> + }
> 
> done_free_sp:
> /* ref: INIT */
> @@ -2261,9 +2291,8 @@ qla2x00_async_tm_cmd(fc_port_t *fcport, uint32_t flags, uint64_t lun,
>     uint32_t tag)
> {
> struct scsi_qla_host *vha = fcport->vha;
> - struct qla_qpair *qpair;
> struct tmf_arg a;
> - int i, rval = QLA_SUCCESS;
> + int rval = QLA_SUCCESS;
> 
> if (TMF_NOT_READY(fcport))
> return QLA_SUSPENDED;
> @@ -2283,34 +2312,9 @@ qla2x00_async_tm_cmd(fc_port_t *fcport, uint32_t flags, uint64_t lun,
> if (qla_get_tmf(&a))
> return QLA_FUNCTION_FAILED;
> 
> - if (vha->hw->mqenable) {
> - for (i = 0; i < vha->hw->num_qpairs; i++) {
> - qpair = vha->hw->queue_pair_map[i];
> - if (!qpair)
> - continue;
> -
> - if (TMF_NOT_READY(fcport)) {
> - ql_log(ql_log_warn, vha, 0x8026,
> -    "Unable to send TM due to disruption.\n");
> - rval = QLA_SUSPENDED;
> - break;
> - }
> -
> - a.qpair = qpair;
> - a.flags = flags|TCF_NOTMCMD_TO_TARGET;
> - rval = __qla2x00_async_tm_cmd(&a);
> - if (rval)
> - break;
> - }
> - }
> -
> - if (rval)
> - goto bailout;
> -
> a.qpair = vha->hw->base_qpair;
> rval = __qla2x00_async_tm_cmd(&a);
> 
> -bailout:
> qla_put_tmf(&a);
> return rval;
> }
> diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
> index a1675f056a5c..1c6e300ed3ab 100644
> --- a/drivers/scsi/qla2xxx/qla_iocb.c
> +++ b/drivers/scsi/qla2xxx/qla_iocb.c
> @@ -3881,6 +3881,7 @@ qla_marker_iocb(srb_t *sp, struct mrk_entry_24xx *mrk)
> {
> mrk->entry_type = MARKER_TYPE;
> mrk->modifier = sp->u.iocb_cmd.u.tmf.modifier;
> + mrk->handle = make_handle(sp->qpair->req->id, sp->handle);
> if (sp->u.iocb_cmd.u.tmf.modifier != MK_SYNC_ALL) {
> mrk->nport_handle = cpu_to_le16(sp->u.iocb_cmd.u.tmf.loop_id);
> int_to_scsilun(sp->u.iocb_cmd.u.tmf.lun, (struct scsi_lun *)&mrk->lun);
> diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
> index 47bbc8b321f8..03bc3a0b45b6 100644
> --- a/drivers/scsi/qla2xxx/qla_os.c
> +++ b/drivers/scsi/qla2xxx/qla_os.c
> @@ -1488,8 +1488,9 @@ qla2xxx_eh_device_reset(struct scsi_cmnd *cmd)
> goto eh_reset_failed;
> }
> err = 3;
> - if (qla2x00_eh_wait_for_pending_commands(vha, sdev->id,
> -    sdev->lun, WAIT_LUN) != QLA_SUCCESS) {
> + if (qla2x00_eh_wait_for_pending_commands(vha, fcport->d_id.b24,
> + cmd->device->lun,
> + WAIT_LUN) != QLA_SUCCESS) {
> ql_log(ql_log_warn, vha, 0x800d,
>    "wait for pending cmds failed for cmd=%p.\n", cmd);
> goto eh_reset_failed;
> @@ -1555,8 +1556,8 @@ qla2xxx_eh_target_reset(struct scsi_cmnd *cmd)
> goto eh_reset_failed;
> }
> err = 3;
> - if (qla2x00_eh_wait_for_pending_commands(vha, sdev->id,
> -    0, WAIT_TARGET) != QLA_SUCCESS) {
> + if (qla2x00_eh_wait_for_pending_commands(vha, fcport->d_id.b24, 0,
> + WAIT_TARGET) != QLA_SUCCESS) {
> ql_log(ql_log_warn, vha, 0x800d,
>    "wait for pending cmds failed for cmd=%p.\n", cmd);
> goto eh_reset_failed;
> -- 
> 2.23.1
> 

Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
diff mbox series

Patch

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 5ec6f01ca635..3b32e65d6260 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -2002,12 +2002,11 @@  qla2x00_tmf_iocb_timeout(void *data)
 	int rc, h;
 	unsigned long flags;
 
-	if (sp->type == SRB_MARKER) {
-		complete(&tmf->u.tmf.comp);
-		return;
-	}
+	if (sp->type == SRB_MARKER)
+		rc = QLA_FUNCTION_FAILED;
+	else
+		rc = qla24xx_async_abort_cmd(sp, false);
 
-	rc = qla24xx_async_abort_cmd(sp, false);
 	if (rc) {
 		spin_lock_irqsave(sp->qpair->qp_lock_ptr, flags);
 		for (h = 1; h < sp->qpair->req->num_outstanding_cmds; h++) {
@@ -2129,6 +2128,17 @@  static void qla2x00_tmf_sp_done(srb_t *sp, int res)
 	complete(&tmf->u.tmf.comp);
 }
 
+static int qla_tmf_wait(struct tmf_arg *arg)
+{
+	/* there are only 2 types of error handling that reaches here, lun or target reset */
+	if (arg->flags & (TCF_LUN_RESET | TCF_ABORT_TASK_SET | TCF_CLEAR_TASK_SET))
+		return qla2x00_eh_wait_for_pending_commands(arg->vha,
+		    arg->fcport->d_id.b24, arg->lun, WAIT_LUN);
+	else
+		return qla2x00_eh_wait_for_pending_commands(arg->vha,
+		    arg->fcport->d_id.b24, arg->lun, WAIT_TARGET);
+}
+
 static int
 __qla2x00_async_tm_cmd(struct tmf_arg *arg)
 {
@@ -2136,8 +2146,9 @@  __qla2x00_async_tm_cmd(struct tmf_arg *arg)
 	struct srb_iocb *tm_iocb;
 	srb_t *sp;
 	int rval = QLA_FUNCTION_FAILED;
-
 	fc_port_t *fcport = arg->fcport;
+	u32 chip_gen, login_gen;
+	u64 jif;
 
 	if (TMF_NOT_READY(arg->fcport)) {
 		ql_dbg(ql_dbg_taskm, vha, 0x8032,
@@ -2182,8 +2193,27 @@  __qla2x00_async_tm_cmd(struct tmf_arg *arg)
 		    "TM IOCB failed (%x).\n", rval);
 	}
 
-	if (!test_bit(UNLOADING, &vha->dpc_flags) && !IS_QLAFX00(vha->hw))
-		rval = qla26xx_marker(arg);
+	if (!test_bit(UNLOADING, &vha->dpc_flags) && !IS_QLAFX00(vha->hw)) {
+		chip_gen = vha->hw->chip_reset;
+		login_gen = fcport->login_gen;
+
+		jif = jiffies;
+		if (qla_tmf_wait(arg)) {
+			ql_log(ql_log_info, vha, 0x803e,
+			       "Waited %u ms Nexus=%ld:%06x:%llu.\n",
+			       jiffies_to_msecs(jiffies - jif), vha->host_no,
+			       fcport->d_id.b24, arg->lun);
+		}
+
+		if (chip_gen == vha->hw->chip_reset && login_gen == fcport->login_gen) {
+			rval = qla26xx_marker(arg);
+		} else {
+			ql_log(ql_log_info, vha, 0x803e,
+			       "Skip Marker due to disruption. Nexus=%ld:%06x:%llu.\n",
+			       vha->host_no, fcport->d_id.b24, arg->lun);
+			rval = QLA_FUNCTION_FAILED;
+		}
+	}
 
 done_free_sp:
 	/* ref: INIT */
@@ -2261,9 +2291,8 @@  qla2x00_async_tm_cmd(fc_port_t *fcport, uint32_t flags, uint64_t lun,
 		     uint32_t tag)
 {
 	struct scsi_qla_host *vha = fcport->vha;
-	struct qla_qpair *qpair;
 	struct tmf_arg a;
-	int i, rval = QLA_SUCCESS;
+	int rval = QLA_SUCCESS;
 
 	if (TMF_NOT_READY(fcport))
 		return QLA_SUSPENDED;
@@ -2283,34 +2312,9 @@  qla2x00_async_tm_cmd(fc_port_t *fcport, uint32_t flags, uint64_t lun,
 	if (qla_get_tmf(&a))
 		return QLA_FUNCTION_FAILED;
 
-	if (vha->hw->mqenable) {
-		for (i = 0; i < vha->hw->num_qpairs; i++) {
-			qpair = vha->hw->queue_pair_map[i];
-			if (!qpair)
-				continue;
-
-			if (TMF_NOT_READY(fcport)) {
-				ql_log(ql_log_warn, vha, 0x8026,
-				    "Unable to send TM due to disruption.\n");
-				rval = QLA_SUSPENDED;
-				break;
-			}
-
-			a.qpair = qpair;
-			a.flags = flags|TCF_NOTMCMD_TO_TARGET;
-			rval = __qla2x00_async_tm_cmd(&a);
-			if (rval)
-				break;
-		}
-	}
-
-	if (rval)
-		goto bailout;
-
 	a.qpair = vha->hw->base_qpair;
 	rval = __qla2x00_async_tm_cmd(&a);
 
-bailout:
 	qla_put_tmf(&a);
 	return rval;
 }
diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index a1675f056a5c..1c6e300ed3ab 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -3881,6 +3881,7 @@  qla_marker_iocb(srb_t *sp, struct mrk_entry_24xx *mrk)
 {
 	mrk->entry_type = MARKER_TYPE;
 	mrk->modifier = sp->u.iocb_cmd.u.tmf.modifier;
+	mrk->handle = make_handle(sp->qpair->req->id, sp->handle);
 	if (sp->u.iocb_cmd.u.tmf.modifier != MK_SYNC_ALL) {
 		mrk->nport_handle = cpu_to_le16(sp->u.iocb_cmd.u.tmf.loop_id);
 		int_to_scsilun(sp->u.iocb_cmd.u.tmf.lun, (struct scsi_lun *)&mrk->lun);
diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
index 47bbc8b321f8..03bc3a0b45b6 100644
--- a/drivers/scsi/qla2xxx/qla_os.c
+++ b/drivers/scsi/qla2xxx/qla_os.c
@@ -1488,8 +1488,9 @@  qla2xxx_eh_device_reset(struct scsi_cmnd *cmd)
 		goto eh_reset_failed;
 	}
 	err = 3;
-	if (qla2x00_eh_wait_for_pending_commands(vha, sdev->id,
-	    sdev->lun, WAIT_LUN) != QLA_SUCCESS) {
+	if (qla2x00_eh_wait_for_pending_commands(vha, fcport->d_id.b24,
+						 cmd->device->lun,
+						 WAIT_LUN) != QLA_SUCCESS) {
 		ql_log(ql_log_warn, vha, 0x800d,
 		    "wait for pending cmds failed for cmd=%p.\n", cmd);
 		goto eh_reset_failed;
@@ -1555,8 +1556,8 @@  qla2xxx_eh_target_reset(struct scsi_cmnd *cmd)
 		goto eh_reset_failed;
 	}
 	err = 3;
-	if (qla2x00_eh_wait_for_pending_commands(vha, sdev->id,
-	    0, WAIT_TARGET) != QLA_SUCCESS) {
+	if (qla2x00_eh_wait_for_pending_commands(vha, fcport->d_id.b24, 0,
+						 WAIT_TARGET) != QLA_SUCCESS) {
 		ql_log(ql_log_warn, vha, 0x800d,
 		    "wait for pending cmds failed for cmd=%p.\n", cmd);
 		goto eh_reset_failed;