diff mbox series

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

Message ID 20240710171057.35066-11-njavali@marvell.com (mailing list archive)
State Accepted
Headers show
Series qla2xxx misc. bug fixes | expand

Commit Message

Nilesh Javali July 10, 2024, 5:10 p.m. UTC 
From: Quinn Tran <qutran@marvell.com>

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@vger.kernel.org
Signed-off-by: Quinn Tran <qutran@marvell.com>
Signed-off-by: Nilesh Javali <njavali@marvell.com>
---
 drivers/scsi/qla2xxx/qla_bsg.c | 96 ++++++++++++++++++++--------------
 1 file changed, 57 insertions(+), 39 deletions(-)

Comments

Himanshu Madhani July 10, 2024, 10:20 p.m. UTC | #1 
On 7/10/24 10:10 AM, Nilesh Javali wrote:
> From: Quinn Tran <qutran@marvell.com>
> 
> 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@vger.kernel.org
> Signed-off-by: Quinn Tran <qutran@marvell.com>
> Signed-off-by: Nilesh Javali <njavali@marvell.com>
> ---
>   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@oracle.com>
diff mbox series

Patch

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;
 }