scsi: lpfc: resolve lockdep warnings

There were a number of erroneous comments and incorrect older lockdep
checks that were causing a number of warnings.

Resolve the following:

 - Inconsistent lock state warnings in lpfc_nvme_info_show().

 - Fixed comments and code on sequences where ring lock is now held instead
   of hbalock.

 - Reworked calling sequences around lpfc_sli_iocbq_lookup(). Rather than
   locking prior to the routine and have routine guess on what lock, take
   the lock within the routine. The lockdep check becomes unnecessary.

 - Fixed comments and removed erroneous hbalock checks.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
CC: Bart Van Assche <bvanassche@acm.org>
Tested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index e9adb3f1961d86967c53d82c3149a0693ac4fbb0..31b963ae72896e046969d4d4cbc277379d0857ae 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -176,6 +176,7 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
        int i;
        int len = 0;
        char tmp[LPFC_MAX_NVME_INFO_TMP_LEN] = {0};
+       unsigned long iflags = 0;
 
        if (!(vport->cfg_enable_fc4_type & LPFC_ENABLE_NVME)) {
                len = scnprintf(buf, PAGE_SIZE, "NVME Disabled\n");
@@ -374,11 +375,11 @@ lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 
        list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) {
                nrport = NULL;
-               spin_lock(&vport->phba->hbalock);
+               spin_lock_irqsave(&vport->phba->hbalock, iflags);
                rport = lpfc_ndlp_get_nrport(ndlp);
                if (rport)
                        nrport = rport->remoteport;
-               spin_unlock(&vport->phba->hbalock);
+               spin_unlock_irqrestore(&vport->phba->hbalock, iflags);
                if (!nrport)
                        continue;
 

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 2acda188b0dcfcf33c84fc9f0af4283a0ee7d42b..d1512e4f9791732acc225f234024630d8ec4e0c2 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -991,15 +991,14 @@ lpfc_cleanup_vports_rrqs(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp)
  * @ndlp: Targets nodelist pointer for this exchange.
  * @xritag the xri in the bitmap to test.
  *
- * This function is called with hbalock held. This function
- * returns 0 = rrq not active for this xri
- *         1 = rrq is valid for this xri.
+ * This function returns:
+ * 0 = rrq not active for this xri
+ * 1 = rrq is valid for this xri.
  **/
 int
 lpfc_test_rrq_active(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp,
                        uint16_t  xritag)
 {
-       lockdep_assert_held(&phba->hbalock);
        if (!ndlp)
                return 0;
        if (!ndlp->active_rrqs_xri_bitmap)
@@ -1102,10 +1101,11 @@ out:
  * @phba: Pointer to HBA context object.
  * @piocb: Pointer to the iocbq.
  *
- * This function is called with the ring lock held. This function
- * gets a new driver sglq object from the sglq list. If the
- * list is not empty then it is successful, it returns pointer to the newly
- * allocated sglq object else it returns NULL.
+ * The driver calls this function with either the nvme ls ring lock
+ * or the fc els ring lock held depending on the iocb usage.  This function
+ * gets a new driver sglq object from the sglq list. If the list is not empty
+ * then it is successful, it returns pointer to the newly allocated sglq
+ * object else it returns NULL.
  **/
 static struct lpfc_sglq *
 __lpfc_sli_get_els_sglq(struct lpfc_hba *phba, struct lpfc_iocbq *piocbq)
@@ -1115,9 +1115,15 @@ __lpfc_sli_get_els_sglq(struct lpfc_hba *phba, struct lpfc_iocbq *piocbq)
        struct lpfc_sglq *start_sglq = NULL;
        struct lpfc_io_buf *lpfc_cmd;
        struct lpfc_nodelist *ndlp;
+       struct lpfc_sli_ring *pring = NULL;
        int found = 0;
 
-       lockdep_assert_held(&phba->hbalock);
+       if (piocbq->iocb_flag & LPFC_IO_NVME_LS)
+               pring =  phba->sli4_hba.nvmels_wq->pring;
+       else
+               pring = lpfc_phba_elsring(phba);
+
+       lockdep_assert_held(&pring->ring_lock);
 
        if (piocbq->iocb_flag &  LPFC_IO_FCP) {
                lpfc_cmd = (struct lpfc_io_buf *) piocbq->context1;
@@ -1560,7 +1566,8 @@ lpfc_sli_ring_map(struct lpfc_hba *phba)
  * @pring: Pointer to driver SLI ring object.
  * @piocb: Pointer to the driver iocb object.
  *
- * This function is called with hbalock held. The function adds the
+ * The driver calls this function with the hbalock held for SLI3 ports or
+ * the ring lock held for SLI4 ports. The function adds the
  * new iocb to txcmplq of the given ring. This function always returns
  * 0. If this function is called for ELS ring, this function checks if
  * there is a vport associated with the ELS command. This function also
@@ -1570,7 +1577,10 @@ static int
 lpfc_sli_ringtxcmpl_put(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
                        struct lpfc_iocbq *piocb)
 {
-       lockdep_assert_held(&phba->hbalock);
+       if (phba->sli_rev == LPFC_SLI_REV4)
+               lockdep_assert_held(&pring->ring_lock);
+       else
+               lockdep_assert_held(&phba->hbalock);
 
        BUG_ON(!piocb);
 
@@ -2967,8 +2977,8 @@ lpfc_sli_process_unsol_iocb(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
  *
  * This function looks up the iocb_lookup table to get the command iocb
  * corresponding to the given response iocb using the iotag of the
- * response iocb. This function is called with the hbalock held
- * for sli3 devices or the ring_lock for sli4 devices.
+ * response iocb. The driver calls this function with the hbalock held
+ * for SLI3 ports or the ring lock held for SLI4 ports.
  * This function returns the command iocb object if it finds the command
  * iocb else returns NULL.
  **/
@@ -2979,8 +2989,15 @@ lpfc_sli_iocbq_lookup(struct lpfc_hba *phba,
 {
        struct lpfc_iocbq *cmd_iocb = NULL;
        uint16_t iotag;
-       lockdep_assert_held(&phba->hbalock);
+       spinlock_t *temp_lock = NULL;
+       unsigned long iflag = 0;
 
+       if (phba->sli_rev == LPFC_SLI_REV4)
+               temp_lock = &pring->ring_lock;
+       else
+               temp_lock = &phba->hbalock;
+
+       spin_lock_irqsave(temp_lock, iflag);
        iotag = prspiocb->iocb.ulpIoTag;
 
        if (iotag != 0 && iotag <= phba->sli.last_iotag) {
@@ -2990,10 +3007,12 @@ lpfc_sli_iocbq_lookup(struct lpfc_hba *phba,
                        list_del_init(&cmd_iocb->list);
                        cmd_iocb->iocb_flag &= ~LPFC_IO_ON_TXCMPLQ;
                        pring->txcmplq_cnt--;
+                       spin_unlock_irqrestore(temp_lock, iflag);
                        return cmd_iocb;
                }
        }
 
+       spin_unlock_irqrestore(temp_lock, iflag);
        lpfc_printf_log(phba, KERN_ERR, LOG_SLI,
                        "0317 iotag x%x is out of "
                        "range: max iotag x%x wd0 x%x\n",
@@ -3009,8 +3028,8 @@ lpfc_sli_iocbq_lookup(struct lpfc_hba *phba,
  * @iotag: IOCB tag.
  *
  * This function looks up the iocb_lookup table to get the command iocb
- * corresponding to the given iotag. This function is called with the
- * hbalock held.
+ * corresponding to the given iotag. The driver calls this function with
+ * the ring lock held because this function is an SLI4 port only helper.
  * This function returns the command iocb object if it finds the command
  * iocb else returns NULL.
  **/
@@ -3019,8 +3038,15 @@ lpfc_sli_iocbq_lookup_by_tag(struct lpfc_hba *phba,
                             struct lpfc_sli_ring *pring, uint16_t iotag)
 {
        struct lpfc_iocbq *cmd_iocb = NULL;
+       spinlock_t *temp_lock = NULL;
+       unsigned long iflag = 0;
 
-       lockdep_assert_held(&phba->hbalock);
+       if (phba->sli_rev == LPFC_SLI_REV4)
+               temp_lock = &pring->ring_lock;
+       else
+               temp_lock = &phba->hbalock;
+
+       spin_lock_irqsave(temp_lock, iflag);
        if (iotag != 0 && iotag <= phba->sli.last_iotag) {
                cmd_iocb = phba->sli.iocbq_lookup[iotag];
                if (cmd_iocb->iocb_flag & LPFC_IO_ON_TXCMPLQ) {
@@ -3028,10 +3054,12 @@ lpfc_sli_iocbq_lookup_by_tag(struct lpfc_hba *phba,
                        list_del_init(&cmd_iocb->list);
                        cmd_iocb->iocb_flag &= ~LPFC_IO_ON_TXCMPLQ;
                        pring->txcmplq_cnt--;
+                       spin_unlock_irqrestore(temp_lock, iflag);
                        return cmd_iocb;
                }
        }
 
+       spin_unlock_irqrestore(temp_lock, iflag);
        lpfc_printf_log(phba, KERN_ERR, LOG_SLI,
                        "0372 iotag x%x lookup error: max iotag (x%x) "
                        "iocb_flag x%x\n",
@@ -3065,17 +3093,7 @@ lpfc_sli_process_sol_iocb(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
        int rc = 1;
        unsigned long iflag;
 
-       /* Based on the iotag field, get the cmd IOCB from the txcmplq */
-       if (phba->sli_rev == LPFC_SLI_REV4)
-               spin_lock_irqsave(&pring->ring_lock, iflag);
-       else
-               spin_lock_irqsave(&phba->hbalock, iflag);
        cmdiocbp = lpfc_sli_iocbq_lookup(phba, pring, saveq);
-       if (phba->sli_rev == LPFC_SLI_REV4)
-               spin_unlock_irqrestore(&pring->ring_lock, iflag);
-       else
-               spin_unlock_irqrestore(&phba->hbalock, iflag);
-
        if (cmdiocbp) {
                if (cmdiocbp->iocb_cmpl) {
                        /*
@@ -3406,8 +3424,10 @@ lpfc_sli_handle_fast_ring_event(struct lpfc_hba *phba,
                                break;
                        }
 
+                       spin_unlock_irqrestore(&phba->hbalock, iflag);
                        cmdiocbq = lpfc_sli_iocbq_lookup(phba, pring,
                                                         &rspiocbq);
+                       spin_lock_irqsave(&phba->hbalock, iflag);
                        if (unlikely(!cmdiocbq))
                                break;
                        if (cmdiocbq->iocb_flag & LPFC_DRIVER_ABORTED)
@@ -3601,9 +3621,12 @@ lpfc_sli_sp_handle_rspiocb(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
 
                case LPFC_ABORT_IOCB:
                        cmdiocbp = NULL;
-                       if (irsp->ulpCommand != CMD_XRI_ABORTED_CX)
+                       if (irsp->ulpCommand != CMD_XRI_ABORTED_CX) {
+                               spin_unlock_irqrestore(&phba->hbalock, iflag);
                                cmdiocbp = lpfc_sli_iocbq_lookup(phba, pring,
                                                                 saveq);
+                               spin_lock_irqsave(&phba->hbalock, iflag);
+                       }
                        if (cmdiocbp) {
                                /* Call the specified completion routine */
                                if (cmdiocbp->iocb_cmpl) {
@@ -12976,13 +12999,11 @@ lpfc_sli4_els_wcqe_to_rspiocbq(struct lpfc_hba *phba,
                return NULL;
 
        wcqe = &irspiocbq->cq_event.cqe.wcqe_cmpl;
-       spin_lock_irqsave(&pring->ring_lock, iflags);
        pring->stats.iocb_event++;
        /* Look up the ELS command IOCB and create pseudo response IOCB */
        cmdiocbq = lpfc_sli_iocbq_lookup_by_tag(phba, pring,
                                bf_get(lpfc_wcqe_c_request_tag, wcqe));
        if (unlikely(!cmdiocbq)) {
-               spin_unlock_irqrestore(&pring->ring_lock, iflags);
                lpfc_printf_log(phba, KERN_WARNING, LOG_SLI,
                                "0386 ELS complete with no corresponding "
                                "cmdiocb: 0x%x 0x%x 0x%x 0x%x\n",
@@ -12992,6 +13013,7 @@ lpfc_sli4_els_wcqe_to_rspiocbq(struct lpfc_hba *phba,
                return NULL;
        }
 
+       spin_lock_irqsave(&pring->ring_lock, iflags);
        /* Put the iocb back on the txcmplq */
        lpfc_sli_ringtxcmpl_put(phba, pring, cmdiocbq);
        spin_unlock_irqrestore(&pring->ring_lock, iflags);
@@ -13762,9 +13784,9 @@ lpfc_sli4_fp_handle_fcp_wcqe(struct lpfc_hba *phba, struct lpfc_queue *cq,
        /* Look up the FCP command IOCB and create pseudo response IOCB */
        spin_lock_irqsave(&pring->ring_lock, iflags);
        pring->stats.iocb_event++;
+       spin_unlock_irqrestore(&pring->ring_lock, iflags);
        cmdiocbq = lpfc_sli_iocbq_lookup_by_tag(phba, pring,
                                bf_get(lpfc_wcqe_c_request_tag, wcqe));
-       spin_unlock_irqrestore(&pring->ring_lock, iflags);
        if (unlikely(!cmdiocbq)) {
                lpfc_printf_log(phba, KERN_WARNING, LOG_SLI,
                                "0374 FCP complete with no corresponding "