[PATCH v4 05/10] mpi3mr: Fix performance regression caused by extended IRQ poll sleep

Commit 24d7071d9645 ("scsi: mpi3mr: A performance fix") increased the
threaded IRQ poll sleep range from 2-20 us to 20-21 us to work around a
timer slack issue.

On kernels unaffected by the timer slack issue, the longer sleep interval
reduces reply queue processing efficiency and causes an approximately 7%
throughput regression on NVMe direct-attached RAID10 configurations.

Restore the IRQ poll sleep base to 2 us (widening the usleep_range() upper
bound to 10x the base instead of a fixed +1 us) to recover the lost
throughput, and skip the sleep entirely once pend_ios reaches 0 so the
poll loop exits immediately at the tail of a completion burst.

Additionally, resolve the following issues in the reply queue processing
and polling logic:

1. Add missing dma_rmb() memory barriers in the admin and operational
   reply queue processing loops. This ensures that the descriptor
   payload is only read after the phase bit check is complete, preventing
   weakly ordered architectures from speculatively processing stale data.

2. Add bounds checking for `request_queue_id` in
   mpi3mr_process_op_reply_q(). An out-of-range id is now logged and the
   descriptor is retired (consumer index advanced, phase toggled on
   wraparound, pend_ios/threshold accounted) rather than aborting the
   loop in place, which previously left the same corrupted descriptor
   at the head of the ring forever and stalled polling indefinitely.

3. Recheck for a late-arriving descriptor via dma_rmb() while still
   holding op_reply_q->in_use, instead of releasing it and reclaiming
   it afterward, which could race and reprocess a descriptor with
   stale indices or double-decrement in_use.

4. Replace a direct panic() call with a safe ioc_err() log and abort in
   mpi3mr_process_op_reply_desc() when mpi3mr_get_reply_virt_addr()
   returns NULL. This prevents a single malformed DMA reply address from
   crashing the entire host OS. The reply_dma output parameter is also
   cleared before returning, since it was already populated with the
   unvalidated address before the NULL check. Leaving it set would make
   the caller repost that unvalidated address back to the hardware.

Note: The unbounded busy-wait loop (usleep_range) in mpi3mr_isr_poll()
flagged by automated review is intentionally retained. This short sleep
polling mechanism is critical for batching completions and achieving the
target throughput on high-performance NVMe configurations.

Reported-by: Sashiko <[email protected]>
Closes: https://sashiko.dev/#/patchset/[email protected]?part=5
Closes: https://sashiko.dev/#/patchset/[email protected]?part=5
Closes: https://sashiko.dev/#/patchset/[email protected]?part=5
Signed-off-by: Chandrakanth Patil <[email protected]>
Signed-off-by: Ranjan Kumar <[email protected]>
---
 drivers/scsi/mpi3mr/mpi3mr.h    |  2 +-
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 52 ++++++++++++++++++++++++++++++---
 drivers/scsi/mpi3mr/mpi3mr_os.c |  8 +++--
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 6128b30112e2..4d19a9460d38 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -179,7 +179,7 @@ extern atomic64_t event_counter;
 #define MPI3MR_DEFAULT_SDEV_QD	32
 
 /* Definitions for Threaded IRQ poll*/
-#define MPI3MR_IRQ_POLL_SLEEP			20
+#define MPI3MR_IRQ_POLL_SLEEP			2
 #define MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT		8
 
 /* Definitions for the controller security status*/
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 434b66f7b502..e6050b41e15a 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -473,6 +473,12 @@ int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc)
 		return 0;
 	}
 
+	/*
+	 * Ensure that the descriptor payload is read only after
+	 * the phase bit check is complete.
+	 */
+	dma_rmb();
+
 	do {
 		if (mrioc->unrecoverable || mrioc->io_admin_reset_sync)
 			break;
@@ -493,6 +499,13 @@ int mpi3mr_process_admin_reply_q(struct mpi3mr_ioc *mrioc)
 		if ((le16_to_cpu(reply_desc->reply_flags) &
 		    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase)
 			break;
+
+		/*
+		 * Ensure that the descriptor payload is read only after
+		 * the phase bit check is complete.
+		 */
+		dma_rmb();
+
 		if (threshold_comps == MPI3MR_THRESHOLD_REPLY_COUNT) {
 			writel(admin_reply_ci,
 			    &mrioc->sysif_regs->admin_reply_queue_ci);
@@ -564,15 +577,33 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 	reply_desc = mpi3mr_get_reply_desc(op_reply_q, reply_ci);
 	if ((le16_to_cpu(reply_desc->reply_flags) &
 	    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase) {
+		/* Recheck under in_use before releasing, to avoid a reclaim race */
+		dma_rmb();
+		if ((le16_to_cpu(reply_desc->reply_flags) &
+		    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) == exp_phase)
+			goto process_desc;
 		atomic_dec(&op_reply_q->in_use);
 		return 0;
 	}
+process_desc:
+	/*
+	 * Ensure that the descriptor payload is read only after
+	 * the phase bit check is complete.
+	 */
+	dma_rmb();
 
 	do {
 		if (mrioc->unrecoverable || mrioc->io_admin_reset_sync)
 			break;
 
 		req_q_idx = le16_to_cpu(reply_desc->request_queue_id) - 1;
+
+		if (unlikely(req_q_idx >= mrioc->num_op_req_q)) {
+			ioc_err(mrioc, "Invalid request queue id %d, skipping reply\n",
+			    req_q_idx + 1);
+			goto next_reply;
+		}
+
 		op_req_q = &mrioc->req_qinfo[req_q_idx];
 
 		WRITE_ONCE(op_req_q->ci, le16_to_cpu(reply_desc->request_queue_ci));
@@ -581,6 +612,7 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 
 		if (reply_dma)
 			mpi3mr_repost_reply_buf(mrioc, reply_dma);
+next_reply:
 		num_op_reply++;
 		threshold_comps++;
 
@@ -592,8 +624,19 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 		reply_desc = mpi3mr_get_reply_desc(op_reply_q, reply_ci);
 
 		if ((le16_to_cpu(reply_desc->reply_flags) &
-		    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase)
+		    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) != exp_phase) {
+			dma_rmb();
+			if ((le16_to_cpu(reply_desc->reply_flags) &
+			    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) == exp_phase)
+				goto reply_ready;
 			break;
+		}
+reply_ready:
+		/*
+		 * Ensure that the descriptor payload is read only after
+		 * the phase bit check is complete.
+		 */
+		dma_rmb();
 #ifndef CONFIG_PREEMPT_RT
 		/*
 		 * Exit completion loop to avoid CPU lockup
@@ -743,11 +786,12 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
 			num_op_reply +=
 			    mpi3mr_process_op_reply_q(mrioc,
 				intr_info->op_reply_q);
+		if (!atomic_read(&intr_info->op_reply_q->pend_ios))
+			break;
 
-		usleep_range(MPI3MR_IRQ_POLL_SLEEP, MPI3MR_IRQ_POLL_SLEEP + 1);
+		usleep_range(MPI3MR_IRQ_POLL_SLEEP, 10 * MPI3MR_IRQ_POLL_SLEEP);
 
-	} while (atomic_read(&intr_info->op_reply_q->pend_ios) &&
-	    (num_op_reply < mrioc->max_host_ios));
+	} while (num_op_reply < mrioc->max_host_ios);
 
 	intr_info->op_reply_q->enable_irq_poll = false;
 	enable_irq(intr_info->os_irq);
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 88b1d6360dac..23a6a5e3df5f 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -3430,8 +3430,12 @@ void mpi3mr_process_op_reply_desc(struct mpi3mr_ioc *mrioc,
 		scsi_reply = mpi3mr_get_reply_virt_addr(mrioc,
 		    *reply_dma);
 		if (!scsi_reply) {
-			panic("%s: scsi_reply is NULL, this shouldn't happen\n",
-			    mrioc->name);
+			ioc_err(mrioc, "scsi_reply is NULL, invalid reply_frame_address\n");
+			/*
+			 * Do not let the caller repost an address that
+			 * failed virt-addr lookup back to the hardware.
+			 */
+			*reply_dma = 0;
 			goto out;
 		}
 		host_tag = le16_to_cpu(scsi_reply->host_tag);
-- 
2.47.3