> On Mar 15, 2021, at 10:56 PM, Bart Van Assche <bvanassche@xxxxxxx> 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 <qutran@xxxxxxxxxxx>
> Cc: Mike Christie <michael.christie@xxxxxxxxxx>
> Cc: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>
> Cc: Daniel Wagner <dwagner@xxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> 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;
Curious …. What triggered this revert? Can you share your motivation for this revert.
--
Himanshu Madhani Oracle Linux Engineering
