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

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

On 9/1/26 12:13, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Path routing algorithms completely ignore ALUA path priorities due to reliance on a static state field.
> - [High] I/Os stranded in the requeue list during path unavailability will hang indefinitely because ALUA state recovery does not trigger a requeue list flush.
> --
> 
> commit 8deb21c730fe5e2ebafeae61cfbcd392c6ab5948
> Author: John Garry <linux.dev>
> 
> scsi-multipath: provide callbacks for path state
> 
> This commit provides callbacks for is_disabled, is_optimized, and
> available_path for SCSI multipath devices. The callbacks rely on the
> device's ALUA state and running state to dynamically determine path
> availability and routing priorities.
> 
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index fd28667792472..b9f6b1eb3c883 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
> 
> [ ... ]
> 
>> +static bool scsi_mpath_is_disabled(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;
>> +	unsigned char access_state = READ_ONCE(sdev->access_state) &
>> +					SCSI_ACCESS_STATE_MASK;
>> +
>> +	if (READ_ONCE(sdev->sdev_state) != SDEV_RUNNING)
>> +		return true;
>> +
>> +	if (access_state == SCSI_ACCESS_STATE_OPTIMAL ||
>> +	    access_state == SCSI_ACCESS_STATE_ACTIVE)
>> +		return false;
>> +
>> +	return true;
>> +}
> 
> [Severity: High]
> When this function correctly returns true (for example, when an ALUA-managed
> storage path transitions to STANDBY), incoming I/Os are deferred to the
> multipath head's requeue list.
> 
> However, when the target recovers and the ALUA state changes back, is there a
> mechanism to wake up and flush these stranded I/Os?
> 
> The ALUA device handlers update sdev->access_state upon receiving an RTPG,
> but they do not appear to notify the core multipath layer to flush the requeue
> list. In NVMe, this is handled by kblockd_schedule_work(&head->requeue_work)
> when ANA state changes. Because sdev_state remains SDEV_RUNNING, unblock
> handlers are not triggered either. Will this lead to I/Os hanging
> indefinitely in the requeue list without a corresponding wake-up trigger?

All this will be improved with proper scsi-multipath ALUA support.

> 
>> +
>> +static bool scsi_mpath_is_optimized(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;
>> +	unsigned char access_state = READ_ONCE(sdev->access_state) &
>> +					SCSI_ACCESS_STATE_MASK;
>> +
>> +	if (READ_ONCE(sdev->sdev_state) != SDEV_RUNNING)
>> +		return false;
>> +
>> +	return access_state == SCSI_ACCESS_STATE_OPTIMAL;
>> +}
> 
> [Severity: High]
> Does the multipath core actually use this callback for path selection routing?
> 
> Looking at the core path routing algorithms in lib/multipath.c, such as
> __mpath_find_path, mpath_round_robin_path, or mpath_queue_depth_path, they
> appear to check the static struct field instead of invoking the callback:
> 
> lib/multipath.c:__mpath_find_path() {
>      ...
>      switch(mpath_device->access_state) {
>      ...
> }
> 
> Because mpath_device->access_state is hardcoded to MPATH_STATE_OPTIMIZED
> during device allocation in scsi_multipath_sdev_init() and is not dynamically
> synced with the ALUA state, won't the routing algorithms blindly place
> non-optimized active paths in the same preference bucket as optimized paths?
> This seems to bypass the newly introduced ALUA callbacks entirely for
> queue-depth or round-robin policies.

As above

> 
> [ ... ]
>