[PATCH 0/2] scsi: mpi3mr: Stop IRQ polling when no reply is ready

[PATCH 0/2] scsi: mpi3mr: Stop IRQ polling when no reply is ready

[Usama Arif]

Once a reply queue has more than 8 I/Os pending, mpi3mr's IRQ thread
polls it every ~20us until nothing is pending or it has processed
max_host_ios replies. On busy HDDs that means about 45 seconds of
polling at a time, with nearly every wakeup finding nothing. On Meta's
HDD storage hosts, these threads used about 15% of the non-idle CPU
on polling.

Patch 2 goes back to interrupts once a poll finds the queue empty while
owning it. It relies on patch 1: an interrupt that finds the queue owned
by another context now hands it to the thread, instead of leaving a
reply until the next interrupt or a command timeout.

On a machine configured like those hosts (36 SATA HDDs behind a SAS40xx
HBA), HDD random reads now wake the threads twice per I/O instead of 236
times, and a co-located CPU-bound task loses 0.6% instead of 30%.
Drive-cache reads still reach 270K IOPS, and a single reply queue whose
CPU is saturated loses about 5%. With a kthread injected to hold a reply
queue, replies waited up to 50ms, and timed out under light load,
without patch 1, and at most 233us with it.

Based on mkp/scsi 7.4/scsi-staging (f09d2c7485b32).
 
Usama Arif (2):
  scsi: mpi3mr: Poll a reply queue that an interrupt found busy
  scsi: mpi3mr: Stop IRQ polling when no reply is ready

 drivers/scsi/mpi3mr/mpi3mr.h    |  2 +-
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 65 ++++++++++++++++++++++++---------
 drivers/scsi/mpi3mr/mpi3mr_os.c |  2 +-
 3 files changed, 50 insertions(+), 19 deletions(-)


base-commit: f09d2c7485b32adb82336d0d748935c8237a649e
-- 
2.53.0-Meta

[PATCH 1/2] scsi: mpi3mr: Poll a reply queue that an interrupt found busy

[Usama Arif]

mpi3mr_process_op_reply_q() returns without looking at the queue when
another context owns it. Task management polls all reply queues with
interrupts enabled, and a submitter drains the reply queue when the request
queue is full, so either can own the queue when an interrupt comes in. If
the owner made its last check just before the reply arrived, the handler
finds the queue busy and, unless enable_irq_poll happens to be set,
returns. Nobody looks at the reply until the next interrupt on that queue
or the command timeout.

Let's set enable_irq_poll when the queue is busy, so that mpi3mr_isr()
hands over to the IRQ thread, which polls until it has processed the
reply. The flag cannot be cleared before mpi3mr_isr() reads it, as the
thread only writes it while the interrupt is disabled.

Fixes: 463429f8dd5c ("scsi: mpi3mr: Add support for threaded ISR")
Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index f0d3cd398dd00..102f84667a5cf 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -588,8 +588,11 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 
 	reply_qidx = op_reply_q->qid - 1;
 
-	if (!atomic_add_unless(&op_reply_q->in_use, 1, 1))
+	if (!atomic_add_unless(&op_reply_q->in_use, 1, 1)) {
+		/* The owner may have missed a reply, let the thread poll */
+		WRITE_ONCE(op_reply_q->enable_irq_poll, true);
 		return 0;
+	}
 
 	exp_phase = op_reply_q->ephase;
 	reply_ci = op_reply_q->ci;
-- 
2.53.0-Meta

[PATCH 2/2] scsi: mpi3mr: Stop IRQ polling when no reply is ready

[Usama Arif]

Once more than MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT I/Os are pending on a reply
queue, the next interrupt hands the queue to the SCHED_FIFO IRQ thread,
which polls it until no I/O is pending or it has processed max_host_ios
replies. Busy HDD queues almost always have I/O pending, so polling runs
for about 45 seconds, waking every ~20us for replies that arrive ~5ms
apart.

On a machine configured like Meta's HDD storage hosts, with 36 SATA HDDs
behind a SAS40xx HBA, 4k random reads at 16 I/Os per disk make the IRQ
threads wake up 236 times per I/O. They use 2.2 CPUs, plus 2.3 CPUs of
timer interrupts, for 6.4K IOPS, and a CPU-bound task on the same CPUs
loses 30% of its throughput.

Let's go back to interrupts once a poll after a sleep finds the queue
empty. A later reply raises an interrupt, which enable_irq() replays if it
arrived while polling, and a handler that finds the queue busy hands it
back to the thread. The empty check must be made while owning the queue:
a busy queue's owner may have missed the reply whose interrupt woke the
thread, leaving nothing to replay. If more than
MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT I/Os remain, enable_irq_poll is set again
so the next interrupt resumes polling. It is cleared first and pend_ios
is rechecked after a full barrier pairing with atomic_inc_return() in
mpi3mr_op_request_post(), so a concurrent submitter's trigger is not lost.

In the HDD test, the IRQ threads now wake up twice per I/O instead of 236
times and use 0.03 CPUs instead of 2.2, and the CPU-bound task loses 0.6%
instead of 30%. Faster queues are mostly unaffected: one that finds a
reply on every poll keeps polling, and one whose replies are further apart
than the poll interval switches to interrupts without losing throughput,
as drive-cache reads at 270K IOPS show. The exception is a single reply
queue whose CPU is saturated by both submission and completion: its polls
sometimes find the queue empty, and the extra interrupts cost about 5% of
its IOPS.

Also mark all enable_irq_poll accesses with READ_ONCE() and WRITE_ONCE().
Submitters, the hard IRQ and the IRQ thread read and write
the flag on different CPUs without a common lock. Plain accesses would
let the compiler assume that no other CPU changes it, and the memory
model only guarantees the barrier pairing above for marked accesses.

Signed-off-by: Usama Arif <usama.arif@linux.dev>
---
 drivers/scsi/mpi3mr/mpi3mr.h    |  2 +-
 drivers/scsi/mpi3mr/mpi3mr_fw.c | 60 ++++++++++++++++++++++++---------
 drivers/scsi/mpi3mr/mpi3mr_os.c |  2 +-
 3 files changed, 46 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index d6e16707fd97c..4012757d2cf3b 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -1525,7 +1525,7 @@ void mpi3mr_check_rh_fault_ioc(struct mpi3mr_ioc *mrioc, u32 reason_code);
 void mpi3mr_print_fault_info(struct mpi3mr_ioc *mrioc);
 void mpi3mr_check_rh_fault_ioc(struct mpi3mr_ioc *mrioc, u32 reason_code);
 int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
-	struct op_reply_qinfo *op_reply_q);
+	struct op_reply_qinfo *op_reply_q, bool *checked_empty);
 int mpi3mr_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num);
 void mpi3mr_bsg_init(struct mpi3mr_ioc *mrioc);
 void mpi3mr_bsg_exit(struct mpi3mr_ioc *mrioc);
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 102f84667a5cf..6f113d8625bb4 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -564,16 +564,18 @@ mpi3mr_get_reply_desc(struct op_reply_qinfo *op_reply_q, u32 reply_ci)
  * mpi3mr_process_op_reply_q - Operational reply queue handler
  * @mrioc: Adapter instance reference
  * @op_reply_q: Operational reply queue info
+ * @checked_empty: Optional, set to true if the queue was owned and had no
+ *		   reply ready, false otherwise
  *
  * Checks the specific operational reply queue and drains the
  * reply queue entries until the queue is empty and process the
  * individual reply descriptors.
  *
- * Return: 0 if queue is already processed,or number of reply
- *	    descriptors processed.
+ * Return: Number of reply descriptors processed, 0 if no reply was ready or
+ *	   another context is processing the queue.
  */
 int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
-	struct op_reply_qinfo *op_reply_q)
+	struct op_reply_qinfo *op_reply_q, bool *checked_empty)
 {
 	struct op_req_qinfo *op_req_q;
 	u32 exp_phase;
@@ -583,6 +585,9 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 	struct mpi3_default_reply_descriptor *reply_desc;
 	u16 req_q_idx = 0, reply_qidx, threshold_comps = 0;
 
+	if (checked_empty)
+		*checked_empty = false;
+
 	if (!op_reply_q)
 		return 0;
 
@@ -605,6 +610,8 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 		if ((le16_to_cpu(reply_desc->reply_flags) &
 		    MPI3_REPLY_DESCRIPT_FLAGS_PHASE_MASK) == exp_phase)
 			goto process_desc;
+		if (checked_empty)
+			*checked_empty = true;
 		atomic_dec(&op_reply_q->in_use);
 		return 0;
 	}
@@ -667,7 +674,7 @@ int mpi3mr_process_op_reply_q(struct mpi3mr_ioc *mrioc,
 		 */
 		if ((num_op_reply > mrioc->max_host_ios) &&
 			(threaded_isr_poll == true)) {
-			op_reply_q->enable_irq_poll = true;
+			WRITE_ONCE(op_reply_q->enable_irq_poll, true);
 			break;
 		}
 #endif
@@ -712,7 +719,7 @@ int mpi3mr_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num)
 		return 0;
 
 	num_entries = mpi3mr_process_op_reply_q(mrioc,
-			&mrioc->op_reply_qinfo[queue_num]);
+			&mrioc->op_reply_qinfo[queue_num], NULL);
 
 	return num_entries;
 }
@@ -739,7 +746,8 @@ static irqreturn_t mpi3mr_isr_primary(int irq, void *privdata)
 		num_admin_replies = mpi3mr_process_admin_reply_q(mrioc);
 	op_reply_q = READ_ONCE(intr_info->op_reply_q);
 	if (op_reply_q)
-		num_op_reply = mpi3mr_process_op_reply_q(mrioc, op_reply_q);
+		num_op_reply = mpi3mr_process_op_reply_q(mrioc, op_reply_q,
+							 NULL);
 
 	if (num_admin_replies || num_op_reply)
 		return IRQ_HANDLED;
@@ -769,7 +777,7 @@ static irqreturn_t mpi3mr_isr(int irq, void *privdata)
 	if ((threaded_isr_poll == false) || !op_reply_q)
 		return ret;
 
-	if (!op_reply_q->enable_irq_poll ||
+	if (!READ_ONCE(op_reply_q->enable_irq_poll) ||
 	    !atomic_read(&op_reply_q->pend_ios))
 		return ret;
 
@@ -783,8 +791,9 @@ static irqreturn_t mpi3mr_isr(int irq, void *privdata)
  * @irq: IRQ
  * @privdata: Interrupt info
  *
- * poll for pending I/O completions in a loop until pending I/Os
- * present or controller queue depth I/Os are processed.
+ * Poll for pending I/O completions until no I/Os are pending, a post-sleep
+ * check finds no reply ready while owning the queue, or controller queue
+ * depth I/Os are processed.
  *
  * Return: IRQ_NONE or IRQ_HANDLED
  */
@@ -795,6 +804,7 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
 	struct op_reply_qinfo *op_reply_q;
 	u16 midx;
 	u32 num_op_reply = 0;
+	bool slept = false, idle = false, checked_empty;
 
 	if (!intr_info)
 		return IRQ_NONE;
@@ -819,17 +829,34 @@ static irqreturn_t mpi3mr_isr_poll(int irq, void *privdata)
 
 		if (!midx)
 			mpi3mr_process_admin_reply_q(mrioc);
-		num_op_reply +=
-		    mpi3mr_process_op_reply_q(mrioc, op_reply_q);
+		num_op_reply += mpi3mr_process_op_reply_q(mrioc, op_reply_q,
+							  &checked_empty);
+		/* Stop only on an empty check made while owning the queue */
+		if (slept && checked_empty) {
+			idle = true;
+			break;
+		}
 		if (!atomic_read(&op_reply_q->pend_ios))
 			break;
 
 		usleep_range(MPI3MR_IRQ_POLL_SLEEP, 10 * MPI3MR_IRQ_POLL_SLEEP);
+		slept = true;
 
 	} while (num_op_reply < mrioc->max_host_ios);
 
-	if (op_reply_q)
-		op_reply_q->enable_irq_poll = false;
+	if (op_reply_q) {
+		WRITE_ONCE(op_reply_q->enable_irq_poll, false);
+		if (idle) {
+			/*
+			 * Recheck after clearing, pairs with atomic_inc_return()
+			 * in mpi3mr_op_request_post().
+			 */
+			smp_mb();
+			if (atomic_read(&op_reply_q->pend_ios) >
+			    MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT)
+				WRITE_ONCE(op_reply_q->enable_irq_poll, true);
+		}
+	}
 	enable_irq(intr_info->os_irq);
 
 	return IRQ_HANDLED;
@@ -2337,7 +2364,7 @@ static int mpi3mr_create_op_reply_q(struct mpi3mr_ioc *mrioc, u16 qidx)
 	op_reply_q->ephase = 1;
 	atomic_set(&op_reply_q->pend_ios, 0);
 	atomic_set(&op_reply_q->in_use, 0);
-	op_reply_q->enable_irq_poll = false;
+	WRITE_ONCE(op_reply_q->enable_irq_poll, false);
 	op_reply_q->qfull_watermark =
 		op_reply_q->num_replies - (MPI3MR_THRESHOLD_REPLY_COUNT * 2);
 
@@ -2688,7 +2715,7 @@ int mpi3mr_op_request_post(struct mpi3mr_ioc *mrioc,
 		midx = REPLY_QUEUE_IDX_TO_MSIX_IDX(
 		    reply_qidx, mrioc->op_reply_q_offset);
 		mpi3mr_process_op_reply_q(mrioc,
-		    READ_ONCE(mrioc->intr_info[midx].op_reply_q));
+		    READ_ONCE(mrioc->intr_info[midx].op_reply_q), NULL);
 
 		if (mpi3mr_check_req_qfull(op_req_q)) {
 
@@ -2740,7 +2767,8 @@ int mpi3mr_op_request_post(struct mpi3mr_ioc *mrioc,
 #ifndef CONFIG_PREEMPT_RT
 	if (atomic_inc_return(&mrioc->op_reply_qinfo[reply_qidx].pend_ios)
 	    > MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT)
-		mrioc->op_reply_qinfo[reply_qidx].enable_irq_poll = true;
+		WRITE_ONCE(mrioc->op_reply_qinfo[reply_qidx].enable_irq_poll,
+			   true);
 #else
 	atomic_inc_return(&mrioc->op_reply_qinfo[reply_qidx].pend_ios);
 #endif
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 6b0156e54d454..ec5e0ddb04deb 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -4063,7 +4063,7 @@ inline void mpi3mr_poll_pend_io_completions(struct mpi3mr_ioc *mrioc)
 
 	for (i = mrioc->op_reply_q_offset; i < num_of_reply_queues; i++)
 		mpi3mr_process_op_reply_q(mrioc,
-		    READ_ONCE(mrioc->intr_info[i].op_reply_q));
+		    READ_ONCE(mrioc->intr_info[i].op_reply_q), NULL);
 }
 
 /**
-- 
2.53.0-Meta

Re: [PATCH 0/2] scsi: mpi3mr: Stop IRQ polling when no reply is ready

[Usama Arif]

On Wed, 30 Sep 2026 07:55:47 -0700 Usama Arif <usama.arif@linux.dev> wrote:

> Once a reply queue has more than 8 I/Os pending, mpi3mr's IRQ thread
> polls it every ~20us until nothing is pending or it has processed
> max_host_ios replies. On busy HDDs that means about 45 seconds of
> polling at a time, with nearly every wakeup finding nothing. On Meta's
> HDD storage hosts, these threads used about 15% of the non-idle CPU
> on polling.
> 
> Patch 2 goes back to interrupts once a poll finds the queue empty while
> owning it. It relies on patch 1: an interrupt that finds the queue owned
> by another context now hands it to the thread, instead of leaving a
> reply until the next interrupt or a command timeout.
> 
> On a machine configured like those hosts (36 SATA HDDs behind a SAS40xx
> HBA), HDD random reads now wake the threads twice per I/O instead of 236
> times, and a co-located CPU-bound task loses 0.6% instead of 30%.
> Drive-cache reads still reach 270K IOPS, and a single reply queue whose
> CPU is saturated loses about 5%. With a kthread injected to hold a reply
> queue, replies waited up to 50ms, and timed out under light load,
> without patch 1, and at most 233us with it.

I believe all of the sashiko reviews are pre-existing conditions and not
introduced by these patches. I have replied in [1] and [2]

[1] https://lore.kernel.org/all/bdf2781e-fbcb-4356-b1eb-ccdbfb39c1b9@linux.dev/
[2] https://lore.kernel.org/all/0b1bb7ce-4720-4d12-96ff-ab04cd1b85bc@linux.dev/

> 
> Based on mkp/scsi 7.4/scsi-staging (f09d2c7485b32).
>  
> Usama Arif (2):
>   scsi: mpi3mr: Poll a reply queue that an interrupt found busy
>   scsi: mpi3mr: Stop IRQ polling when no reply is ready
> 
>  drivers/scsi/mpi3mr/mpi3mr.h    |  2 +-
>  drivers/scsi/mpi3mr/mpi3mr_fw.c | 65 ++++++++++++++++++++++++---------
>  drivers/scsi/mpi3mr/mpi3mr_os.c |  2 +-
>  3 files changed, 50 insertions(+), 19 deletions(-)
> 
> 
> base-commit: f09d2c7485b32adb82336d0d748935c8237a649e
> -- 
> 2.53.0-Meta
> 
>