Re: [PATCH v6 20/27] scsi-multipath: add scsi_mpath_{start,end}_request()

On 29/07/2026 17:12, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Permanent leak of mpath_nr_active counter and master block device in-flight IO stats when a requeued multipath request fails with a hard error.
> --
> 
> --- Patch [20]: [PATCH v6 20/27] scsi-multipath: add scsi_mpath_{start,end}_request() ---
> commit 1a27bdf4453d197f11533d04e7997e7873652eb4
> Author: John Garry <[email protected]>
> 
> scsi-multipath: add scsi_mpath_{start,end}_request()
> 
> Add scsi_mpath_{start,end}_request() to handle updating private multipath
> request data, like nvme_mpath_{start,end}_request().
> 
> New member Scsi_Host.mpath_nr_active is added. This is required for queue
> depth multipath iopolicy.
> 
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index 6a2d7dbfbe8c3..ce9a2c7fbe707 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
> [ ... ]
>> @@ -1917,6 +1920,9 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
>>   	memset(cmd->sense_buffer, 0, SCSI_SENSE_BUFFERSIZE);
>>   	cmd->submitter = SUBMITTED_BY_BLOCK_LAYER;
>>   
>> +	if (sdev->scsi_mpath_dev && is_mpath_request(req))
>> +		scsi_mpath_start_request(req);
>> +
>>   	blk_mq_start_request(req);
>>   	if (blk_mq_is_reserved_rq(req)) {
>>   		reason = shost->hostt->queue_reserved_command(shost, cmd);
> 
> [Severity: High]
> Can this permanently leak the mpath_nr_active counter and master block device
> in-flight IO stats when a requeued multipath request fails with a hard error?
> 
> When a multipath request is dispatched, scsi_mpath_start_request() increments
> mpath_nr_active and sets SCMD_MPATH_CNT_ACTIVE. If the dispatch then fails
> with BLK_STS_RESOURCE (e.g. due to a full queue), the request is requeued and
> the multipath flags are preserved by SCMD_PRESERVED_FLAGS.
> 
> On a subsequent retry, if the device has transitioned to an offline state
> (which is common during path failure), the early state check in
> scsi_queue_rq() returns an error before scsi_mpath_start_request() is
> reached:
> 
> 	if (unlikely(sdev->sdev_state != SDEV_RUNNING)) {
> 		ret = scsi_device_state_check(sdev, req);
> 		if (ret != BLK_STS_OK)
> 			goto out_put_budget;
> 	}
> 
> Returning an error here causes the block layer to directly complete the
> request, bypassing the driver's normal scsi_end_request() completion path.
> The block layer will eventually call the scsi_cleanup_rq() hook, which merely
> zeroes cmd->flags:
> 
> 	cmd->flags = 0;
> 
> but fails to decrement mpath_nr_active or call bdev_end_io_acct().
> 
> Would this leave the queue-depth multipath I/O policy perceiving an
> artificially inflated load on the controller, permanently degrading load
> balancing?

Can be improved later

>