Re: [PATCH 20/25] qla2xxx: Remove redundant code

[Bart Van Assche <Bart.VanAssche@sandisk.com>]

On Fri, 2017-05-19 at 14:53 -0700, Himanshu Madhani wrote:
> From: Quinn Tran <quinn.tran@cavium.com>
> 
> During ABTS or Abort task, qla2xxx does a pre-search for
> the se_cmd, based on command's tag. The same search is
> performed by TCM. Remove the extra search from qla2xxx.
> 
> Signed-off-by: Quinn Tran <quinn.tran@cavium.com>
> Signed-off-by: Himanshu Madhani <himanshu.madhani@cavium.com>
> ---
>  drivers/scsi/qla2xxx/qla_target.c | 29 ++++-------------------------
>  1 file changed, 4 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
> index 21e8993baf4b..b8e609ae6cff 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -1836,34 +1836,13 @@ static int __qlt_24xx_handle_abts(struct scsi_qla_host *vha,
>  	struct abts_recv_from_24xx *abts, struct fc_port *sess)
>  {
>  	struct qla_hw_data *ha = vha->hw;
> -	struct se_session *se_sess = sess->se_sess;
>  	struct qla_tgt_mgmt_cmd *mcmd;
> -	struct se_cmd *se_cmd;
>  	int rc;
> -	bool found_lun = false;
> -	unsigned long flags;
> -
> -	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
> -	list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {
> -		if (se_cmd->tag == abts->exchange_addr_to_abort) {
> -			found_lun = true;
> -			break;
> -		}
> -	}
> -	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
>  
> -	/* cmd not in LIO lists, look in qla list */
> -	if (!found_lun) {
> -		if (abort_cmd_for_tag(vha, abts->exchange_addr_to_abort)) {
> -			/* send TASK_ABORT response immediately */
> -			qlt_24xx_send_abts_resp(vha, abts, FCP_TMF_CMPL, false);
> -			return 0;
> -		} else {
> -			ql_dbg(ql_dbg_tgt_mgt, vha, 0xf081,
> -			    "unable to find cmd in driver or LIO for tag 0x%x\n",
> -			    abts->exchange_addr_to_abort);
> -			return -ENOENT;
> -		}
> +	if (abort_cmd_for_tag(vha, abts->exchange_addr_to_abort)) {
> +		/* send TASK_ABORT response immediately */
> +		qlt_24xx_send_abts_resp(vha, abts, FCP_TMF_CMPL, false);
> +		return 0;
>  	}
>  
>  	ql_dbg(ql_dbg_tgt_mgt, vha, 0xf00f,

Hello Himanshu and Quinn,

Please drop this patch. If a command has already been submitted to the LIO
core and an ABTS is received then the LIO core should be requested to perform
the abort. This patch changes the behavior of the qla2xxx target driver such
that the LIO core is not informed at all if abort_cmd_for_tag() finds the
command that has to be aborted in one of the command lists maintained by the
qla2xxx driver. That can lead to the presence of overlapping writes in the
command set on the target system and hence to data corruption. Please note
that I had proposed a better approach on the target-devel mailing list and
that I'm still waiting for someone from Cavium to review these patches:
* [PATCH v6 09/33] target: Make it possible to specify I_T nexus for SCSI
  abort (http://www.spinics.net/lists/target-devel/msg14534.html).
* [PATCH v6 10/33] tcm_qla2xxx: Let the target core look up the LUN of the
  aborted cmd (http://www.spinics.net/lists/target-devel/msg14563.html).

Bart.