[PATCH v2 0/4] scsi: Bind sd to LUs in the ALUA unavailable state

[PATCH v2 0/4] scsi: Bind sd to LUs in the ALUA unavailable state

[Brian Bunker]

A LU that is in the ALUA unavailable state on a target port reports
peripheral qualifier 001b in its INQUIRY data. scsi_bus_match() only
matches devices with peripheral qualifier 000b, so if a host scans such
a path, no upper-level driver is ever bound to it. When the target port
later becomes available, nothing binds sd and the path stays unusable
until the device is deleted and rescanned. A host that was already
running when the path became unavailable keeps sd bound, so the result
depends on when the host scanned.

This series fixes that at scan time. The SCSI core reads the ALUA access
state of a device with REPORT TARGET PORT GROUPS before the device is
registered, and matches sd to PQ 001b devices whose access state could
be read. SPC-4 requires PQ 001b in the unavailable state, and the
command fails for an unmapped LUN, so these are taken to be mapped LUs
that are unavailable. This is the same situation as a path in the
standby state, to which sd already binds. The path then stays in the
multipath map as a
failed path, and the existing UNIT ATTENTION handling brings it back
when the target port becomes available.

PQ 001b cannot be matched on its own: an unmapped LUN reports it as
well, and some targets report every possible LUN in REPORT LUNS. Binding
sd to all of those would create disks with no backing LU. An unmapped
LUN fails REPORT TARGET PORT GROUPS, so the access state is what tells
the two cases apart.

  1/4 moves submit_rtpg() out of scsi_dh_alua into the SCSI core as
      scsi_report_tpgs(), so the core can read the ALUA state of a
      device itself. No functional change.
  2/4 sends REPORT TARGET PORT GROUPS from scsi_add_lun() for a device
      that reports TPGS support and sets sdev->access_state from the
      descriptor of the target port group of this path.
  3/4 starts a new port group in scsi_dh_alua in the access state the
      core read, rather than active/optimized, so that no I/O is let
      through to an unavailable path before the handler's first RTPG
      completes.
  4/4 matches PQ 001b devices whose access state could be read, which
      are taken to be mapped LUs in the unavailable state.

This is an alternative to the series that refreshes the INQUIRY data
and reprobes the device on rescan ("scsi: Refresh INQUIRY data and
reprobe on rescan", v8 last posted). That series needs a way to update
the INQUIRY data of a live device and to reprobe it. This one does not
change the INQUIRY data after the device is discovered, and has no
reprobe, so none of the locking and ordering issues that came with
refreshing it.

The INQUIRY data in sysfs is unchanged by this series. It is read and
created once, when the device is discovered, and keeps the peripheral
qualifier reported at that time like the rest of the INQUIRY data. At
any given time that cached peripheral qualifier can be out of sync with
the device, for example after the target port moves between available
and unavailable. An INQUIRY sent to the device returns the current
value, and the access_state attribute shows the current ALUA state.

This does not make every target work. Today none does: a host that scans
a path while it is unavailable never binds sd to it. With this series a
target that answers sd's probe commands in that state is handled
correctly, and the others are no worse off. What it relies on, and the
known limits:
  - SPC-4 requires PQ 001b in the unavailable state and requires REPORT
    TARGET PORT GROUPS there. It does not say that PQ 001b is reported
    only in that state, so a PQ 001b device whose access state could be
    read is taken to be a mapped LU that is unavailable: an unmapped
    LUN fails the command.
  - The decision is made once, at scan, like the peripheral qualifier
    itself. If REPORT TARGET PORT GROUPS fails (other than with UNIT
    ATTENTION, which is retried), or the INQUIRY data and VPD page 0x83
    do not allow it to be sent, nothing is matched, as before.
  - SPC-4 requires neither READ CAPACITY nor, in the unavailable state,
    MODE SENSE. alua_prep_fn() already fails normal I/O to such a path
    without sending it, but sd's probe commands are passthrough
    commands and are sent to the target. If a target rejects them, sd
    keeps default values, such as a capacity of 0, until a later
    revalidate, as it does for a standby path today. If the path
    changes state while sd probes, it can also end up with a mix.
  - The ALUA device handler starts a new port group in the state read at
    scan. If the path changed since, that is stale until the first RTPG
    of the handler completes.
  - The standard INQUIRY data, including the peripheral qualifier and
    what sd uses from it, is only read when the device is discovered;
    the kernel has no mechanism to refresh it. A rescan, e.g. on the
    INQUIRY DATA HAS CHANGED UNIT ATTENTION, reads the VPD pages again
    and revalidates sd, but does not read the INQUIRY data again.

Changes since v1 (prompted by the Sashiko review of 4/4):
  - 4/4: scsi_bus_match() no longer tests sdev->access_state. The
    peripheral qualifier is read once when the device is scanned, but
    access_state follows the ALUA state, so a device scanned in the
    unavailable state stopped matching as soon as its path became
    available: when an upper-level driver module is loaded later, when
    a driver is rebound through sysfs, or when the ALUA device handler
    updated access_state before sd was probed. scsi_add_lun() now
    records in a new sdev->match_unavailable flag whether the device
    reported PQ 001b and its access state could be read, and
    scsi_bus_match() uses that. SPC-4 requires PQ 001b in the unavailable
    state and an unmapped LUN fails the command, so this does not
    depend on the state the command returns, which can already be
    available if the path recovered between the INQUIRY and the
    command.
  - 1/4 to 3/4: No changes.

v1 cover-letter Message-ID: <20261028184216.25453-1-brian@purestorage.com>

Brian Bunker (4):
  scsi: core: Move REPORT TARGET PORT GROUPS into the SCSI core
  scsi: core: Read the ALUA access state when a device is scanned
  scsi: scsi_dh_alua: Start a port group in the scanned access state
  scsi: core: Match ULDs to logical units in the ALUA unavailable state

 drivers/scsi/device_handler/scsi_dh_alua.c |  72 +++++---------
 drivers/scsi/scsi_lib.c                    |  52 +++++++++++
 drivers/scsi/scsi_scan.c                   | 103 +++++++++++++++++++++
 drivers/scsi/scsi_sysfs.c                  |  14 ++-
 include/scsi/scsi_device.h                 |  10 ++
 include/scsi/scsi_proto.h                  |   3 +
 6 files changed, 204 insertions(+), 50 deletions(-)

-- 
2.56.0

[PATCH v2 1/4] scsi: core: Move REPORT TARGET PORT GROUPS into the SCSI core

[Brian Bunker]

ALUA is a standard SCSI feature, independent of multipathing, but only
the ALUA device handler can currently send REPORT TARGET PORT GROUPS.
Move submit_rtpg() into the SCSI core as scsi_report_tpgs(), together
with the resubmission with a larger buffer that alua_rtpg() did when
the parameter data did not fit, so that the core can read the ALUA
state of a device itself. The parameter data format definitions move to
scsi_proto.h with it.

scsi_dh_alua calls scsi_report_tpgs() with the same timeout, retries
and parameter data format as before.

No functional change.

Suggested-by: Hannes Reinecke <hare@kernel.org>
Co-developed-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Brian Bunker <brian@purestorage.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 63 ++++++----------------
 drivers/scsi/scsi_lib.c                    | 52 ++++++++++++++++++
 include/scsi/scsi_device.h                 |  3 ++
 include/scsi/scsi_proto.h                  |  3 ++
 4 files changed, 73 insertions(+), 48 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 87ac0a6d63e5..06eb35cc9d7c 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -29,9 +29,6 @@
 #define TPGS_SUPPORT_TRANSITION		0x80
 #define TPGS_SUPPORT_ALL		0xdf
 
-#define RTPG_FMT_MASK			0x70
-#define RTPG_FMT_EXT_HDR		0x10
-
 #define TPGS_MODE_UNINITIALIZED		 -1
 #define TPGS_MODE_NONE			0x0
 #define TPGS_MODE_IMPLICIT		0x1
@@ -120,34 +117,6 @@ static void release_port_group(struct kref *kref)
 	kfree_rcu(pg, rcu);
 }
 
-/*
- * submit_rtpg - Issue a REPORT TARGET GROUP STATES command
- * @sdev: sdev the command should be sent to
- */
-static int submit_rtpg(struct scsi_device *sdev, unsigned char *buff,
-		       int bufflen, struct scsi_sense_hdr *sshdr, int flags)
-{
-	u8 cdb[MAX_COMMAND_SIZE];
-	blk_opf_t opf = REQ_OP_DRV_IN | REQ_FAILFAST_DEV |
-				REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER;
-	const struct scsi_exec_args exec_args = {
-		.sshdr = sshdr,
-	};
-
-	/* Prepare the command. */
-	memset(cdb, 0x0, MAX_COMMAND_SIZE);
-	cdb[0] = MAINTENANCE_IN;
-	if (!(flags & ALUA_RTPG_EXT_HDR_UNSUPP))
-		cdb[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
-	else
-		cdb[1] = MI_REPORT_TARGET_PGS;
-	put_unaligned_be32(bufflen, &cdb[6]);
-
-	return scsi_execute_cmd(sdev, cdb, opf, buff, bufflen,
-				ALUA_FAILOVER_TIMEOUT * HZ,
-				ALUA_FAILOVER_RETRIES, &exec_args);
-}
-
 /*
  * submit_stpg - Issue a SET TARGET PORT GROUP command
  *
@@ -518,6 +487,9 @@ static int alua_tur(struct scsi_device *sdev)
 static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 {
 	struct scsi_sense_hdr sense_hdr;
+	const struct scsi_exec_args exec_args = {
+		.sshdr = &sense_hdr,
+	};
 	struct alua_port_group *tmp_pg;
 	int len, k, off, bufflen = ALUA_RTPG_SIZE;
 	int group_id_old, state_old, pref_old, valid_states_old;
@@ -549,7 +521,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 
  retry:
 	err = 0;
-	retval = submit_rtpg(sdev, buff, bufflen, &sense_hdr, pg->flags);
+	retval = scsi_report_tpgs(sdev, &buff, &bufflen,
+				  !(pg->flags & ALUA_RTPG_EXT_HDR_UNSUPP),
+				  ALUA_FAILOVER_TIMEOUT * HZ,
+				  ALUA_FAILOVER_RETRIES, &exec_args);
+	if (!buff) {
+		sdev_printk(KERN_WARNING, sdev,
+			    "%s: kmalloc buffer failed\n", __func__);
+		/* Temporary failure, bypass */
+		pg->expiry = 0;
+		return SCSI_DH_DEV_TEMP_BUSY;
+	}
 
 	if (retval) {
 		/*
@@ -581,7 +563,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		}
 
 		/*
-		 * submit_rtpg() has failed on existing arrays
+		 * scsi_report_tpgs() has failed on existing arrays
 		 * when requesting extended header info, and
 		 * the array doesn't support extended headers,
 		 * even though it shouldn't according to T10.
@@ -629,21 +611,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 
 	len = get_unaligned_be32(&buff[0]) + 4;
 
-	if (len > bufflen) {
-		/* Resubmit with the correct length */
-		kfree(buff);
-		bufflen = len;
-		buff = kmalloc(bufflen, GFP_KERNEL);
-		if (!buff) {
-			sdev_printk(KERN_WARNING, sdev,
-				    "%s: kmalloc buffer failed\n",__func__);
-			/* Temporary failure, bypass */
-			pg->expiry = 0;
-			return SCSI_DH_DEV_TEMP_BUSY;
-		}
-		goto retry;
-	}
-
 	orig_transition_tmo = pg->transition_tmo;
 	if ((buff[4] & RTPG_FMT_MASK) == RTPG_FMT_EXT_HDR && buff[5] != 0)
 		pg->transition_tmo = buff[5];
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index f695a6605815..ebbfe3238d97 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3581,6 +3581,58 @@ int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
 }
 EXPORT_SYMBOL(scsi_vpd_tpg_id);
 
+/**
+ * scsi_report_tpgs - Issue a REPORT TARGET PORT GROUPS command
+ * @sdev: SCSI device to send the command to
+ * @buff: buffer for the parameter data, replaced if it is too small
+ * @bufflen: length of @buff
+ * @ext_hdr: request the extended header parameter data format
+ * @timeout: command timeout in jiffies
+ * @retries: number of times to retry the command in the midlayer
+ * @args: optional arguments for scsi_execute_cmd()
+ *
+ * Resend the command with a larger buffer if the parameter data does not
+ * fit.
+ *
+ * Return: the result of the last command, or -ENOMEM with *@buff set to
+ * NULL if a larger buffer could not be allocated.
+ */
+int scsi_report_tpgs(struct scsi_device *sdev, unsigned char **buff,
+		     int *bufflen, bool ext_hdr, int timeout, int retries,
+		     const struct scsi_exec_args *args)
+{
+	blk_opf_t opf = REQ_OP_DRV_IN | REQ_FAILFAST_DEV |
+			REQ_FAILFAST_TRANSPORT | REQ_FAILFAST_DRIVER;
+	u8 cdb[MAX_COMMAND_SIZE] = { };
+	int ret, len;
+
+	cdb[0] = MAINTENANCE_IN;
+	cdb[1] = MI_REPORT_TARGET_PGS;
+	if (ext_hdr)
+		cdb[1] |= MI_EXT_HDR_PARAM_FMT;
+
+ retry:
+	put_unaligned_be32(*bufflen, &cdb[6]);
+	ret = scsi_execute_cmd(sdev, cdb, opf, *buff, *bufflen, timeout,
+			       retries, args);
+	if (ret)
+		return ret;
+
+	len = get_unaligned_be32(&(*buff)[0]) + 4;
+	if (len > *bufflen) {
+		/* Resubmit with the correct length */
+		kfree(*buff);
+		*bufflen = len;
+		*buff = kmalloc(len, GFP_KERNEL);
+		if (!*buff)
+			return -ENOMEM;
+		goto retry;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(scsi_report_tpgs);
+
 /**
  * scsi_set_sense - build sense data for a command
  * @scmd:	scsi command for which the sense should be formatted
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index e6fd6bc30644..691275db0677 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -583,6 +583,9 @@ extern void sdev_enable_disk_events(struct scsi_device *sdev);
 extern int scsi_vpd_lun_id(struct scsi_device *, char *, size_t);
 extern int scsi_vpd_lun_serial(struct scsi_device *, char *, size_t);
 extern int scsi_vpd_tpg_id(struct scsi_device *, int *);
+int scsi_report_tpgs(struct scsi_device *sdev, unsigned char **buff,
+		     int *bufflen, bool ext_hdr, int timeout, int retries,
+		     const struct scsi_exec_args *args);
 
 #ifdef CONFIG_PM
 extern int scsi_autopm_get_device(struct scsi_device *);
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index 9c7d31aa6bae..6826f2d74407 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -145,6 +145,9 @@
 #define MI_MANAGEMENT_PROTOCOL_IN 0x10
 /* value for MI_REPORT_TARGET_PGS ext header */
 #define MI_EXT_HDR_PARAM_FMT  0x20
+/* format field in byte 4 of the REPORT TARGET PORT GROUPS parameter data */
+#define RTPG_FMT_MASK         0x70
+#define RTPG_FMT_EXT_HDR      0x10
 /* values for maintenance out */
 #define MO_SET_IDENTIFYING_INFORMATION 0x06
 #define MO_SET_TARGET_PGS     0x0a
-- 
2.56.0

[PATCH v2 2/4] scsi: core: Read the ALUA access state when a device is scanned

[Brian Bunker]

sdev->access_state is only set by a device handler, and the ALUA
device handler reads it with an RTPG it queues when it attaches, which
normally completes after the device has been registered and matched to
a driver. The SCSI core therefore has no ALUA state for a device while
it decides whether to bind an upper-level driver to it.

For a device that reports TPGS support, send REPORT TARGET PORT GROUPS
from scsi_add_lun(), after the VPD pages have been read, and set
sdev->access_state from the descriptor of the target port group of
this path. UNIT ATTENTIONs, which a target typically reports first
after a login or an ALUA state change, are retried, and the command is
resent without the extended header if the target rejects it. If the
command fails, e.g. with LOGICAL UNIT NOT SUPPORTED because no LU is
mapped at the LUN, sdev->access_state is left as it is.

The command uses the same timeout as the INQUIRY of the scan. The ALUA
device handler keeps updating sdev->access_state afterwards as before.

Suggested-by: Hannes Reinecke <hare@kernel.org>
Co-developed-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Brian Bunker <brian@purestorage.com>
---
 drivers/scsi/scsi_scan.c | 94 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 8aae12fd38f3..a21dee1d9e9e 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -861,6 +861,93 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	return 0;
 }
 
+/**
+ * scsi_read_access_state - Read the ALUA state of a device
+ * @sdev: SCSI device that supports target port groups
+ *
+ * Send REPORT TARGET PORT GROUPS and set sdev->access_state to the
+ * asymmetric access state that the device reports for the target port
+ * group of this path, as identified in VPD page 0x83. If the command
+ * fails, e.g. with LOGICAL UNIT NOT SUPPORTED because no LU is mapped at
+ * this LUN for this I_T nexus, sdev->access_state is left unchanged. The
+ * ALUA device handler, if attached, keeps the state up to date later.
+ */
+static void scsi_read_access_state(struct scsi_device *sdev)
+{
+	/*
+	 * Each UNIT ATTENTION reports a different condition queued for this
+	 * I_T nexus, e.g. a power on or reset after login followed by ALUA
+	 * state and INQUIRY data changes, so retry until the target answers
+	 * the command itself.
+	 */
+	struct scsi_failure failure_defs[] = {
+		{
+			.sense_key = UNIT_ATTENTION,
+			.sense_code = SCMD_FAILURE_SENSE_CODE_ANY,
+			.allowed = 5,
+			.result = SAM_STAT_CHECK_CONDITION,
+		},
+		{}
+	};
+	struct scsi_failures failures = {
+		.failure_definitions = failure_defs,
+	};
+	struct scsi_sense_hdr sshdr;
+	const struct scsi_exec_args exec_args = {
+		.sshdr = &sshdr,
+		.failures = &failures,
+	};
+	/* scsi_report_tpgs() replaces the buffer if this is too small */
+	int bufflen = 128;
+	int group_id, len, k, off;
+	unsigned char *buff, *desc;
+	bool ext_hdr = true;
+	int ret;
+
+	group_id = scsi_vpd_tpg_id(sdev, NULL);
+	if (group_id < 0)
+		return;
+
+	buff = kzalloc(bufflen, GFP_KERNEL);
+	if (!buff)
+		return;
+
+ retry:
+	ret = scsi_report_tpgs(sdev, &buff, &bufflen, ext_hdr,
+			       HZ / 2 + HZ * scsi_inq_timeout, 3, &exec_args);
+	if (ret) {
+		/*
+		 * Some targets reject the extended header format with
+		 * ILLEGAL REQUEST (see alua_rtpg()), so retry without it,
+		 * unless the reason is that no LU is mapped at this LUN.
+		 */
+		if (ret > 0 && ext_hdr && scsi_sense_valid(&sshdr) &&
+		    sshdr.sense_key == ILLEGAL_REQUEST &&
+		    sshdr.sense_code != LU_NOT_SUPPORTED) {
+			ext_hdr = false;
+			goto retry;
+		}
+		goto out;
+	}
+
+	len = get_unaligned_be32(&buff[0]) + 4;
+	if ((buff[4] & RTPG_FMT_MASK) == RTPG_FMT_EXT_HDR)
+		k = 8;
+	else
+		k = 4;
+
+	for (desc = buff + k; k + 8 <= len; k += off, desc += off) {
+		off = 8 + (desc[7] * 4);
+		if (get_unaligned_be16(&desc[2]) == group_id) {
+			sdev->access_state = desc[0];
+			break;
+		}
+	}
+
+ out:
+	kfree(buff);
+}
+
 /**
  * scsi_add_lun - allocate and fully initialize a scsi_device
  * @sdev:	holds information to be stored in the new scsi_device
@@ -1126,6 +1213,13 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 
 	scsi_cdl_check(sdev);
 
+	/*
+	 * Read the ALUA state before the device is registered, so that
+	 * scsi_bus_match() can use it. This needs VPD page 0x83.
+	 */
+	if (scsi_device_tpgs(sdev))
+		scsi_read_access_state(sdev);
+
 	sdev->max_queue_depth = sdev->queue_depth;
 	WARN_ON_ONCE(sdev->max_queue_depth > sdev->budget_map.depth);
 
-- 
2.56.0

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

[Brian Bunker]

A new port group starts out active/optimized until the first RTPG of
the handler completes, so alua_prep_fn() lets I/O through to a path
that is actually unavailable. For a device whose driver is bound
straight away, the first I/O, typically the partition table read, then
reaches the target and fails with LOGICAL UNIT NOT ACCESSIBLE, TARGET
PORT IN UNAVAILABLE STATE, logging an I/O error.

The SCSI core now reads sdev->access_state when the device is scanned,
before the handler attaches. Start a new port group in that state. If
the core did not read one, sdev->access_state is zero, which is
active/optimized, so nothing changes for those devices.

Co-developed-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Brian Bunker <brian@purestorage.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 06eb35cc9d7c..15471e7bf19a 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);
 	pg->valid_states = TPGS_SUPPORT_ALL;
 	if (optimize_stpg)
 		pg->flags |= ALUA_OPTIMIZE_STPG;
-- 
2.56.0

[PATCH v2 4/4] scsi: core: Match ULDs to logical units in the ALUA unavailable state

[Brian Bunker]

scsi_bus_match() only matches devices with peripheral qualifier 000b,
so a LU that is in the ALUA unavailable state on a target port when
the host scans it never gets an upper-level driver on that path. When
the target port becomes available again, nothing binds sd and the path
stays unusable until the device is deleted and rescanned. A host that
was already running when the path became unavailable keeps sd bound,
so the result depends on when the host scanned.

Peripheral qualifier 001b cannot be matched on its own: an unmapped
LUN reports it as well, and some targets report every possible LUN in
REPORT LUNS. Binding sd to all of those would create disks with no
backing LU.

The SCSI core now reads the access state of a device that supports
target port groups with REPORT TARGET PORT GROUPS before the device is
registered, and an unmapped LUN fails that command. SPC-4 requires a LU
in the unavailable state to report PQ 001b, and the command shows that
the LU is mapped, so record in the device whether it reported PQ 001b
and its access state could be read, and match such a device. sd then
binds to the unavailable path like it does to a standby path, the path
stays in the multipath map as a failed path, and the existing UNIT
ATTENTION handling brings it back when the target port becomes
available.

The decision is made once, at scan time, and not from access_state when
the match is made. access_state follows the ALUA state, but the
peripheral qualifier it relates to does not change after the scan. If
the match depended on access_state, a device scanned in the unavailable
state would stop matching as soon as its path became available, e.g.
when the upper-level driver module is loaded later, or the driver is
rebound, or the ALUA device handler updates access_state before the
driver is probed. It also does not depend on the state that the command
returns, which can already be available if the path recovered between
the INQUIRY and the command.

This is the same situation as a path in the standby state, to which sd
already binds: SPC-4 requires READ CAPACITY in neither state. The
unavailable state does not require MODE SENSE either, which the standby
state does.

The cached INQUIRY data in sysfs keeps the peripheral qualifier
reported at scan time, like the rest of the INQUIRY data; access_state
shows the current ALUA state.

Co-developed-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Krishna Kant <krishna.kant@purestorage.com>
Signed-off-by: Brian Bunker <brian@purestorage.com>
---
 drivers/scsi/scsi_scan.c   | 26 +++++++++++++++++++++-----
 drivers/scsi/scsi_sysfs.c  | 14 +++++++++++++-
 include/scsi/scsi_device.h |  6 ++++++
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index a21dee1d9e9e..b78b257b4b24 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -871,8 +871,10 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result,
  * fails, e.g. with LOGICAL UNIT NOT SUPPORTED because no LU is mapped at
  * this LUN for this I_T nexus, sdev->access_state is left unchanged. The
  * ALUA device handler, if attached, keeps the state up to date later.
+ *
+ * Returns true if the state was read.
  */
-static void scsi_read_access_state(struct scsi_device *sdev)
+static bool scsi_read_access_state(struct scsi_device *sdev)
 {
 	/*
 	 * Each UNIT ATTENTION reports a different condition queued for this
@@ -902,15 +904,16 @@ static void scsi_read_access_state(struct scsi_device *sdev)
 	int group_id, len, k, off;
 	unsigned char *buff, *desc;
 	bool ext_hdr = true;
+	bool found = false;
 	int ret;
 
 	group_id = scsi_vpd_tpg_id(sdev, NULL);
 	if (group_id < 0)
-		return;
+		return false;
 
 	buff = kzalloc(bufflen, GFP_KERNEL);
 	if (!buff)
-		return;
+		return false;
 
  retry:
 	ret = scsi_report_tpgs(sdev, &buff, &bufflen, ext_hdr,
@@ -940,12 +943,14 @@ static void scsi_read_access_state(struct scsi_device *sdev)
 		off = 8 + (desc[7] * 4);
 		if (get_unaligned_be16(&desc[2]) == group_id) {
 			sdev->access_state = desc[0];
+			found = true;
 			break;
 		}
 	}
 
  out:
 	kfree(buff);
+	return found;
 }
 
 /**
@@ -1217,8 +1222,19 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
 	 * Read the ALUA state before the device is registered, so that
 	 * scsi_bus_match() can use it. This needs VPD page 0x83.
 	 */
-	if (scsi_device_tpgs(sdev))
-		scsi_read_access_state(sdev);
+	if (scsi_device_tpgs(sdev)) {
+		bool read = scsi_read_access_state(sdev);
+
+		/*
+		 * SPC-4 requires PQ 001b in the ALUA unavailable state, and
+		 * an unmapped LUN fails the command, so take this to be a
+		 * mapped LU that is unavailable. Decide once, like the
+		 * peripheral qualifier itself: access_state changes when the
+		 * ALUA state does.
+		 */
+		sdev->match_unavailable =
+			read && sdev->inq_periph_qual == SCSI_INQ_PQ_NOT_CON;
+	}
 
 	sdev->max_queue_depth = sdev->queue_depth;
 	WARN_ON_ONCE(sdev->max_queue_depth > sdev->budget_map.depth);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index fe54792ed69e..4866b5f35aed 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -539,7 +539,19 @@ static int scsi_bus_match(struct device *dev, const struct device_driver *gendrv
 	sdp = to_scsi_device(dev);
 	if (sdp->no_uld_attach)
 		return 0;
-	return (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)? 1: 0;
+	if (sdp->inq_periph_qual == SCSI_INQ_PQ_CON)
+		return 1;
+	/*
+	 * Peripheral qualifier 001b is also what a LU reports while it is in
+	 * the ALUA unavailable state on this target port (SPC-4 5.15.2.4.5).
+	 * scsi_add_lun() reads that state with REPORT TARGET PORT GROUPS
+	 * before the device is registered and records whether to match it.
+	 * A LUN that is not mapped reports 001b too, but fails REPORT TARGET
+	 * PORT GROUPS and so is not matched. The decision is not made from
+	 * access_state here, as that changes with the ALUA state while the
+	 * peripheral qualifier does not.
+	 */
+	return sdp->match_unavailable;
 }
 
 static int scsi_bus_uevent(const struct device *dev, struct kobj_uevent_env *env)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 691275db0677..b9540a412ae8 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -196,6 +196,12 @@ struct scsi_device {
 	 */
 	unsigned is_ata:1;
 
+	/*
+	 * Set if a ULD should be matched to a PQ 001b device in the ALUA
+	 * unavailable state.
+	 */
+	unsigned match_unavailable:1;
+
 	unsigned removable:1;
 	unsigned changed:1;	/* Data invalid due to media change */
 	unsigned busy:1;	/* Used to prevent races */
-- 
2.56.0

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

[sashiko-bot]

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