[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 1/2] scsi: mpi3mr: Poll a reply queue that an interrupt found busy

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: mpi3mr: unhandled missed reply race under PREEMPT_RT
--

commit c0b0a317901b4ce93a734871bf41b604287947d5
Author: Usama Arif <usama.arif@linux.dev>

scsi: mpi3mr: Poll a reply queue that an interrupt found busy

This patch sets a flag to trigger a thread wakeup when an interrupt finds a
reply queue busy. It aims to prevent missed replies when another context
owns the queue.

> 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);

[Severity: High]
When this code executes in mpi3mr_process_op_reply_q() under PREEMPT_RT,
does it leave missed IO replies stranded?

Under CONFIG_PREEMPT_RT, the interrupt handler is mpi3mr_isr_primary(),
which exits without checking the enable_irq_poll flag or returning
IRQ_WAKE_THREAD.

Additionally, looking at how the interrupt is requested:

drivers/scsi/mpi3mr/mpi3mr_fw.c:mpi3mr_request_irq() {
    ...
#else
	retval = request_threaded_irq(pci_irq_vector(pdev, index), mpi3mr_isr_primary,
	    NULL, IRQF_SHARED, intr_info->name, intr_info);
#endif
    ...
}

Because the polling thread handler is explicitly NULL under PREEMPT_RT,
there is no polling thread to wake up to process the missed replies.
Could this cause SCSI command timeouts and host resets during concurrent
IO under PREEMPT_RT?

>  		return 0;
> +	}
>  
>  	exp_phase = op_reply_q->ephase;
>  	reply_ci = op_reply_q->ci;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930145606.2632749-1-usama.arif@linux.dev?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: mpi3mr: dropped IRQ poll trigger causes hard-IRQ storm
--

commit a00593b528c873a4c3ec905f3fa02c054a4e3bf0
Author: Usama Arif <usama.arif@linux.dev>
Subject: scsi: mpi3mr: Stop IRQ polling when no reply is ready

This commit modifies the IRQ polling logic in the mpi3mr driver to stop
polling when a post-sleep check finds the queue empty. It introduces a
barrier and a recheck of pend_ios to avoid losing wakeups from concurrent
submitters.

> 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
[ ... ]
> @@ -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);
> +		}
> +	}

[Severity: Medium]
Could this unconditional clearing of the flag combined with gating the recheck
behind if (idle) allow a concurrent submitter's trigger to be overwritten and
lost?

If the polling thread processes all replies and pend_ios drops to 0, it breaks
the loop with idle = false.

If a concurrent submitter in mpi3mr_op_request_post() then posts a burst of
I/Os, surpasses the trigger threshold, and sets enable_irq_poll = true, the
polling thread will subsequently execute WRITE_ONCE(op_reply_q->enable_irq_poll, false).

Because idle is false, the thread skips the smp_mb() barrier and the pend_ios
recheck. Doesn't this blindly overwrite the submitter's flag and leave
enable_irq_poll disabled, forcing all completions into the hard IRQ handler?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930145606.2632749-1-usama.arif@linux.dev?part=2

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

[Usama Arif]

On 30/09/2026 16:10, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] scsi: mpi3mr: unhandled missed reply race under PREEMPT_RT
> --
> 
> commit c0b0a317901b4ce93a734871bf41b604287947d5
> Author: Usama Arif <usama.arif@linux.dev>
> 
> scsi: mpi3mr: Poll a reply queue that an interrupt found busy
> 
> This patch sets a flag to trigger a thread wakeup when an interrupt finds a
> reply queue busy. It aims to prevent missed replies when another context
> owns the queue.
> 
>> 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);
> 
> [Severity: High]
> When this code executes in mpi3mr_process_op_reply_q() under PREEMPT_RT,
> does it leave missed IO replies stranded?
> 
> Under CONFIG_PREEMPT_RT, the interrupt handler is mpi3mr_isr_primary(),
> which exits without checking the enable_irq_poll flag or returning
> IRQ_WAKE_THREAD.
> 
> Additionally, looking at how the interrupt is requested:
> 
> drivers/scsi/mpi3mr/mpi3mr_fw.c:mpi3mr_request_irq() {
>     ...
> #else
> 	retval = request_threaded_irq(pci_irq_vector(pdev, index), mpi3mr_isr_primary,
> 	    NULL, IRQF_SHARED, intr_info->name, intr_info);
> #endif
>     ...
> }
> 
> Because the polling thread handler is explicitly NULL under PREEMPT_RT,
> there is no polling thread to wake up to process the missed replies.
> Could this cause SCSI command timeouts and host resets during concurrent
> IO under PREEMPT_RT?


This patch doesn't change how PREEMPT_RT behaves. Nothing reads
enable_irq_poll there, so a handler that loses the in_use try-lock
returns just as it did before, and the same goes for
threaded_isr_poll=0.

So the race remains on those configurations, but it is not new. The
reply waits for the next interrupt on that queue, or for the command
timeout if none comes. Fixing it without the polling thread needs a
different approach, which I'd rather keep out of this series.


> 
>>  		return 0;
>> +	}
>>  
>>  	exp_phase = op_reply_q->ephase;
>>  	reply_ci = op_reply_q->ci;
> 

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

[Usama Arif]

On 30/09/2026 16:20, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] scsi: mpi3mr: dropped IRQ poll trigger causes hard-IRQ storm
> --
> 
> commit a00593b528c873a4c3ec905f3fa02c054a4e3bf0
> Author: Usama Arif <usama.arif@linux.dev>
> Subject: scsi: mpi3mr: Stop IRQ polling when no reply is ready
> 
> This commit modifies the IRQ polling logic in the mpi3mr driver to stop
> polling when a post-sleep check finds the queue empty. It introduces a
> barrier and a recheck of pend_ios to avoid losing wakeups from concurrent
> submitters.
> 
>> 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
> [ ... ]
>> @@ -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);
>> +		}
>> +	}
> 
> [Severity: Medium]
> Could this unconditional clearing of the flag combined with gating the recheck
> behind if (idle) allow a concurrent submitter's trigger to be overwritten and
> lost?
> 
> If the polling thread processes all replies and pend_ios drops to 0, it breaks
> the loop with idle = false.
> 
> If a concurrent submitter in mpi3mr_op_request_post() then posts a burst of
> I/Os, surpasses the trigger threshold, and sets enable_irq_poll = true, the
> polling thread will subsequently execute WRITE_ONCE(op_reply_q->enable_irq_poll, false).
> 
> Because idle is false, the thread skips the smp_mb() barrier and the pend_ios
> recheck. Doesn't this blindly overwrite the submitter's flag and leave
> enable_irq_poll disabled, forcing all completions into the hard IRQ handler?
> 

Yes, it can be overwritten on those exits, but that is not new: before
this patch the thread cleared the flag on its exits without a recheck,
and those exits are unchanged. The recheck is only added to the idle
exit this patch introduces, which stops polling while I/Os can still be
pending.

It also only lasts until the next submission: mpi3mr_op_request_post()
sets the flag on every submission that leaves more than
MPI3MR_IRQ_POLL_TRIGGER_IOCOUNT I/Os pending, not just on the one that
crosses it. Until then the hard IRQ handler processes the replies, as it
does below the trigger or with threaded_isr_poll=0, so no reply is lost.

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
> 
>