Re: [PATCH v8 19/27] scsi-multipath: provide callbacks for path state

[John Garry <john.garry@linux.dev>]

On 9/7/26 13:03, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] scsi_mpath_available_path() erroneously returns true for ALUA unavailable/offline states, leading to infinite I/O requeuing.
> --
> 
> commit 4cf5187795e3a1d7ac00b1645759bf9140204feb
> Author: John Garry <john.garry@linux.dev>
> 
> scsi-multipath: provide callbacks for path state
> 
> This patch provides callbacks for .is_disabled, .is_optimized, and
> .available_path. These use scsi_device.sdev_state and access_state, which
> will be driven by ALUA.
> 
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index 12200664ed676..4055b86c83a03 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
>> @@ -306,7 +306,56 @@ static struct bio *scsi_mpath_clone_bio(struct bio *bio)
> [ ... ]
>> +static bool scsi_mpath_available_path(struct mpath_device *mpath_device)
>> +{
>> +	struct scsi_mpath_device *scsi_mpath_dev =
>> +				to_scsi_mpath_device(mpath_device);
>> +	struct scsi_device *sdev = scsi_mpath_dev->sdev;
>> +	enum scsi_device_state sdev_state = READ_ONCE(sdev->sdev_state);
>> +
>> +	if (sdev_state == SDEV_RUNNING || sdev_state == SDEV_QUIESCE ||
>> +	    sdev_state == SDEV_BLOCK || sdev_state == SDEV_CREATED_BLOCK)
>> +		return true;
>> +
>> +	return false;
>> +}
> 
> [Severity: High]
> Does this logic bypass queue_if_no_path limits when devices are in ALUA
> offline or unavailable states?
> 
> Since this only checks sdev_state, if all paths transition to an ALUA
> offline or unavailable state but remain physically connected (SDEV_RUNNING),
> would this function still return true?

ALUA support will be properly added next

> 
> If it returns true for ALUA offline paths, could mpath_bdev_submit_bio()
> indefinitely queue I/O in the requeue_list, ignoring the user's
> configuration for timeout and failure?
> 
> Should this function also check sdev->access_state to prevent treating
> paths as available when they are in ALUA states that can never serve I/O?
>