A scsi host lock is taken on every io completion to check whether
the abort handler is waiting on the io completion. This is an
expensive lock to take on all completion when rarely in an abort
condition.
Replace scsi host lock with command-specific lock. Synchronize
completion and abort paths by new cmd lock. Ensure all flag
changing and nulling of context pointers taken under lock.
When adding lock to task management abort, realized it was
missing other synchronization locks. Added that synchronization
to match normal paths.
Signed-off-by: Dick Kennedy <dick.kennedy@xxxxxxxxxxxx>
Signed-off-by: James Smart <jsmart2021@xxxxxxxxx>
---
drivers/scsi/lpfc/lpfc_init.c | 1 +
drivers/scsi/lpfc/lpfc_nvme.c | 43 ++++++++++++--------
drivers/scsi/lpfc/lpfc_scsi.c | 92 +++++++++++++++++++++----------------------
drivers/scsi/lpfc/lpfc_sli.c | 42 +++++++++++++++-----
drivers/scsi/lpfc/lpfc_sli.h | 1 +
5 files changed, 107 insertions(+), 72 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 7ee9120b10f6..bcae215cd18c 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -4160,6 +4160,7 @@ lpfc_new_io_buf(struct lpfc_hba *phba, int num_to_alloc)
lpfc_ncmd->dma_sgl = lpfc_ncmd->data;
lpfc_ncmd->dma_phys_sgl = lpfc_ncmd->dma_handle;
lpfc_ncmd->cur_iocbq.context1 = lpfc_ncmd;
+ spin_lock_init(&lpfc_ncmd->buf_lock);
/* add the nvme buffer to a post list */
list_add_tail(&lpfc_ncmd->list, &post_nblist);
diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 9480257c5143..4b19be386b7b 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -969,15 +969,19 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn,
uint32_t *ptr;
/* Sanity check on return of outstanding command */
- if (!lpfc_ncmd || !lpfc_ncmd->nvmeCmd) {
- if (!lpfc_ncmd) {
- lpfc_printf_vlog(vport, KERN_ERR,
- LOG_NODE | LOG_NVME_IOERR,
- "6071 Null lpfc_ncmd pointer. No "
- "release, skip completion\n");
- return;
- }
+ if (!lpfc_ncmd) {
+ lpfc_printf_vlog(vport, KERN_ERR,
+ LOG_NODE | LOG_NVME_IOERR,
+ "6071 Null lpfc_ncmd pointer. No "
+ "release, skip completion\n");
+ return;
+ }
+ /* Guard against abort handler being called at same time */
+ spin_lock(&lpfc_ncmd->buf_lock);
+
+ if (!lpfc_ncmd->nvmeCmd) {
+ spin_unlock(&lpfc_ncmd->buf_lock);
lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE | LOG_NVME_IOERR,
"6066 Missing cmpl ptrs: lpfc_ncmd %p, "
"nvmeCmd %p\n",
@@ -1157,6 +1161,7 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn,
nCmd->done(nCmd);
lpfc_ncmd->nvmeCmd = NULL;
}
+ spin_unlock(&lpfc_ncmd->buf_lock);
/* Call release with XB=1 to queue the IO into the abort list. */
lpfc_release_nvme_buf(phba, lpfc_ncmd);
@@ -1781,6 +1786,9 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
}
nvmereq_wqe = &lpfc_nbuf->cur_iocbq;
+ /* Guard against IO completion being called at same time */
+ spin_lock(&lpfc_nbuf->buf_lock);
+
/*
* The lpfc_nbuf and the mapped nvme_fcreq in the driver's
* state must match the nvme_fcreq passed by the nvme
@@ -1789,24 +1797,22 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
* has not seen it yet.
*/
if (lpfc_nbuf->nvmeCmd != pnvme_fcreq) {
- spin_unlock_irqrestore(&phba->hbalock, flags);
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
"6143 NVME req mismatch: "
"lpfc_nbuf %p nvmeCmd %p, "
"pnvme_fcreq %p. Skipping Abort xri x%x\n",
lpfc_nbuf, lpfc_nbuf->nvmeCmd,
pnvme_fcreq, nvmereq_wqe->sli4_xritag);
- return;
+ goto out_unlock;
}
/* Don't abort IOs no longer on the pending queue. */
if (!(nvmereq_wqe->iocb_flag & LPFC_IO_ON_TXCMPLQ)) {
- spin_unlock_irqrestore(&phba->hbalock, flags);
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
"6142 NVME IO req %p not queued - skipping "
"abort req xri x%x\n",
pnvme_fcreq, nvmereq_wqe->sli4_xritag);
- return;
+ goto out_unlock;
}
atomic_inc(&lport->xmt_fcp_abort);
@@ -1816,24 +1822,22 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
/* Outstanding abort is in progress */
if (nvmereq_wqe->iocb_flag & LPFC_DRIVER_ABORTED) {
- spin_unlock_irqrestore(&phba->hbalock, flags);
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
"6144 Outstanding NVME I/O Abort Request "
"still pending on nvme_fcreq %p, "
"lpfc_ncmd %p xri x%x\n",
pnvme_fcreq, lpfc_nbuf,
nvmereq_wqe->sli4_xritag);
- return;
+ goto out_unlock;
}
abts_buf = __lpfc_sli_get_iocbq(phba);
if (!abts_buf) {
- spin_unlock_irqrestore(&phba->hbalock, flags);
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
"6136 No available abort wqes. Skipping "
"Abts req for nvme_fcreq %p xri x%x\n",
pnvme_fcreq, nvmereq_wqe->sli4_xritag);
- return;
+ goto out_unlock;
}
/* Ready - mark outstanding as aborted by driver. */
@@ -1877,6 +1881,7 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
abts_buf->vport = vport;
abts_buf->wqe_cmpl = lpfc_nvme_abort_fcreq_cmpl;
ret_val = lpfc_sli4_issue_wqe(phba, lpfc_nbuf->hdwq, abts_buf);
+ spin_unlock(&lpfc_nbuf->buf_lock);
spin_unlock_irqrestore(&phba->hbalock, flags);
if (ret_val) {
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
@@ -1892,6 +1897,12 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
"ox_id x%x on reqtag x%x\n",
nvmereq_wqe->sli4_xritag,
abts_buf->iotag);
+ return;
+
+out_unlock:
+ spin_unlock(&lpfc_nbuf->buf_lock);
+ spin_unlock_irqrestore(&phba->hbalock, flags);
+ return;
}
/* Declare and initialization an instance of the FC NVME template. */
diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index a827520789f1..eea5d4b3cac4 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -506,6 +506,7 @@ lpfc_new_scsi_buf_s3(struct lpfc_vport *vport, int num_to_alloc)
psb->status = IOSTAT_SUCCESS;
/* Put it back into the SCSI buffer list */
psb->cur_iocbq.context1 = psb;
+ spin_lock_init(&psb->buf_lock);
lpfc_release_scsi_buf_s3(phba, psb);
}
@@ -712,7 +713,6 @@ lpfc_get_scsi_buf_s4(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp,
lpfc_cmd->cur_iocbq.iocb_flag = LPFC_IO_FCP;
lpfc_cmd->prot_seg_cnt = 0;
lpfc_cmd->seg_cnt = 0;
- lpfc_cmd->waitq = NULL;
lpfc_cmd->timeout = 0;
lpfc_cmd->flags = 0;
lpfc_cmd->start_time = jiffies;
@@ -3651,10 +3651,17 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn,
int cpu;
#endif
+ /* Guard against abort handler being called at same time */
+ spin_lock(&lpfc_cmd->buf_lock);
+
/* Sanity check on return of outstanding command */
cmd = lpfc_cmd->pCmd;
- if (!cmd)
+ if (!cmd) {
+ lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
+ "2621 IO completion: Not an active IO\n");
+ spin_unlock(&lpfc_cmd->buf_lock);
return;
+ }
idx = lpfc_cmd->cur_iocbq.hba_wqidx;
if (phba->sli4_hba.hdwq)
@@ -3860,30 +3867,23 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn,
}
lpfc_scsi_unprep_dma_buf(phba, lpfc_cmd);
- /* If pCmd was set to NULL from abort path, do not call scsi_done */
- if (xchg(&lpfc_cmd->pCmd, NULL) == NULL) {
- lpfc_printf_vlog(vport, KERN_INFO, LOG_FCP,
- "5688 FCP cmd already NULL, sid: 0x%06x, "
- "did: 0x%06x, oxid: 0x%04x\n",
- vport->fc_myDID,
- (pnode) ? pnode->nlp_DID : 0,
- phba->sli_rev == LPFC_SLI_REV4 ?
- lpfc_cmd->cur_iocbq.sli4_xritag : 0xffff);
- return;
- }
+ lpfc_cmd->pCmd = NULL;
/* The sdev is not guaranteed to be valid post scsi_done upcall. */
cmd->scsi_done(cmd);
/*
- * If there is a thread waiting for command completion
+ * If there is an abort thread waiting for command completion
* wake up the thread.
*/
- spin_lock_irqsave(shost->host_lock, flags);
- if (lpfc_cmd->waitq)
- wake_up(lpfc_cmd->waitq);
- spin_unlock_irqrestore(shost->host_lock, flags);
+ if (unlikely(lpfc_cmd->cur_iocbq.iocb_flag & LPFC_DRIVER_ABORTED)) {
+ lpfc_cmd->cur_iocbq.iocb_flag &= ~LPFC_DRIVER_ABORTED;
+ if (lpfc_cmd->waitq)
+ wake_up(lpfc_cmd->waitq);
+ lpfc_cmd->waitq = NULL;
+ }
+ spin_unlock(&lpfc_cmd->buf_lock);
lpfc_release_scsi_buf(phba, lpfc_cmd);
}
@@ -4563,24 +4563,29 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
if (status != 0 && status != SUCCESS)
return status;
+ lpfc_cmd = (struct lpfc_io_buf *)cmnd->host_scribble;
+ if (!lpfc_cmd)
+ return ret;
+
spin_lock_irqsave(&phba->hbalock, flags);
/* driver queued commands are in process of being flushed */
if (phba->hba_flag & HBA_FCP_IOQ_FLUSH) {
- spin_unlock_irqrestore(&phba->hbalock, flags);
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
"3168 SCSI Layer abort requested I/O has been "
"flushed by LLD.\n");
- return FAILED;
+ ret = FAILED;
+ goto out_unlock;
}
- lpfc_cmd = (struct lpfc_io_buf *)cmnd->host_scribble;
- if (!lpfc_cmd || !lpfc_cmd->pCmd) {
- spin_unlock_irqrestore(&phba->hbalock, flags);
+ /* Guard against IO completion being called at same time */
+ spin_lock(&lpfc_cmd->buf_lock);
+
+ if (!lpfc_cmd->pCmd) {
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
"2873 SCSI Layer I/O Abort Request IO CMPL Status "
"x%x ID %d LUN %llu\n",
SUCCESS, cmnd->device->id, cmnd->device->lun);
- return SUCCESS;
+ goto out_unlock_buf;
}
iocb = &lpfc_cmd->cur_iocbq;
@@ -4588,19 +4593,17 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
pring_s4 = phba->sli4_hba.hdwq[iocb->hba_wqidx].fcp_wq->pring;
if (!pring_s4) {
ret = FAILED;
- goto out_unlock;
+ goto out_unlock_buf;
}
spin_lock(&pring_s4->ring_lock);
}
/* the command is in process of being cancelled */
if (!(iocb->iocb_flag & LPFC_IO_ON_TXCMPLQ)) {
- if (phba->sli_rev == LPFC_SLI_REV4)
- spin_unlock(&pring_s4->ring_lock);
- spin_unlock_irqrestore(&phba->hbalock, flags);
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
"3169 SCSI Layer abort requested I/O has been "
"cancelled by LLD.\n");
- return FAILED;
+ ret = FAILED;
+ goto out_unlock_ring;
}
/*
* If pCmd field of the corresponding lpfc_io_buf structure
@@ -4609,12 +4612,10 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
* see the completion before the eh fired. Just return SUCCESS.
*/
if (lpfc_cmd->pCmd != cmnd) {
- if (phba->sli_rev == LPFC_SLI_REV4)
- spin_unlock(&pring_s4->ring_lock);
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
"3170 SCSI Layer abort requested I/O has been "
"completed by LLD.\n");
- goto out_unlock;
+ goto out_unlock_ring;
}
BUG_ON(iocb->context1 != lpfc_cmd);
@@ -4625,6 +4626,7 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
"3389 SCSI Layer I/O Abort Request is pending\n");
if (phba->sli_rev == LPFC_SLI_REV4)
spin_unlock(&pring_s4->ring_lock);
+ spin_unlock(&lpfc_cmd->buf_lock);
spin_unlock_irqrestore(&phba->hbalock, flags);
goto wait_for_cmpl;
}
@@ -4632,9 +4634,7 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
abtsiocb = __lpfc_sli_get_iocbq(phba);
if (abtsiocb == NULL) {
ret = FAILED;
- if (phba->sli_rev == LPFC_SLI_REV4)
- spin_unlock(&pring_s4->ring_lock);
- goto out_unlock;
+ goto out_unlock_ring;
}
/* Indicate the IO is being aborted by the driver. */
@@ -4684,24 +4684,18 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
/* no longer need the lock after this point */
spin_unlock_irqrestore(&phba->hbalock, flags);
-
if (ret_val == IOCB_ERROR) {
- if (phba->sli_rev == LPFC_SLI_REV4)
- spin_lock_irqsave(&pring_s4->ring_lock, flags);
- else
- spin_lock_irqsave(&phba->hbalock, flags);
/* Indicate the IO is not being aborted by the driver. */
iocb->iocb_flag &= ~LPFC_DRIVER_ABORTED;
lpfc_cmd->waitq = NULL;
- if (phba->sli_rev == LPFC_SLI_REV4)
- spin_unlock_irqrestore(&pring_s4->ring_lock, flags);
- else
- spin_unlock_irqrestore(&phba->hbalock, flags);
+ spin_unlock(&lpfc_cmd->buf_lock);
lpfc_sli_release_iocbq(phba, abtsiocb);
ret = FAILED;
goto out;
}
+ spin_unlock(&lpfc_cmd->buf_lock);
+
if (phba->cfg_poll & DISABLE_FCP_RING_INT)
lpfc_sli_handle_fast_ring_event(phba,
&phba->sli.sli3_ring[LPFC_FCP_RING], HA_R0RE_REQ);
@@ -4712,9 +4706,7 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
(lpfc_cmd->pCmd != cmnd),
msecs_to_jiffies(2*vport->cfg_devloss_tmo*1000));
- spin_lock_irqsave(shost->host_lock, flags);
- lpfc_cmd->waitq = NULL;
- spin_unlock_irqrestore(shost->host_lock, flags);
+ spin_lock(&lpfc_cmd->buf_lock);
if (lpfc_cmd->pCmd == cmnd) {
ret = FAILED;
@@ -4725,8 +4717,14 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
iocb->sli4_xritag, ret,
cmnd->device->id, cmnd->device->lun);
}
+ spin_unlock(&lpfc_cmd->buf_lock);
goto out;
+out_unlock_ring:
+ if (phba->sli_rev == LPFC_SLI_REV4)
+ spin_unlock(&pring_s4->ring_lock);
+out_unlock_buf:
+ spin_unlock(&lpfc_cmd->buf_lock);
out_unlock:
spin_unlock_irqrestore(&phba->hbalock, flags);
out:
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 41447dd355ec..29cbbad19a56 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -11659,7 +11659,7 @@ lpfc_sli_abort_taskmgmt(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,
IOCB_t *icmd;
int sum, i, ret_val;
unsigned long iflags;
- struct lpfc_sli_ring *pring_s4;
+ struct lpfc_sli_ring *pring_s4 = NULL;
spin_lock_irqsave(&phba->hbalock, iflags);
@@ -11677,17 +11677,46 @@ lpfc_sli_abort_taskmgmt(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,
cmd) != 0)
continue;
+ /* Guard against IO completion being called at same time */
+ lpfc_cmd = container_of(iocbq, struct lpfc_io_buf, cur_iocbq);
+ spin_lock(&lpfc_cmd->buf_lock);
+
+ if (!lpfc_cmd->pCmd) {
+ spin_unlock(&lpfc_cmd->buf_lock);
+ continue;
+ }
+
+ if (phba->sli_rev == LPFC_SLI_REV4) {
+ pring_s4 =
+ phba->sli4_hba.hdwq[iocbq->hba_wqidx].fcp_wq->pring;
+ if (!pring_s4) {
+ spin_unlock(&lpfc_cmd->buf_lock);
+ continue;
+ }
+ /* Note: both hbalock and ring_lock must be set here */
+ spin_lock(&pring_s4->ring_lock);
+ }
+
/*
* If the iocbq is already being aborted, don't take a second
* action, but do count it.
*/
- if (iocbq->iocb_flag & LPFC_DRIVER_ABORTED)
+ if ((iocbq->iocb_flag & LPFC_DRIVER_ABORTED) ||
+ !(iocbq->iocb_flag & LPFC_IO_ON_TXCMPLQ)) {
+ if (phba->sli_rev == LPFC_SLI_REV4)
+ spin_unlock(&pring_s4->ring_lock);
+ spin_unlock(&lpfc_cmd->buf_lock);
continue;
+ }
/* issue ABTS for this IOCB based on iotag */
abtsiocbq = __lpfc_sli_get_iocbq(phba);
- if (abtsiocbq == NULL)
+ if (!abtsiocbq) {
+ if (phba->sli_rev == LPFC_SLI_REV4)
+ spin_unlock(&pring_s4->ring_lock);
+ spin_unlock(&lpfc_cmd->buf_lock);
continue;
+ }
icmd = &iocbq->iocb;
abtsiocbq->iocb.un.acxri.abortType = ABORT_TYPE_ABTS;
@@ -11708,7 +11737,6 @@ lpfc_sli_abort_taskmgmt(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,
if (iocbq->iocb_flag & LPFC_IO_FOF)
abtsiocbq->iocb_flag |= LPFC_IO_FOF;
- lpfc_cmd = container_of(iocbq, struct lpfc_io_buf, cur_iocbq);
ndlp = lpfc_cmd->rdata->pnode;
if (lpfc_is_link_up(phba) &&
@@ -11727,11 +11755,6 @@ lpfc_sli_abort_taskmgmt(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,
iocbq->iocb_flag |= LPFC_DRIVER_ABORTED;
if (phba->sli_rev == LPFC_SLI_REV4) {
- pring_s4 = lpfc_sli4_calc_ring(phba, abtsiocbq);
- if (!pring_s4)
- continue;
- /* Note: both hbalock and ring_lock must be set here */
- spin_lock(&pring_s4->ring_lock);
ret_val = __lpfc_sli_issue_iocb(phba, pring_s4->ringno,
abtsiocbq, 0);
spin_unlock(&pring_s4->ring_lock);
@@ -11740,6 +11763,7 @@ lpfc_sli_abort_taskmgmt(struct lpfc_vport *vport, struct lpfc_sli_ring *pring,
abtsiocbq, 0);
}
+ spin_unlock(&lpfc_cmd->buf_lock);
if (ret_val == IOCB_ERROR)
__lpfc_sli_release_iocbq(phba, abtsiocbq);
diff --git a/drivers/scsi/lpfc/lpfc_sli.h b/drivers/scsi/lpfc/lpfc_sli.h
index 658a40d3b1a2..d203a666497f 100644
--- a/drivers/scsi/lpfc/lpfc_sli.h
+++ b/drivers/scsi/lpfc/lpfc_sli.h
@@ -387,6 +387,7 @@ struct lpfc_io_buf {
* to dma_unmap_sg.
*/
unsigned long start_time;
+ spinlock_t buf_lock; /* lock used in case of simultaneous abort */
bool expedite; /* this is an expedite io_buf */
union {
--
2.13.7
