From 32b938656462dd2c4aa1e91e0b48cd114c2d00f7 Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:48:52 -0700 Subject: scsi: lpfc: Fix nvmet target abort cmd matching After receiving an unsolicited ABTS (meaning rxid is 0xFFFF), the driver used the oxid from the initiator to match against a local xri which may have been allocated for the io. The xri would be the rxid - it's an invalid check resulting in the command not being matched or erroneously matched. Change the lookup to use the oxid and the SID to match against received IO's original values. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index d74bfd264495..c9011579aa0f 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -1497,6 +1497,7 @@ void lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, struct sli4_wcqe_xri_aborted *axri) { +#if (IS_ENABLED(CONFIG_NVME_TARGET_FC)) uint16_t xri = bf_get(lpfc_wcqe_xa_xri, axri); uint16_t rxid = bf_get(lpfc_wcqe_xa_remote_xid, axri); struct lpfc_nvmet_rcv_ctx *ctxp, *next_ctxp; @@ -1562,6 +1563,7 @@ lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, } spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); spin_unlock_irqrestore(&phba->hbalock, iflag); +#endif } int @@ -1572,19 +1574,23 @@ lpfc_nvmet_rcv_unsol_abort(struct lpfc_vport *vport, struct lpfc_hba *phba = vport->phba; struct lpfc_nvmet_rcv_ctx *ctxp, *next_ctxp; struct nvmefc_tgt_fcp_req *rsp; + uint32_t sid; uint16_t xri; unsigned long iflag = 0; xri = be16_to_cpu(fc_hdr->fh_ox_id); + sid = sli4_sid_from_fc_hdr(fc_hdr); spin_lock_irqsave(&phba->hbalock, iflag); spin_lock(&phba->sli4_hba.abts_nvmet_buf_list_lock); list_for_each_entry_safe(ctxp, next_ctxp, &phba->sli4_hba.lpfc_abts_nvmet_ctx_list, list) { - if (ctxp->ctxbuf->sglq->sli4_xritag != xri) + if (ctxp->oxid != xri || ctxp->sid != sid) continue; + xri = ctxp->ctxbuf->sglq->sli4_xritag; + spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); spin_unlock_irqrestore(&phba->hbalock, iflag); @@ -1613,7 +1619,7 @@ lpfc_nvmet_rcv_unsol_abort(struct lpfc_vport *vport, xri, raw_smp_processor_id(), 1); lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6320 NVMET Rcv ABTS:rjt xri x%x\n", xri); + "6320 NVMET Rcv ABTS:rjt xid x%x\n", xri); /* Respond with BA_RJT accordingly */ lpfc_sli4_seq_abort_rsp(vport, fc_hdr, 0); -- cgit From 4767c58af96e1c6a2dad307c6e5bb75d6c646815 Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:48:53 -0700 Subject: scsi: lpfc: Correct nvmet buffer free race condition A race condition resulted in receive buffers being placed in the free list twice. Change the locking and handling to check whether the "other" path will be freeing the entry in a later thread and skip it if it is. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index c9011579aa0f..3a11861b7ad6 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -343,16 +343,23 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct lpfc_nvmet_ctxbuf *ctx_buf) } if (ctxp->rqb_buffer) { - nvmebuf = ctxp->rqb_buffer; spin_lock_irqsave(&ctxp->ctxlock, iflag); - ctxp->rqb_buffer = NULL; - if (ctxp->flag & LPFC_NVMET_CTX_REUSE_WQ) { - ctxp->flag &= ~LPFC_NVMET_CTX_REUSE_WQ; - spin_unlock_irqrestore(&ctxp->ctxlock, iflag); - nvmebuf->hrq->rqbp->rqb_free_buffer(phba, nvmebuf); + nvmebuf = ctxp->rqb_buffer; + /* check if freed in another path whilst acquiring lock */ + if (nvmebuf) { + ctxp->rqb_buffer = NULL; + if (ctxp->flag & LPFC_NVMET_CTX_REUSE_WQ) { + ctxp->flag &= ~LPFC_NVMET_CTX_REUSE_WQ; + spin_unlock_irqrestore(&ctxp->ctxlock, iflag); + nvmebuf->hrq->rqbp->rqb_free_buffer(phba, + nvmebuf); + } else { + spin_unlock_irqrestore(&ctxp->ctxlock, iflag); + /* repost */ + lpfc_rq_buf_free(phba, &nvmebuf->hbuf); + } } else { spin_unlock_irqrestore(&ctxp->ctxlock, iflag); - lpfc_rq_buf_free(phba, &nvmebuf->hbuf); /* repost */ } } ctxp->state = LPFC_NVMET_STE_FREE; -- cgit From d74a89aab9be1df8bceb564258305e0f3bf1c471 Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:48:55 -0700 Subject: scsi: lpfc: Separate CQ processing for nvmet_fc upcalls Currently the driver is notified of new command frame receipt by CQEs. As part of the CQE processing, the driver upcalls the nvmet_fc transport to deliver the command. nvmet_fc, as part of receiving the command builds out a context for it, where one of the first steps is to allocate memory for the io. When running with tests that do large ios (1MB), it was found on some systems, the total number of outstanding I/O's, at 1MB per, completely consumed the system's memory. Thus additional ios were getting blocked in the memory allocator. Given that this blocked the lpfc thread processing CQEs, there were lots of other commands that were received and which are then held up, and given CQEs are serially processed, the aggregate delays for an IO waiting behind the others became cummulative - enough so that the initiator hit timeouts for the ios. The basic fix is to avoid the direct upcall and instead schedule a work item for each io as it is received. This allows the cq processing to complete very quickly, and each io can then run or block on it's own. However, this general solution hurts latency when there are few ios. As such, implemented the fix such that the driver watches how many CQEs it has processed sequentially in one run. As long as the count is below a threshold, the direct nvmet_fc upcall will be made. Only when the count is exceeded will it revert to work scheduling. Given that debug of this showed a surprisingly long delay in cq processing, the io timer stats were updated to better reflect the processing of the different points. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 67 +++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 20 deletions(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index 3a11861b7ad6..95386f90a874 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -395,8 +395,9 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct lpfc_nvmet_ctxbuf *ctx_buf) spin_lock_init(&ctxp->ctxlock); #ifdef CONFIG_SCSI_LPFC_DEBUG_FS - if (ctxp->ts_cmd_nvme) { - ctxp->ts_cmd_nvme = ktime_get_ns(); + /* NOTE: isr time stamp is stale when context is re-assigned*/ + if (ctxp->ts_isr_cmd) { + ctxp->ts_cmd_nvme = 0; ctxp->ts_nvme_data = 0; ctxp->ts_data_wqput = 0; ctxp->ts_isr_data = 0; @@ -1877,6 +1878,10 @@ lpfc_nvmet_process_rcv_fcp_req(struct lpfc_nvmet_ctxbuf *ctx_buf) payload = (uint32_t *)(nvmebuf->dbuf.virt); tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private; +#ifdef CONFIG_SCSI_LPFC_DEBUG_FS + if (ctxp->ts_isr_cmd) + ctxp->ts_cmd_nvme = ktime_get_ns(); +#endif /* * The calling sequence should be: * nvmet_fc_rcv_fcp_req->lpfc_nvmet_xmt_fcp_op/cmp- req->done @@ -2015,6 +2020,8 @@ lpfc_nvmet_replenish_context(struct lpfc_hba *phba, * @phba: pointer to lpfc hba data structure. * @idx: relative index of MRQ vector * @nvmebuf: pointer to lpfc nvme command HBQ data structure. + * @isr_timestamp: in jiffies. + * @cqflag: cq processing information regarding workload. * * This routine is used for processing the WQE associated with a unsolicited * event. It first determines whether there is an existing ndlp that matches @@ -2027,7 +2034,8 @@ static void lpfc_nvmet_unsol_fcp_buffer(struct lpfc_hba *phba, uint32_t idx, struct rqb_dmabuf *nvmebuf, - uint64_t isr_timestamp) + uint64_t isr_timestamp, + uint8_t cqflag) { struct lpfc_nvmet_rcv_ctx *ctxp; struct lpfc_nvmet_tgtport *tgtp; @@ -2136,24 +2144,41 @@ lpfc_nvmet_unsol_fcp_buffer(struct lpfc_hba *phba, spin_lock_init(&ctxp->ctxlock); #ifdef CONFIG_SCSI_LPFC_DEBUG_FS - if (isr_timestamp) { + if (isr_timestamp) ctxp->ts_isr_cmd = isr_timestamp; - ctxp->ts_cmd_nvme = ktime_get_ns(); - ctxp->ts_nvme_data = 0; - ctxp->ts_data_wqput = 0; - ctxp->ts_isr_data = 0; - ctxp->ts_data_nvme = 0; - ctxp->ts_nvme_status = 0; - ctxp->ts_status_wqput = 0; - ctxp->ts_isr_status = 0; - ctxp->ts_status_nvme = 0; - } else { - ctxp->ts_cmd_nvme = 0; - } + ctxp->ts_cmd_nvme = 0; + ctxp->ts_nvme_data = 0; + ctxp->ts_data_wqput = 0; + ctxp->ts_isr_data = 0; + ctxp->ts_data_nvme = 0; + ctxp->ts_nvme_status = 0; + ctxp->ts_status_wqput = 0; + ctxp->ts_isr_status = 0; + ctxp->ts_status_nvme = 0; #endif atomic_inc(&tgtp->rcv_fcp_cmd_in); - lpfc_nvmet_process_rcv_fcp_req(ctx_buf); + /* check for cq processing load */ + if (!cqflag) { + lpfc_nvmet_process_rcv_fcp_req(ctx_buf); + return; + } + + if (!queue_work(phba->wq, &ctx_buf->defer_work)) { + atomic_inc(&tgtp->rcv_fcp_cmd_drop); + lpfc_printf_log(phba, KERN_ERR, LOG_NVME, + "6325 Unable to queue work for oxid x%x. " + "FCP Drop IO [x%x x%x x%x]\n", + ctxp->oxid, + atomic_read(&tgtp->rcv_fcp_cmd_in), + atomic_read(&tgtp->rcv_fcp_cmd_out), + atomic_read(&tgtp->xmt_fcp_release)); + + spin_lock_irqsave(&ctxp->ctxlock, iflag); + lpfc_nvmet_defer_release(phba, ctxp); + spin_unlock_irqrestore(&ctxp->ctxlock, iflag); + lpfc_nvmet_unsol_fcp_issue_abort(phba, ctxp, sid, oxid); + } } /** @@ -2190,6 +2215,8 @@ lpfc_nvmet_unsol_ls_event(struct lpfc_hba *phba, struct lpfc_sli_ring *pring, * @phba: pointer to lpfc hba data structure. * @idx: relative index of MRQ vector * @nvmebuf: pointer to received nvme data structure. + * @isr_timestamp: in jiffies. + * @cqflag: cq processing information regarding workload. * * This routine is used to process an unsolicited event received from a SLI * (Service Level Interface) ring. The actual processing of the data buffer @@ -2201,14 +2228,14 @@ void lpfc_nvmet_unsol_fcp_event(struct lpfc_hba *phba, uint32_t idx, struct rqb_dmabuf *nvmebuf, - uint64_t isr_timestamp) + uint64_t isr_timestamp, + uint8_t cqflag) { if (phba->nvmet_support == 0) { lpfc_rq_buf_free(phba, &nvmebuf->hbuf); return; } - lpfc_nvmet_unsol_fcp_buffer(phba, idx, nvmebuf, - isr_timestamp); + lpfc_nvmet_unsol_fcp_buffer(phba, idx, nvmebuf, isr_timestamp, cqflag); } /** -- cgit From 79d8c4ce01b273348e98335c9a9e405e549a88c6 Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:48:56 -0700 Subject: scsi: lpfc: Fix nvmet handling of received ABTS for unmapped frames The driver currently is relying on firmware to match ABTSs to existing exchanges. This works fine as long as an exchange has been assigned to the io and work posted to it. However, for unmapped frames (rxid=0xFFFF), the driver has yet to assign an xri. The driver was blindly saying it couldn't match the ABTS and sending the BA_xxx. However, the command frame may have been in queues waiting on xri's before posting to the nvmet_fc layer. When xri's became available, the command frame would still be pushed to the transport and that io would execute, even though the io had been killed by ABTS. The initiator, seeing the io ABTS'd, would reuse the exchange for a different io which would be received on the target and pushed up. If the "zombie" io then came back down and started transmitting, the initiator would match the oxid and accept erroneous data. Bad things happened. Add tracking of active exchanges in the target to allow matching of a received ABTS against active or pending IO requests. If the ABTS is matched to a pending or active IO, the drive initiates cleanup and conditionally notifies the transport. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 234 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 209 insertions(+), 25 deletions(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index 95386f90a874..a943b2a20001 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -220,19 +220,66 @@ lpfc_nvmet_cmd_template(void) /* Word 12, 13, 14, 15 - is zero */ } +struct lpfc_nvmet_rcv_ctx * +lpfc_nvmet_get_ctx_for_xri(struct lpfc_hba *phba, u16 xri) +{ + struct lpfc_nvmet_rcv_ctx *ctxp; + unsigned long iflag; + bool found = false; + + spin_lock_irqsave(&phba->sli4_hba.t_active_list_lock, iflag); + list_for_each_entry(ctxp, &phba->sli4_hba.t_active_ctx_list, list) { + if (ctxp->ctxbuf->sglq->sli4_xritag != xri) + continue; + + found = true; + break; + } + spin_unlock_irqrestore(&phba->sli4_hba.t_active_list_lock, iflag); + if (found) + return ctxp; + + return NULL; +} + +struct lpfc_nvmet_rcv_ctx * +lpfc_nvmet_get_ctx_for_oxid(struct lpfc_hba *phba, u16 oxid, u32 sid) +{ + struct lpfc_nvmet_rcv_ctx *ctxp; + unsigned long iflag; + bool found = false; + + spin_lock_irqsave(&phba->sli4_hba.t_active_list_lock, iflag); + list_for_each_entry(ctxp, &phba->sli4_hba.t_active_ctx_list, list) { + if (ctxp->oxid != oxid || ctxp->sid != sid) + continue; + + found = true; + break; + } + spin_unlock_irqrestore(&phba->sli4_hba.t_active_list_lock, iflag); + if (found) + return ctxp; + + return NULL; +} + static void lpfc_nvmet_defer_release(struct lpfc_hba *phba, struct lpfc_nvmet_rcv_ctx *ctxp) { lockdep_assert_held(&ctxp->ctxlock); lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6313 NVMET Defer ctx release xri x%x flg x%x\n", + "6313 NVMET Defer ctx release oxid x%x flg x%x\n", ctxp->oxid, ctxp->flag); if (ctxp->flag & LPFC_NVMET_CTX_RLS) return; ctxp->flag |= LPFC_NVMET_CTX_RLS; + spin_lock(&phba->sli4_hba.t_active_list_lock); + list_del(&ctxp->list); + spin_unlock(&phba->sli4_hba.t_active_list_lock); spin_lock(&phba->sli4_hba.abts_nvmet_buf_list_lock); list_add_tail(&ctxp->list, &phba->sli4_hba.lpfc_abts_nvmet_ctx_list); spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); @@ -410,9 +457,7 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct lpfc_nvmet_ctxbuf *ctx_buf) #endif atomic_inc(&tgtp->rcv_fcp_cmd_in); - /* flag new work queued, replacement buffer has already - * been reposted - */ + /* Indicate that a replacement buffer has been posted */ spin_lock_irqsave(&ctxp->ctxlock, iflag); ctxp->flag |= LPFC_NVMET_CTX_REUSE_WQ; spin_unlock_irqrestore(&ctxp->ctxlock, iflag); @@ -441,6 +486,9 @@ lpfc_nvmet_ctxbuf_post(struct lpfc_hba *phba, struct lpfc_nvmet_ctxbuf *ctx_buf) * Use the CPU context list, from the MRQ the IO was received on * (ctxp->idx), to save context structure. */ + spin_lock_irqsave(&phba->sli4_hba.t_active_list_lock, iflag); + list_del_init(&ctxp->list); + spin_unlock_irqrestore(&phba->sli4_hba.t_active_list_lock, iflag); cpu = raw_smp_processor_id(); infop = lpfc_get_ctx_list(phba, cpu, ctxp->idx); spin_lock_irqsave(&infop->nvmet_ctx_list_lock, iflag); @@ -708,8 +756,10 @@ lpfc_nvmet_xmt_fcp_op_cmp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdwqe, } lpfc_printf_log(phba, KERN_INFO, logerr, - "6315 IO Error Cmpl xri x%x: %x/%x XBUSY:x%x\n", - ctxp->oxid, status, result, ctxp->flag); + "6315 IO Error Cmpl oxid: x%x xri: x%x %x/%x " + "XBUSY:x%x\n", + ctxp->oxid, ctxp->ctxbuf->sglq->sli4_xritag, + status, result, ctxp->flag); } else { rsp->fcp_error = NVME_SC_SUCCESS; @@ -930,7 +980,7 @@ lpfc_nvmet_xmt_fcp_op(struct nvmet_fc_target_port *tgtport, (ctxp->state == LPFC_NVMET_STE_ABORT)) { atomic_inc(&lpfc_nvmep->xmt_fcp_drop); lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR, - "6102 IO xri x%x aborted\n", + "6102 IO oxid x%x aborted\n", ctxp->oxid); rc = -ENXIO; goto aerr; @@ -1030,7 +1080,7 @@ lpfc_nvmet_xmt_fcp_abort(struct nvmet_fc_target_port *tgtport, ctxp->hdwq = &phba->sli4_hba.hdwq[0]; lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6103 NVMET Abort op: oxri x%x flg x%x ste %d\n", + "6103 NVMET Abort op: oxid x%x flg x%x ste %d\n", ctxp->oxid, ctxp->flag, ctxp->state); lpfc_nvmeio_data(phba, "NVMET FCP ABRT: xri x%x flg x%x ste x%x\n", @@ -1043,7 +1093,7 @@ lpfc_nvmet_xmt_fcp_abort(struct nvmet_fc_target_port *tgtport, /* Since iaab/iaar are NOT set, we need to check * if the firmware is in process of aborting IO */ - if (ctxp->flag & LPFC_NVMET_XBUSY) { + if (ctxp->flag & (LPFC_NVMET_XBUSY | LPFC_NVMET_ABORT_OP)) { spin_unlock_irqrestore(&ctxp->ctxlock, flags); return; } @@ -1106,6 +1156,7 @@ lpfc_nvmet_xmt_fcp_release(struct nvmet_fc_target_port *tgtport, ctxp->state, aborting); atomic_inc(&lpfc_nvmep->xmt_fcp_release); + ctxp->flag &= ~LPFC_NVMET_TNOTIFY; if (aborting) return; @@ -1130,7 +1181,7 @@ lpfc_nvmet_defer_rcv(struct nvmet_fc_target_port *tgtport, if (!nvmebuf) { lpfc_printf_log(phba, KERN_INFO, LOG_NVME_IOERR, - "6425 Defer rcv: no buffer xri x%x: " + "6425 Defer rcv: no buffer oxid x%x: " "flg %x ste %x\n", ctxp->oxid, ctxp->flag, ctxp->state); return; @@ -1510,6 +1561,7 @@ lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, uint16_t rxid = bf_get(lpfc_wcqe_xa_remote_xid, axri); struct lpfc_nvmet_rcv_ctx *ctxp, *next_ctxp; struct lpfc_nvmet_tgtport *tgtp; + struct nvmefc_tgt_fcp_req *req = NULL; struct lpfc_nodelist *ndlp; unsigned long iflag = 0; int rrq_empty = 0; @@ -1540,7 +1592,7 @@ lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, */ if (ctxp->flag & LPFC_NVMET_CTX_RLS && !(ctxp->flag & LPFC_NVMET_ABORT_OP)) { - list_del(&ctxp->list); + list_del_init(&ctxp->list); released = true; } ctxp->flag &= ~LPFC_NVMET_XBUSY; @@ -1560,7 +1612,7 @@ lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, } lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6318 XB aborted oxid %x flg x%x (%x)\n", + "6318 XB aborted oxid x%x flg x%x (%x)\n", ctxp->oxid, ctxp->flag, released); if (released) lpfc_nvmet_ctxbuf_post(phba, ctxp->ctxbuf); @@ -1571,6 +1623,32 @@ lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, } spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); spin_unlock_irqrestore(&phba->hbalock, iflag); + + ctxp = lpfc_nvmet_get_ctx_for_xri(phba, xri); + if (ctxp) { + /* + * Abort already done by FW, so BA_ACC sent. + * However, the transport may be unaware. + */ + lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, + "6323 NVMET Rcv ABTS xri x%x ctxp state x%x " + "flag x%x oxid x%x rxid x%x\n", + xri, ctxp->state, ctxp->flag, ctxp->oxid, + rxid); + + spin_lock_irqsave(&ctxp->ctxlock, iflag); + ctxp->flag |= LPFC_NVMET_ABTS_RCV; + ctxp->state = LPFC_NVMET_STE_ABORT; + spin_unlock_irqrestore(&ctxp->ctxlock, iflag); + + lpfc_nvmeio_data(phba, + "NVMET ABTS RCV: xri x%x CPU %02x rjt %d\n", + xri, smp_processor_id(), 0); + + req = &ctxp->ctx.fcp_req; + if (req) + nvmet_fc_rcv_fcp_abort(phba->targetport, req); + } #endif } @@ -1583,18 +1661,18 @@ lpfc_nvmet_rcv_unsol_abort(struct lpfc_vport *vport, struct lpfc_nvmet_rcv_ctx *ctxp, *next_ctxp; struct nvmefc_tgt_fcp_req *rsp; uint32_t sid; - uint16_t xri; + uint16_t oxid, xri; unsigned long iflag = 0; - xri = be16_to_cpu(fc_hdr->fh_ox_id); sid = sli4_sid_from_fc_hdr(fc_hdr); + oxid = be16_to_cpu(fc_hdr->fh_ox_id); spin_lock_irqsave(&phba->hbalock, iflag); spin_lock(&phba->sli4_hba.abts_nvmet_buf_list_lock); list_for_each_entry_safe(ctxp, next_ctxp, &phba->sli4_hba.lpfc_abts_nvmet_ctx_list, list) { - if (ctxp->oxid != xri || ctxp->sid != sid) + if (ctxp->oxid != oxid || ctxp->sid != sid) continue; xri = ctxp->ctxbuf->sglq->sli4_xritag; @@ -1623,11 +1701,92 @@ lpfc_nvmet_rcv_unsol_abort(struct lpfc_vport *vport, spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); spin_unlock_irqrestore(&phba->hbalock, iflag); - lpfc_nvmeio_data(phba, "NVMET ABTS RCV: xri x%x CPU %02x rjt %d\n", - xri, raw_smp_processor_id(), 1); + /* check the wait list */ + if (phba->sli4_hba.nvmet_io_wait_cnt) { + struct rqb_dmabuf *nvmebuf; + struct fc_frame_header *fc_hdr_tmp; + u32 sid_tmp; + u16 oxid_tmp; + bool found = false; + + spin_lock_irqsave(&phba->sli4_hba.nvmet_io_wait_lock, iflag); + + /* match by oxid and s_id */ + list_for_each_entry(nvmebuf, + &phba->sli4_hba.lpfc_nvmet_io_wait_list, + hbuf.list) { + fc_hdr_tmp = (struct fc_frame_header *) + (nvmebuf->hbuf.virt); + oxid_tmp = be16_to_cpu(fc_hdr_tmp->fh_ox_id); + sid_tmp = sli4_sid_from_fc_hdr(fc_hdr_tmp); + if (oxid_tmp != oxid || sid_tmp != sid) + continue; + + lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, + "6321 NVMET Rcv ABTS oxid x%x from x%x " + "is waiting for a ctxp\n", + oxid, sid); + + list_del_init(&nvmebuf->hbuf.list); + phba->sli4_hba.nvmet_io_wait_cnt--; + found = true; + break; + } + spin_unlock_irqrestore(&phba->sli4_hba.nvmet_io_wait_lock, + iflag); + + /* free buffer since already posted a new DMA buffer to RQ */ + if (found) { + nvmebuf->hrq->rqbp->rqb_free_buffer(phba, nvmebuf); + /* Respond with BA_ACC accordingly */ + lpfc_sli4_seq_abort_rsp(vport, fc_hdr, 1); + return 0; + } + } + + /* check active list */ + ctxp = lpfc_nvmet_get_ctx_for_oxid(phba, oxid, sid); + if (ctxp) { + xri = ctxp->ctxbuf->sglq->sli4_xritag; + + spin_lock_irqsave(&ctxp->ctxlock, iflag); + ctxp->flag |= (LPFC_NVMET_ABTS_RCV | LPFC_NVMET_ABORT_OP); + spin_unlock_irqrestore(&ctxp->ctxlock, iflag); + + lpfc_nvmeio_data(phba, + "NVMET ABTS RCV: xri x%x CPU %02x rjt %d\n", + xri, raw_smp_processor_id(), 0); + + lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, + "6322 NVMET Rcv ABTS:acc oxid x%x xri x%x " + "flag x%x state x%x\n", + ctxp->oxid, xri, ctxp->flag, ctxp->state); + + if (ctxp->flag & LPFC_NVMET_TNOTIFY) { + /* Notify the transport */ + nvmet_fc_rcv_fcp_abort(phba->targetport, + &ctxp->ctx.fcp_req); + } else { + spin_lock_irqsave(&ctxp->ctxlock, iflag); + lpfc_nvmet_defer_release(phba, ctxp); + spin_unlock_irqrestore(&ctxp->ctxlock, iflag); + } + if (ctxp->state == LPFC_NVMET_STE_RCV) + lpfc_nvmet_unsol_fcp_issue_abort(phba, ctxp, ctxp->sid, + ctxp->oxid); + else + lpfc_nvmet_sol_fcp_issue_abort(phba, ctxp, ctxp->sid, + ctxp->oxid); + + lpfc_sli4_seq_abort_rsp(vport, fc_hdr, 0); + return 0; + } + + lpfc_nvmeio_data(phba, "NVMET ABTS RCV: oxid x%x CPU %02x rjt %d\n", + oxid, raw_smp_processor_id(), 1); lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6320 NVMET Rcv ABTS:rjt xid x%x\n", xri); + "6320 NVMET Rcv ABTS:rjt oxid x%x\n", oxid); /* Respond with BA_RJT accordingly */ lpfc_sli4_seq_abort_rsp(vport, fc_hdr, 0); @@ -1711,6 +1870,18 @@ lpfc_nvmet_wqfull_process(struct lpfc_hba *phba, spin_unlock_irqrestore(&pring->ring_lock, iflags); return; } + if (rc == WQE_SUCCESS) { +#ifdef CONFIG_SCSI_LPFC_DEBUG_FS + if (ctxp->ts_cmd_nvme) { + if (ctxp->ctx.fcp_req.op == NVMET_FCOP_RSP) + ctxp->ts_status_wqput = ktime_get_ns(); + else + ctxp->ts_data_wqput = ktime_get_ns(); + } +#endif + } else { + WARN_ON(rc); + } } wq->q_flag &= ~HBA_NVMET_WQFULL; spin_unlock_irqrestore(&pring->ring_lock, iflags); @@ -1876,8 +2047,16 @@ lpfc_nvmet_process_rcv_fcp_req(struct lpfc_nvmet_ctxbuf *ctx_buf) return; } + if (ctxp->flag & LPFC_NVMET_ABTS_RCV) { + lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR, + "6324 IO oxid x%x aborted\n", + ctxp->oxid); + return; + } + payload = (uint32_t *)(nvmebuf->dbuf.virt); tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private; + ctxp->flag |= LPFC_NVMET_TNOTIFY; #ifdef CONFIG_SCSI_LPFC_DEBUG_FS if (ctxp->ts_isr_cmd) ctxp->ts_cmd_nvme = ktime_get_ns(); @@ -1931,6 +2110,7 @@ lpfc_nvmet_process_rcv_fcp_req(struct lpfc_nvmet_ctxbuf *ctx_buf) phba->sli4_hba.nvmet_mrq_data[qno], 1, qno); return; } + ctxp->flag &= ~LPFC_NVMET_TNOTIFY; atomic_inc(&tgtp->rcv_fcp_cmd_drop); lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR, "2582 FCP Drop IO x%x: err x%x: x%x x%x x%x\n", @@ -2122,6 +2302,9 @@ lpfc_nvmet_unsol_fcp_buffer(struct lpfc_hba *phba, sid = sli4_sid_from_fc_hdr(fc_hdr); ctxp = (struct lpfc_nvmet_rcv_ctx *)ctx_buf->context; + spin_lock_irqsave(&phba->sli4_hba.t_active_list_lock, iflag); + list_add_tail(&ctxp->list, &phba->sli4_hba.t_active_ctx_list); + spin_unlock_irqrestore(&phba->sli4_hba.t_active_list_lock, iflag); if (ctxp->state != LPFC_NVMET_STE_FREE) { lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR, "6414 NVMET Context corrupt %d %d oxid x%x\n", @@ -2773,7 +2956,7 @@ lpfc_nvmet_sol_fcp_abort_cmp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdwqe, if ((ctxp->flag & LPFC_NVMET_CTX_RLS) && !(ctxp->flag & LPFC_NVMET_XBUSY)) { spin_lock(&phba->sli4_hba.abts_nvmet_buf_list_lock); - list_del(&ctxp->list); + list_del_init(&ctxp->list); spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); released = true; } @@ -2782,7 +2965,7 @@ lpfc_nvmet_sol_fcp_abort_cmp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdwqe, atomic_inc(&tgtp->xmt_abort_rsp); lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6165 ABORT cmpl: xri x%x flg x%x (%d) " + "6165 ABORT cmpl: oxid x%x flg x%x (%d) " "WCQE: %08x %08x %08x %08x\n", ctxp->oxid, ctxp->flag, released, wcqe->word0, wcqe->total_data_placed, @@ -2857,7 +3040,7 @@ lpfc_nvmet_unsol_fcp_abort_cmp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdwqe, if ((ctxp->flag & LPFC_NVMET_CTX_RLS) && !(ctxp->flag & LPFC_NVMET_XBUSY)) { spin_lock(&phba->sli4_hba.abts_nvmet_buf_list_lock); - list_del(&ctxp->list); + list_del_init(&ctxp->list); spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); released = true; } @@ -2866,7 +3049,7 @@ lpfc_nvmet_unsol_fcp_abort_cmp(struct lpfc_hba *phba, struct lpfc_iocbq *cmdwqe, atomic_inc(&tgtp->xmt_abort_rsp); lpfc_printf_log(phba, KERN_INFO, LOG_NVME_ABTS, - "6316 ABTS cmpl xri x%x flg x%x (%x) " + "6316 ABTS cmpl oxid x%x flg x%x (%x) " "WCQE: %08x %08x %08x %08x\n", ctxp->oxid, ctxp->flag, released, wcqe->word0, wcqe->total_data_placed, @@ -3237,7 +3420,7 @@ aerr: spin_lock_irqsave(&ctxp->ctxlock, flags); if (ctxp->flag & LPFC_NVMET_CTX_RLS) { spin_lock(&phba->sli4_hba.abts_nvmet_buf_list_lock); - list_del(&ctxp->list); + list_del_init(&ctxp->list); spin_unlock(&phba->sli4_hba.abts_nvmet_buf_list_lock); released = true; } @@ -3246,8 +3429,9 @@ aerr: atomic_inc(&tgtp->xmt_abort_rsp_error); lpfc_printf_log(phba, KERN_ERR, LOG_NVME_ABTS, - "6135 Failed to Issue ABTS for oxid x%x. Status x%x\n", - ctxp->oxid, rc); + "6135 Failed to Issue ABTS for oxid x%x. Status x%x " + "(%x)\n", + ctxp->oxid, rc, released); if (released) lpfc_nvmet_ctxbuf_post(phba, ctxp->ctxbuf); return 1; -- cgit From 51d23fb28ccb355ee4d26dedacca24c171c2f664 Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:48:59 -0700 Subject: scsi: lpfc: Prevent 'use after free' memory overwrite in nvmet LS handling Use-after-free memory overwrite detected. Problem reported by Ewan Milne at Red Hat after running lpfc target with additional memory checking enabled. Race condition when lpfc_nvmet_xmt_ls_rsp_cmp frees the ctxp memory in interrupt context before lpfc_nvmet_xmt_ls_rsp clears a field in the ctxp after successfully issuing the wqe. Remove the unnecessary ctxp write after reposting the rq buffer. The ctxp->rqb_buffer field is not checked in LS handling after the wqe is submitted. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Reported-by: Ewan Milne Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index a943b2a20001..08c2c4e3515b 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -907,7 +907,6 @@ lpfc_nvmet_xmt_ls_rsp(struct nvmet_fc_target_port *tgtport, * before freeing ctxp and iocbq. */ lpfc_in_buf_free(phba, &nvmebuf->dbuf); - ctxp->rqb_buffer = 0; atomic_inc(&nvmep->xmt_ls_rsp); return 0; } -- cgit From 6594d31bab02e4a1d02355ff2f16a87dfc11b34f Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:49:00 -0700 Subject: scsi: lpfc: Cancel queued work for an IO when processing a received ABTS When queued work is executed posting a new command to the transport the driver is reporting a null buffer. The driver had received an ABTS which matched a command that had been scheduled for delivery to the transport. The driver proceeded to cancel the command, but the work item was never cancelled. Fix by cancelling the queued work item. Also turns out the ABTS response was not properly sending a BA_ACC, so set the flag to send the ACC. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index 08c2c4e3515b..36e8d842d973 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -1766,6 +1766,7 @@ lpfc_nvmet_rcv_unsol_abort(struct lpfc_vport *vport, nvmet_fc_rcv_fcp_abort(phba->targetport, &ctxp->ctx.fcp_req); } else { + cancel_work_sync(&ctxp->ctxbuf->defer_work); spin_lock_irqsave(&ctxp->ctxlock, iflag); lpfc_nvmet_defer_release(phba, ctxp); spin_unlock_irqrestore(&ctxp->ctxlock, iflag); @@ -1777,7 +1778,7 @@ lpfc_nvmet_rcv_unsol_abort(struct lpfc_vport *vport, lpfc_nvmet_sol_fcp_issue_abort(phba, ctxp, ctxp->sid, ctxp->oxid); - lpfc_sli4_seq_abort_rsp(vport, fc_hdr, 0); + lpfc_sli4_seq_abort_rsp(vport, fc_hdr, 1); return 0; } -- cgit From 01d53c04637f96bbb753e9c7f61392c40671b564 Mon Sep 17 00:00:00 2001 From: James Smart Date: Tue, 21 May 2019 17:49:10 -0700 Subject: scsi: lpfc: Fix kernel warnings related to smp_processor_id() Kernel warnings may be seen with preempt debugging enabled. Replace smp_processor_id calls with raw_smp_processor_id or cpu information stored in hdwq structures. Signed-off-by: Dick Kennedy Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index 36e8d842d973..eb93189f4544 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -1642,7 +1642,7 @@ lpfc_sli4_nvmet_xri_aborted(struct lpfc_hba *phba, lpfc_nvmeio_data(phba, "NVMET ABTS RCV: xri x%x CPU %02x rjt %d\n", - xri, smp_processor_id(), 0); + xri, raw_smp_processor_id(), 0); req = &ctxp->ctx.fcp_req; if (req) -- cgit From d7b761b0694986ea811c0daaa1178bfaaddf036d Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Fri, 31 May 2019 23:28:41 +0800 Subject: scsi: lpfc: Make some symbols static Fix sparse warnings: drivers/scsi/lpfc/lpfc_sli.c:115:1: warning: symbol 'lpfc_sli4_pcimem_bcopy' was not declared. Should it be static? drivers/scsi/lpfc/lpfc_sli.c:7854:1: warning: symbol 'lpfc_sli4_process_missed_mbox_completions' was not declared. Should it be static? drivers/scsi/lpfc/lpfc_nvmet.c:223:27: warning: symbol 'lpfc_nvmet_get_ctx_for_xri' was not declared. Should it be static? drivers/scsi/lpfc/lpfc_nvmet.c:245:27: warning: symbol 'lpfc_nvmet_get_ctx_for_oxid' was not declared. Should it be static? drivers/scsi/lpfc/lpfc_init.c:75:10: warning: symbol 'lpfc_present_cpu' was not declared. Should it be static? Reported-by: Hulk Robot Signed-off-by: YueHaibing Acked-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index eb93189f4544..e471bbcca838 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -220,7 +220,7 @@ lpfc_nvmet_cmd_template(void) /* Word 12, 13, 14, 15 - is zero */ } -struct lpfc_nvmet_rcv_ctx * +static struct lpfc_nvmet_rcv_ctx * lpfc_nvmet_get_ctx_for_xri(struct lpfc_hba *phba, u16 xri) { struct lpfc_nvmet_rcv_ctx *ctxp; @@ -242,7 +242,7 @@ lpfc_nvmet_get_ctx_for_xri(struct lpfc_hba *phba, u16 xri) return NULL; } -struct lpfc_nvmet_rcv_ctx * +static struct lpfc_nvmet_rcv_ctx * lpfc_nvmet_get_ctx_for_oxid(struct lpfc_hba *phba, u16 oxid, u32 sid) { struct lpfc_nvmet_rcv_ctx *ctxp; -- cgit From 336df6eb628298e27e40e23d1eb00a0fb7083269 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Wed, 5 Jun 2019 22:24:21 -0700 Subject: scsi: lpfc: Avoid unused function warnings When building powerpc pseries_defconfig or powernv_defconfig: drivers/scsi/lpfc/lpfc_nvmet.c:224:1: error: unused function 'lpfc_nvmet_get_ctx_for_xri' [-Werror,-Wunused-function] drivers/scsi/lpfc/lpfc_nvmet.c:246:1: error: unused function 'lpfc_nvmet_get_ctx_for_oxid' [-Werror,-Wunused-function] These functions are only compiled when CONFIG_NVME_TARGET_FC is enabled. Use that same condition so there is no more warning. While the fixes commit did not introduce these functions, it caused these warnings. Fixes: 4064b27417a7 ("scsi: lpfc: Make some symbols static") Signed-off-by: Nathan Chancellor Acked-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_nvmet.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/scsi/lpfc/lpfc_nvmet.c') diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c index e471bbcca838..f3d9a5545164 100644 --- a/drivers/scsi/lpfc/lpfc_nvmet.c +++ b/drivers/scsi/lpfc/lpfc_nvmet.c @@ -220,6 +220,7 @@ lpfc_nvmet_cmd_template(void) /* Word 12, 13, 14, 15 - is zero */ } +#if (IS_ENABLED(CONFIG_NVME_TARGET_FC)) static struct lpfc_nvmet_rcv_ctx * lpfc_nvmet_get_ctx_for_xri(struct lpfc_hba *phba, u16 xri) { @@ -263,6 +264,7 @@ lpfc_nvmet_get_ctx_for_oxid(struct lpfc_hba *phba, u16 oxid, u32 sid) return NULL; } +#endif static void lpfc_nvmet_defer_release(struct lpfc_hba *phba, struct lpfc_nvmet_rcv_ctx *ctxp) -- cgit