Skip to content

Commit c201726

Browse files
jsmart-ghmartinkpetersen
jsmart-gh
authored and
martinkpetersen
committed
scsi: lpfc: Rework locking on SCSI io completion
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@broadcom.com> Signed-off-by: James Smart <jsmart2021@gmail.com> Reviewed-by: Hannes Reinecke <hare@suse.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent b1684a0 commit c201726

File tree

5 files changed

+112
-74
lines changed

5 files changed

+112
-74
lines changed

‎drivers/scsi/lpfc/lpfc_init.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4160,6 +4160,7 @@ lpfc_new_io_buf(struct lpfc_hba *phba, int num_to_alloc)
41604160
lpfc_ncmd->dma_sgl = lpfc_ncmd->data;
41614161
lpfc_ncmd->dma_phys_sgl = lpfc_ncmd->dma_handle;
41624162
lpfc_ncmd->cur_iocbq.context1 = lpfc_ncmd;
4163+
spin_lock_init(&lpfc_ncmd->buf_lock);
41634164

41644165
/* add the nvme buffer to a post list */
41654166
list_add_tail(&lpfc_ncmd->list, &post_nblist);

‎drivers/scsi/lpfc/lpfc_nvme.c

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -969,15 +969,19 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn,
969969
uint32_t *ptr;
970970

971971
/* Sanity check on return of outstanding command */
972-
if (!lpfc_ncmd || !lpfc_ncmd->nvmeCmd) {
973-
if (!lpfc_ncmd) {
974-
lpfc_printf_vlog(vport, KERN_ERR,
975-
LOG_NODE | LOG_NVME_IOERR,
976-
"6071 Null lpfc_ncmd pointer. No "
977-
"release, skip completion\n");
978-
return;
979-
}
972+
if (!lpfc_ncmd) {
973+
lpfc_printf_vlog(vport, KERN_ERR,
974+
LOG_NODE | LOG_NVME_IOERR,
975+
"6071 Null lpfc_ncmd pointer. No "
976+
"release, skip completion\n");
977+
return;
978+
}
979+
980+
/* Guard against abort handler being called at same time */
981+
spin_lock(&lpfc_ncmd->buf_lock);
980982

983+
if (!lpfc_ncmd->nvmeCmd) {
984+
spin_unlock(&lpfc_ncmd->buf_lock);
981985
lpfc_printf_vlog(vport, KERN_ERR, LOG_NODE | LOG_NVME_IOERR,
982986
"6066 Missing cmpl ptrs: lpfc_ncmd %p, "
983987
"nvmeCmd %p\n",
@@ -1154,9 +1158,11 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn,
11541158
if (!(lpfc_ncmd->flags & LPFC_SBUF_XBUSY)) {
11551159
freqpriv = nCmd->private;
11561160
freqpriv->nvme_buf = NULL;
1157-
nCmd->done(nCmd);
11581161
lpfc_ncmd->nvmeCmd = NULL;
1159-
}
1162+
spin_unlock(&lpfc_ncmd->buf_lock);
1163+
nCmd->done(nCmd);
1164+
} else
1165+
spin_unlock(&lpfc_ncmd->buf_lock);
11601166

11611167
/* Call release with XB=1 to queue the IO into the abort list. */
11621168
lpfc_release_nvme_buf(phba, lpfc_ncmd);
@@ -1781,6 +1787,9 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
17811787
}
17821788
nvmereq_wqe = &lpfc_nbuf->cur_iocbq;
17831789

1790+
/* Guard against IO completion being called at same time */
1791+
spin_lock(&lpfc_nbuf->buf_lock);
1792+
17841793
/*
17851794
* The lpfc_nbuf and the mapped nvme_fcreq in the driver's
17861795
* state must match the nvme_fcreq passed by the nvme
@@ -1789,24 +1798,22 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
17891798
* has not seen it yet.
17901799
*/
17911800
if (lpfc_nbuf->nvmeCmd != pnvme_fcreq) {
1792-
spin_unlock_irqrestore(&phba->hbalock, flags);
17931801
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
17941802
"6143 NVME req mismatch: "
17951803
"lpfc_nbuf %p nvmeCmd %p, "
17961804
"pnvme_fcreq %p. Skipping Abort xri x%x\n",
17971805
lpfc_nbuf, lpfc_nbuf->nvmeCmd,
17981806
pnvme_fcreq, nvmereq_wqe->sli4_xritag);
1799-
return;
1807+
goto out_unlock;
18001808
}
18011809

18021810
/* Don't abort IOs no longer on the pending queue. */
18031811
if (!(nvmereq_wqe->iocb_flag & LPFC_IO_ON_TXCMPLQ)) {
1804-
spin_unlock_irqrestore(&phba->hbalock, flags);
18051812
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
18061813
"6142 NVME IO req %p not queued - skipping "
18071814
"abort req xri x%x\n",
18081815
pnvme_fcreq, nvmereq_wqe->sli4_xritag);
1809-
return;
1816+
goto out_unlock;
18101817
}
18111818

18121819
atomic_inc(&lport->xmt_fcp_abort);
@@ -1816,24 +1823,22 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
18161823

18171824
/* Outstanding abort is in progress */
18181825
if (nvmereq_wqe->iocb_flag & LPFC_DRIVER_ABORTED) {
1819-
spin_unlock_irqrestore(&phba->hbalock, flags);
18201826
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
18211827
"6144 Outstanding NVME I/O Abort Request "
18221828
"still pending on nvme_fcreq %p, "
18231829
"lpfc_ncmd %p xri x%x\n",
18241830
pnvme_fcreq, lpfc_nbuf,
18251831
nvmereq_wqe->sli4_xritag);
1826-
return;
1832+
goto out_unlock;
18271833
}
18281834

18291835
abts_buf = __lpfc_sli_get_iocbq(phba);
18301836
if (!abts_buf) {
1831-
spin_unlock_irqrestore(&phba->hbalock, flags);
18321837
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
18331838
"6136 No available abort wqes. Skipping "
18341839
"Abts req for nvme_fcreq %p xri x%x\n",
18351840
pnvme_fcreq, nvmereq_wqe->sli4_xritag);
1836-
return;
1841+
goto out_unlock;
18371842
}
18381843

18391844
/* Ready - mark outstanding as aborted by driver. */
@@ -1877,6 +1882,7 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
18771882
abts_buf->vport = vport;
18781883
abts_buf->wqe_cmpl = lpfc_nvme_abort_fcreq_cmpl;
18791884
ret_val = lpfc_sli4_issue_wqe(phba, lpfc_nbuf->hdwq, abts_buf);
1885+
spin_unlock(&lpfc_nbuf->buf_lock);
18801886
spin_unlock_irqrestore(&phba->hbalock, flags);
18811887
if (ret_val) {
18821888
lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_ABTS,
@@ -1892,6 +1898,12 @@ lpfc_nvme_fcp_abort(struct nvme_fc_local_port *pnvme_lport,
18921898
"ox_id x%x on reqtag x%x\n",
18931899
nvmereq_wqe->sli4_xritag,
18941900
abts_buf->iotag);
1901+
return;
1902+
1903+
out_unlock:
1904+
spin_unlock(&lpfc_nbuf->buf_lock);
1905+
spin_unlock_irqrestore(&phba->hbalock, flags);
1906+
return;
18951907
}
18961908

18971909
/* Declare and initialization an instance of the FC NVME template. */

‎drivers/scsi/lpfc/lpfc_scsi.c

Lines changed: 47 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ lpfc_new_scsi_buf_s3(struct lpfc_vport *vport, int num_to_alloc)
506506
psb->status = IOSTAT_SUCCESS;
507507
/* Put it back into the SCSI buffer list */
508508
psb->cur_iocbq.context1 = psb;
509+
spin_lock_init(&psb->buf_lock);
509510
lpfc_release_scsi_buf_s3(phba, psb);
510511

511512
}
@@ -712,7 +713,6 @@ lpfc_get_scsi_buf_s4(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp,
712713
lpfc_cmd->cur_iocbq.iocb_flag = LPFC_IO_FCP;
713714
lpfc_cmd->prot_seg_cnt = 0;
714715
lpfc_cmd->seg_cnt = 0;
715-
lpfc_cmd->waitq = NULL;
716716
lpfc_cmd->timeout = 0;
717717
lpfc_cmd->flags = 0;
718718
lpfc_cmd->start_time = jiffies;
@@ -3651,10 +3651,17 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn,
36513651
int cpu;
36523652
#endif
36533653

3654+
/* Guard against abort handler being called at same time */
3655+
spin_lock(&lpfc_cmd->buf_lock);
3656+
36543657
/* Sanity check on return of outstanding command */
36553658
cmd = lpfc_cmd->pCmd;
3656-
if (!cmd)
3659+
if (!cmd) {
3660+
lpfc_printf_vlog(vport, KERN_ERR, LOG_INIT,
3661+
"2621 IO completion: Not an active IO\n");
3662+
spin_unlock(&lpfc_cmd->buf_lock);
36573663
return;
3664+
}
36583665

36593666
idx = lpfc_cmd->cur_iocbq.hba_wqidx;
36603667
if (phba->sli4_hba.hdwq)
@@ -3860,29 +3867,24 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pIocbIn,
38603867
}
38613868
lpfc_scsi_unprep_dma_buf(phba, lpfc_cmd);
38623869

3863-
/* If pCmd was set to NULL from abort path, do not call scsi_done */
3864-
if (xchg(&lpfc_cmd->pCmd, NULL) == NULL) {
3865-
lpfc_printf_vlog(vport, KERN_INFO, LOG_FCP,
3866-
"5688 FCP cmd already NULL, sid: 0x%06x, "
3867-
"did: 0x%06x, oxid: 0x%04x\n",
3868-
vport->fc_myDID,
3869-
(pnode) ? pnode->nlp_DID : 0,
3870-
phba->sli_rev == LPFC_SLI_REV4 ?
3871-
lpfc_cmd->cur_iocbq.sli4_xritag : 0xffff);
3872-
return;
3873-
}
3870+
lpfc_cmd->pCmd = NULL;
3871+
spin_unlock(&lpfc_cmd->buf_lock);
38743872

38753873
/* The sdev is not guaranteed to be valid post scsi_done upcall. */
38763874
cmd->scsi_done(cmd);
38773875

38783876
/*
3879-
* If there is a thread waiting for command completion
3877+
* If there is an abort thread waiting for command completion
38803878
* wake up the thread.
38813879
*/
3882-
spin_lock_irqsave(shost->host_lock, flags);
3883-
if (lpfc_cmd->waitq)
3884-
wake_up(lpfc_cmd->waitq);
3885-
spin_unlock_irqrestore(shost->host_lock, flags);
3880+
spin_lock(&lpfc_cmd->buf_lock);
3881+
if (unlikely(lpfc_cmd->cur_iocbq.iocb_flag & LPFC_DRIVER_ABORTED)) {
3882+
lpfc_cmd->cur_iocbq.iocb_flag &= ~LPFC_DRIVER_ABORTED;
3883+
if (lpfc_cmd->waitq)
3884+
wake_up(lpfc_cmd->waitq);
3885+
lpfc_cmd->waitq = NULL;
3886+
}
3887+
spin_unlock(&lpfc_cmd->buf_lock);
38863888

38873889
lpfc_release_scsi_buf(phba, lpfc_cmd);
38883890
}
@@ -4563,44 +4565,47 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
45634565
if (status != 0 && status != SUCCESS)
45644566
return status;
45654567

4568+
lpfc_cmd = (struct lpfc_io_buf *)cmnd->host_scribble;
4569+
if (!lpfc_cmd)
4570+
return ret;
4571+
45664572
spin_lock_irqsave(&phba->hbalock, flags);
45674573
/* driver queued commands are in process of being flushed */
45684574
if (phba->hba_flag & HBA_FCP_IOQ_FLUSH) {
4569-
spin_unlock_irqrestore(&phba->hbalock, flags);
45704575
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
45714576
"3168 SCSI Layer abort requested I/O has been "
45724577
"flushed by LLD.\n");
4573-
return FAILED;
4578+
ret = FAILED;
4579+
goto out_unlock;
45744580
}
45754581

4576-
lpfc_cmd = (struct lpfc_io_buf *)cmnd->host_scribble;
4577-
if (!lpfc_cmd || !lpfc_cmd->pCmd) {
4578-
spin_unlock_irqrestore(&phba->hbalock, flags);
4582+
/* Guard against IO completion being called at same time */
4583+
spin_lock(&lpfc_cmd->buf_lock);
4584+
4585+
if (!lpfc_cmd->pCmd) {
45794586
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
45804587
"2873 SCSI Layer I/O Abort Request IO CMPL Status "
45814588
"x%x ID %d LUN %llu\n",
45824589
SUCCESS, cmnd->device->id, cmnd->device->lun);
4583-
return SUCCESS;
4590+
goto out_unlock_buf;
45844591
}
45854592

45864593
iocb = &lpfc_cmd->cur_iocbq;
45874594
if (phba->sli_rev == LPFC_SLI_REV4) {
45884595
pring_s4 = phba->sli4_hba.hdwq[iocb->hba_wqidx].fcp_wq->pring;
45894596
if (!pring_s4) {
45904597
ret = FAILED;
4591-
goto out_unlock;
4598+
goto out_unlock_buf;
45924599
}
45934600
spin_lock(&pring_s4->ring_lock);
45944601
}
45954602
/* the command is in process of being cancelled */
45964603
if (!(iocb->iocb_flag & LPFC_IO_ON_TXCMPLQ)) {
4597-
if (phba->sli_rev == LPFC_SLI_REV4)
4598-
spin_unlock(&pring_s4->ring_lock);
4599-
spin_unlock_irqrestore(&phba->hbalock, flags);
46004604
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
46014605
"3169 SCSI Layer abort requested I/O has been "
46024606
"cancelled by LLD.\n");
4603-
return FAILED;
4607+
ret = FAILED;
4608+
goto out_unlock_ring;
46044609
}
46054610
/*
46064611
* If pCmd field of the corresponding lpfc_io_buf structure
@@ -4609,12 +4614,10 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
46094614
* see the completion before the eh fired. Just return SUCCESS.
46104615
*/
46114616
if (lpfc_cmd->pCmd != cmnd) {
4612-
if (phba->sli_rev == LPFC_SLI_REV4)
4613-
spin_unlock(&pring_s4->ring_lock);
46144617
lpfc_printf_vlog(vport, KERN_WARNING, LOG_FCP,
46154618
"3170 SCSI Layer abort requested I/O has been "
46164619
"completed by LLD.\n");
4617-
goto out_unlock;
4620+
goto out_unlock_ring;
46184621
}
46194622

46204623
BUG_ON(iocb->context1 != lpfc_cmd);
@@ -4625,16 +4628,15 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
46254628
"3389 SCSI Layer I/O Abort Request is pending\n");
46264629
if (phba->sli_rev == LPFC_SLI_REV4)
46274630
spin_unlock(&pring_s4->ring_lock);
4631+
spin_unlock(&lpfc_cmd->buf_lock);
46284632
spin_unlock_irqrestore(&phba->hbalock, flags);
46294633
goto wait_for_cmpl;
46304634
}
46314635

46324636
abtsiocb = __lpfc_sli_get_iocbq(phba);
46334637
if (abtsiocb == NULL) {
46344638
ret = FAILED;
4635-
if (phba->sli_rev == LPFC_SLI_REV4)
4636-
spin_unlock(&pring_s4->ring_lock);
4637-
goto out_unlock;
4639+
goto out_unlock_ring;
46384640
}
46394641

46404642
/* Indicate the IO is being aborted by the driver. */
@@ -4684,24 +4686,18 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
46844686
/* no longer need the lock after this point */
46854687
spin_unlock_irqrestore(&phba->hbalock, flags);
46864688

4687-
46884689
if (ret_val == IOCB_ERROR) {
4689-
if (phba->sli_rev == LPFC_SLI_REV4)
4690-
spin_lock_irqsave(&pring_s4->ring_lock, flags);
4691-
else
4692-
spin_lock_irqsave(&phba->hbalock, flags);
46934690
/* Indicate the IO is not being aborted by the driver. */
46944691
iocb->iocb_flag &= ~LPFC_DRIVER_ABORTED;
46954692
lpfc_cmd->waitq = NULL;
4696-
if (phba->sli_rev == LPFC_SLI_REV4)
4697-
spin_unlock_irqrestore(&pring_s4->ring_lock, flags);
4698-
else
4699-
spin_unlock_irqrestore(&phba->hbalock, flags);
4693+
spin_unlock(&lpfc_cmd->buf_lock);
47004694
lpfc_sli_release_iocbq(phba, abtsiocb);
47014695
ret = FAILED;
47024696
goto out;
47034697
}
47044698

4699+
spin_unlock(&lpfc_cmd->buf_lock);
4700+
47054701
if (phba->cfg_poll & DISABLE_FCP_RING_INT)
47064702
lpfc_sli_handle_fast_ring_event(phba,
47074703
&phba->sli.sli3_ring[LPFC_FCP_RING], HA_R0RE_REQ);
@@ -4712,9 +4708,7 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
47124708
(lpfc_cmd->pCmd != cmnd),
47134709
msecs_to_jiffies(2*vport->cfg_devloss_tmo*1000));
47144710

4715-
spin_lock_irqsave(shost->host_lock, flags);
4716-
lpfc_cmd->waitq = NULL;
4717-
spin_unlock_irqrestore(shost->host_lock, flags);
4711+
spin_lock(&lpfc_cmd->buf_lock);
47184712

47194713
if (lpfc_cmd->pCmd == cmnd) {
47204714
ret = FAILED;
@@ -4725,8 +4719,14 @@ lpfc_abort_handler(struct scsi_cmnd *cmnd)
47254719
iocb->sli4_xritag, ret,
47264720
cmnd->device->id, cmnd->device->lun);
47274721
}
4722+
spin_unlock(&lpfc_cmd->buf_lock);
47284723
goto out;
47294724

4725+
out_unlock_ring:
4726+
if (phba->sli_rev == LPFC_SLI_REV4)
4727+
spin_unlock(&pring_s4->ring_lock);
4728+
out_unlock_buf:
4729+
spin_unlock(&lpfc_cmd->buf_lock);
47304730
out_unlock:
47314731
spin_unlock_irqrestore(&phba->hbalock, flags);
47324732
out:

0 commit comments

Comments
 (0)