Re: [PATCH v2 10/11] qla2xxx: Use QP lock to search for bsg

[Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>]

On 7/10/24 10:10 AM, Nilesh Javali wrote:
> From: Quinn Tran <qutran@xxxxxxxxxxx>
>
> On bsg timeout, hardware_lock is used as part of search for the srb.
> Instead, qpair lock should be used to iterate through different qpair.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Quinn Tran <qutran@xxxxxxxxxxx>
> Signed-off-by: Nilesh Javali <njavali@xxxxxxxxxxx>
> ---
>   drivers/scsi/qla2xxx/qla_bsg.c | 96 ++++++++++++++++++++--------------
>   1 file changed, 57 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
> index 8d1e45b883cd..52dc9604f567 100644
> --- a/drivers/scsi/qla2xxx/qla_bsg.c
> +++ b/drivers/scsi/qla2xxx/qla_bsg.c
> @@ -3059,17 +3059,61 @@ qla24xx_bsg_request(struct bsg_job *bsg_job)
>   	return ret;
>   }
>   
> -int
> -qla24xx_bsg_timeout(struct bsg_job *bsg_job)
> +static bool qla_bsg_found(struct qla_qpair *qpair, struct bsg_job *bsg_job)
>   {
> +	bool found = false;
>   	struct fc_bsg_reply *bsg_reply = bsg_job->reply;
>   	scsi_qla_host_t *vha = shost_priv(fc_bsg_to_shost(bsg_job));
>   	struct qla_hw_data *ha = vha->hw;
> -	srb_t *sp;
> -	int cnt, que;
> +	srb_t *sp = NULL;
> +	int cnt;
>   	unsigned long flags;
>   	struct req_que *req;
>   
> +	spin_lock_irqsave(qpair->qp_lock_ptr, flags);
> +	req = qpair->req;
> +
> +	for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) {
> +		sp = req->outstanding_cmds[cnt];
> +		if (sp &&
> +		    (sp->type == SRB_CT_CMD ||
> +		     sp->type == SRB_ELS_CMD_HST ||
> +		     sp->type == SRB_ELS_CMD_HST_NOLOGIN) &&
> +		    sp->u.bsg_job == bsg_job) {
> +			req->outstanding_cmds[cnt] = NULL;
> +			spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
> +
> +			if (!ha->flags.eeh_busy && ha->isp_ops->abort_command(sp)) {
> +				ql_log(ql_log_warn, vha, 0x7089,
> +						"mbx abort_command failed.\n");
> +				bsg_reply->result = -EIO;
> +			} else {
> +				ql_dbg(ql_dbg_user, vha, 0x708a,
> +						"mbx abort_command success.\n");
> +				bsg_reply->result = 0;
> +			}
> +			/* ref: INIT */
> +			kref_put(&sp->cmd_kref, qla2x00_sp_release);
> +
> +			found = true;
> +			goto done;
> +		}
> +	}
> +	spin_unlock_irqrestore(qpair->qp_lock_ptr, flags);
> +
> +done:
> +	return found;
> +}
> +
> +int
> +qla24xx_bsg_timeout(struct bsg_job *bsg_job)
> +{
> +	struct fc_bsg_reply *bsg_reply = bsg_job->reply;
> +	scsi_qla_host_t *vha = shost_priv(fc_bsg_to_shost(bsg_job));
> +	struct qla_hw_data *ha = vha->hw;
> +	int i;
> +	struct qla_qpair *qpair;
> +
>   	ql_log(ql_log_info, vha, 0x708b, "%s CMD timeout. bsg ptr %p.\n",
>   	    __func__, bsg_job);
>   
> @@ -3079,48 +3123,22 @@ qla24xx_bsg_timeout(struct bsg_job *bsg_job)
>   		qla_pci_set_eeh_busy(vha);
>   	}
>   
> +	if (qla_bsg_found(ha->base_qpair, bsg_job))
> +		goto done;
> +
>   	/* find the bsg job from the active list of commands */
> -	spin_lock_irqsave(&ha->hardware_lock, flags);
> -	for (que = 0; que < ha->max_req_queues; que++) {
> -		req = ha->req_q_map[que];
> -		if (!req)
> +	for (i = 0; i < ha->max_qpairs; i++) {
> +		qpair = vha->hw->queue_pair_map[i];
> +		if (!qpair)
>   			continue;
> -
> -		for (cnt = 1; cnt < req->num_outstanding_cmds; cnt++) {
> -			sp = req->outstanding_cmds[cnt];
> -			if (sp &&
> -			    (sp->type == SRB_CT_CMD ||
> -			     sp->type == SRB_ELS_CMD_HST ||
> -			     sp->type == SRB_ELS_CMD_HST_NOLOGIN ||
> -			     sp->type == SRB_FXIOCB_BCMD) &&
> -			    sp->u.bsg_job == bsg_job) {
> -				req->outstanding_cmds[cnt] = NULL;
> -				spin_unlock_irqrestore(&ha->hardware_lock, flags);
> -
> -				if (!ha->flags.eeh_busy && ha->isp_ops->abort_command(sp)) {
> -					ql_log(ql_log_warn, vha, 0x7089,
> -					    "mbx abort_command failed.\n");
> -					bsg_reply->result = -EIO;
> -				} else {
> -					ql_dbg(ql_dbg_user, vha, 0x708a,
> -					    "mbx abort_command success.\n");
> -					bsg_reply->result = 0;
> -				}
> -				spin_lock_irqsave(&ha->hardware_lock, flags);
> -				goto done;
> -
> -			}
> -		}
> +		if (qla_bsg_found(qpair, bsg_job))
> +			goto done;
>   	}
> -	spin_unlock_irqrestore(&ha->hardware_lock, flags);
> +
>   	ql_log(ql_log_info, vha, 0x708b, "SRB not found to abort.\n");
>   	bsg_reply->result = -ENXIO;
> -	return 0;
>   
>   done:
> -	spin_unlock_irqrestore(&ha->hardware_lock, flags);
> -	/* ref: INIT */
> -	kref_put(&sp->cmd_kref, qla2x00_sp_release);
>   	return 0;
>   }
>  

Reviewed-by: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>

--
Himanshu Madhani                                Oracle Linux Engineering