[PATCH 00/18] ALUA device handler update, part 1

[PATCH 00/18] ALUA device handler update, part 1

[Hannes Reinecke]

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.

Patch is relative to 4.4/scsi-queue by mkp, and based upon the
'scsi: rescan VPD attributes' patch I send earlier.

As usual, comments and reviews are welcome.

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 'device_id' 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                     |  68 +------
 drivers/scsi/scsi_lib.c                    | 188 ++++++++++++++++++
 drivers/scsi/scsi_sysfs.c                  |  82 +++++++-
 include/scsi/scsi_dbg.h                    |   2 -
 include/scsi/scsi_device.h                 |   2 +
 6 files changed, 373 insertions(+), 263 deletions(-)

-- 
1.8.5.6

[PATCH 01/18] scsi_dh: move 'dh_state' sysfs attribute to generic code

[Hannes Reinecke]

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

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_dh.c    | 68 +----------------------------------------------
 drivers/scsi/scsi_sysfs.c | 58 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 67 deletions(-)

diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
index 0a2168e..57ad0f3 100644
--- a/drivers/scsi/scsi_dh.c
+++ b/drivers/scsi/scsi_dh.c
@@ -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)
@@ -236,7 +171,6 @@ void scsi_dh_remove_device(struct scsi_device *sdev)
 {
 	if (sdev->handler)
 		scsi_dh_handler_detach(sdev);
-	device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
 }
 
 /*
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index f021423..13a5ede 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -17,6 +17,7 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_tcq.h>
+#include <scsi/scsi_dh.h>
 #include <scsi/scsi_transport.h>
 #include <scsi/scsi_driver.h>
 
@@ -902,6 +903,60 @@ sdev_show_function(queue_depth, "%d\n");
 static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
 		   sdev_store_queue_depth);
 
+#ifdef CONFIG_SCSI_DH
+static ssize_t
+sdev_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 ssize_t
+sdev_store_dh_state(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	int err = -EINVAL;
+
+	if (sdev->sdev_state == SDEV_CANCEL ||
+	    sdev->sdev_state == SDEV_DEL)
+		return -ENODEV;
+
+	if (!sdev->handler) {
+		/*
+		 * Attach to a device handler
+		 */
+		err = scsi_dh_attach(sdev->request_queue, buf);
+	} else if (!strncmp(buf, "activate", 8)) {
+		/*
+		 * Activate a device handler
+		 */
+		if (sdev->handler->activate)
+			err = sdev->handler->activate(sdev, NULL, NULL);
+		else
+			err = 0;
+	} 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;
+	}
+
+	return err < 0 ? err : count;
+}
+
+static DEVICE_ATTR(dh_state, S_IRUGO | S_IWUSR, sdev_show_dh_state,
+		   sdev_store_dh_state);
+#endif
+
 static ssize_t
 sdev_show_queue_ramp_up_period(struct device *dev,
 			       struct device_attribute *attr,
@@ -971,6 +1026,9 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_modalias.attr,
 	&dev_attr_queue_depth.attr,
 	&dev_attr_queue_type.attr,
+#ifdef CONFIG_SCSI_DH
+	&dev_attr_dh_state.attr,
+#endif
 	&dev_attr_queue_ramp_up_period.attr,
 	REF_EVT(media_change),
 	REF_EVT(inquiry_change_reported),
-- 
1.8.5.6

[PATCH 02/18] scsi: ignore errors from scsi_dh_add_device()

[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().

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_sysfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 13a5ede..7b41b2c 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1118,11 +1118,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);
-- 
1.8.5.6

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

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

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index cc2773b..7d01ef0 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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:
-- 
1.8.5.6

[PATCH 04/18] scsi_dh_alua: Use vpd_pg83 information

[Hannes Reinecke]

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

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 87 +++++++-----------------------
 1 file changed, 18 insertions(+), 69 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 7d01ef0..9b3b2f7 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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;
 
-- 
1.8.5.6

[PATCH 05/18] scsi_dh_alua: improved logging

[Hannes Reinecke]

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

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 9b3b2f7..c63f304 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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;
-- 
1.8.5.6

[PATCH 06/18] scsi_dh_alua: sanitze sense code handling

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

Reviewed-by: Bart van Assche <bvanassche@sandisk.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index c63f304..240a5dc 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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;
 
 		/*
-- 
1.8.5.6

[PATCH 07/18] scsi_dh_alua: use standard logging functions

[Hannes Reinecke]

Use standard logging functions instead of hand-crafted ones.

Reviewed-by: Bart Van Assche <bvanassche@sandisk.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 240a5dc..b817963 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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;
-- 
1.8.5.6

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

[Hannes Reinecke]

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

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 33 +++++++++++++++---------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index b817963..50fe87c 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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;
-- 
1.8.5.6

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

[Hannes Reinecke]

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

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 50fe87c..ea626732 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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)
 {
-- 
1.8.5.6

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

[Hannes Reinecke]

Last caller is gone, so remove it.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 include/scsi/scsi_dbg.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
index f8170e9..56710e0 100644
--- a/include/scsi/scsi_dbg.h
+++ b/include/scsi/scsi_dbg.h
@@ -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 *);
-- 
1.8.5.6

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

[Hannes Reinecke]

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

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index ea626732..03f8190 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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;
 		}
 
-- 
1.8.5.6

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

[Hannes Reinecke]

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

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 03f8190..d54f42e 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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];
-- 
1.8.5.6

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

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

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index d54f42e..c9751c9 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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);
-- 
1.8.5.6

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

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

Reviewed-by: Ewan Milne <emilne@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 45 +++++++++++-------------------
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index c9751c9..abf05b4 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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);
-- 
1.8.5.6

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

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

Reviewed-by: Ewan Milne <emilne@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_lib.c    | 140 +++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |   1 +
 2 files changed, 141 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index cbfc599..3cb295c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -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);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index bde4077..4c49cfa 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -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 *);
-- 
1.8.5.6

[PATCH 16/18] scsi: export 'device_id' to sysfs

[Hannes Reinecke]

Use scsi_vpd_lun_id() to export the device id to sysfs.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_sysfs.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 7b41b2c..aaa38c2 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -903,6 +903,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_device_id(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(device_id, S_IRUGO, sdev_show_device_id, NULL);
+
 #ifdef CONFIG_SCSI_DH
 static ssize_t
 sdev_show_dh_state(struct device *dev, struct device_attribute *attr,
@@ -1026,6 +1042,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_modalias.attr,
 	&dev_attr_queue_depth.attr,
 	&dev_attr_queue_type.attr,
+	&dev_attr_device_id.attr,
 #ifdef CONFIG_SCSI_DH
 	&dev_attr_dh_state.attr,
 #endif
-- 
1.8.5.6

[PATCH 17/18] scsi: Add scsi_vpd_tpg_id()

[Hannes Reinecke]

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

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_lib.c    | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h |  1 +
 2 files changed, 49 insertions(+)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 3cb295c..d12edc8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -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);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 4c49cfa..f63a167 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -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 *);
-- 
1.8.5.6

[PATCH 18/18] scsi_dh_alua: use scsi_vpd_tpg_id()

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 39 +++++-------------------------
 1 file changed, 6 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index abf05b4..20fe981 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -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);
-- 
1.8.5.6

Re: [PATCH 00/18] ALUA device handler update, part 1

[Christoph Hellwig]

Can you push out a git branch with this code for easier review as well?

Re: [PATCH 00/18] ALUA device handler update, part 1

[Hannes Reinecke]

On 11/19/2015 09:35 AM, Christoph Hellwig wrote:
> Can you push out a git branch with this code for easier review as well?
> 
Pushed to kernel.org:hare/scsi-devel.git branch alua.v6
Branch is based upon mkp/scsi.git branch 4.5/scsi-queue

Thanks for reviewing.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/18] ALUA device handler update, part 1

[Christoph Hellwig]

On Mon, Nov 09, 2015 at 04:08:05PM +0100, Hannes Reinecke wrote:
> 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.

I've already reviewed most patches, but the other ones looks fine as
well:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Let's get this into the 4.5 queue ASAP an then tackle the actually
interesting bits next!

Re: [PATCH 00/18] ALUA device handler update, part 1

[Hannes Reinecke]

On 11/20/2015 11:47 AM, Christoph Hellwig wrote:
> On Mon, Nov 09, 2015 at 04:08:05PM +0100, Hannes Reinecke wrote:
>> 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.
> 
> I've already reviewed most patches, but the other ones looks fine as
> well:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Let's get this into the 4.5 queue ASAP an then tackle the actually
> interesting bits next!
> 
I'm all for it.

One thing, though: I don't really agree with Barts objection that
moving to a workqueue would tie in too many resources.
Thing is, I'm not convinces that using a work queue is allocating
too many resources (we're speaking of 460 vs 240 bytes here).
Also we have to retry commands for quite some time (cite the
infamous NetApp takeover/giveback, which can take minutes).
If we were to handle that without workqueue we'd have to initiate
the retry from the end_io callback, causing a quite deep stack
recursion. Which I'm not really fond of.

But if anyone has a better idea on how to handle retries without the
need for workqueues I'm all ears :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/18] ALUA device handler update, part 1

[Christoph Hellwig]

On Fri, Nov 20, 2015 at 11:52:21AM +0100, Hannes Reinecke wrote:
> One thing, though: I don't really agree with Barts objection that
> moving to a workqueue would tie in too many resources.
> Thing is, I'm not convinces that using a work queue is allocating
> too many resources (we're speaking of 460 vs 240 bytes here).
> Also we have to retry commands for quite some time (cite the
> infamous NetApp takeover/giveback, which can take minutes).
> If we were to handle that without workqueue we'd have to initiate
> the retry from the end_io callback, causing a quite deep stack
> recursion. Which I'm not really fond of.
> 
> But if anyone has a better idea on how to handle retries without the
> need for workqueues I'm all ears :-)

I tend to agree with you, but you better warm up that discussion again
on the old thread that actually has Bart on Cc.  Or just resend once
Martin has merged this patch, and have discussion around that version.

Re: [PATCH 00/18] ALUA device handler update, part 1

[Hannes Reinecke]

On 11/20/2015 11:54 AM, Christoph Hellwig wrote:
> On Fri, Nov 20, 2015 at 11:52:21AM +0100, Hannes Reinecke wrote:
>> One thing, though: I don't really agree with Barts objection that
>> moving to a workqueue would tie in too many resources.
>> Thing is, I'm not convinces that using a work queue is allocating
>> too many resources (we're speaking of 460 vs 240 bytes here).
>> Also we have to retry commands for quite some time (cite the
>> infamous NetApp takeover/giveback, which can take minutes).
>> If we were to handle that without workqueue we'd have to initiate
>> the retry from the end_io callback, causing a quite deep stack
>> recursion. Which I'm not really fond of.
>>
>> But if anyone has a better idea on how to handle retries without the
>> need for workqueues I'm all ears :-)
> 
> I tend to agree with you, but you better warm up that discussion again
> on the old thread that actually has Bart on Cc.  Or just resend once
> Martin has merged this patch, and have discussion around that version.
> 
Fully agree. So I'm waiting for Martin to pick up the patches first.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/18] ALUA device handler update, part 1

[Bart Van Assche]

On 11/20/2015 02:52 AM, Hannes Reinecke wrote:
> One thing, though: I don't really agree with Barts objection that
> moving to a workqueue would tie in too many resources.
> Thing is, I'm not convinces that using a work queue is allocating
> too many resources (we're speaking of 460 vs 240 bytes here).
> Also we have to retry commands for quite some time (cite the
> infamous NetApp takeover/giveback, which can take minutes).
> If we were to handle that without workqueue we'd have to initiate
> the retry from the end_io callback, causing a quite deep stack
> recursion. Which I'm not really fond of.

Hello Hannes,

Sorry if I wasn't clear enough in my previous e-mail about this topic 
but I'm more concerned about the additional memory needed for thread 
stacks and thread control data structures than about the additional 
memory needed for the workqueue. I'd like to see the ALUA device handler 
implementation scale to thousands of LUNs and target port groups. In 
case all connections between an initiator and a target port group fail, 
with a synchronous implementation of STPG we will either need a large 
number of threads (in case of one thread per STPG command) or the STPG 
commands will be serialized (if there are fewer threads than portal 
groups). Neither alternative looks attractive to me.

BTW, not all storage arrays need STPG retries. Some arrays are able to 
process an STPG command quickly (this means within a few seconds).

A previous discussion about this topic is available e.g. at 
http://thread.gmane.org/gmane.linux.scsi/105340/focus=105601.

Bart.

Re: [PATCH 00/18] ALUA device handler update, part 1

[Hannes Reinecke]

On 11/20/2015 11:58 PM, Bart Van Assche wrote:
> On 11/20/2015 02:52 AM, Hannes Reinecke wrote:
>> One thing, though: I don't really agree with Barts objection that
>> moving to a workqueue would tie in too many resources.
>> Thing is, I'm not convinces that using a work queue is allocating
>> too many resources (we're speaking of 460 vs 240 bytes here).
>> Also we have to retry commands for quite some time (cite the
>> infamous NetApp takeover/giveback, which can take minutes).
>> If we were to handle that without workqueue we'd have to initiate
>> the retry from the end_io callback, causing a quite deep stack
>> recursion. Which I'm not really fond of.
> 
> Hello Hannes,
> 
> Sorry if I wasn't clear enough in my previous e-mail about this
> topic but I'm more concerned about the additional memory needed for
> thread stacks and thread control data structures than about the
> additional memory needed for the workqueue. I'd like to see the ALUA
> device handler implementation scale to thousands of LUNs and target
> port groups. In case all connections between an initiator and a
> target port group fail, with a synchronous implementation of STPG we
> will either need a large number of threads (in case of one thread
> per STPG command) or the STPG commands will be serialized (if there
> are fewer threads than portal groups). Neither alternative looks
> attractive to me.
> 
> BTW, not all storage arrays need STPG retries. Some arrays are able
> to process an STPG command quickly (this means within a few seconds).
> 
> A previous discussion about this topic is available e.g. at
> http://thread.gmane.org/gmane.linux.scsi/105340/focus=105601.
> 
Well, one could argue that the whole point of this patchset is to
allow you to serialize STPGs :-)

We definitely need to serialize STPGs for the same target port
group; the current implementation is far too limited to take that
into account.

But the main problem I'm facing with the current implementation is
that we cannot handle retries. An RTPG or an STPG might fail, at
which point we need to re-run RTPG to figure out the current status.
(We also need to send RTPGs when we receive an "ALUA state changed"
 UA, but that's slightly beside the point).
The retry cannot be send directly, as we're evaluating the status
from end_io context. So to instantiate a retry we need to move it
over to a workqueue.

Or, at least, that's the solution I'm able to come up with.
If you have other ideas it'd be most welcome.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/18] ALUA device handler update, part 1

[Bart Van Assche]

On 11/23/2015 08:10 AM, Hannes Reinecke wrote:
> On 11/20/2015 11:58 PM, Bart Van Assche wrote:
>> On 11/20/2015 02:52 AM, Hannes Reinecke wrote:
>>> One thing, though: I don't really agree with Barts objection that
>>> moving to a workqueue would tie in too many resources.
>>> Thing is, I'm not convinces that using a work queue is allocating
>>> too many resources (we're speaking of 460 vs 240 bytes here).
>>> Also we have to retry commands for quite some time (cite the
>>> infamous NetApp takeover/giveback, which can take minutes).
>>> If we were to handle that without workqueue we'd have to initiate
>>> the retry from the end_io callback, causing a quite deep stack
>>> recursion. Which I'm not really fond of.
>>
>> Hello Hannes,
>>
>> Sorry if I wasn't clear enough in my previous e-mail about this
>> topic but I'm more concerned about the additional memory needed for
>> thread stacks and thread control data structures than about the
>> additional memory needed for the workqueue. I'd like to see the ALUA
>> device handler implementation scale to thousands of LUNs and target
>> port groups. In case all connections between an initiator and a
>> target port group fail, with a synchronous implementation of STPG we
>> will either need a large number of threads (in case of one thread
>> per STPG command) or the STPG commands will be serialized (if there
>> are fewer threads than portal groups). Neither alternative looks
>> attractive to me.
>>
>> BTW, not all storage arrays need STPG retries. Some arrays are able
>> to process an STPG command quickly (this means within a few seconds).
>>
>> A previous discussion about this topic is available e.g. at
>> http://thread.gmane.org/gmane.linux.scsi/105340/focus=105601.
>>
> Well, one could argue that the whole point of this patchset is to
> allow you to serialize STPGs :-)
>
> We definitely need to serialize STPGs for the same target port
> group; the current implementation is far too limited to take that
> into account.
>
> But the main problem I'm facing with the current implementation is
> that we cannot handle retries. An RTPG or an STPG might fail, at
> which point we need to re-run RTPG to figure out the current status.
> (We also need to send RTPGs when we receive an "ALUA state changed"
>   UA, but that's slightly beside the point).
> The retry cannot be send directly, as we're evaluating the status
> from end_io context. So to instantiate a retry we need to move it
> over to a workqueue.
>
> Or, at least, that's the solution I'm able to come up with.
> If you have other ideas it'd be most welcome.

Hello Hannes,

I agree that retries have to be handled from workqueue context instead 
of end_io context. But in workqueue context we can choose whether to 
submit the retry synchronously or asynchronously. Unless I overlooked 
something I don't see why the retry should be submitted synchronously.

Bart.

Re: [PATCH 01/18] scsi_dh: move 'dh_state' sysfs attribute to generic code

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> As scsi_dh.c is now always compiled in we should be moving
> the 'dh_state' attribute to the generic code.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_dh.c    | 68 +----------------------------------
> ------------
>  drivers/scsi/scsi_sysfs.c | 58
> ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
> index 0a2168e..57ad0f3 100644
> --- a/drivers/scsi/scsi_dh.c
> +++ b/drivers/scsi/scsi_dh.c
> @@ -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)
> @@ -236,7 +171,6 @@ void scsi_dh_remove_device(struct scsi_device
> *sdev)
>  {
>  	if (sdev->handler)
>  		scsi_dh_handler_detach(sdev);
> -	device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
>  }
>  
>  /*
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index f021423..13a5ede 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -17,6 +17,7 @@
>  #include <scsi/scsi_device.h>
>  #include <scsi/scsi_host.h>
>  #include <scsi/scsi_tcq.h>
> +#include <scsi/scsi_dh.h>
>  #include <scsi/scsi_transport.h>
>  #include <scsi/scsi_driver.h>
>  
> @@ -902,6 +903,60 @@ sdev_show_function(queue_depth, "%d\n");
>  static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR,
> sdev_show_queue_depth,
>  		   sdev_store_queue_depth);
>  
> +#ifdef CONFIG_SCSI_DH
> +static ssize_t
> +sdev_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 ssize_t
> +sdev_store_dh_state(struct device *dev, struct device_attribute
> *attr,
> +		    const char *buf, size_t count)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +	int err = -EINVAL;
> +
> +	if (sdev->sdev_state == SDEV_CANCEL ||
> +	    sdev->sdev_state == SDEV_DEL)
> +		return -ENODEV;
> +
> +	if (!sdev->handler) {
> +		/*
> +		 * Attach to a device handler
> +		 */
> +		err = scsi_dh_attach(sdev->request_queue, buf);
> +	} else if (!strncmp(buf, "activate", 8)) {
> +		/*
> +		 * Activate a device handler
> +		 */
> +		if (sdev->handler->activate)
> +			err = sdev->handler->activate(sdev, NULL,
> NULL);
> +		else
> +			err = 0;
> +	} 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;
> +	}
> +
> +	return err < 0 ? err : count;
> +}
> +
> +static DEVICE_ATTR(dh_state, S_IRUGO | S_IWUSR, sdev_show_dh_state,
> +		   sdev_store_dh_state);
> +#endif
> +
>  static ssize_t
>  sdev_show_queue_ramp_up_period(struct device *dev,
>  			       struct device_attribute *attr,
> @@ -971,6 +1026,9 @@ static struct attribute *scsi_sdev_attrs[] = {
>  	&dev_attr_modalias.attr,
>  	&dev_attr_queue_depth.attr,
>  	&dev_attr_queue_type.attr,
> +#ifdef CONFIG_SCSI_DH
> +	&dev_attr_dh_state.attr,
> +#endif
>  	&dev_attr_queue_ramp_up_period.attr,
>  	REF_EVT(media_change),
>  	REF_EVT(inquiry_change_reported),

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 02/18] scsi: ignore errors from scsi_dh_add_device()

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> 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().
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_sysfs.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 13a5ede..7b41b2c 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1118,11 +1118,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);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> 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.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index cc2773b..7d01ef0 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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:

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> The SCSI device now has the VPD page 0x83 information attached,
> so there is no need to query it again.
> 
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 87 +++++++-------------
> ----------
>  1 file changed, 18 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 7d01ef0..9b3b2f7 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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;
>  

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Issue different logging messages if ALUA is not supported
> or the TPGS setting is invalid.
> 
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 9b3b2f7..c63f304 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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;

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 06/18] scsi_dh_alua: sanitze sense code handling

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> 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.
> 
> Reviewed-by: Bart van Assche <bvanassche@sandisk.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 18 +++++++-----------
>  1 file changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index c63f304..240a5dc 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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;
>  
>  		/*

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 07/18] scsi_dh_alua: use standard logging functions

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Use standard logging functions instead of hand-crafted ones.
> 
> Reviewed-by: Bart Van Assche <bvanassche@sandisk.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 27 +++++++++++++-------
> -------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 240a5dc..b817963 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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;

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Fixup submit_rtpg() to always return a standard SCSI return code.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 33 +++++++++++++++-----
> ----------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index b817963..50fe87c 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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;

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Fixup copy-and-paste error in the description of stpg_endio().
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 50fe87c..ea626732 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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)
>  {

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Last caller is gone, so remove it.
> 
> Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  include/scsi/scsi_dbg.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/scsi/scsi_dbg.h b/include/scsi/scsi_dbg.h
> index f8170e9..56710e0 100644
> --- a/include/scsi/scsi_dbg.h
> +++ b/include/scsi/scsi_dbg.h
> @@ -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 *);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> We should be using a flag when RTPG extended header is not
> supported, that saves us sending RTPG twice for older arrays.
> 
> Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index ea626732..03f8190 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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;
>  		}
>  

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Use 'get_unaligned_XX' and 'put_unaligned_XX' instead of
> open-coding it.
> 
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 23 ++++++++----------
> -----
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index 03f8190..d54f42e 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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];

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 13/18] scsi_dh_alua: rework alua_check_tpgs() to return the tpgs mode

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Instead of returning an error code in alua_check_tpgs() we should
> rather return the tpgs mode directly and have a cleaner syntax.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 25 +++++++++++---------
> -----
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index d54f42e..c9751c9 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> 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.
> 
> Reviewed-by: Ewan Milne <emilne@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 45 +++++++++++---------
> ----------
>  1 file changed, 17 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index c9751c9..abf05b4 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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

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

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> 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.
> 
> Reviewed-by: Ewan Milne <emilne@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_lib.c    | 140
> +++++++++++++++++++++++++++++++++++++++++++++
>  include/scsi/scsi_device.h |   1 +
>  2 files changed, 141 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index cbfc599..3cb295c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -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);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index bde4077..4c49cfa 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -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 *);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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

Re: [PATCH 16/18] scsi: export 'device_id' to sysfs

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Use scsi_vpd_lun_id() to export the device id to sysfs.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_sysfs.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index 7b41b2c..aaa38c2 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -903,6 +903,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_device_id(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(device_id, S_IRUGO, sdev_show_device_id, NULL);
> +
>  #ifdef CONFIG_SCSI_DH
>  static ssize_t
>  sdev_show_dh_state(struct device *dev, struct device_attribute
> *attr,
> @@ -1026,6 +1042,7 @@ static struct attribute *scsi_sdev_attrs[] = {
>  	&dev_attr_modalias.attr,
>  	&dev_attr_queue_depth.attr,
>  	&dev_attr_queue_type.attr,
> +	&dev_attr_device_id.attr,
>  #ifdef CONFIG_SCSI_DH
>  	&dev_attr_dh_state.attr,
>  #endif

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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

Re: [PATCH 17/18] scsi: Add scsi_vpd_tpg_id()

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Implement scsi_vpd_tpg_id() to extract the target
> port group id and the relative port id from
> SCSI VPD page 0x83.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_lib.c    | 48
> ++++++++++++++++++++++++++++++++++++++++++++++
>  include/scsi/scsi_device.h |  1 +
>  2 files changed, 49 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 3cb295c..d12edc8 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -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);
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 4c49cfa..f63a167 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -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 *);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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

Re: [PATCH 18/18] scsi_dh_alua: use scsi_vpd_tpg_id()

[Johannes Thumshirn]

On Mon, 2015-11-09 at 16:08 +0100, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/device_handler/scsi_dh_alua.c | 39 +++++---------------
> ----------
>  1 file changed, 6 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c
> b/drivers/scsi/device_handler/scsi_dh_alua.c
> index abf05b4..20fe981 100644
> --- a/drivers/scsi/device_handler/scsi_dh_alua.c
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -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);

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>

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

Re: [PATCH 00/18] ALUA device handler update, part 1

[Hannes Reinecke]

On 11/23/2015 05:18 PM, Bart Van Assche wrote:
> 
> 
> On 11/23/2015 08:10 AM, Hannes Reinecke wrote:
>> On 11/20/2015 11:58 PM, Bart Van Assche wrote:
[ .. ]
>>> BTW, not all storage arrays need STPG retries. Some arrays are able
>>> to process an STPG command quickly (this means within a few
>>> seconds).
>>>
>>> A previous discussion about this topic is available e.g. at
>>> http://thread.gmane.org/gmane.linux.scsi/105340/focus=105601.
>>>
>> Well, one could argue that the whole point of this patchset is to
>> allow you to serialize STPGs :-)
>>
>> We definitely need to serialize STPGs for the same target port
>> group; the current implementation is far too limited to take that
>> into account.
>>
>> But the main problem I'm facing with the current implementation is
>> that we cannot handle retries. An RTPG or an STPG might fail, at
>> which point we need to re-run RTPG to figure out the current status.
>> (We also need to send RTPGs when we receive an "ALUA state changed"
>>   UA, but that's slightly beside the point).
>> The retry cannot be send directly, as we're evaluating the status
>> from end_io context. So to instantiate a retry we need to move it
>> over to a workqueue.
>>
>> Or, at least, that's the solution I'm able to come up with.
>> If you have other ideas it'd be most welcome.
> 
> Hello Hannes,
> 
> I agree that retries have to be handled from workqueue context
> instead of end_io context. But in workqueue context we can choose
> whether to submit the retry synchronously or asynchronously. Unless
> I overlooked something I don't see why the retry should be submitted
> synchronously.
> 
Good point. Sure we can do this; the primary difference would be
whether the workqueue is ordered or not.
An unordered workqueue would be resulting in not serializing
retries, hence achieving pretty much what you asked.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/18] scsi_dh: move 'dh_state' sysfs attribute to generic code

[Martin K. Petersen]

>>>>> "Hannes" == Hannes Reinecke <hare@suse.de> writes:

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

This patch conflicts with 23695e41a1ca ("scsi_dh: fix use-after-free
when removing scsi device").

Looked simple enough and I started to fix it up but then decided to let
you do since you'll have to check it anyway.

Please rebase on top of latest 4.5/scsi-queue (which has your VPD rescan
patch in place).

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 01/18] scsi_dh: move 'dh_state' sysfs attribute to generic code

[Hannes Reinecke]

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

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 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(-)

diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
index e7649ed..54d446c 100644
--- a/drivers/scsi/scsi_dh.c
+++ b/drivers/scsi/scsi_dh.c
@@ -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.
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 4d01cdb1..27b4d0a 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -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
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 158f1b5..fc3cd26 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -17,6 +17,7 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_tcq.h>
+#include <scsi/scsi_dh.h>
 #include <scsi/scsi_transport.h>
 #include <scsi/scsi_driver.h>
 
@@ -904,6 +905,60 @@ sdev_show_function(queue_depth, "%d\n");
 static DEVICE_ATTR(queue_depth, S_IRUGO | S_IWUSR, sdev_show_queue_depth,
 		   sdev_store_queue_depth);
 
+#ifdef CONFIG_SCSI_DH
+static ssize_t
+sdev_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 ssize_t
+sdev_store_dh_state(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	int err = -EINVAL;
+
+	if (sdev->sdev_state == SDEV_CANCEL ||
+	    sdev->sdev_state == SDEV_DEL)
+		return -ENODEV;
+
+	if (!sdev->handler) {
+		/*
+		 * Attach to a device handler
+		 */
+		err = scsi_dh_attach(sdev->request_queue, buf);
+	} else if (!strncmp(buf, "activate", 8)) {
+		/*
+		 * Activate a device handler
+		 */
+		if (sdev->handler->activate)
+			err = sdev->handler->activate(sdev, NULL, NULL);
+		else
+			err = 0;
+	} 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;
+	}
+
+	return err < 0 ? err : count;
+}
+
+static DEVICE_ATTR(dh_state, S_IRUGO | S_IWUSR, sdev_show_dh_state,
+		   sdev_store_dh_state);
+#endif
+
 static ssize_t
 sdev_show_queue_ramp_up_period(struct device *dev,
 			       struct device_attribute *attr,
@@ -973,6 +1028,9 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_modalias.attr,
 	&dev_attr_queue_depth.attr,
 	&dev_attr_queue_type.attr,
+#ifdef CONFIG_SCSI_DH
+	&dev_attr_dh_state.attr,
+#endif
 	&dev_attr_queue_ramp_up_period.attr,
 	REF_EVT(media_change),
 	REF_EVT(inquiry_change_reported),
-- 
1.8.5.6