scsi: lpfc: Fix locking on mailbox command completion

Symptoms were seen of the driver not having valid data for mailbox
commands. After debugging, the following sequence was found:

The driver maintains a port-wide pointer of the mailbox command that is
currently in execution. Once finished, the port-wide pointer is cleared
(done in lpfc_sli4_mq_release()). The next mailbox command issued will set
the next pointer and so on.

The mailbox response data is only copied if there is a valid port-wide
pointer.

In the failing case, it was seen that a new mailbox command was being
attempted in parallel with the completion.  The parallel path was seeing
the mailbox no long in use (flag check under lock) and thus set the port
pointer.  The completion path had cleared the active flag under lock, but
had not touched the port pointer.  The port pointer is cleared after the
lock is released. In this case, the completion path cleared the just-set
value by the parallel path.

Fix by making the calls that clear mbox state/port pointer while under
lock.  Also slightly cleaned up the error path.

Link: https://lore.kernel.org/r/20190922035906.10977-8-jsmart2021@gmail.com
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 24d6779a99f8e30f441d0161f1bb25bb2e8276b3..313441a3c4cff648bbf9a6b0f852a51282c5d3cb 100644 (file)
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -13165,13 +13165,19 @@ send_current_mbox:
        phba->sli.sli_flag &= ~LPFC_SLI_MBOX_ACTIVE;
        /* Setting active mailbox pointer need to be in sync to flag clear */
        phba->sli.mbox_active = NULL;
+       if (bf_get(lpfc_trailer_consumed, mcqe))
+               lpfc_sli4_mq_release(phba->sli4_hba.mbx_wq);
        spin_unlock_irqrestore(&phba->hbalock, iflags);
        /* Wake up worker thread to post the next pending mailbox command */
        lpfc_worker_wake_up(phba);
+       return workposted;
+
 out_no_mqe_complete:
+       spin_lock_irqsave(&phba->hbalock, iflags);
        if (bf_get(lpfc_trailer_consumed, mcqe))
                lpfc_sli4_mq_release(phba->sli4_hba.mbx_wq);
-       return workposted;
+       spin_unlock_irqrestore(&phba->hbalock, iflags);
+       return false;
 }
 
 /**