New sparse warning in lpfc_sli.c
Subsystems (2)
James, somewhere between 2.6.25-rc1 and -rc2 this new warning appeared, care to give the locking a quick once-over here?
drivers/scsi/lpfc/lpfc_sli.c:645:1: warning: context imbalance in 'lpfc_sli_hbqbuf_fill_hbqs' - wrong count at exit
Full log available if you want it.
Harvey
Quoted 4 lines · Harvey Harrison
James, somewhere between 2.6.25-rc1 and -rc2 this new warning appeared, care to give the locking a quick once-over here?
drivers/scsi/lpfc/lpfc_sli.c:645:1: warning: context imbalance in 'lpfc_sli_hbqbuf_fill_hbqs' - wrong count at exit
Easy enough to spot by hand:
/* Check whether HBQ is still in use */
spin_lock_irqsave(&phba->hbalock, flags);
if (!phba->hbq_in_use) {
spin_unlock_irqrestore(&phba->hbalock, flags);
return 0;
}
/* Populate HBQ entries */
for (i = start; i < end; i++) {
hbq_buffer = (phba->hbqs[hbqno].hbq_alloc_buffer)(phba);
if (!hbq_buffer)---> return 1;
Here's a patch:
lpfc: Balance locking
Commit 3163f725a5d071eea1830bbbfab78cfe3fc9baaf introduced locking in lpfc_sli_hbqbuf_fill_hbqs, but missed unlocking on one exit.
drivers/scsi/lpfc/lpfc_sli.c +9 -9
@@ -648,28 +648,24 @@ lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *phba, uint32_t hbqno, uint32_t count)unsigned long flags;struct hbq_dmabuf *hbq_buffer; -if (!phba->hbqs[hbqno].hbq_alloc_buffer) { +if (!phba->hbqs[hbqno].hbq_alloc_buffer)return 0 ; - } start = phba->hbqs[hbqno].buffer_count; end = count + start; -if (end > lpfc_hbq_defs[hbqno]->entry_count) { +if (end > lpfc_hbq_defs[hbqno]->entry_count) end = lpfc_hbq_defs[hbqno]->entry_count; - }/* Check whether HBQ is still in use */ spin_lock_irqsave (&phba->hbalock, flags); -if (!phba->hbq_in_use) { -spin_unlock_irqrestore (&phba->hbalock, flags); -return 0 ; - } +if (!phba->hbq_in_use) + goto out;/* Populate HBQ entries */ for (i = start; i < end; i++) { hbq_buffer = (phba->hbqs[hbqno].hbq_alloc_buffer)(phba);if (!hbq_buffer) -return 1 ; + goto err; hbq_buffer->tag = (i | (hbqno <<16 ));if (lpfc_sli_hbq_to_firmware (phba, hbqno, hbq_buffer)) phba->hbqs[hbqno].buffer_count++;
@@ -677,8 +673,12 @@ lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *phba, uint32_t hbqno, uint32_t count) (phba->hbqs[hbqno].hbq_free_buffer)(phba, hbq_buffer); } + out:spin_unlock_irqrestore (&phba->hbalock, flags);return 0 ; + err: +spin_unlock_irqrestore (&phba->hbalock, flags); +return 1 ; }int
Signature · 4 lines
Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Thanks Matt.
ACK
-- james s
Quoted 84 lines · Matthew Wilcox
Quoted 4 lines · Harvey Harrison
James, somewhere between 2.6.25-rc1 and -rc2 this new warning appeared, care to give the locking a quick once-over here?
drivers/scsi/lpfc/lpfc_sli.c:645:1: warning: context imbalance in 'lpfc_sli_hbqbuf_fill_hbqs' - wrong count at exitEasy enough to spot by hand:
/* Check whether HBQ is still in use */ spin_lock_irqsave(&phba->hbalock, flags); if (!phba->hbq_in_use) { spin_unlock_irqrestore(&phba->hbalock, flags); return 0; } /* Populate HBQ entries */ for (i = start; i < end; i++) { hbq_buffer = (phba->hbqs[hbqno].hbq_alloc_buffer)(phba); if (!hbq_buffer)---> return 1;
Here's a patch:
lpfc: Balance locking
Commit
3163f725a5d071eea1830bbbfab78cfe3fc9baafintroduced locking inlpfc_sli_hbqbuf_fill_hbqs, but missed unlocking on one exit.Reported-by Harvey Harrison <[email protected]>Signed-off-by Matthew Wilcox <[email protected]>Cc James Smart <[email protected]>drivers/scsi/lpfc/lpfc_sli.c +9
-9@@ -648,28 +648,24 @@ lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *phba, uint32_t hbqno, uint32_t count)unsigned long flags;struct hbq_dmabuf *hbq_buffer; -if (!phba->hbqs[hbqno].hbq_alloc_buffer) { +if (!phba->hbqs[hbqno].hbq_alloc_buffer)return 0 ; - } start = phba->hbqs[hbqno].buffer_count; end = count + start; -if (end > lpfc_hbq_defs[hbqno]->entry_count) { +if (end > lpfc_hbq_defs[hbqno]->entry_count) end = lpfc_hbq_defs[hbqno]->entry_count; - }/* Check whether HBQ is still in use */ spin_lock_irqsave (&phba->hbalock, flags); -if (!phba->hbq_in_use) { -spin_unlock_irqrestore (&phba->hbalock, flags); -return 0 ; - } +if (!phba->hbq_in_use) + goto out;/* Populate HBQ entries */ for (i = start; i < end; i++) { hbq_buffer = (phba->hbqs[hbqno].hbq_alloc_buffer)(phba);if (!hbq_buffer) -return 1 ; + goto err; hbq_buffer->tag = (i | (hbqno <<16 ));if (lpfc_sli_hbq_to_firmware (phba, hbqno, hbq_buffer)) phba->hbqs[hbqno].buffer_count++;@@ -677,8 +673,12 @@ lpfc_sli_hbqbuf_fill_hbqs(struct lpfc_hba *phba, uint32_t hbqno, uint32_t count) (phba->hbqs[hbqno].hbq_free_buffer)(phba, hbq_buffer); } + out:spin_unlock_irqrestore (&phba->hbalock, flags);return 0 ; + err: +spin_unlock_irqrestore (&phba->hbalock, flags); +return 1 ; }int