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

[Usama Arif <usama.arif@linux.dev>]

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