Skip to contentSkip to conversation

ALUA device handler update, part 1

PATCH Applied Hannes Reinecke 3 participants 30 messages Dec 1, 2015 – Dec 2, 2015 SCSI

lore
Share & tools
lore
#0 lore raw

v2 cover ALUA device handler update, part 1

Hi all,

here's the first part of my ALUA device handler update. It's mainly bugfixes and minor improvements; the two important things are the addition of VPD parsing functions scsi_vpd_lun_id() and scsi_vpd_tpg_id().
This series has been split off from the original 'Asynchronous ALUA' patchset, as these bits are pretty uncontroversial and have a good chance of being merged reasonably soon.

The second part of the ALUA device handler update will be posted (and, doubtlessly, discussed) once this part is in.

Patchset is relative to 4.5/scsi-queue by mkp.
The entire tree can be found at

kernel.org:/pub/scm/linux/kernel/git/hare/scsi-devel.git branch alua.v7

As usual, comments and reviews are welcome.

Changes to v1:
- Update to 4.5/scsi-queue
- Rename 'device_id' to 'wwid'

Hannes Reinecke (18):
scsi_dh: move 'dh_state' sysfs attribute to generic code
scsi: ignore errors from scsi_dh_add_device()
scsi_dh_alua: Disable ALUA handling for non-disk devices
scsi_dh_alua: Use vpd_pg83 information
scsi_dh_alua: improved logging
scsi_dh_alua: sanitze sense code handling
scsi_dh_alua: use standard logging functions
scsi_dh_alua: return standard SCSI return codes in submit_rtpg
scsi_dh_alua: fixup description of stpg_endio()
scsi: remove scsi_show_sense_hdr()
scsi_dh_alua: use flag for RTPG extended header
scsi_dh_alua: use unaligned access macros
scsi_dh_alua: rework alua_check_tpgs() to return the tpgs mode
scsi_dh_alua: simplify sense code handling
scsi: Add scsi_vpd_lun_id()
scsi: export 'wwid' to sysfs
scsi: Add scsi_vpd_tpg_id()
scsi_dh_alua: use scsi_vpd_tpg_id()

 drivers/scsi/device_handler/scsi_dh_alua.c | 294 ++++++++++-------------------
 drivers/scsi/scsi_dh.c                     |  72 +------
 drivers/scsi/scsi_lib.c                    | 188 ++++++++++++++++++
 drivers/scsi/scsi_priv.h                   |   3 +-
 drivers/scsi/scsi_sysfs.c                  |  82 +++++++-
 include/scsi/scsi_dbg.h                    |   2 -
 include/scsi/scsi_device.h                 |   2 +
 7 files changed, 374 insertions(+), 269 deletions(-)
Signature · 1 line
1.8.5.6
#1 lore raw

PATCH v2 1/18 scsi_dh: move 'dh_state' sysfs attribute to generic code

As scsi_dh.c is now always compiled in we should be moving the 'dh_state' attribute to the generic code.

---
 drivers/scsi/scsi_dh.c    | 72 +----------------------------------------------
 drivers/scsi/scsi_priv.h  |  3 +-
 drivers/scsi/scsi_sysfs.c | 58 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+), 73 deletions(-)
drivers/scsi/scsi_dh.c +1 -71
index e7649ed..54d446c 100644
@@ -153,76 +153,11 @@ static void scsi_dh_handler_detach(struct scsi_device *sdev)
 	module_put(sdev->handler->module);
 }
 
-/*
- * Functions for sysfs attribute 'dh_state'
- */
-static ssize_t
-store_dh_state(struct device *dev, struct device_attribute *attr,
-	       const char *buf, size_t count)
-{
-	struct scsi_device *sdev = to_scsi_device(dev);
-	struct scsi_device_handler *scsi_dh;
-	int err = -EINVAL;
-
-	if (sdev->sdev_state == SDEV_CANCEL ||
-	    sdev->sdev_state == SDEV_DEL)
-		return -ENODEV;
-
-	if (!sdev->handler) {
-		/*
-		 * Attach to a device handler
-		 */
-		scsi_dh = scsi_dh_lookup(buf);
-		if (!scsi_dh)
-			return err;
-		err = scsi_dh_handler_attach(sdev, scsi_dh);
-	} else {
-		if (!strncmp(buf, "detach", 6)) {
-			/*
-			 * Detach from a device handler
-			 */
-			sdev_printk(KERN_WARNING, sdev,
-				    "can't detach handler %s.\n",
-				    sdev->handler->name);
-			err = -EINVAL;
-		} else if (!strncmp(buf, "activate", 8)) {
-			/*
-			 * Activate a device handler
-			 */
-			if (sdev->handler->activate)
-				err = sdev->handler->activate(sdev, NULL, NULL);
-			else
-				err = 0;
-		}
-	}
-
-	return err<0?err:count;
-}
-
-static ssize_t
-show_dh_state(struct device *dev, struct device_attribute *attr, char *buf)
-{
-	struct scsi_device *sdev = to_scsi_device(dev);
-
-	if (!sdev->handler)
-		return snprintf(buf, 20, "detached\n");
-
-	return snprintf(buf, 20, "%s\n", sdev->handler->name);
-}
-
-static struct device_attribute scsi_dh_state_attr =
-	__ATTR(dh_state, S_IRUGO | S_IWUSR, show_dh_state,
-	       store_dh_state);
-
 int scsi_dh_add_device(struct scsi_device *sdev)
 {
 	struct scsi_device_handler *devinfo = NULL;
 	const char *drv;
-	int err;
-
-	err = device_create_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
-	if (err)
-		return err;
+	int err = 0;
 
 	drv = scsi_dh_find_driver(sdev);
 	if (drv)
@@ -238,11 +173,6 @@ void scsi_dh_release_device(struct scsi_device *sdev)
 		scsi_dh_handler_detach(sdev);
 }
 
-void scsi_dh_remove_device(struct scsi_device *sdev)
-{
-	device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
-}
-
 /*
  * scsi_register_device_handler - register a device handler personality
  *      module.
drivers/scsi/scsi_priv.h +1 -2
index 4d01cdb1..27b4d0a 100644
@@ -174,12 +174,11 @@ extern struct async_domain scsi_sd_probe_domain;
 #ifdef CONFIG_SCSI_DH
 int scsi_dh_add_device(struct scsi_device *sdev);
 void scsi_dh_release_device(struct scsi_device *sdev);
-void scsi_dh_remove_device(struct scsi_device *sdev);
 #else
 static inline int scsi_dh_add_device(struct scsi_device *sdev) { return 0; }
 static inline void scsi_dh_release_device(struct scsi_device *sdev) { }
-static inline void scsi_dh_remove_device(struct scsi_device *sdev) { }
 #endif
+static inline void scsi_dh_remove_device(struct scsi_device *sdev) { }
 
 /* 
  * internal scsi timeout functions: for use by mid-layer and transport
drivers/scsi/scsi_sysfs.c +58 -0

76 lines not on this page · raw message

Signature · 1 line
1.8.5.6
#2 lore raw

PATCH v2 2/18 scsi: ignore errors from scsi_dh_add_device()

In reply to Hannes Reinecke

device handler initialisation might fail due to a number of reasons. But as device_handlers are optional this shouldn't cause us to disable the device entirely.
So just ignore errors from scsi_dh_add_device().

---
 drivers/scsi/scsi_sysfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/scsi/scsi_sysfs.c +4 -3
index fc3cd26..d015374 100644
@@ -1120,11 +1120,12 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 	}
 
 	error = scsi_dh_add_device(sdev);
-	if (error) {
+	if (error)
+		/*
+		 * device_handler is optional, so any error can be ignored
+		 */
 		sdev_printk(KERN_INFO, sdev,
 				"failed to add device handler: %d\n", error);
-		return error;
-	}
 
 	device_enable_async_suspend(&sdev->sdev_dev);
 	error = device_add(&sdev->sdev_dev);
Signature · 1 line
1.8.5.6
#3 lore raw

PATCH v2 3/18 scsi_dh_alua: Disable ALUA handling for non-disk devices

In reply to Hannes Reinecke

Non-disk devices might support ALUA, but the firmware implementation is untested and frequently broken. As we're don't actually need it disable ALUA support for non-disk device for now.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
drivers/scsi/device_handler/scsi_dh_alua.c +12 -0
index cc2773b..7d01ef0 100644
@@ -320,6 +320,18 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h)
 {
 	int err = SCSI_DH_OK;
 
+	/*
+	 * ALUA support for non-disk devices is fraught with
+	 * difficulties, so disable it for now.
+	 */
+	if (sdev->type != TYPE_DISK) {
+		h->tpgs = TPGS_MODE_NONE;
+		sdev_printk(KERN_INFO, sdev,
+			    "%s: disable for non-disk devices\n",
+			    ALUA_DH_NAME);
+		return SCSI_DH_DEV_UNSUPP;
+	}
+
 	h->tpgs = scsi_device_tpgs(sdev);
 	switch (h->tpgs) {
 	case TPGS_MODE_EXPLICIT|TPGS_MODE_IMPLICIT:
Signature · 1 line
1.8.5.6
Reviewed-by on v2 3/18
#4 lore raw

Re: [PATCH 03/18] scsi_dh_alua: Disable ALUA handling for non-disk devices

Quoted 35 lines · Hannes Reinecke

Non-disk devices might support ALUA, but the firmware implementation is untested and frequently broken. As we're don't actually need it disable ALUA support for non-disk device for now.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++++++++

 1 file changed, 12 insertions(+)
drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
index cc2773b..7d01ef0 100644
@@ -320,6 +320,18 @@ static int alua_check_tpgs(struct scsi_device *sdev,
 struct alua_dh_data *h)
  {
  	int err = SCSI_DH_OK;

drivers/scsi/device_handler/scsi_dh_alua.c +12 -0
+	/*
+	 * ALUA support for non-disk devices is fraught with
+	 * difficulties, so disable it for now.
+	 */
+	if (sdev->type != TYPE_DISK) {
+		h->tpgs = TPGS_MODE_NONE;
+		sdev_printk(KERN_INFO, sdev,
+			    "%s: disable for non-disk devices\n",
+			    ALUA_DH_NAME);
+		return SCSI_DH_DEV_UNSUPP;
+	}
+
Quoted 3 lines

  h->tpgs = scsi_device_tpgs(sdev); switch (h->tpgs) {
  case TPGS_MODE_EXPLICIT|TPGS_MODE_IMPLICIT:

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

PATCH v2 4/18 scsi_dh_alua: Use vpd_pg83 information

In reply to Hannes Reinecke

The SCSI device now has the VPD page 0x83 information attached, so there is no need to query it again.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 87 +++++++-----------------------
 1 file changed, 18 insertions(+), 69 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +18 -69
index 7d01ef0..9b3b2f7 100644
@@ -131,43 +131,6 @@ static struct request *get_alua_req(struct scsi_device *sdev,
 }
 
 /*
- * submit_vpd_inquiry - Issue an INQUIRY VPD page 0x83 command
- * @sdev: sdev the command should be sent to
- */
-static int submit_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
-{
-	struct request *rq;
-	int err = SCSI_DH_RES_TEMP_UNAVAIL;
-
-	rq = get_alua_req(sdev, h->buff, h->bufflen, READ);
-	if (!rq)
-		goto done;
-
-	/* Prepare the command. */
-	rq->cmd[0] = INQUIRY;
-	rq->cmd[1] = 1;
-	rq->cmd[2] = 0x83;
-	rq->cmd[4] = h->bufflen;
-	rq->cmd_len = COMMAND_SIZE(INQUIRY);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
-
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: evpd inquiry failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
-		h->senselen = rq->sense_len;
-		err = SCSI_DH_IO;
-	}
-	blk_put_request(rq);
-done:
-	return err;
-}
-
-/*
  * submit_rtpg - Issue a REPORT TARGET GROUP STATES command
  * @sdev: sdev the command should be sent to
  */
@@ -359,43 +322,29 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h)
 }
 
 /*
- * alua_vpd_inquiry - Evaluate INQUIRY vpd page 0x83
+ * alua_check_vpd - Evaluate INQUIRY vpd page 0x83
  * @sdev: device to be checked
  *
  * Extract the relative target port and the target port group
  * descriptor from the list of identificators.
  */
-static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
+static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 {
-	int len;
-	unsigned err;
 	unsigned char *d;
+	unsigned char __rcu *vpd_pg83;
 
- retry:
-	err = submit_vpd_inquiry(sdev, h);
-
-	if (err != SCSI_DH_OK)
-		return err;
-
-	/* Check if vpd page exceeds initial buffer */
-	len = (h->buff[2] << 8) + h->buff[3] + 4;
-	if (len > h->bufflen) {
-		/* Resubmit with the correct length */
-		if (realloc_buffer(h, len)) {
-			sdev_printk(KERN_WARNING, sdev,
-				    "%s: kmalloc buffer failed\n",
-				    ALUA_DH_NAME);
-			/* Temporary failure, bypass */
-			return SCSI_DH_DEV_TEMP_BUSY;
-		}
-		goto retry;
+	rcu_read_lock();
+	if (!rcu_dereference(sdev->vpd_pg83)){
+		rcu_read_unlock();
+		return SCSI_DH_DEV_UNSUPP;
 	}
 
 	/*
-	 * Now look for the correct descriptor.
+	 * Look for the correct descriptor.
 	 */
-	d = h->buff + 4;
-	while (d < h->buff + len) {
+	vpd_pg83 = rcu_dereference(sdev->vpd_pg83);
+	d = vpd_pg83 + 4;
+	while (d < vpd_pg83 + sdev->vpd_pg83_len) {
 		switch (d[1] & 0xf) {
 		case 0x4:
 			/* Relative target port */
@@ -410,6 +359,7 @@ static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
 		}
 		d += d[3] + 4;
 	}
+	rcu_read_unlock();
 
 	if (h->group_id == -1) {
 		/*
@@ -422,14 +372,13 @@ static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
 			    ALUA_DH_NAME);
 		h->state = TPGS_STATE_OPTIMIZED;
 		h->tpgs = TPGS_MODE_NONE;
-		err = SCSI_DH_DEV_UNSUPP;
-	} else {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: port group %02x rel port %02x\n",
-			    ALUA_DH_NAME, h->group_id, h->rel_port);
+		return SCSI_DH_DEV_UNSUPP;
 	}
+	sdev_printk(KERN_INFO, sdev,
+		    "%s: port group %02x rel port %02x\n",
+		    ALUA_DH_NAME, h->group_id, h->rel_port);
 
-	return err;
+	return 0;
 }
 
 static char print_alua_state(int state)
@@ -692,7 +641,7 @@ static int alua_initialize(struct scsi_device *sdev, struct alua_dh_data *h)
 	if (err != SCSI_DH_OK)
 		goto out;
 
-	err = alua_vpd_inquiry(sdev, h);
+	err = alua_check_vpd(sdev, h);
 	if (err != SCSI_DH_OK)
 		goto out;
 
Signature · 1 line
1.8.5.6
Reviewed-by on v2 4/18
#6 lore raw

Re: [PATCH 04/18] scsi_dh_alua: Use vpd_pg83 information

Quoted 156 lines · Hannes Reinecke

The SCSI device now has the VPD page 0x83 information attached, so there is no need to query it again.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 87 +++++++--------------------- --

 1 file changed, 18 insertions(+), 69 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
index 7d01ef0..9b3b2f7 100644
@@ -131,43 +131,6 @@ static struct request *get_alua_req(struct scsi_device
 *sdev,
  }


 /*
- * submit_vpd_inquiry - Issue an INQUIRY VPD page 0x83 command
- * @sdev: sdev the command should be sent to
- */
-static int submit_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)

drivers/scsi/device_handler/scsi_dh_alua.c +0 -33
-{
-	struct request *rq;
-	int err = SCSI_DH_RES_TEMP_UNAVAIL;
-
-	rq = get_alua_req(sdev, h->buff, h->bufflen, READ);
-	if (!rq)
-		goto done;
-
-	/* Prepare the command. */
-	rq->cmd[0] = INQUIRY;
-	rq->cmd[1] = 1;
-	rq->cmd[2] = 0x83;
-	rq->cmd[4] = h->bufflen;
-	rq->cmd_len = COMMAND_SIZE(INQUIRY);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
-
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: evpd inquiry failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
-		h->senselen = rq->sense_len;
-		err = SCSI_DH_IO;
-	}
-	blk_put_request(rq);
-done:
-	return err;
-}
-
-/*

  * submit_rtpg - Issue a REPORT TARGET GROUP STATES command
  * @sdev: sdev the command should be sent to
  */

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -359,43 +322,29 @@ static int alua_check_tpgs(struct scsi_device *sdev,
 struct alua_dh_data *h)
  }


 /*
- * alua_vpd_inquiry - Evaluate INQUIRY vpd page 0x83
+ * alua_check_vpd - Evaluate INQUIRY vpd page 0x83
  * @sdev: device to be checked
  *
  * Extract the relative target port and the target port group
  * descriptor from the list of identificators.
  */
-static int alua_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
+static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h) {

drivers/scsi/device_handler/scsi_dh_alua.c +0 -2
-	int len;
-	unsigned err;

  unsigned char *d;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -0
+	unsigned char __rcu *vpd_pg83;


- retry:

drivers/scsi/device_handler/scsi_dh_alua.c +4 -17
-	err = submit_vpd_inquiry(sdev, h);
-
-	if (err != SCSI_DH_OK)
-		return err;
-
-	/* Check if vpd page exceeds initial buffer */
-	len = (h->buff[2] << 8) + h->buff[3] + 4;
-	if (len > h->bufflen) {
-		/* Resubmit with the correct length */
-		if (realloc_buffer(h, len)) {
-			sdev_printk(KERN_WARNING, sdev,
-				    "%s: kmalloc buffer failed\n",
-				    ALUA_DH_NAME);
-			/* Temporary failure, bypass */
-			return SCSI_DH_DEV_TEMP_BUSY;
-		}
-		goto retry;
+	rcu_read_lock();
+	if (!rcu_dereference(sdev->vpd_pg83)){
+		rcu_read_unlock();
+		return SCSI_DH_DEV_UNSUPP;

  }

  /*

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-	 * Now look for the correct descriptor.
+	 * Look for the correct descriptor.

   */

drivers/scsi/device_handler/scsi_dh_alua.c +3 -2
-	d = h->buff + 4;
-	while (d < h->buff + len) {
+	vpd_pg83 = rcu_dereference(sdev->vpd_pg83);
+	d = vpd_pg83 + 4;
+	while (d < vpd_pg83 + sdev->vpd_pg83_len) {

  switch (d[1] & 0xf) {
  case 0x4:
  /* Relative target port */

drivers/scsi/device_handler/scsi_dh_alua.c +1 -0
@@ -410,6 +359,7 @@ static int alua_vpd_inquiry(struct scsi_device *sdev,
 struct alua_dh_data *h)
  		}
  		d += d[3] + 4;
  	}
+	rcu_read_unlock();


  if (h->group_id == -1) {
  /*

drivers/scsi/device_handler/scsi_dh_alua.c +4 -5
@@ -422,14 +372,13 @@ static int alua_vpd_inquiry(struct scsi_device *sdev,
 struct alua_dh_data *h)
  			    ALUA_DH_NAME);
  		h->state = TPGS_STATE_OPTIMIZED;
  		h->tpgs = TPGS_MODE_NONE;
-		err = SCSI_DH_DEV_UNSUPP;
-	} else {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: port group %02x rel port %02x\n",
-			    ALUA_DH_NAME, h->group_id, h->rel_port);
+		return SCSI_DH_DEV_UNSUPP;
  	}
+	sdev_printk(KERN_INFO, sdev,
+		    "%s: port group %02x rel port %02x\n",
+		    ALUA_DH_NAME, h->group_id, h->rel_port);

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-	return err;
+	return 0;

 }

 static char print_alua_state(int state)

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -692,7 +641,7 @@ static int alua_initialize(struct scsi_device *sdev,
 struct alua_dh_data *h)
  	if (err != SCSI_DH_OK)
  		goto out;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-	err = alua_vpd_inquiry(sdev, h);
+	err = alua_check_vpd(sdev, h);
Quoted 3 lines

  if (err != SCSI_DH_OK)
  goto out;

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#7 lore raw

PATCH v2 5/18 scsi_dh_alua: improved logging

In reply to Hannes Reinecke

Issue different logging messages if ALUA is not supported or the TPGS setting is invalid.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +8 -2
index 9b3b2f7..c63f304 100644
@@ -310,12 +310,18 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h)
 		sdev_printk(KERN_INFO, sdev, "%s: supports implicit TPGS\n",
 			    ALUA_DH_NAME);
 		break;
-	default:
-		h->tpgs = TPGS_MODE_NONE;
+	case TPGS_MODE_NONE:
 		sdev_printk(KERN_INFO, sdev, "%s: not supported\n",
 			    ALUA_DH_NAME);
 		err = SCSI_DH_DEV_UNSUPP;
 		break;
+	default:
+		sdev_printk(KERN_INFO, sdev,
+			    "%s: unsupported TPGS setting %d\n",
+			    ALUA_DH_NAME, h->tpgs);
+		h->tpgs = TPGS_MODE_NONE;
+		err = SCSI_DH_DEV_UNSUPP;
+		break;
 	}
 
 	return err;
Signature · 1 line
1.8.5.6
Reviewed-by on v2 5/18
#8 lore raw

Re: [PATCH 05/18] scsi_dh_alua: improved logging

Quoted 35 lines · Hannes Reinecke

Issue different logging messages if ALUA is not supported or the TPGS setting is invalid.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 10 ++++++++--

 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +8 -2
index 9b3b2f7..c63f304 100644
@@ -310,12 +310,18 @@ static int alua_check_tpgs(struct scsi_device *sdev,
 struct alua_dh_data *h)
  		sdev_printk(KERN_INFO, sdev, "%s: supports implicit TPGS\n",
  			    ALUA_DH_NAME);
  		break;
-	default:
-		h->tpgs = TPGS_MODE_NONE;
+	case TPGS_MODE_NONE:
  		sdev_printk(KERN_INFO, sdev, "%s: not supported\n",
  			    ALUA_DH_NAME);
  		err = SCSI_DH_DEV_UNSUPP;
  		break;
+	default:
+		sdev_printk(KERN_INFO, sdev,
+			    "%s: unsupported TPGS setting %d\n",
+			    ALUA_DH_NAME, h->tpgs);
+		h->tpgs = TPGS_MODE_NONE;
+		err = SCSI_DH_DEV_UNSUPP;
+		break;
  	}
Quoted 2 lines


  return err;

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#9 lore raw

PATCH v2 6/18 scsi_dh_alua: sanitze sense code handling

In reply to Hannes Reinecke

The only check for a valid sense code is calling scsi_normalize_sense() and check the return value. So drop the pointless checks and rely on scsi_normalize_sense() to figure out if the sense code is valid. With that we can also remove the 'senselen' field.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +7 -11
index c63f304..240a5dc 100644
@@ -73,7 +73,6 @@ struct alua_dh_data {
 	int			bufflen;
 	unsigned char		transition_tmo;
 	unsigned char		sense[SCSI_SENSE_BUFFERSIZE];
-	int			senselen;
 	struct scsi_device	*sdev;
 	activate_complete	callback_fn;
 	void			*callback_data;
@@ -158,14 +157,13 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h,
 
 	rq->sense = h->sense;
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
+	rq->sense_len = 0;
 
 	err = blk_execute_rq(rq->q, NULL, rq, 1);
 	if (err == -EIO) {
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: rtpg failed with %x\n",
 			    ALUA_DH_NAME, rq->errors);
-		h->senselen = rq->sense_len;
 		err = SCSI_DH_IO;
 	}
 	blk_put_request(rq);
@@ -194,9 +192,8 @@ static void stpg_endio(struct request *req, int error)
 		goto done;
 	}
 
-	if (req->sense_len > 0) {
-		err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
-					   &sense_hdr);
+	if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
+				 &sense_hdr)) {
 		if (!err) {
 			err = SCSI_DH_IO;
 			goto done;
@@ -265,7 +262,7 @@ static unsigned submit_stpg(struct alua_dh_data *h)
 
 	rq->sense = h->sense;
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
+	rq->sense_len = 0;
 	rq->end_io_data = h;
 
 	blk_execute_rq_nowait(rq->q, NULL, rq, 1, stpg_endio);
@@ -514,10 +511,9 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
  retry:
 	err = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
 
-	if (err == SCSI_DH_IO && h->senselen > 0) {
-		err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
-					   &sense_hdr);
-		if (!err)
+	if (err == SCSI_DH_IO) {
+		if (!scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
+					  &sense_hdr))
 			return SCSI_DH_IO;
 
 		/*
Signature · 1 line
1.8.5.6
#10 lore raw

PATCH v2 7/18 scsi_dh_alua: use standard logging functions

In reply to Hannes Reinecke

Use standard logging functions instead of hand-crafted ones.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +13 -14
index 240a5dc..b817963 100644
@@ -23,6 +23,7 @@
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <scsi/scsi.h>
+#include <scsi/scsi_dbg.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_dh.h>
 
@@ -194,19 +195,14 @@ static void stpg_endio(struct request *req, int error)
 
 	if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
 				 &sense_hdr)) {
-		if (!err) {
-			err = SCSI_DH_IO;
-			goto done;
-		}
 		err = alua_check_sense(h->sdev, &sense_hdr);
 		if (err == ADD_TO_MLQUEUE) {
 			err = SCSI_DH_RETRY;
 			goto done;
 		}
-		sdev_printk(KERN_INFO, h->sdev,
-			    "%s: stpg sense code: %02x/%02x/%02x\n",
-			    ALUA_DH_NAME, sense_hdr.sense_key,
-			    sense_hdr.asc, sense_hdr.ascq);
+		sdev_printk(KERN_INFO, h->sdev, "%s: stpg failed\n",
+			    ALUA_DH_NAME);
+		scsi_print_sense_hdr(h->sdev, ALUA_DH_NAME, &sense_hdr);
 		err = SCSI_DH_IO;
 	} else if (error)
 		err = SCSI_DH_IO;
@@ -532,13 +528,16 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 		}
 
 		err = alua_check_sense(sdev, &sense_hdr);
-		if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry))
+		if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) {
+			sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n",
+				    ALUA_DH_NAME);
+			scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
 			goto retry;
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: rtpg sense code %02x/%02x/%02x\n",
-			    ALUA_DH_NAME, sense_hdr.sense_key,
-			    sense_hdr.asc, sense_hdr.ascq);
-		err = SCSI_DH_IO;
+		}
+		sdev_printk(KERN_ERR, sdev, "%s: rtpg failed\n",
+			    ALUA_DH_NAME);
+		scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
+		return SCSI_DH_IO;
 	}
 	if (err != SCSI_DH_OK)
 		return err;
Signature · 1 line
1.8.5.6
#11 lore raw

PATCH v2 8/18 scsi_dh_alua: return standard SCSI return codes in submit_rtpg

In reply to Hannes Reinecke

Fixup submit_rtpg() to always return a standard SCSI return code.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 33 +++++++++++++++---------------
 1 file changed, 17 insertions(+), 16 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +17 -16
index b817963..50fe87c 100644
@@ -138,11 +138,13 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h,
 			    bool rtpg_ext_hdr_req)
 {
 	struct request *rq;
-	int err = SCSI_DH_RES_TEMP_UNAVAIL;
+	int err = 0;
 
 	rq = get_alua_req(sdev, h->buff, h->bufflen, READ);
-	if (!rq)
+	if (!rq) {
+		err = DRIVER_BUSY << 24;
 		goto done;
+	}
 
 	/* Prepare the command. */
 	rq->cmd[0] = MAINTENANCE_IN;
@@ -160,13 +162,9 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h,
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
 	rq->sense_len = 0;
 
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: rtpg failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
-		err = SCSI_DH_IO;
-	}
+	blk_execute_rq(rq->q, NULL, rq, 1);
+	if (rq->errors)
+		err = rq->errors;
 	blk_put_request(rq);
 done:
 	return err;
@@ -493,7 +491,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 	struct scsi_sense_hdr sense_hdr;
 	int len, k, off, valid_states = 0;
 	unsigned char *ucp;
-	unsigned err;
+	unsigned err, retval;
 	bool rtpg_ext_hdr_req = 1;
 	unsigned long expiry, interval = 0;
 	unsigned int tpg_desc_tbl_off;
@@ -505,12 +503,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 		expiry = round_jiffies_up(jiffies + h->transition_tmo * HZ);
 
  retry:
-	err = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
-
-	if (err == SCSI_DH_IO) {
+	retval = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
+	if (retval) {
 		if (!scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
-					  &sense_hdr))
+					  &sense_hdr)) {
+			sdev_printk(KERN_INFO, sdev,
+				    "%s: rtpg failed, result %d\n",
+				    ALUA_DH_NAME, retval);
+			if (driver_byte(retval) == DRIVER_BUSY)
+				return SCSI_DH_DEV_TEMP_BUSY;
 			return SCSI_DH_IO;
+		}
 
 		/*
 		 * submit_rtpg() has failed on existing arrays
@@ -539,8 +542,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 		scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
 		return SCSI_DH_IO;
 	}
-	if (err != SCSI_DH_OK)
-		return err;
 
 	len = (h->buff[0] << 24) + (h->buff[1] << 16) +
 		(h->buff[2] << 8) + h->buff[3] + 4;
Signature · 1 line
1.8.5.6
Reviewed-by on v2 8/18
#12 lore raw

Re: [PATCH 08/18] scsi_dh_alua: return standard SCSI return codes in submit_rtpg

Quoted 91 lines · Hannes Reinecke

Fixup submit_rtpg() to always return a standard SCSI return code.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 33 +++++++++++++++------------- --

 1 file changed, 17 insertions(+), 16 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
index b817963..50fe87c 100644
@@ -138,11 +138,13 @@ static unsigned submit_rtpg(struct scsi_device *sdev,
 struct alua_dh_data *h,
  			    bool rtpg_ext_hdr_req)
  {
  	struct request *rq;
-	int err = SCSI_DH_RES_TEMP_UNAVAIL;
+	int err = 0;


  rq = get_alua_req(sdev, h->buff, h->bufflen, READ);

drivers/scsi/device_handler/scsi_dh_alua.c +2 -1
-	if (!rq)
+	if (!rq) {
+		err = DRIVER_BUSY << 24;

  goto done;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -0
+	}


  /* Prepare the command. */
  rq->cmd[0] = MAINTENANCE_IN;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -160,13 +162,9 @@ static unsigned submit_rtpg(struct scsi_device *sdev,
 struct alua_dh_data *h,
  	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
  	rq->sense_len = 0;

drivers/scsi/device_handler/scsi_dh_alua.c +3 -7
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: rtpg failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
-		err = SCSI_DH_IO;
-	}
+	blk_execute_rq(rq->q, NULL, rq, 1);
+	if (rq->errors)
+		err = rq->errors;

  blk_put_request(rq);
 done:
  return err;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -493,7 +491,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  struct scsi_sense_hdr sense_hdr;
  int len, k, off, valid_states = 0; unsigned char *ucp;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-	unsigned err;
+	unsigned err, retval;

  bool rtpg_ext_hdr_req = 1;
  unsigned long expiry, interval = 0; unsigned int tpg_desc_tbl_off;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -505,12 +503,17 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  expiry = round_jiffies_up(jiffies + h->transition_tmo * HZ);
  retry:

drivers/scsi/device_handler/scsi_dh_alua.c +2 -3
-	err = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
-
-	if (err == SCSI_DH_IO) {
+	retval = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
+	if (retval) {

  if (!scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,

drivers/scsi/device_handler/scsi_dh_alua.c +6 -1
-					  &sense_hdr))
+					  &sense_hdr)) {
+			sdev_printk(KERN_INFO, sdev,
+				    "%s: rtpg failed, result %d\n",
+				    ALUA_DH_NAME, retval);
+			if (driver_byte(retval) == DRIVER_BUSY)
+				return SCSI_DH_DEV_TEMP_BUSY;

  return SCSI_DH_IO;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -0
+		}


  /*
   * submit_rtpg() has failed on existing arrays

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -539,8 +542,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr); return SCSI_DH_IO;
  }

drivers/scsi/device_handler/scsi_dh_alua.c +0 -2
-	if (err != SCSI_DH_OK)
-		return err;
Quoted 3 lines


  len = (h->buff[0] << 24) + (h->buff[1] << 16) + (h->buff[2] << 8) + h->buff[3] + 4;

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#13 lore raw

PATCH v2 9/18 scsi_dh_alua: fixup description of stpg_endio()

In reply to Hannes Reinecke

Fixup copy-and-paste error in the description of stpg_endio().

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +2 -4
index 50fe87c..ea626732 100644
@@ -171,13 +171,11 @@ done:
 }
 
 /*
- * alua_stpg - Evaluate SET TARGET GROUP STATES
+ * stpg_endio - Evaluate SET TARGET GROUP STATES
  * @sdev: the device to be evaluated
  * @state: the new target group state
  *
- * Send a SET TARGET GROUP STATES command to the device.
- * We only have to test here if we should resubmit the command;
- * any other error is assumed as a failure.
+ * Evaluate a SET TARGET GROUP STATES command response.
  */
 static void stpg_endio(struct request *req, int error)
 {
Signature · 1 line
1.8.5.6
Reviewed-by on v2 9/18
#14 lore raw

Re: [PATCH 09/18] scsi_dh_alua: fixup description of stpg_endio()

Quoted 22 lines · Hannes Reinecke

Fixup copy-and-paste error in the description of stpg_endio().

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 6 ++----

 1 file changed, 2 insertions(+), 4 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
index 50fe87c..ea626732 100644
@@ -171,13 +171,11 @@ done:
  }


 /*
- * alua_stpg - Evaluate SET TARGET GROUP STATES
+ * stpg_endio - Evaluate SET TARGET GROUP STATES
  * @sdev: the device to be evaluated
  * @state: the new target group state

Quoted 8 lines

  *
- * Send a SET TARGET GROUP STATES command to the device.
- * We only have to test here if we should resubmit the command;
- * any other error is assumed as a failure.
+ * Evaluate a SET TARGET GROUP STATES command response. */
 static void stpg_endio(struct request *req, int error) {

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#15 lore raw

PATCH v2 10/18 scsi: remove scsi_show_sense_hdr()

In reply to Hannes Reinecke

Last caller is gone, so remove it.

---
 include/scsi/scsi_dbg.h | 2 --
 1 file changed, 2 deletions(-)
include/scsi/scsi_dbg.h +0 -2
index f8170e9..56710e0 100644
@@ -12,8 +12,6 @@ extern size_t __scsi_format_command(char *, size_t,
 				   const unsigned char *, size_t);
 extern void scsi_show_extd_sense(const struct scsi_device *, const char *,
 				 unsigned char, unsigned char);
-extern void scsi_show_sense_hdr(const struct scsi_device *, const char *,
-				const struct scsi_sense_hdr *);
 extern void scsi_print_sense_hdr(const struct scsi_device *, const char *,
 				 const struct scsi_sense_hdr *);
 extern void scsi_print_sense(const struct scsi_cmnd *);
Signature · 1 line
1.8.5.6
Reviewed-by on v2 10/18
#16 lore raw

Re: [PATCH 10/18] scsi: remove scsi_show_sense_hdr()

Quoted 15 lines · Hannes Reinecke

Last caller is gone, so remove it.

---
 include/scsi/scsi_dbg.h | 2 --

 1 file changed, 2 deletions(-)
include/scsi/scsi_dbg.h +0 -0
index f8170e9..56710e0 100644

include/scsi/scsi_dbg.h extern size_t __scsi_format_command(char *, size_t, · line 17

Quoted 8 lines
include/scsi/scsi_dbg.h +0 -2
@@ -12,8 +12,6 @@ extern size_t __scsi_format_command(char *, size_t,
  				   const unsigned char *, size_t);
  extern void scsi_show_extd_sense(const struct scsi_device *, const char *,
  				 unsigned char, unsigned char);
-extern void scsi_show_sense_hdr(const struct scsi_device *, const char *,
-				const struct scsi_sense_hdr *);
  extern void scsi_print_sense_hdr(const struct scsi_device *, const char *,
  				 const struct scsi_sense_hdr *);
  extern void scsi_print_sense(const struct scsi_cmnd *);

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#17 lore raw

PATCH v2 11/18 scsi_dh_alua: use flag for RTPG extended header

In reply to Hannes Reinecke

We should be using a flag when RTPG extended header is not supported, that saves us sending RTPG twice for older arrays.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +7 -8
index ea626732..03f8190 100644
@@ -59,8 +59,9 @@
 #define ALUA_FAILOVER_TIMEOUT		60
 #define ALUA_FAILOVER_RETRIES		5
 
-/* flags passed from user level */
+/* device handler flags */
 #define ALUA_OPTIMIZE_STPG		1
+#define ALUA_RTPG_EXT_HDR_UNSUPP	2
 
 struct alua_dh_data {
 	int			group_id;
@@ -134,8 +135,7 @@ static struct request *get_alua_req(struct scsi_device *sdev,
  * submit_rtpg - Issue a REPORT TARGET GROUP STATES command
  * @sdev: sdev the command should be sent to
  */
-static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h,
-			    bool rtpg_ext_hdr_req)
+static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h)
 {
 	struct request *rq;
 	int err = 0;
@@ -148,7 +148,7 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h,
 
 	/* Prepare the command. */
 	rq->cmd[0] = MAINTENANCE_IN;
-	if (rtpg_ext_hdr_req)
+	if (!(h->flags & ALUA_RTPG_EXT_HDR_UNSUPP))
 		rq->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
 	else
 		rq->cmd[1] = MI_REPORT_TARGET_PGS;
@@ -490,7 +490,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 	int len, k, off, valid_states = 0;
 	unsigned char *ucp;
 	unsigned err, retval;
-	bool rtpg_ext_hdr_req = 1;
 	unsigned long expiry, interval = 0;
 	unsigned int tpg_desc_tbl_off;
 	unsigned char orig_transition_tmo;
@@ -501,7 +500,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 		expiry = round_jiffies_up(jiffies + h->transition_tmo * HZ);
 
  retry:
-	retval = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
+	retval = submit_rtpg(sdev, h);
 	if (retval) {
 		if (!scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
 					  &sense_hdr)) {
@@ -521,10 +520,10 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 		 * The retry without rtpg_ext_hdr_req set
 		 * handles this.
 		 */
-		if (rtpg_ext_hdr_req == 1 &&
+		if (!(h->flags & ALUA_RTPG_EXT_HDR_UNSUPP) &&
 		    sense_hdr.sense_key == ILLEGAL_REQUEST &&
 		    sense_hdr.asc == 0x24 && sense_hdr.ascq == 0) {
-			rtpg_ext_hdr_req = 0;
+			h->flags |= ALUA_RTPG_EXT_HDR_UNSUPP;
 			goto retry;
 		}
 
Signature · 1 line
1.8.5.6
Reviewed-by on v2 11/18
#18 lore raw

Re: [PATCH 11/18] scsi_dh_alua: use flag for RTPG extended header

Quoted 80 lines · Hannes Reinecke

We should be using a flag when RTPG extended header is not supported, that saves us sending RTPG twice for older arrays.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 15 +++++++--------

 1 file changed, 7 insertions(+), 8 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
index ea626732..03f8190 100644
@@ -59,8 +59,9 @@
  #define ALUA_FAILOVER_TIMEOUT		60
  #define ALUA_FAILOVER_RETRIES		5

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-/* flags passed from user level */
+/* device handler flags */

 #define ALUA_OPTIMIZE_STPG 1
+#define ALUA_RTPG_EXT_HDR_UNSUPP 2

 struct alua_dh_data {
  int group_id;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -2
@@ -134,8 +135,7 @@ static struct request *get_alua_req(struct scsi_device
 *sdev,
   * submit_rtpg - Issue a REPORT TARGET GROUP STATES command
   * @sdev: sdev the command should be sent to
   */
-static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data
 *h,
-			    bool rtpg_ext_hdr_req)
+static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data
 *h)

 {
  struct request *rq;
  int err = 0;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -148,7 +148,7 @@ static unsigned submit_rtpg(struct scsi_device *sdev,
 struct alua_dh_data *h,


  /* Prepare the command. */
  rq->cmd[0] = MAINTENANCE_IN;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-	if (rtpg_ext_hdr_req)
+	if (!(h->flags & ALUA_RTPG_EXT_HDR_UNSUPP))

  rq->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT; else
  rq->cmd[1] = MI_REPORT_TARGET_PGS;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -490,7 +490,6 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  int len, k, off, valid_states = 0;
  unsigned char *ucp;
  unsigned err, retval;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -1
-	bool rtpg_ext_hdr_req = 1;

  unsigned long expiry, interval = 0;
  unsigned int tpg_desc_tbl_off;
  unsigned char orig_transition_tmo;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -501,7 +500,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  expiry = round_jiffies_up(jiffies + h->transition_tmo * HZ);
  retry:

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-	retval = submit_rtpg(sdev, h, rtpg_ext_hdr_req);
+	retval = submit_rtpg(sdev, h);

  if (retval) {
  if (!scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE, &sense_hdr)) {

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -521,10 +520,10 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
   * The retry without rtpg_ext_hdr_req set
   * handles this.
   */

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-		if (rtpg_ext_hdr_req == 1 &&
+		if (!(h->flags & ALUA_RTPG_EXT_HDR_UNSUPP) &&

      sense_hdr.sense_key == ILLEGAL_REQUEST && sense_hdr.asc == 0x24 && sense_hdr.ascq == 0) {

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-			rtpg_ext_hdr_req = 0;
+			h->flags |= ALUA_RTPG_EXT_HDR_UNSUPP;
Quoted 3 lines

  goto retry;
  }

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#19 lore raw

PATCH v2 12/18 scsi_dh_alua: use unaligned access macros

In reply to Hannes Reinecke

Use 'get_unaligned_XX' and 'put_unaligned_XX' instead of open-coding it.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +8 -15
index 03f8190..d54f42e 100644
@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <asm/unaligned.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_eh.h>
@@ -152,10 +153,7 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h)
 		rq->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
 	else
 		rq->cmd[1] = MI_REPORT_TARGET_PGS;
-	rq->cmd[6] = (h->bufflen >> 24) & 0xff;
-	rq->cmd[7] = (h->bufflen >> 16) & 0xff;
-	rq->cmd[8] = (h->bufflen >>  8) & 0xff;
-	rq->cmd[9] = h->bufflen & 0xff;
+	put_unaligned_be32(h->bufflen, &rq->cmd[6]);
 	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_IN);
 
 	rq->sense = h->sense;
@@ -236,8 +234,7 @@ static unsigned submit_stpg(struct alua_dh_data *h)
 	/* Prepare the data buffer */
 	memset(h->buff, 0, stpg_len);
 	h->buff[4] = TPGS_STATE_OPTIMIZED & 0x0f;
-	h->buff[6] = (h->group_id >> 8) & 0xff;
-	h->buff[7] = h->group_id & 0xff;
+	put_unaligned_be16(h->group_id, &h->buff[6]);
 
 	rq = get_alua_req(sdev, h->buff, stpg_len, WRITE);
 	if (!rq)
@@ -246,10 +243,7 @@ static unsigned submit_stpg(struct alua_dh_data *h)
 	/* Prepare the command. */
 	rq->cmd[0] = MAINTENANCE_OUT;
 	rq->cmd[1] = MO_SET_TARGET_PGS;
-	rq->cmd[6] = (stpg_len >> 24) & 0xff;
-	rq->cmd[7] = (stpg_len >> 16) & 0xff;
-	rq->cmd[8] = (stpg_len >>  8) & 0xff;
-	rq->cmd[9] = stpg_len & 0xff;
+	put_unaligned_be32(stpg_len, &rq->cmd[6]);
 	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_OUT);
 
 	rq->sense = h->sense;
@@ -343,11 +337,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 		switch (d[1] & 0xf) {
 		case 0x4:
 			/* Relative target port */
-			h->rel_port = (d[6] << 8) + d[7];
+			h->rel_port = get_unaligned_be16(&d[6]);
 			break;
 		case 0x5:
 			/* Target port group */
-			h->group_id = (d[6] << 8) + d[7];
+			h->group_id = get_unaligned_be16(&d[6]);
 			break;
 		default:
 			break;
@@ -540,8 +534,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 		return SCSI_DH_IO;
 	}
 
-	len = (h->buff[0] << 24) + (h->buff[1] << 16) +
-		(h->buff[2] << 8) + h->buff[3] + 4;
+	len = get_unaligned_be32(&h->buff[0]) + 4;
 
 	if (len > h->bufflen) {
 		/* Resubmit with the correct length */
@@ -576,7 +569,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 	     k < len;
 	     k += off, ucp += off) {
 
-		if (h->group_id == (ucp[2] << 8) + ucp[3]) {
+		if (h->group_id == get_unaligned_be16(&ucp[2])) {
 			h->state = ucp[0] & 0x0f;
 			h->pref = ucp[0] >> 7;
 			valid_states = ucp[1];
Signature · 1 line
1.8.5.6
Reviewed-by on v2 12/18
#20 lore raw

Re: [PATCH 12/18] scsi_dh_alua: use unaligned access macros

Quoted 91 lines · Hannes Reinecke

Use 'get_unaligned_XX' and 'put_unaligned_XX' instead of open-coding it.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 23 ++++++++---------------

 1 file changed, 8 insertions(+), 15 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +1 -0
index 03f8190..d54f42e 100644
@@ -22,6 +22,7 @@
  #include <linux/slab.h>
  #include <linux/delay.h>
  #include <linux/module.h>
+#include <asm/unaligned.h>
  #include <scsi/scsi.h>
  #include <scsi/scsi_dbg.h>
  #include <scsi/scsi_eh.h>
@@ -152,10 +153,7 @@ static unsigned submit_rtpg(struct scsi_device *sdev,
 struct alua_dh_data *h)
  		rq->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;

  else
  rq->cmd[1] = MI_REPORT_TARGET_PGS;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -4
-	rq->cmd[6] = (h->bufflen >> 24) & 0xff;
-	rq->cmd[7] = (h->bufflen >> 16) & 0xff;
-	rq->cmd[8] = (h->bufflen >>  8) & 0xff;
-	rq->cmd[9] = h->bufflen & 0xff;
+	put_unaligned_be32(h->bufflen, &rq->cmd[6]);

  rq->cmd_len = COMMAND_SIZE(MAINTENANCE_IN);
  rq->sense = h->sense;

drivers/scsi/device_handler/scsi_dh_alua.c +1 -2
@@ -236,8 +234,7 @@ static unsigned submit_stpg(struct alua_dh_data *h)
  	/* Prepare the data buffer */
  	memset(h->buff, 0, stpg_len);
  	h->buff[4] = TPGS_STATE_OPTIMIZED & 0x0f;
-	h->buff[6] = (h->group_id >> 8) & 0xff;
-	h->buff[7] = h->group_id & 0xff;
+	put_unaligned_be16(h->group_id, &h->buff[6]);


  rq = get_alua_req(sdev, h->buff, stpg_len, WRITE); if (!rq)

drivers/scsi/device_handler/scsi_dh_alua.c +1 -4
@@ -246,10 +243,7 @@ static unsigned submit_stpg(struct alua_dh_data *h)
  	/* Prepare the command. */
  	rq->cmd[0] = MAINTENANCE_OUT;
  	rq->cmd[1] = MO_SET_TARGET_PGS;
-	rq->cmd[6] = (stpg_len >> 24) & 0xff;
-	rq->cmd[7] = (stpg_len >> 16) & 0xff;
-	rq->cmd[8] = (stpg_len >>  8) & 0xff;
-	rq->cmd[9] = stpg_len & 0xff;
+	put_unaligned_be32(stpg_len, &rq->cmd[6]);
  	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_OUT);


  rq->sense = h->sense;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -343,11 +337,11 @@ static int alua_check_vpd(struct scsi_device *sdev,
 struct alua_dh_data *h)
  		switch (d[1] & 0xf) {

  case 0x4:
  /* Relative target port */

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-			h->rel_port = (d[6] << 8) + d[7];
+			h->rel_port = get_unaligned_be16(&d[6]);

  break;
  case 0x5:
  /* Target port group */

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-			h->group_id = (d[6] << 8) + d[7];
+			h->group_id = get_unaligned_be16(&d[6]);

  break;
  default:
  break;

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -540,8 +534,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  return SCSI_DH_IO;
  }

drivers/scsi/device_handler/scsi_dh_alua.c +1 -2
-	len = (h->buff[0] << 24) + (h->buff[1] << 16) +
-		(h->buff[2] << 8) + h->buff[3] + 4;
+	len = get_unaligned_be32(&h->buff[0]) + 4;


  if (len > h->bufflen) {
  /* Resubmit with the correct length */

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -576,7 +569,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
       k < len;
       k += off, ucp += off) {

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1
-		if (h->group_id == (ucp[2] << 8) + ucp[3]) {
+		if (h->group_id == get_unaligned_be16(&ucp[2])) {
Quoted 3 lines

  h->state = ucp[0] & 0x0f;
  h->pref = ucp[0] >> 7;
  valid_states = ucp[1];

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#21 lore raw

PATCH v2 13/18 scsi_dh_alua: rework alua_check_tpgs() to return the tpgs mode

In reply to Hannes Reinecke

Instead of returning an error code in alua_check_tpgs() we should rather return the tpgs mode directly and have a cleaner syntax.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +11 -14
index d54f42e..c9751c9 100644
@@ -262,24 +262,23 @@ static unsigned submit_stpg(struct alua_dh_data *h)
  * Examine the TPGS setting of the sdev to find out if ALUA
  * is supported.
  */
-static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h)
+static int alua_check_tpgs(struct scsi_device *sdev)
 {
-	int err = SCSI_DH_OK;
+	int tpgs = TPGS_MODE_NONE;
 
 	/*
 	 * ALUA support for non-disk devices is fraught with
 	 * difficulties, so disable it for now.
 	 */
 	if (sdev->type != TYPE_DISK) {
-		h->tpgs = TPGS_MODE_NONE;
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: disable for non-disk devices\n",
 			    ALUA_DH_NAME);
-		return SCSI_DH_DEV_UNSUPP;
+		return tpgs;
 	}
 
-	h->tpgs = scsi_device_tpgs(sdev);
-	switch (h->tpgs) {
+	tpgs = scsi_device_tpgs(sdev);
+	switch (tpgs) {
 	case TPGS_MODE_EXPLICIT|TPGS_MODE_IMPLICIT:
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: supports implicit and explicit TPGS\n",
@@ -296,18 +295,16 @@ static int alua_check_tpgs(struct scsi_device *sdev, struct alua_dh_data *h)
 	case TPGS_MODE_NONE:
 		sdev_printk(KERN_INFO, sdev, "%s: not supported\n",
 			    ALUA_DH_NAME);
-		err = SCSI_DH_DEV_UNSUPP;
 		break;
 	default:
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: unsupported TPGS setting %d\n",
-			    ALUA_DH_NAME, h->tpgs);
-		h->tpgs = TPGS_MODE_NONE;
-		err = SCSI_DH_DEV_UNSUPP;
+			    ALUA_DH_NAME, tpgs);
+		tpgs = TPGS_MODE_NONE;
 		break;
 	}
 
-	return err;
+	return tpgs;
 }
 
 /*
@@ -627,10 +624,10 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
  */
 static int alua_initialize(struct scsi_device *sdev, struct alua_dh_data *h)
 {
-	int err;
+	int err = SCSI_DH_DEV_UNSUPP;
 
-	err = alua_check_tpgs(sdev, h);
-	if (err != SCSI_DH_OK)
+	h->tpgs = alua_check_tpgs(sdev);
+	if (h->tpgs == TPGS_MODE_NONE)
 		goto out;
 
 	err = alua_check_vpd(sdev, h);
Signature · 1 line
1.8.5.6
#22 lore raw

PATCH v2 14/18 scsi_dh_alua: simplify sense code handling

In reply to Hannes Reinecke

Most sense code is already handled in the generic code, so we shouldn't be adding special cases here. However, when doing so we need to check for unit attention whenever we're sending an internal command.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 45 +++++++++++-------------------
 1 file changed, 17 insertions(+), 28 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +17 -28
index c9751c9..abf05b4 100644
@@ -85,7 +85,6 @@ struct alua_dh_data {
 #define ALUA_POLICY_SWITCH_ALL		1
 
 static char print_alua_state(int);
-static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *);
 
 static int realloc_buffer(struct alua_dh_data *h, unsigned len)
 {
@@ -189,8 +188,13 @@ static void stpg_endio(struct request *req, int error)
 
 	if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
 				 &sense_hdr)) {
-		err = alua_check_sense(h->sdev, &sense_hdr);
-		if (err == ADD_TO_MLQUEUE) {
+		if (sense_hdr.sense_key == NOT_READY &&
+		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) {
+			/* ALUA state transition already in progress */
+			err = SCSI_DH_OK;
+			goto done;
+		}
+		if (sense_hdr.sense_key == UNIT_ATTENTION) {
 			err = SCSI_DH_RETRY;
 			goto done;
 		}
@@ -399,28 +403,6 @@ static int alua_check_sense(struct scsi_device *sdev,
 			 * LUN Not Accessible - ALUA state transition
 			 */
 			return ADD_TO_MLQUEUE;
-		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0b)
-			/*
-			 * LUN Not Accessible -- Target port in standby state
-			 */
-			return SUCCESS;
-		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0c)
-			/*
-			 * LUN Not Accessible -- Target port in unavailable state
-			 */
-			return SUCCESS;
-		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x12)
-			/*
-			 * LUN Not Ready -- Offline
-			 */
-			return SUCCESS;
-		if (sdev->allow_restart &&
-		    sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x02)
-			/*
-			 * if the device is not started, we need to wake
-			 * the error handler to start the motor
-			 */
-			return FAILED;
 		break;
 	case UNIT_ATTENTION:
 		if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00)
@@ -517,9 +499,16 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_dh_data *h, int wait_
 			h->flags |= ALUA_RTPG_EXT_HDR_UNSUPP;
 			goto retry;
 		}
-
-		err = alua_check_sense(sdev, &sense_hdr);
-		if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) {
+		/*
+		 * Retry on ALUA state transition or if any
+		 * UNIT ATTENTION occurred.
+		 */
+		if (sense_hdr.sense_key == NOT_READY &&
+		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a)
+			err = SCSI_DH_RETRY;
+		else if (sense_hdr.sense_key == UNIT_ATTENTION)
+			err = SCSI_DH_RETRY;
+		if (err == SCSI_DH_RETRY && time_before(jiffies, expiry)) {
 			sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n",
 				    ALUA_DH_NAME);
 			scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);
Signature · 1 line
1.8.5.6
Reviewed-by on v2 14/18
#23 lore raw

Re: [PATCH 14/18] scsi_dh_alua: simplify sense code handling

Quoted 92 lines · Hannes Reinecke

Most sense code is already handled in the generic code, so we shouldn't be adding special cases here. However, when doing so we need to check for unit attention whenever we're sending an internal command.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 45 +++++++++++----------------- --

 1 file changed, 17 insertions(+), 28 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
index c9751c9..abf05b4 100644
@@ -85,7 +85,6 @@ struct alua_dh_data {
  #define ALUA_POLICY_SWITCH_ALL		1


 static char print_alua_state(int);
-static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *);
 static int realloc_buffer(struct alua_dh_data *h, unsigned len) {

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -189,8 +188,13 @@ static void stpg_endio(struct request *req, int error)


  if (scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE, &sense_hdr)) {

drivers/scsi/device_handler/scsi_dh_alua.c +7 -2
-		err = alua_check_sense(h->sdev, &sense_hdr);
-		if (err == ADD_TO_MLQUEUE) {
+		if (sense_hdr.sense_key == NOT_READY &&
+		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a) {
+			/* ALUA state transition already in progress */
+			err = SCSI_DH_OK;
+			goto done;
+		}
+		if (sense_hdr.sense_key == UNIT_ATTENTION) {

  err = SCSI_DH_RETRY; goto done;
  }

drivers/scsi/device_handler/scsi_dh_alua.c +0 -3
@@ -399,28 +403,6 @@ static int alua_check_sense(struct scsi_device *sdev,
  			 * LUN Not Accessible - ALUA state transition
  			 */
  			return ADD_TO_MLQUEUE;
-		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0b)
-			/*
-			 * LUN Not Accessible -- Target port in standby

state

drivers/scsi/device_handler/scsi_dh_alua.c +0 -5
-			 */
-			return SUCCESS;
-		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x0c)
-			/*
-			 * LUN Not Accessible -- Target port in unavailable

state

drivers/scsi/device_handler/scsi_dh_alua.c +0 -14
-			 */
-			return SUCCESS;
-		if (sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x12)
-			/*
-			 * LUN Not Ready -- Offline
-			 */
-			return SUCCESS;
-		if (sdev->allow_restart &&
-		    sense_hdr->asc == 0x04 && sense_hdr->ascq == 0x02)
-			/*
-			 * if the device is not started, we need to wake
-			 * the error handler to start the motor
-			 */
-			return FAILED;

  break;
  case UNIT_ATTENTION:
  if (sense_hdr->asc == 0x29 && sense_hdr->ascq == 0x00)

drivers/scsi/device_handler/scsi_dh_alua.c +0 -0
@@ -517,9 +499,16 @@ static int alua_rtpg(struct scsi_device *sdev, struct

alua_dh_data *h, int wait_
  h->flags |= ALUA_RTPG_EXT_HDR_UNSUPP; goto retry;
  }

drivers/scsi/device_handler/scsi_dh_alua.c +10 -3
-
-		err = alua_check_sense(sdev, &sense_hdr);
-		if (err == ADD_TO_MLQUEUE && time_before(jiffies, expiry)) {
+		/*
+		 * Retry on ALUA state transition or if any
+		 * UNIT ATTENTION occurred.
+		 */
+		if (sense_hdr.sense_key == NOT_READY &&
+		    sense_hdr.asc == 0x04 && sense_hdr.ascq == 0x0a)
+			err = SCSI_DH_RETRY;
+		else if (sense_hdr.sense_key == UNIT_ATTENTION)
+			err = SCSI_DH_RETRY;
+		if (err == SCSI_DH_RETRY && time_before(jiffies, expiry)) {
Quoted 4 lines

  sdev_printk(KERN_ERR, sdev, "%s: rtpg retry\n", ALUA_DH_NAME); scsi_print_sense_hdr(sdev, ALUA_DH_NAME, &sense_hdr);

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#24 lore raw

PATCH v2 15/18 scsi: Add scsi_vpd_lun_id()

In reply to Hannes Reinecke

Add a function scsi_vpd_lun_id() to return a unique device identifcation based on the designation descriptors of VPD page 0x83.

As devices might implement several descriptors the order of preference is:
- NAA IEE Registered Extended
- EUI-64 based 16-byte
- EUI-64 based 12-byte
- NAA IEEE Registered
- NAA IEEE Extended
A SCSI name string descriptor is preferred to all of them if the identification is longer than 16 bytes.

The returned unique device identification will be formatted as a SCSI Name string to avoid clashes between different designator types.

---
 drivers/scsi/scsi_lib.c    | 140 +++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |   1 +
 2 files changed, 141 insertions(+)
drivers/scsi/scsi_lib.c +140 -0
index dd8ad2a..5bd8a19 100644
@@ -3154,3 +3154,143 @@ void sdev_enable_disk_events(struct scsi_device *sdev)
 	atomic_dec(&sdev->disk_events_disable_depth);
 }
 EXPORT_SYMBOL(sdev_enable_disk_events);
+
+/*
+ * scsi_vpd_lun_id - return a unique device identification
+ * @sdev: SCSI device
+ * @id:   buffer for the identification
+ * @id_len:  length of the buffer
+ *
+ * Copies a unique device identification into @id based
+ * on the information in the VPD page 0x83 of the device.
+ * The string will be formatted as a SCSI name string.
+ *
+ * Returns the length of the identification or error on failure.
+ * If the identifier is longer than the supplied buffer the actual
+ * identifier length is returned and the buffer is not zero-padded.
+ */
+int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, size_t id_len)
+{
+	u8 cur_id_type = 0xff;
+	u8 cur_id_size = 0;
+	unsigned char *d, *cur_id_str;
+	unsigned char __rcu *vpd_pg83;
+	int id_size = -EINVAL;
+
+	rcu_read_lock();
+	vpd_pg83 = rcu_dereference(sdev->vpd_pg83);
+	if (!vpd_pg83) {
+		rcu_read_unlock();
+		return -ENXIO;
+	}
+
+	/*
+	 * Look for the correct descriptor.
+	 * Order of preference for lun descriptor:
+	 * - SCSI name string
+	 * - NAA IEEE Registered Extended
+	 * - EUI-64 based 16-byte
+	 * - EUI-64 based 12-byte
+	 * - NAA IEEE Registered
+	 * - NAA IEEE Extended
+	 * as longer descriptors reduce the likelyhood
+	 * of identification clashes.
+	 */
+
+	/* The id string must be at least 20 bytes + terminating NULL byte */
+	if (id_len < 21) {
+		rcu_read_unlock();
+		return -EINVAL;
+	}
+
+	memset(id, 0, id_len);
+	d = vpd_pg83 + 4;
+	while (d < vpd_pg83 + sdev->vpd_pg83_len) {
+		/* Skip designators not referring to the LUN */
+		if ((d[1] & 0x30) != 0x00)
+			goto next_desig;
+
+		switch (d[1] & 0xf) {
+		case 0x2:
+			/* EUI-64 */
+			if (cur_id_size > d[3])
+				break;
+			/* Prefer NAA IEEE Registered Extended */
+			if (cur_id_type == 0x3 &&
+			    cur_id_size == d[3])
+				break;
+			cur_id_size = d[3];
+			cur_id_str = d + 4;
+			cur_id_type = d[1] & 0xf;
+			switch (cur_id_size) {
+			case 8:
+				id_size = snprintf(id, id_len,
+						   "eui.%8phN",
+						   cur_id_str);
+				break;
+			case 12:
+				id_size = snprintf(id, id_len,
+						   "eui.%12phN",
+						   cur_id_str);
+				break;
+			case 16:
+				id_size = snprintf(id, id_len,
+						   "eui.%16phN",
+						   cur_id_str);
+				break;
+			default:
+				cur_id_size = 0;
+				break;
+			}
+			break;
+		case 0x3:
+			/* NAA */
+			if (cur_id_size > d[3])
+				break;
+			cur_id_size = d[3];
+			cur_id_str = d + 4;
+			cur_id_type = d[1] & 0xf;
+			switch (cur_id_size) {
+			case 8:
+				id_size = snprintf(id, id_len,
+						   "naa.%8phN",
+						   cur_id_str);
+				break;
+			case 16:
+				id_size = snprintf(id, id_len,
+						   "naa.%16phN",
+						   cur_id_str);
+				break;
+			default:
+				cur_id_size = 0;
+				break;
+			}
+			break;
+		case 0x8:
+			/* SCSI name string */
+			if (cur_id_size + 4 > d[3])
+				break;
+			/* Prefer others for truncated descriptor */
+			if (cur_id_size && d[3] > id_len)
+				break;
+			cur_id_size = id_size = d[3];
+			cur_id_str = d + 4;
+			cur_id_type = d[1] & 0xf;
+			if (cur_id_size >= id_len)
+				cur_id_size = id_len - 1;
+			memcpy(id, cur_id_str, cur_id_size);
+			/* Decrease priority for truncated descriptor */
+			if (cur_id_size != id_size)
+				cur_id_size = 6;
+			break;
+		default:
+			break;
+		}
+next_desig:
+		d += d[3] + 4;
+	}
+	rcu_read_unlock();
+
+	return id_size;
+}
+EXPORT_SYMBOL(scsi_vpd_lun_id);
include/scsi/scsi_device.h +1 -0
index bde4077..4c49cfa 100644
@@ -415,6 +415,7 @@ static inline int scsi_execute_req(struct scsi_device *sdev,
 }
 extern void sdev_disable_disk_events(struct scsi_device *sdev);
 extern void sdev_enable_disk_events(struct scsi_device *sdev);
+extern int scsi_vpd_lun_id(struct scsi_device *, char *, size_t);
 
 #ifdef CONFIG_PM
 extern int scsi_autopm_get_device(struct scsi_device *);
Signature · 1 line
1.8.5.6
Reviewed-by on v2 15/18
#25 lore raw

Re: [PATCH 15/18] scsi: Add scsi_vpd_lun_id()

Quoted 35 lines · Hannes Reinecke

Add a function scsi_vpd_lun_id() to return a unique device identifcation based on the designation descriptors of VPD page 0x83.

As devices might implement several descriptors the order of preference is:
- NAA IEE Registered Extended
- EUI-64 based 16-byte
- EUI-64 based 12-byte
- NAA IEEE Registered
- NAA IEEE Extended
A SCSI name string descriptor is preferred to all of them if the identification is longer than 16 bytes.

The returned unique device identification will be formatted as a SCSI Name string to avoid clashes between different designator types.

---
 drivers/scsi/scsi_lib.c    | 140
+++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |   1 +

 2 files changed, 141 insertions(+)
drivers/scsi/scsi_lib.c +0 -0
index dd8ad2a..5bd8a19 100644
@@ -3154,3 +3154,143 @@ void sdev_enable_disk_events(struct scsi_device
 *sdev)
  	atomic_dec(&sdev->disk_events_disable_depth);
  }
Quoted 1 line

 EXPORT_SYMBOL(sdev_enable_disk_events);

Nitpick:
kerneldoc comments start with /**

Quoted 7 lines
drivers/scsi/scsi_lib.c +7 -0
+
+/*
+ * scsi_vpd_lun_id - return a unique device identification
+ * @sdev: SCSI device
+ * @id:   buffer for the identification
+ * @id_len:  length of the buffer
+ *

drivers/scsi/scsi_lib.c void sdev_enable_disk_events(struct scsi_device *sdev) · line 3173

Quoted 10 lines
drivers/scsi/scsi_lib.c +10 -0
+ * Copies a unique device identification into @id based
+ * on the information in the VPD page 0x83 of the device.
+ * The string will be formatted as a SCSI name string.
+ *
+ * Returns the length of the identification or error on failure.
+ * If the identifier is longer than the supplied buffer the actual
+ * identifier length is returned and the buffer is not zero-padded.
+ */
+int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, size_t id_len)
+{

Otherwise

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#26 lore raw

PATCH v2 16/18 scsi: export 'wwid' to sysfs

In reply to Hannes Reinecke

Use scsi_vpd_lun_id() to export the world-wide unique id (wwid) to sysfs. Note that this is the 'best' wwid according to the rules in scsi_vpd_lun_id(), not every possible wwid presented by the drive.

---
 drivers/scsi/scsi_sysfs.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
drivers/scsi/scsi_sysfs.c +17 -0
index d015374..ef36053 100644
@@ -905,6 +905,22 @@ sdev_show_function(queue_depth, "%d\n");
 static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
 		   sdev_store_queue_depth);
 
+static ssize_t
+sdev_show_wwid(struct device *dev, struct device_attribute *attr,
+		    char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	ssize_t count;
+
+	count = scsi_vpd_lun_id(sdev, buf, PAGE_SIZE);
+	if (count > 0) {
+		buf[count] = '\n';
+		count++;
+	}
+	return count;
+}
+static DEVICE_ATTR(wwid, S_IRUGO, sdev_show_wwid, NULL);
+
 #ifdef CONFIG_SCSI_DH
 static ssize_t
 sdev_show_dh_state(struct device *dev, struct device_attribute *attr,
@@ -1028,6 +1044,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_modalias.attr,
 	&dev_attr_queue_depth.attr,
 	&dev_attr_queue_type.attr,
+	&dev_attr_wwid.attr,
 #ifdef CONFIG_SCSI_DH
 	&dev_attr_dh_state.attr,
 #endif
Signature · 1 line
1.8.5.6
#27 lore raw

PATCH v2 17/18 scsi: Add scsi_vpd_tpg_id()

In reply to Hannes Reinecke

Implement scsi_vpd_tpg_id() to extract the target port group id and the relative port id from SCSI VPD page 0x83.

---
 drivers/scsi/scsi_lib.c    | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |  1 +
 2 files changed, 49 insertions(+)
drivers/scsi/scsi_lib.c +48 -0
index 5bd8a19..99d087b 100644
@@ -23,6 +23,7 @@
 #include <linux/scatterlist.h>
 #include <linux/blk-mq.h>
 #include <linux/ratelimit.h>
+#include <asm/unaligned.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -3294,3 +3295,50 @@ next_desig:
 	return id_size;
 }
 EXPORT_SYMBOL(scsi_vpd_lun_id);
+
+/*
+ * scsi_vpd_tpg_id - return a target port group identifier
+ * @sdev: SCSI device
+ *
+ * Returns the Target Port Group identifier from the information
+ * froom VPD page 0x83 of the device.
+ *
+ * Returns the identifier or error on failure.
+ */
+int scsi_vpd_tpg_id(struct scsi_device *sdev, int *rel_id)
+{
+	unsigned char *d;
+	unsigned char __rcu *vpd_pg83;
+	int group_id = -EAGAIN, rel_port = -1;
+
+	rcu_read_lock();
+	vpd_pg83 = rcu_dereference(sdev->vpd_pg83);
+	if (!vpd_pg83) {
+		rcu_read_unlock();
+		return -ENXIO;
+	}
+
+	d = sdev->vpd_pg83 + 4;
+	while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
+		switch (d[1] & 0xf) {
+		case 0x4:
+			/* Relative target port */
+			rel_port = get_unaligned_be16(&d[6]);
+			break;
+		case 0x5:
+			/* Target port group */
+			group_id = get_unaligned_be16(&d[6]);
+			break;
+		default:
+			break;
+		}
+		d += d[3] + 4;
+	}
+	rcu_read_unlock();
+
+	if (group_id >= 0 && rel_id && rel_port != -1)
+		*rel_id = rel_port;
+
+	return group_id;
+}
+EXPORT_SYMBOL(scsi_vpd_tpg_id);
include/scsi/scsi_device.h +1 -0
index 4c49cfa..f63a167 100644
@@ -416,6 +416,7 @@ static inline int scsi_execute_req(struct scsi_device *sdev,
 extern void sdev_disable_disk_events(struct scsi_device *sdev);
 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_tpg_id(struct scsi_device *, int *);
 
 #ifdef CONFIG_PM
 extern int scsi_autopm_get_device(struct scsi_device *);
Signature · 1 line
1.8.5.6
#28 lore raw

PATCH v2 18/18 scsi_dh_alua: use scsi_vpd_tpg_id()

In reply to Hannes Reinecke

Use the common function 'scsi_vpd_tpg_id()' instead of open-coding it in scsi_dh_alua.

---
 drivers/scsi/device_handler/scsi_dh_alua.c | 39 +++++-------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)
drivers/scsi/device_handler/scsi_dh_alua.c +6 -33
index abf05b4..20fe981 100644
@@ -320,38 +320,10 @@ static int alua_check_tpgs(struct scsi_device *sdev)
  */
 static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 {
-	unsigned char *d;
-	unsigned char __rcu *vpd_pg83;
+	int rel_port = -1, group_id;
 
-	rcu_read_lock();
-	if (!rcu_dereference(sdev->vpd_pg83)){
-		rcu_read_unlock();
-		return SCSI_DH_DEV_UNSUPP;
-	}
-
-	/*
-	 * Look for the correct descriptor.
-	 */
-	vpd_pg83 = rcu_dereference(sdev->vpd_pg83);
-	d = vpd_pg83 + 4;
-	while (d < vpd_pg83 + sdev->vpd_pg83_len) {
-		switch (d[1] & 0xf) {
-		case 0x4:
-			/* Relative target port */
-			h->rel_port = get_unaligned_be16(&d[6]);
-			break;
-		case 0x5:
-			/* Target port group */
-			h->group_id = get_unaligned_be16(&d[6]);
-			break;
-		default:
-			break;
-		}
-		d += d[3] + 4;
-	}
-	rcu_read_unlock();
-
-	if (h->group_id == -1) {
+	group_id = scsi_vpd_tpg_id(sdev, &rel_port);
+	if (group_id < 0) {
 		/*
 		 * Internal error; TPGS supported but required
 		 * VPD identification descriptors not present.
@@ -360,10 +332,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: No target port descriptors found\n",
 			    ALUA_DH_NAME);
-		h->state = TPGS_STATE_OPTIMIZED;
-		h->tpgs = TPGS_MODE_NONE;
 		return SCSI_DH_DEV_UNSUPP;
 	}
+	h->rel_port = rel_port;
+	h->group_id = group_id;
+
 	sdev_printk(KERN_INFO, sdev,
 		    "%s: port group %02x rel port %02x\n",
 		    ALUA_DH_NAME, h->group_id, h->rel_port);
Signature · 1 line
1.8.5.6
#29 lore raw
In reply to Hannes Reinecke
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Hannes" == Hannes Reinecke <[email protected]> writes:

Hannes> Hi all, here's the first part of my ALUA device handler update. Hannes> It's mainly bugfixes and minor improvements; the two important Hannes> things are the addition of VPD parsing functions Hannes> scsi_vpd_lun_id() and scsi_vpd_tpg_id(). This series has been Hannes> split off from the original 'Asynchronous ALUA' patchset, as Hannes> these bits are pretty uncontroversial and have a good chance of Hannes> being merged reasonably soon.

Applied to 4.5/scsi-queue.

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering