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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Bypassing `scsi_end_request()` on hard errors in `scsi_queue_rq()` permanently leaks multipath active counters and corrupts state for recycled blk-mq tags.
--

commit d6985f792a0819a7b56a04e827a71ff5a8dd2172
Author: John Garry <john.garry@linux.dev>

scsi-multipath: add scsi_mpath_{start,end}_request()

This commit adds scsi_mpath_start_request() and scsi_mpath_end_request() to
handle private multipath request data. It also introduces mpath_nr_active
per SCSI host to track active requests for queue depth io-policies.

> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 189263ce56624..5916dead5422e 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
[ ... ]
> @@ -1918,6 +1921,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);

[ ... ]

> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 8ecfb94049db5..52227e2cd9749 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
[ ... ]
> @@ -58,8 +58,11 @@ struct scsi_pointer {
>   */
>  #define SCMD_FORCE_EH_SUCCESS	(1 << 3)
>  #define SCMD_FAIL_IF_RECOVERING	(1 << 4)
> +#define SCMD_MPATH_IO_STATS	(1 << 5)
> +#define SCMD_MPATH_CNT_ACTIVE	(1 << 6)
>  /* flags preserved across unprep / reprep */
> -#define SCMD_PRESERVED_FLAGS	(SCMD_INITIALIZED | SCMD_FAIL_IF_RECOVERING)
> +#define SCMD_PRESERVED_FLAGS	(SCMD_INITIALIZED | SCMD_FAIL_IF_RECOVERING | \
> +				SCMD_MPATH_IO_STATS | SCMD_MPATH_CNT_ACTIVE)

[Severity: High]
Does preserving these multipath flags across requeue operations cause a leak 
of the mpath_nr_active counter and stale state for recycled tags?

If a multipath request is dispatched, it calls scsi_mpath_start_request() 
which increments the host's mpath_nr_active counter and sets these flags. 

If this request is requeued (for example, if the host is busy) and then 
re-dispatched while the device is offline or entering recovery, 
scsi_device_state_check() in scsi_queue_rq() will fail early. This causes 
scsi_queue_rq() to return a hard error directly to the block layer.

Because the block layer completes the request natively on hard dispatch 
errors, it bypasses scsi_end_request(), leaving scsi_mpath_end_request() 
uncalled. The mpath_nr_active counter is leaked. 

Furthermore, since SCMD_MPATH_IO_STATS and SCMD_MPATH_CNT_ACTIVE are 
preserved in SCMD_PRESERVED_FLAGS, any new request reusing this tag might 
inherit the stale flags, bypassing initialization in 
scsi_mpath_start_request().

Could there be a mechanism to clear these flags and decrement the counter 
when a requeued request encounters a hard failure during re-dispatch?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901104526.3850621-1-john.g.garry@oracle.com?part=20