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

[sashiko-bot@kernel.org]

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