scsi: lpfc: Refactor and clean up mailbox command memory free

A lot of repeated clean up code exists when freeing mailbox commands in
lpfc_mem_free_all().

Introduce a lpfc_mem_free_sli_mbox() helper routine to refactor the
copy-paste code.  Additionally, reinitialize the mailbox command structure
context pointers to NULL in lpfc_sli4_mbox_cmd_free().

Signed-off-by: Justin Tee <justin.tee@broadcom.com>
Link: https://lore.kernel.org/r/20231031191224.150862-7-justintee8345@gmail.com
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/lpfc/lpfc_mbox.c b/drivers/scsi/lpfc/lpfc_mbox.c
index 0dfdc0c4c08c65ef67895db061fcfc5490bbd7e6..b5ab44234f20b4c737b1d1cfb714c8bc7f5f3b70 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_mbox.c
+++ b/drivers/scsi/lpfc/lpfc_mbox.c
@@ -1814,7 +1814,9 @@ lpfc_sli4_mbox_cmd_free(struct lpfc_hba *phba, struct lpfcMboxq *mbox)
                dma_free_coherent(&phba->pcidev->dev, SLI4_PAGE_SIZE,
                                  mbox->sge_array->addr[sgentry], phyaddr);
        }
-       /* Free the sge address array memory */
+       /* Reinitialize the context pointers to avoid stale usage. */
+       mbox->ctx_buf = NULL;
+       mbox->context3 = NULL;
        kfree(mbox->sge_array);
        /* Finally, free the mailbox command itself */
        mempool_free(mbox, phba->mbox_mem_pool);

diff --git a/drivers/scsi/lpfc/lpfc_mem.c b/drivers/scsi/lpfc/lpfc_mem.c
index 89cbeba06aeab675f9f81b86ba3f57ac53253f9b..ef6e5256a35e5c769693e7d052243203d64e82b8 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_mem.c
+++ b/drivers/scsi/lpfc/lpfc_mem.c
@@ -48,6 +48,29 @@
 #define LPFC_RRQ_POOL_SIZE     256     /* max elements in non-DMA  pool */
 #define LPFC_MBX_POOL_SIZE     256     /* max elements in MBX non-DMA pool */
 
+/* lpfc_mbox_free_sli_mbox
+ *
+ * @phba: HBA to free memory for
+ * @mbox: mailbox command to free
+ *
+ * This routine detects the mbox type and calls the correct
+ * free routine to fully release all associated memory.
+ */
+static void
+lpfc_mem_free_sli_mbox(struct lpfc_hba *phba, LPFC_MBOXQ_t *mbox)
+{
+       /* Detect if the caller's mbox is an SLI4_CONFIG type.  If so, this
+        * mailbox type requires a different cleanup routine.  Otherwise, the
+        * mailbox is just an mbuf and mem_pool release.
+        */
+       if (phba->sli_rev == LPFC_SLI_REV4 &&
+           bf_get(lpfc_mqe_command, &mbox->u.mqe) == MBX_SLI4_CONFIG) {
+               lpfc_sli4_mbox_cmd_free(phba, mbox);
+       } else {
+               lpfc_mbox_rsrc_cleanup(phba, mbox, MBOX_THD_UNLOCKED);
+       }
+}
+
 int
 lpfc_mem_alloc_active_rrq_pool_s4(struct lpfc_hba *phba) {
        size_t bytes;
@@ -288,27 +311,16 @@ lpfc_mem_free_all(struct lpfc_hba *phba)
 {
        struct lpfc_sli *psli = &phba->sli;
        LPFC_MBOXQ_t *mbox, *next_mbox;
-       struct lpfc_dmabuf   *mp;
 
        /* Free memory used in mailbox queue back to mailbox memory pool */
        list_for_each_entry_safe(mbox, next_mbox, &psli->mboxq, list) {
-               mp = (struct lpfc_dmabuf *)(mbox->ctx_buf);
-               if (mp) {
-                       lpfc_mbuf_free(phba, mp->virt, mp->phys);
-                       kfree(mp);
-               }
                list_del(&mbox->list);
-               mempool_free(mbox, phba->mbox_mem_pool);
+               lpfc_mem_free_sli_mbox(phba, mbox);
        }
        /* Free memory used in mailbox cmpl list back to mailbox memory pool */
        list_for_each_entry_safe(mbox, next_mbox, &psli->mboxq_cmpl, list) {
-               mp = (struct lpfc_dmabuf *)(mbox->ctx_buf);
-               if (mp) {
-                       lpfc_mbuf_free(phba, mp->virt, mp->phys);
-                       kfree(mp);
-               }
                list_del(&mbox->list);
-               mempool_free(mbox, phba->mbox_mem_pool);
+               lpfc_mem_free_sli_mbox(phba, mbox);
        }
        /* Free the active mailbox command back to the mailbox memory pool */
        spin_lock_irq(&phba->hbalock);
@@ -316,12 +328,7 @@ lpfc_mem_free_all(struct lpfc_hba *phba)
        spin_unlock_irq(&phba->hbalock);
        if (psli->mbox_active) {
                mbox = psli->mbox_active;
-               mp = (struct lpfc_dmabuf *)(mbox->ctx_buf);
-               if (mp) {
-                       lpfc_mbuf_free(phba, mp->virt, mp->phys);
-                       kfree(mp);
-               }
-               mempool_free(mbox, phba->mbox_mem_pool);
+               lpfc_mem_free_sli_mbox(phba, mbox);
                psli->mbox_active = NULL;
        }
 

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 2cb327efd57dc3900822b8ead7d26545b83ef27c..bfbc23248692e6f224c7188eb7b212483e056d19 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -22170,6 +22170,12 @@ struct lpfc_io_buf *lpfc_get_io_buf(struct lpfc_hba *phba,
  * The data will be truncated if datasz is not large enough.
  * Version 1 is not supported with Embedded mbox cmd, so we must use version 0.
  * Returns the actual bytes read from the object.
+ *
+ * This routine is hard coded to use a poll completion.  Unlike other
+ * sli4_config mailboxes, it uses lpfc_mbuf memory which is not
+ * cleaned up in lpfc_sli4_cmd_mbox_free.  If this routine is modified
+ * to use interrupt-based completions, code is needed to fully cleanup
+ * the memory.
  */
 int
 lpfc_read_object(struct lpfc_hba *phba, char *rdobject, uint32_t *datap,

diff --git a/drivers/scsi/lpfc/lpfc_sli.h b/drivers/scsi/lpfc/lpfc_sli.h
index cd33dfec758c06f76a7ce96646ff0aeb45191d91..14b8b5a3addf2d7ac23b9520b7490f9a6e3d47f6 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_sli.h
+++ b/drivers/scsi/lpfc/lpfc_sli.h
@@ -182,9 +182,11 @@ typedef struct lpfcMboxq {
                struct lpfc_mqe mqe;
        } u;
        struct lpfc_vport *vport; /* virtual port pointer */
-       void *ctx_ndlp;           /* caller ndlp information */
-       void *ctx_buf;            /* caller buffer information */
-       void *context3;
+       void *ctx_ndlp;           /* an lpfc_nodelist pointer */
+       void *ctx_buf;            /* an lpfc_dmabuf pointer */
+       void *context3;           /* a generic pointer.  Code must
+                                  * accommodate the actual datatype.
+                                  */
 
        void (*mbox_cmpl) (struct lpfc_hba *, struct lpfcMboxq *);
        uint8_t mbox_flag;