From 304ee43238fed517faa123e034b593905b8679f8 Mon Sep 17 00:00:00 2001 From: James Smart Date: Sun, 11 Apr 2021 18:31:17 -0700 Subject: scsi: lpfc: Fix error handling for mailboxes completed in MBX_POLL mode In SLI-4, when performing a mailbox command with MBX_POLL, the driver uses the BMBX register to send the command rather than the MQ. A flag is set indicating the BMBX register is active and saves the mailbox job struct (mboxq) in the mbox_active element of the adapter. The routine then waits for completion or timeout. The mailbox job struct is not freed by the routine. In cases of timeout, the adapter will be reset. The lpfc_sli_mbox_sys_flush() routine will clean up the mbox in preparation for the reset. It clears the BMBX active flag and marks the job structure as MBX_NOT_FINISHED. But, it never frees the mboxq job structure. Expectation in both normal completion and timeout cases is that the issuer of the mbx command will free the structure. Unfortunately, not all calling paths are freeing the memory in cases of error. All calling paths were looked at and updated, if missing, to free the mboxq memory regardless of completion status. Link: https://lore.kernel.org/r/20210412013127.2387-7-jsmart2021@gmail.com Co-developed-by: Justin Tee Signed-off-by: Justin Tee Signed-off-by: James Smart Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_attr.c | 75 ++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 30 deletions(-) (limited to 'drivers/scsi/lpfc/lpfc_attr.c') diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c index 59ca32d850e3..e723b716608a 100644 --- a/drivers/scsi/lpfc/lpfc_attr.c +++ b/drivers/scsi/lpfc/lpfc_attr.c @@ -1685,8 +1685,7 @@ lpfc_set_trunking(struct lpfc_hba *phba, char *buff_out) lpfc_printf_log(phba, KERN_ERR, LOG_MBOX, "0071 Set trunk mode failed with status: %d", rc); - if (rc != MBX_TIMEOUT) - mempool_free(mbox, phba->mbox_mem_pool); + mempool_free(mbox, phba->mbox_mem_pool); return 0; } @@ -6791,15 +6790,19 @@ lpfc_get_stats(struct Scsi_Host *shost) pmboxq->ctx_buf = NULL; pmboxq->vport = vport; - if (vport->fc_flag & FC_OFFLINE_MODE) + if (vport->fc_flag & FC_OFFLINE_MODE) { rc = lpfc_sli_issue_mbox(phba, pmboxq, MBX_POLL); - else - rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); - - if (rc != MBX_SUCCESS) { - if (rc != MBX_TIMEOUT) + if (rc != MBX_SUCCESS) { mempool_free(pmboxq, phba->mbox_mem_pool); - return NULL; + return NULL; + } + } else { + rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); + if (rc != MBX_SUCCESS) { + if (rc != MBX_TIMEOUT) + mempool_free(pmboxq, phba->mbox_mem_pool); + return NULL; + } } memset(hs, 0, sizeof (struct fc_host_statistics)); @@ -6823,15 +6826,19 @@ lpfc_get_stats(struct Scsi_Host *shost) pmboxq->ctx_buf = NULL; pmboxq->vport = vport; - if (vport->fc_flag & FC_OFFLINE_MODE) + if (vport->fc_flag & FC_OFFLINE_MODE) { rc = lpfc_sli_issue_mbox(phba, pmboxq, MBX_POLL); - else - rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); - - if (rc != MBX_SUCCESS) { - if (rc != MBX_TIMEOUT) + if (rc != MBX_SUCCESS) { mempool_free(pmboxq, phba->mbox_mem_pool); - return NULL; + return NULL; + } + } else { + rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); + if (rc != MBX_SUCCESS) { + if (rc != MBX_TIMEOUT) + mempool_free(pmboxq, phba->mbox_mem_pool); + return NULL; + } } hs->link_failure_count = pmb->un.varRdLnk.linkFailureCnt; @@ -6904,15 +6911,19 @@ lpfc_reset_stats(struct Scsi_Host *shost) pmboxq->vport = vport; if ((vport->fc_flag & FC_OFFLINE_MODE) || - (!(psli->sli_flag & LPFC_SLI_ACTIVE))) + (!(psli->sli_flag & LPFC_SLI_ACTIVE))) { rc = lpfc_sli_issue_mbox(phba, pmboxq, MBX_POLL); - else - rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); - - if (rc != MBX_SUCCESS) { - if (rc != MBX_TIMEOUT) + if (rc != MBX_SUCCESS) { mempool_free(pmboxq, phba->mbox_mem_pool); - return; + return; + } + } else { + rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); + if (rc != MBX_SUCCESS) { + if (rc != MBX_TIMEOUT) + mempool_free(pmboxq, phba->mbox_mem_pool); + return; + } } memset(pmboxq, 0, sizeof(LPFC_MBOXQ_t)); @@ -6922,15 +6933,19 @@ lpfc_reset_stats(struct Scsi_Host *shost) pmboxq->vport = vport; if ((vport->fc_flag & FC_OFFLINE_MODE) || - (!(psli->sli_flag & LPFC_SLI_ACTIVE))) + (!(psli->sli_flag & LPFC_SLI_ACTIVE))) { rc = lpfc_sli_issue_mbox(phba, pmboxq, MBX_POLL); - else + if (rc != MBX_SUCCESS) { + mempool_free(pmboxq, phba->mbox_mem_pool); + return; + } + } else { rc = lpfc_sli_issue_mbox_wait(phba, pmboxq, phba->fc_ratov * 2); - - if (rc != MBX_SUCCESS) { - if (rc != MBX_TIMEOUT) - mempool_free( pmboxq, phba->mbox_mem_pool); - return; + if (rc != MBX_SUCCESS) { + if (rc != MBX_TIMEOUT) + mempool_free(pmboxq, phba->mbox_mem_pool); + return; + } } lso->link_failure_count = pmb->un.varRdLnk.linkFailureCnt; -- cgit v1.2.3