From:   James Smart <jsmart2021@gmail.com>
To:     linux-scsi@vger.kernel.org
Cc:     James Smart <jsmart2021@gmail.com>,
        Dick Kennedy <dick.kennedy@broadcom.com>
Subject: [PATCH 07/20] lpfc: Fix locking on mailbox command completion
Date:   Sat, 21 Sep 2019 20:58:53 -0700
Message-Id: <20190922035906.10977-8-jsmart2021@gmail.com>
In-Reply-To: <20190922035906.10977-1-jsmart2021@gmail.com>
References: <20190922035906.10977-1-jsmart2021@gmail.com>
Sender: linux-scsi-owner@vger.kernel.org
Precedence: bulk

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.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/lpfc/lpfc_sli.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 24d6779a99f8..313441a3c4cf 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -13165,13 +13165,19 @@ lpfc_sli4_sp_handle_mbox_event(struct lpfc_hba *phba, struct lpfc_mcqe *mcqe)
 	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;
 }
 
 /**
-- 
2.13.7