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

[sashiko-bot@kernel.org]

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?

> +
> +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.

[ ... ]

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