Re: [PATCH v2 3/4] scsi: scsi_dh_alua: Start a port group in the scanned access state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: scsi_dh_alua: initial RTPG dropped in alua_bus_attach() leaves pg->state stale
--

commit 82a8c8c9b2d2916e1a4b1a2ea0671b7c6f85346c
Author: Brian Bunker <brian@purestorage.com>

scsi: scsi_dh_alua: Start a port group in the scanned access state

This commit initializes a newly allocated ALUA port group's state and
preferred bit from the scan-time access_state stored on the scsi_device
rather than always starting in SCSI_ACCESS_STATE_OPTIMAL.

> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 06eb35cc9d7cb..15471e7bf19a5 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -208,7 +208,14 @@ static struct alua_port_group *alua_alloc_pg(struct scsi_device *sdev,
>  	}
>  	pg->group_id = group_id;
>  	pg->tpgs = tpgs;
> -	pg->state = SCSI_ACCESS_STATE_OPTIMAL;
> +	/*
> +	 * Start in the access state the SCSI core read when the device was
> +	 * scanned. That is active/optimized if it did not read one, as it
> +	 * was before. alua_prep_fn() then already fails I/O to a path that
> +	 * is unavailable before the first RTPG of the handler completes.
> +	 */
> +	pg->state = FIELD_GET(SCSI_ACCESS_STATE_MASK, sdev->access_state);
> +	pg->pref = FIELD_GET(SCSI_ACCESS_STATE_PREFERRED, sdev->access_state);

[Severity: High]
Does the initial RTPG of the handler actually get queued when attaching via
alua_bus_attach()?

In drivers/scsi/device_handler/scsi_dh_alua.c, alua_bus_attach() calls
alua_initialize() before assigning sdev->handler_data:

alua_bus_attach()
	err = alua_initialize(sdev, h);
	if (err != SCSI_DH_OK && err != SCSI_DH_DEV_OFFLINED)
		goto failed;

	sdev->handler_data = h;

During alua_initialize() -> alua_check_vpd(), after alua_alloc_pg() sets
pg->state from sdev->access_state, alua_check_vpd() calls
alua_rtpg_queue(pg, sdev, NULL, true). However, alua_rtpg_queue() checks
sdev->handler_data before scheduling pg->rtpg_work:

	if (pg->rtpg_sdev == NULL) {
		struct alua_dh_data *h = sdev->handler_data;

		rcu_read_lock();
		if (h && rcu_dereference(h->pg) == pg) {
			pg->interval = 0;
			pg->flags |= ALUA_PG_RUN_RTPG;
			kref_get(&pg->kref);
			pg->rtpg_sdev = sdev;
			start_queue = 1;
		}
		rcu_read_unlock();
	}

Because sdev->handler_data is still NULL at this point, h is NULL and the
initial RTPG work is silently dropped.

Previously, pg->state started as SCSI_ACCESS_STATE_OPTIMAL, so I/O was
allowed through alua_prep_fn() and any sense data could trigger
alua_check_sense() after sdev->handler_data was populated.

With pg->state now starting in the scan-time sdev->access_state (and with
scan-time RTPG having already consumed pending unit attentions):

1. If sdev->access_state was UNAVAILABLE or STANDBY at scan time and the
port group transitions to OPTIMAL before alua_bus_attach() runs (or when
the handler is attached later via scsi_dh_attach()), pg->state starts as
UNAVAILABLE or STANDBY. Because the initial RTPG is dropped and
alua_prep_fn() immediately rejects non-passthrough I/O with BLK_STS_IOERR
before it reaches the target, can the path remain stuck in the stale
state?

2. If sdev->access_state was SCSI_ACCESS_STATE_TRANSITIONING at scan time,
won't dropping the initial RTPG prevent alua_rtpg_work from polling the
port group or enforcing the transition timeout?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028230807.61578-1-brian@purestorage.com?part=3