Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

James Smart and committed by
Martin K. Petersen
e2a8be56 d0adee5d

+56 -33
+3 -2
drivers/scsi/lpfc/lpfc_attr.c
··· 176 176 int i; 177 177 int len = 0; 178 178 char tmp[LPFC_MAX_NVME_INFO_TMP_LEN] = {0}; 179 + unsigned long iflags = 0; 179 180 180 181 if (!(vport->cfg_enable_fc4_type & LPFC_ENABLE_NVME)) { 181 182 len = scnprintf(buf, PAGE_SIZE, "NVME Disabled\n"); ··· 375 374 376 375 list_for_each_entry(ndlp, &vport->fc_nodes, nlp_listp) { 377 376 nrport = NULL; 378 - spin_lock(&vport->phba->hbalock); 377 + spin_lock_irqsave(&vport->phba->hbalock, iflags); 379 378 rport = lpfc_ndlp_get_nrport(ndlp); 380 379 if (rport) 381 380 nrport = rport->remoteport; 382 - spin_unlock(&vport->phba->hbalock); 381 + spin_unlock_irqrestore(&vport->phba->hbalock, iflags); 383 382 if (!nrport) 384 383 continue; 385 384
+53 -31
drivers/scsi/lpfc/lpfc_sli.c
··· 991 991 * @ndlp: Targets nodelist pointer for this exchange. 992 992 * @xritag the xri in the bitmap to test. 993 993 * 994 - * This function is called with hbalock held. This function 995 - * returns 0 = rrq not active for this xri 996 - * 1 = rrq is valid for this xri. 994 + * This function returns: 995 + * 0 = rrq not active for this xri 996 + * 1 = rrq is valid for this xri. 997 997 **/ 998 998 int 999 999 lpfc_test_rrq_active(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp, 1000 1000 uint16_t xritag) 1001 1001 { 1002 - lockdep_assert_held(&phba->hbalock); 1003 1002 if (!ndlp) 1004 1003 return 0; 1005 1004 if (!ndlp->active_rrqs_xri_bitmap) ··· 1101 1102 * @phba: Pointer to HBA context object. 1102 1103 * @piocb: Pointer to the iocbq. 1103 1104 * 1104 - * This function is called with the ring lock held. This function 1105 - * gets a new driver sglq object from the sglq list. If the 1106 - * list is not empty then it is successful, it returns pointer to the newly 1107 - * allocated sglq object else it returns NULL. 1105 + * The driver calls this function with either the nvme ls ring lock 1106 + * or the fc els ring lock held depending on the iocb usage. This function 1107 + * gets a new driver sglq object from the sglq list. If the list is not empty 1108 + * then it is successful, it returns pointer to the newly allocated sglq 1109 + * object else it returns NULL. 1108 1110 **/ 1109 1111 static struct lpfc_sglq * 1110 1112 __lpfc_sli_get_els_sglq(struct lpfc_hba *phba, struct lpfc_iocbq *piocbq) ··· 1115 1115 struct lpfc_sglq *start_sglq = NULL; 1116 1116 struct lpfc_io_buf *lpfc_cmd; 1117 1117 struct lpfc_nodelist *ndlp; 1118 + struct lpfc_sli_ring *pring = NULL; 1118 1119 int found = 0; 1119 1120 1120 - lockdep_assert_held(&phba->hbalock); 1121 + if (piocbq->iocb_flag & LPFC_IO_NVME_LS) 1122 + pring = phba->sli4_hba.nvmels_wq->pring; 1123 + else 1124 + pring = lpfc_phba_elsring(phba); 1125 + 1126 + lockdep_assert_held(&pring->ring_lock); 1121 1127 1122 1128 if (piocbq->iocb_flag & LPFC_IO_FCP) { 1123 1129 lpfc_cmd = (struct lpfc_io_buf *) piocbq->context1; ··· 1566 1560 * @pring: Pointer to driver SLI ring object. 1567 1561 * @piocb: Pointer to the driver iocb object. 1568 1562 * 1569 - * This function is called with hbalock held. The function adds the 1563 + * The driver calls this function with the hbalock held for SLI3 ports or 1564 + * the ring lock held for SLI4 ports. The function adds the 1570 1565 * new iocb to txcmplq of the given ring. This function always returns 1571 1566 * 0. If this function is called for ELS ring, this function checks if 1572 1567 * there is a vport associated with the ELS command. This function also ··· 1577 1570 lpfc_sli_ringtxcmpl_put(struct lpfc_hba *phba, struct lpfc_sli_ring *pring, 1578 1571 struct lpfc_iocbq *piocb) 1579 1572 { 1580 - lockdep_assert_held(&phba->hbalock); 1573 + if (phba->sli_rev == LPFC_SLI_REV4) 1574 + lockdep_assert_held(&pring->ring_lock); 1575 + else 1576 + lockdep_assert_held(&phba->hbalock); 1581 1577 1582 1578 BUG_ON(!piocb); 1583 1579 ··· 2977 2967 * 2978 2968 * This function looks up the iocb_lookup table to get the command iocb 2979 2969 * corresponding to the given response iocb using the iotag of the 2980 - * response iocb. This function is called with the hbalock held 2981 - * for sli3 devices or the ring_lock for sli4 devices. 2970 + * response iocb. The driver calls this function with the hbalock held 2971 + * for SLI3 ports or the ring lock held for SLI4 ports. 2982 2972 * This function returns the command iocb object if it finds the command 2983 2973 * iocb else returns NULL. 2984 2974 **/ ··· 2989 2979 { 2990 2980 struct lpfc_iocbq *cmd_iocb = NULL; 2991 2981 uint16_t iotag; 2992 - lockdep_assert_held(&phba->hbalock); 2982 + spinlock_t *temp_lock = NULL; 2983 + unsigned long iflag = 0; 2993 2984 2985 + if (phba->sli_rev == LPFC_SLI_REV4) 2986 + temp_lock = &pring->ring_lock; 2987 + else 2988 + temp_lock = &phba->hbalock; 2989 + 2990 + spin_lock_irqsave(temp_lock, iflag); 2994 2991 iotag = prspiocb->iocb.ulpIoTag; 2995 2992 2996 2993 if (iotag != 0 && iotag <= phba->sli.last_iotag) { ··· 3007 2990 list_del_init(&cmd_iocb->list); 3008 2991 cmd_iocb->iocb_flag &= ~LPFC_IO_ON_TXCMPLQ; 3009 2992 pring->txcmplq_cnt--; 2993 + spin_unlock_irqrestore(temp_lock, iflag); 3010 2994 return cmd_iocb; 3011 2995 } 3012 2996 } 3013 2997 2998 + spin_unlock_irqrestore(temp_lock, iflag); 3014 2999 lpfc_printf_log(phba, KERN_ERR, LOG_SLI, 3015 3000 "0317 iotag x%x is out of " 3016 3001 "range: max iotag x%x wd0 x%x\n", ··· 3028 3009 * @iotag: IOCB tag. 3029 3010 * 3030 3011 * This function looks up the iocb_lookup table to get the command iocb 3031 - * corresponding to the given iotag. This function is called with the 3032 - * hbalock held. 3012 + * corresponding to the given iotag. The driver calls this function with 3013 + * the ring lock held because this function is an SLI4 port only helper. 3033 3014 * This function returns the command iocb object if it finds the command 3034 3015 * iocb else returns NULL. 3035 3016 **/ ··· 3038 3019 struct lpfc_sli_ring *pring, uint16_t iotag) 3039 3020 { 3040 3021 struct lpfc_iocbq *cmd_iocb = NULL; 3022 + spinlock_t *temp_lock = NULL; 3023 + unsigned long iflag = 0; 3041 3024 3042 - lockdep_assert_held(&phba->hbalock); 3025 + if (phba->sli_rev == LPFC_SLI_REV4) 3026 + temp_lock = &pring->ring_lock; 3027 + else 3028 + temp_lock = &phba->hbalock; 3029 + 3030 + spin_lock_irqsave(temp_lock, iflag); 3043 3031 if (iotag != 0 && iotag <= phba->sli.last_iotag) { 3044 3032 cmd_iocb = phba->sli.iocbq_lookup[iotag]; 3045 3033 if (cmd_iocb->iocb_flag & LPFC_IO_ON_TXCMPLQ) { ··· 3054 3028 list_del_init(&cmd_iocb->list); 3055 3029 cmd_iocb->iocb_flag &= ~LPFC_IO_ON_TXCMPLQ; 3056 3030 pring->txcmplq_cnt--; 3031 + spin_unlock_irqrestore(temp_lock, iflag); 3057 3032 return cmd_iocb; 3058 3033 } 3059 3034 } 3060 3035 3036 + spin_unlock_irqrestore(temp_lock, iflag); 3061 3037 lpfc_printf_log(phba, KERN_ERR, LOG_SLI, 3062 3038 "0372 iotag x%x lookup error: max iotag (x%x) " 3063 3039 "iocb_flag x%x\n", ··· 3093 3065 int rc = 1; 3094 3066 unsigned long iflag; 3095 3067 3096 - /* Based on the iotag field, get the cmd IOCB from the txcmplq */ 3097 - if (phba->sli_rev == LPFC_SLI_REV4) 3098 - spin_lock_irqsave(&pring->ring_lock, iflag); 3099 - else 3100 - spin_lock_irqsave(&phba->hbalock, iflag); 3101 3068 cmdiocbp = lpfc_sli_iocbq_lookup(phba, pring, saveq); 3102 - if (phba->sli_rev == LPFC_SLI_REV4) 3103 - spin_unlock_irqrestore(&pring->ring_lock, iflag); 3104 - else 3105 - spin_unlock_irqrestore(&phba->hbalock, iflag); 3106 - 3107 3069 if (cmdiocbp) { 3108 3070 if (cmdiocbp->iocb_cmpl) { 3109 3071 /* ··· 3424 3406 break; 3425 3407 } 3426 3408 3409 + spin_unlock_irqrestore(&phba->hbalock, iflag); 3427 3410 cmdiocbq = lpfc_sli_iocbq_lookup(phba, pring, 3428 3411 &rspiocbq); 3412 + spin_lock_irqsave(&phba->hbalock, iflag); 3429 3413 if (unlikely(!cmdiocbq)) 3430 3414 break; 3431 3415 if (cmdiocbq->iocb_flag & LPFC_DRIVER_ABORTED) ··· 3621 3601 3622 3602 case LPFC_ABORT_IOCB: 3623 3603 cmdiocbp = NULL; 3624 - if (irsp->ulpCommand != CMD_XRI_ABORTED_CX) 3604 + if (irsp->ulpCommand != CMD_XRI_ABORTED_CX) { 3605 + spin_unlock_irqrestore(&phba->hbalock, iflag); 3625 3606 cmdiocbp = lpfc_sli_iocbq_lookup(phba, pring, 3626 3607 saveq); 3608 + spin_lock_irqsave(&phba->hbalock, iflag); 3609 + } 3627 3610 if (cmdiocbp) { 3628 3611 /* Call the specified completion routine */ 3629 3612 if (cmdiocbp->iocb_cmpl) { ··· 12999 12976 return NULL; 13000 12977 13001 12978 wcqe = &irspiocbq->cq_event.cqe.wcqe_cmpl; 13002 - spin_lock_irqsave(&pring->ring_lock, iflags); 13003 12979 pring->stats.iocb_event++; 13004 12980 /* Look up the ELS command IOCB and create pseudo response IOCB */ 13005 12981 cmdiocbq = lpfc_sli_iocbq_lookup_by_tag(phba, pring, 13006 12982 bf_get(lpfc_wcqe_c_request_tag, wcqe)); 13007 12983 if (unlikely(!cmdiocbq)) { 13008 - spin_unlock_irqrestore(&pring->ring_lock, iflags); 13009 12984 lpfc_printf_log(phba, KERN_WARNING, LOG_SLI, 13010 12985 "0386 ELS complete with no corresponding " 13011 12986 "cmdiocb: 0x%x 0x%x 0x%x 0x%x\n", ··· 13013 12992 return NULL; 13014 12993 } 13015 12994 12995 + spin_lock_irqsave(&pring->ring_lock, iflags); 13016 12996 /* Put the iocb back on the txcmplq */ 13017 12997 lpfc_sli_ringtxcmpl_put(phba, pring, cmdiocbq); 13018 12998 spin_unlock_irqrestore(&pring->ring_lock, iflags); ··· 13784 13762 /* Look up the FCP command IOCB and create pseudo response IOCB */ 13785 13763 spin_lock_irqsave(&pring->ring_lock, iflags); 13786 13764 pring->stats.iocb_event++; 13765 + spin_unlock_irqrestore(&pring->ring_lock, iflags); 13787 13766 cmdiocbq = lpfc_sli_iocbq_lookup_by_tag(phba, pring, 13788 13767 bf_get(lpfc_wcqe_c_request_tag, wcqe)); 13789 - spin_unlock_irqrestore(&pring->ring_lock, iflags); 13790 13768 if (unlikely(!cmdiocbq)) { 13791 13769 lpfc_printf_log(phba, KERN_WARNING, LOG_SLI, 13792 13770 "0374 FCP complete with no corresponding "