Re: [PATCH 1/7] Revert "qla2xxx: Make sure that aborted commands are freed"

On Mar 15, 2021, at 10:56 PM, Bart Van Assche [off-list ref] wrote:

Calling vha->hw->tgt.tgt_ops->free_cmd() from qlt_xmit_response() is wrong
since the command for which a response is sent must remain valid until the
SCSI target core calls .release_cmd().

Fixes: 0dcec41acb85 ("scsi: qla2xxx: Make sure that aborted commands are freed")
Cc: Quinn Tran <redacted>
Cc: Mike Christie <michael.christie@oracle.com>
Cc: Himanshu Madhani <redacted>
Cc: Daniel Wagner <redacted>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/qla2xxx/qla_target.c  | 13 +++++--------
drivers/scsi/qla2xxx/tcm_qla2xxx.c |  4 ----
2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index d6366a46283e..5e8b2653e134 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c

@@ -3222,8 +3222,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,

	if (!qpair->fw_started || (cmd->reset_count != qpair->chip_reset) ||
	    (cmd->sess && cmd->sess->deleted)) {
		cmd->state = QLA_TGT_STATE_PROCESSED;
-		res = 0;
-		goto free;
+		return 0;
	}

	ql_dbg_qp(ql_dbg_tgt, qpair, 0xe018,

@@ -3234,8 +3233,9 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,

	res = qlt_pre_xmit_response(cmd, &prm, xmit_type, scsi_status,
	    &full_req_cnt);
-	if (unlikely(res != 0))
-		goto free;
+	if (unlikely(res != 0)) {
+		return res;
+	}

	spin_lock_irqsave(qpair->qp_lock_ptr, flags);

@@ -3255,8 +3255,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,

			vha->flags.online, qla2x00_reset_active(vha),
			cmd->reset_count, qpair->chip_reset);
		spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
-		res = 0;
-		goto free;
+		return 0;
	}

	/* Does F/W have an IOCBs for this request */

@@ -3359,8 +3358,6 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,

	qlt_unmap_sg(vha, cmd);
	spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);

-free:
-	vha->hw->tgt.tgt_ops->free_cmd(cmd);
	return res;
}
EXPORT_SYMBOL(qlt_xmit_response);

diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 30959f8da065..15650a0bde09 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c

@@ -653,7 +653,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)

{
	struct qla_tgt_cmd *cmd = container_of(se_cmd,
				struct qla_tgt_cmd, se_cmd);
-	struct scsi_qla_host *vha = cmd->vha;

	if (cmd->aborted) {
		/* Cmd can loop during Q-full.  tcm_qla2xxx_aborted_task

@@ -666,7 +665,6 @@ static int tcm_qla2xxx_queue_data_in(struct se_cmd *se_cmd)

			cmd->se_cmd.transport_state,
			cmd->se_cmd.t_state,
			cmd->se_cmd.se_cmd_flags);
-		vha->hw->tgt.tgt_ops->free_cmd(cmd);
		return 0;
	}

@@ -694,7 +692,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)

{
	struct qla_tgt_cmd *cmd = container_of(se_cmd,
				struct qla_tgt_cmd, se_cmd);
-	struct scsi_qla_host *vha = cmd->vha;
	int xmit_type = QLA_TGT_XMIT_STATUS;

	if (cmd->aborted) {

@@ -708,7 +705,6 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)

		    cmd, kref_read(&cmd->se_cmd.cmd_kref),
		    cmd->se_cmd.transport_state, cmd->se_cmd.t_state,
		    cmd->se_cmd.se_cmd_flags);
-		vha->hw->tgt.tgt_ops->free_cmd(cmd);
		return 0;
	}
	cmd->bufflen = se_cmd->data_length;