[PATCH 0/5] device handler interface update

[PATCH 0/5] device handler interface update

[Hannes Reinecke]

Hi all,

the current scsi_dh interface is very limited in functionality.
In particular it's not possible to update the internal state
without triggering a failover, and it not possible to figure
out the internal state of the device handler.

This patchset adds the functionality to rescan the device handler
if something has changed and adds a new sysfs attribute 'access_state'
to display the internal state.

To make this work the first patch implements functionality to
rescan the VPD information, as it might have changed, too.

The patchset is relative to my asynchronous ALUA update posted
earlier.

The entire tree can be found on git.kernel.org:

kernel/git/hare/scsi-devel.git branch alua.v3

As usual, reviews and comments are welcome.

Hannes Reinecke (5):
  scsi: rescan VPD attributes
  scsi_dh: add 'rescan' callback
  scsi: Add 'access_state' attribute
  scsi_dh_alua: add 'state' callback function
  scsi_dh_rdac: Add 'state' callback

 drivers/scsi/device_handler/scsi_dh_alua.c | 111 +++++++++++++++++++++++++----
 drivers/scsi/device_handler/scsi_dh_rdac.c |  17 +++++
 drivers/scsi/scsi.c                        |  20 +++++-
 drivers/scsi/scsi_lib.c                    |   1 +
 drivers/scsi/scsi_scan.c                   |  10 +++
 drivers/scsi/scsi_sysfs.c                  |  61 +++++++++++++++-
 drivers/scsi/ses.c                         |   6 +-
 include/scsi/scsi_device.h                 |  14 ++++
 include/scsi/scsi_dh.h                     |   2 +
 9 files changed, 220 insertions(+), 22 deletions(-)

-- 
1.8.5.2

[PATCH 1/5] scsi: rescan VPD attributes

[Hannes Reinecke]

This patch implements a VPD page rescan if the 'rescan' sysfs
attribute is triggered.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 11 +++++++----
 drivers/scsi/scsi.c                        | 20 +++++++++++++++++---
 drivers/scsi/scsi_scan.c                   |  4 ++++
 drivers/scsi/scsi_sysfs.c                  |  8 ++++++--
 drivers/scsi/ses.c                         |  6 ++++--
 include/scsi/scsi_device.h                 |  1 +
 6 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index d077014..e4cabc8 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -264,8 +264,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 	int device_id_size, device_id_type = 0;
 	struct alua_port_group *tmp_pg, *pg = NULL;
 
-	if (!sdev->vpd_pg83)
+	rcu_read_lock();
+	if (!rcu_dereference(sdev->vpd_pg83)) {
+		rcu_read_unlock();
 		return SCSI_DH_DEV_UNSUPP;
+	}
 
 	/*
 	 * Look for the correct descriptor.
@@ -281,8 +284,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 	 */
 	memset(device_id_str, 0, 256);
 	device_id_size = 0;
-	d = sdev->vpd_pg83 + 4;
-	while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
+	d = rcu_dereference(sdev->vpd_pg83) + 4;
+	while (d < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {
 		switch (d[1] & 0xf) {
 		case 0x2:
 			/* EUI-64 */
@@ -366,7 +369,7 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 		}
 		d += d[3] + 4;
 	}
-
+	rcu_read_unlock();
 	if (group_id == -1) {
 		/*
 		 * Internal error; TPGS supported but required
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 207d6a7..f1c0fb5 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -803,7 +803,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
 	int vpd_len = SCSI_VPD_PG_LEN;
 	int pg80_supported = 0;
 	int pg83_supported = 0;
-	unsigned char *vpd_buf;
+	unsigned char *vpd_buf, *orig_vpd_buf = NULL;
 
 	if (sdev->skip_vpd_pages)
 		return;
@@ -849,8 +849,16 @@ retry_pg80:
 			kfree(vpd_buf);
 			goto retry_pg80;
 		}
+		mutex_lock(&sdev->inquiry_mutex);
+		orig_vpd_buf = sdev->vpd_pg80;
 		sdev->vpd_pg80_len = result;
-		sdev->vpd_pg80 = vpd_buf;
+		rcu_assign_pointer(sdev->vpd_pg80, vpd_buf);
+		mutex_unlock(&sdev->inquiry_mutex);
+		synchronize_rcu();
+		if (orig_vpd_buf) {
+			kfree(orig_vpd_buf);
+			orig_vpd_buf = NULL;
+		}
 		vpd_len = SCSI_VPD_PG_LEN;
 	}
 
@@ -870,8 +878,14 @@ retry_pg83:
 			kfree(vpd_buf);
 			goto retry_pg83;
 		}
+		mutex_lock(&sdev->inquiry_mutex);
+		orig_vpd_buf = sdev->vpd_pg83;
 		sdev->vpd_pg83_len = result;
-		sdev->vpd_pg83 = vpd_buf;
+		rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
+		mutex_unlock(&sdev->inquiry_mutex);
+		synchronize_rcu();
+		if (orig_vpd_buf)
+			kfree(orig_vpd_buf);
 	}
 }
 
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index f9f3f82..190d743 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -235,6 +235,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	INIT_LIST_HEAD(&sdev->starved_entry);
 	INIT_LIST_HEAD(&sdev->event_list);
 	spin_lock_init(&sdev->list_lock);
+	mutex_init(&sdev->inquiry_mutex);
 	INIT_WORK(&sdev->event_work, scsi_evt_thread);
 	INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
 
@@ -1516,6 +1517,9 @@ EXPORT_SYMBOL(scsi_add_device);
 void scsi_rescan_device(struct device *dev)
 {
 	device_lock(dev);
+
+	scsi_attach_vpd(to_scsi_device(dev));
+
 	if (dev->driver && try_module_get(dev->driver->owner)) {
 		struct scsi_driver *drv = to_scsi_driver(dev->driver);
 
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index e3c3b86..b4de776 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -759,11 +759,15 @@ show_vpd_##_page(struct file *filp, struct kobject *kobj,	\
 {									\
 	struct device *dev = container_of(kobj, struct device, kobj);	\
 	struct scsi_device *sdev = to_scsi_device(dev);			\
+	int ret;							\
 	if (!sdev->vpd_##_page)						\
 		return -EINVAL;						\
-	return memory_read_from_buffer(buf, count, &off,		\
-				       sdev->vpd_##_page,		\
+	rcu_read_lock();						\
+	ret = memory_read_from_buffer(buf, count, &off,			\
+				      rcu_dereference(sdev->vpd_##_page), \
 				       sdev->vpd_##_page##_len);	\
+	rcu_read_unlock();						\
+	return ret;						\
 }									\
 static struct bin_attribute dev_attr_vpd_##_page = {		\
 	.attr =	{.name = __stringify(vpd_##_page), .mode = S_IRUGO },	\
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index dcb0d76..1fc372e 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -563,8 +563,9 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
 	if (!sdev->vpd_pg83_len)
 		return;
 
-	desc = sdev->vpd_pg83 + 4;
-	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
+	rcu_read_lock();
+	desc = rcu_dereference(sdev->vpd_pg83) + 4;
+	while (desc < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {
 		enum scsi_protocol proto = desc[0] >> 4;
 		u8 code_set = desc[0] & 0x0f;
 		u8 piv = desc[1] & 0x80;
@@ -578,6 +579,7 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
 
 		desc += len + 4;
 	}
+	rcu_read_unlock();
 	if (efd.addr) {
 		efd.dev = &sdev->sdev_gendev;
 
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 2725f8f..d2f8b7a 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -109,6 +109,7 @@ struct scsi_device {
 	char type;
 	char scsi_level;
 	char inq_periph_qual;	/* PQ from INQUIRY data */	
+	struct mutex inquiry_mutex;
 	unsigned char inquiry_len;	/* valid bytes in 'inquiry' */
 	unsigned char * inquiry;	/* INQUIRY response data */
 	const char * vendor;		/* [back_compat] point into 'inquiry' ... */
-- 
1.8.5.2

[PATCH 2/5] scsi_dh: add 'rescan' callback

[Hannes Reinecke]

If a device needs to be rescanned the device_handler might need
to be rechecked, too.
So add a 'rescan' callback to the device handler and call it
upon scsi_rescan_device().

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 53 +++++++++++++++++++++++-------
 drivers/scsi/scsi_lib.c                    |  1 +
 drivers/scsi/scsi_scan.c                   |  8 ++++-
 include/scsi/scsi_dh.h                     |  1 +
 4 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index e4cabc8..5b52017 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -262,7 +262,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 	int group_id = -1;
 	char device_id_str[256], *device_id = NULL;
 	int device_id_size, device_id_type = 0;
-	struct alua_port_group *tmp_pg, *pg = NULL;
+	struct alua_port_group *tmp_pg, *pg = NULL, *old_pg = NULL;
+	bool pg_found = false;
 
 	rcu_read_lock();
 	if (!rcu_dereference(sdev->vpd_pg83)) {
@@ -407,17 +408,25 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 		if (memcmp(tmp_pg->device_id, device_id,
 			   device_id_size))
 			continue;
-		kref_get(&tmp_pg->kref);
 		spin_lock(&h->pg_lock);
-		rcu_assign_pointer(h->pg, tmp_pg);
+		pg = rcu_dereference(h->pg);
+		if (pg) {
+			/*
+			 * This can happen if the VPD information changed
+			 */
+			if (tmp_pg != pg) {
+				old_pg = pg;
+				kref_get(&tmp_pg->kref);
+				rcu_assign_pointer(h->pg, tmp_pg);
+			}
+			pg_found = true;
+		}
 		spin_unlock(&h->pg_lock);
 		break;
 	}
 	spin_unlock(&port_group_lock);
-	if (h->pg) {
-		synchronize_rcu();
-		return SCSI_DH_OK;
-	}
+	if (pg_found)
+		goto out;
 
 	pg = kzalloc(sizeof(struct alua_port_group), GFP_KERNEL);
 	if (!pg) {
@@ -466,12 +475,17 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 		if (memcmp(tmp_pg->device_id, pg->device_id,
 			   device_id_size))
 			continue;
-		kref_get(&tmp_pg->kref);
 		spin_lock(&h->pg_lock);
-		rcu_assign_pointer(h->pg, tmp_pg);
+		pg = rcu_dereference(h->pg);
+		if (pg) {
+			if (tmp_pg != pg) {
+				old_pg = pg;
+				kref_get(&tmp_pg->kref);
+				rcu_assign_pointer(h->pg, tmp_pg);
+			}
+			pg = NULL;
+		}
 		spin_unlock(&h->pg_lock);
-		kfree(pg);
-		pg = NULL;
 		break;
 	}
 	if (pg) {
@@ -481,6 +495,13 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
 		spin_unlock(&h->pg_lock);
 	}
 	spin_unlock(&port_group_lock);
+out:
+	if (old_pg) {
+		synchronize_rcu();
+		if (old_pg->rtpg_sdev)
+			flush_workqueue(old_pg->work_q);
+		kref_put(&pg->kref, release_port_group);
+	}
 
 	return SCSI_DH_OK;
 }
@@ -1011,6 +1032,8 @@ static int alua_initialize(struct scsi_device *sdev, struct alua_dh_data *h)
 			kref_get(&pg->kref);
 			rcu_read_unlock();
 		}
+	} else {
+		WARN_ON(rcu_dereference(h->pg));
 	}
 	complete(&h->init_complete);
 	if (pg) {
@@ -1195,6 +1218,13 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
 
 }
 
+static void alua_rescan(struct scsi_device *sdev)
+{
+	struct alua_dh_data *h = sdev->handler_data;
+
+	alua_initialize(sdev, h);
+}
+
 /*
  * alua_bus_attach - Attach device handler
  * @sdev: device to be attached to
@@ -1256,6 +1286,7 @@ static struct scsi_device_handler alua_dh = {
 	.prep_fn = alua_prep_fn,
 	.check_sense = alua_check_sense,
 	.activate = alua_activate,
+	.rescan = alua_rescan,
 	.set_params = alua_set_params,
 };
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 18ab4ad..991b9e1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2704,6 +2704,7 @@ static void scsi_evt_emit(struct scsi_device *sdev, struct scsi_event *evt)
 		envp[idx++] = "SDEV_MEDIA_CHANGE=1";
 		break;
 	case SDEV_EVT_INQUIRY_CHANGE_REPORTED:
+		scsi_rescan_device(&sdev->sdev_gendev);
 		envp[idx++] = "SDEV_UA=INQUIRY_DATA_HAS_CHANGED";
 		break;
 	case SDEV_EVT_CAPACITY_CHANGE_REPORTED:
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 190d743..5d3e2ae 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -43,6 +43,7 @@
 #include <scsi/scsi_devinfo.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport.h>
+#include <scsi/scsi_dh.h>
 #include <scsi/scsi_eh.h>
 
 #include "scsi_priv.h"
@@ -1516,9 +1517,14 @@ EXPORT_SYMBOL(scsi_add_device);
 
 void scsi_rescan_device(struct device *dev)
 {
+	struct scsi_device *sdev = to_scsi_device(dev);
+
 	device_lock(dev);
 
-	scsi_attach_vpd(to_scsi_device(dev));
+	scsi_attach_vpd(sdev);
+
+	if (sdev->handler && sdev->handler->rescan)
+		sdev->handler->rescan(sdev);
 
 	if (dev->driver && try_module_get(dev->driver->owner)) {
 		struct scsi_driver *drv = to_scsi_driver(dev->driver);
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index 85d7317..5c73062 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -70,6 +70,7 @@ struct scsi_device_handler {
 	int (*activate)(struct scsi_device *, activate_complete, void *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
 	int (*set_params)(struct scsi_device *, const char *);
+	void (*rescan)(struct scsi_device *);
 };
 
 #ifdef CONFIG_SCSI_DH
-- 
1.8.5.2

[PATCH 3/5] scsi: Add 'access_state' attribute

[Hannes Reinecke]

Add an 'access_state' attribute to display the LUN access state.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi_sysfs.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 include/scsi/scsi_device.h | 13 ++++++++++++
 include/scsi/scsi_dh.h     |  1 +
 3 files changed, 67 insertions(+)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index b4de776..5f1a981 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -81,6 +81,34 @@ const char *scsi_host_state_name(enum scsi_host_state state)
 	return name;
 }
 
+static const struct {
+	enum scsi_access_state	value;
+	char			*name;
+} sdev_access_states[] = {
+	{ SCSI_ACCESS_STATE_UNKNOWN, "unknown" },
+	{ SCSI_ACCESS_STATE_OPTIMAL, "optimal" },
+	{ SCSI_ACCESS_STATE_ACTIVE, "active" },
+	{ SCSI_ACCESS_STATE_PASSIVE, "passive" },
+	{ SCSI_ACCESS_STATE_UNAVAILABLE, "unavailable" },
+	{ SCSI_ACCESS_STATE_LBA, "lba-dependent" },
+	{ SCSI_ACCESS_STATE_OFFLINE, "offline" },
+	{ SCSI_ACCESS_STATE_TRANSITIONING, "transitioning" },
+	{ SCSI_ACCESS_STATE_PREFERRED, "preferred" },
+};
+const char *scsi_access_state_name(enum scsi_access_state state)
+{
+	int i;
+	char *name = NULL;
+
+	for (i = 0; i < ARRAY_SIZE(sdev_access_states); i++) {
+		if (sdev_access_states[i].value == state) {
+			name = sdev_access_states[i].name;
+			break;
+		}
+	}
+	return name;
+}
+
 static int check_set(unsigned long long *val, char *src)
 {
 	char *last;
@@ -934,6 +962,30 @@ sdev_store_dh_state(struct device *dev, struct device_attribute *attr,
 
 static DEVICE_ATTR(dh_state, S_IRUGO | S_IWUSR, sdev_show_dh_state,
 		   sdev_store_dh_state);
+
+static ssize_t
+sdev_show_access_state(struct device *dev,
+		       struct device_attribute *attr,
+		       char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+	enum scsi_access_state access_state = SCSI_ACCESS_STATE_UNKNOWN;
+	bool pref = false;
+
+	if (sdev->handler && sdev->handler->state) {
+		int state = sdev->handler->state(sdev);
+
+		if (state & SCSI_ACCESS_STATE_PREFERRED)
+			pref = true;
+
+		access_state = (state & SCSI_ACCESS_STATE_MASK);
+	}
+
+	return snprintf(buf, 32, "%s%s\n",
+			scsi_access_state_name(access_state),
+			pref ? " preferred" :"");
+}
+static DEVICE_ATTR(access_state, S_IRUGO, sdev_show_access_state, NULL);
 #endif
 
 static ssize_t
@@ -1007,6 +1059,7 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_queue_type.attr,
 #ifdef CONFIG_SCSI_DH
 	&dev_attr_dh_state.attr,
+	&dev_attr_access_state.attr,
 #endif
 	&dev_attr_queue_ramp_up_period.attr,
 	REF_EVT(media_change),
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d2f8b7a..4a10737 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -50,6 +50,19 @@ enum scsi_device_state {
 	SDEV_CREATED_BLOCK,	/* same as above but for created devices */
 };
 
+enum scsi_access_state {
+	SCSI_ACCESS_STATE_UNKNOWN = 0,
+	SCSI_ACCESS_STATE_OPTIMAL,
+	SCSI_ACCESS_STATE_ACTIVE,
+	SCSI_ACCESS_STATE_PASSIVE,
+	SCSI_ACCESS_STATE_UNAVAILABLE,
+	SCSI_ACCESS_STATE_LBA,
+	SCSI_ACCESS_STATE_OFFLINE,
+	SCSI_ACCESS_STATE_TRANSITIONING,
+	SCSI_ACCESS_STATE_PREFERRED = 0x80
+};
+#define SCSI_ACCESS_STATE_MASK 0x7f
+
 enum scsi_device_event {
 	SDEV_EVT_MEDIA_CHANGE	= 1,	/* media has changed */
 	SDEV_EVT_INQUIRY_CHANGE_REPORTED,		/* 3F 03  UA reported */
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index 5c73062..bc1b9f0 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -71,6 +71,7 @@ struct scsi_device_handler {
 	int (*prep_fn)(struct scsi_device *, struct request *);
 	int (*set_params)(struct scsi_device *, const char *);
 	void (*rescan)(struct scsi_device *);
+	int (*state)(struct scsi_device *);
 };
 
 #ifdef CONFIG_SCSI_DH
-- 
1.8.5.2

[PATCH 4/5] scsi_dh_alua: add 'state' callback function

[Hannes Reinecke]

Add a 'state' callback function to display the current path
status in sysfs.

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

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 5b52017..99c4b0f 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -1182,6 +1182,52 @@ static void alua_check(struct scsi_device *sdev, bool force)
 }
 
 /*
+ * alua_state - report path status
+ * @sdev: device on the path to be checked
+ *
+ * Check the device status
+ */
+static int alua_state(struct scsi_device *sdev)
+{
+	struct alua_dh_data *h = sdev->handler_data;
+	struct alua_port_group *pg = NULL;
+	int access_state = SCSI_ACCESS_STATE_UNKNOWN;
+
+	rcu_read_lock();
+	if (h)
+		pg = rcu_dereference(h->pg);
+	if (pg) {
+		switch (pg->state) {
+		case TPGS_STATE_OPTIMIZED:
+			access_state = SCSI_ACCESS_STATE_OPTIMAL;
+			break;
+		case TPGS_STATE_NONOPTIMIZED:
+			access_state = SCSI_ACCESS_STATE_ACTIVE;
+			break;
+		case TPGS_STATE_STANDBY:
+			access_state = SCSI_ACCESS_STATE_PASSIVE;
+			break;
+		case TPGS_STATE_UNAVAILABLE:
+			access_state = SCSI_ACCESS_STATE_UNAVAILABLE;
+			break;
+		case TPGS_STATE_LBA_DEPENDENT:
+			access_state = SCSI_ACCESS_STATE_LBA;
+			break;
+		case TPGS_STATE_OFFLINE:
+			access_state = SCSI_ACCESS_STATE_OFFLINE;
+			break;
+		case TPGS_STATE_TRANSITIONING:
+			access_state = SCSI_ACCESS_STATE_TRANSITIONING;
+			break;
+		}
+		if (pg->pref)
+			access_state |= SCSI_ACCESS_STATE_PREFERRED;
+	}
+	rcu_read_unlock();
+	return access_state;
+}
+
+/*
  * alua_prep_fn - request callback
  *
  * Fail I/O to all paths not in state
@@ -1288,6 +1334,7 @@ static struct scsi_device_handler alua_dh = {
 	.activate = alua_activate,
 	.rescan = alua_rescan,
 	.set_params = alua_set_params,
+	.state = alua_state,
 };
 
 static int __init alua_init(void)
-- 
1.8.5.2

[PATCH 5/5] scsi_dh_rdac: Add 'state' callback

[Hannes Reinecke]

Add a 'state' callback to display the current LUN access state
to sysfs.

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

diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 3613581..151b736 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -834,6 +834,22 @@ static void rdac_bus_detach( struct scsi_device *sdev )
 	kfree(h);
 }
 
+static int rdac_state(struct scsi_device *sdev)
+{
+	struct rdac_dh_data *h = sdev->handler_data;
+	int access_state = SCSI_ACCESS_STATE_OPTIMAL;
+
+	if (h->lun_state == RDAC_LUN_UNOWNED)
+		access_state = SCSI_ACCESS_STATE_ACTIVE;
+	if (h->state == RDAC_STATE_PASSIVE)
+		access_state = SCSI_ACCESS_STATE_PASSIVE;
+
+	if (h->preferred == RDAC_PREFERRED)
+		access_state |= SCSI_ACCESS_STATE_PREFERRED;
+
+	return access_state;
+}
+
 static struct scsi_device_handler rdac_dh = {
 	.name = RDAC_NAME,
 	.module = THIS_MODULE,
@@ -842,6 +858,7 @@ static struct scsi_device_handler rdac_dh = {
 	.attach = rdac_bus_attach,
 	.detach = rdac_bus_detach,
 	.activate = rdac_activate,
+	.state = rdac_state,
 };
 
 static int __init rdac_init(void)
-- 
1.8.5.2

Re: [PATCH 3/5] scsi: Add 'access_state' attribute

[Christoph Hellwig]

On Wed, Jul 08, 2015 at 01:09:46PM +0200, Hannes Reinecke wrote:
> Add an 'access_state' attribute to display the LUN access state.

Should we just reuse the ALUA values here as they part of the spec
and map the legacy implemetation values to it?

I'd also really love to store the access_state variable in
struct scsi_device so we can perform the checks for it in core code
instead of the device handlers.

Re: [PATCH 3/5] scsi: Add 'access_state' attribute

[Hannes Reinecke]

On 07/09/2015 10:22 AM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2015 at 01:09:46PM +0200, Hannes Reinecke wrote:
>> Add an 'access_state' attribute to display the LUN access state.
> 
> Should we just reuse the ALUA values here as they part of the spec
> and map the legacy implemetation values to it?
> 
That's what I've attempted to; only I've displayed them as text,
not as a numeric value.
But sure, the idea was to take the ALUA values and map legacy
implementation onto it.

> I'd also really love to store the access_state variable in
> struct scsi_device so we can perform the checks for it in core code
> instead of the device handlers.
> 
That would be a good idea indeed. I had been pondering the idea of a
revamped multipath integration, where this would be come in handy.

I can easily adapt the patches here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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 1/5] scsi: rescan VPD attributes

[Christoph Hellwig]

On Wed, Jul 08, 2015 at 01:09:44PM +0200, Hannes Reinecke wrote:
> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
> @@ -264,8 +264,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
>  	int device_id_size, device_id_type = 0;
>  	struct alua_port_group *tmp_pg, *pg = NULL;
>  
> -	if (!sdev->vpd_pg83)
> +	rcu_read_lock();
> +	if (!rcu_dereference(sdev->vpd_pg83)) {
> +		rcu_read_unlock();
>  		return SCSI_DH_DEV_UNSUPP;
> +	}
>  
>  	/*
>  	 * Look for the correct descriptor.
> @@ -281,8 +284,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
>  	 */
>  	memset(device_id_str, 0, 256);
>  	device_id_size = 0;
> -	d = sdev->vpd_pg83 + 4;
> -	while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
> +	d = rcu_dereference(sdev->vpd_pg83) + 4;
> +	while (d < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {

Seem like this code would benefit from a local variable in favor of the
repeated rcu_dereference() calls.

> @@ -803,7 +803,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)

I think this function could use a new name, e.g. scsi_read_vpd_pages?

> @@ -563,8 +563,9 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
>  	if (!sdev->vpd_pg83_len)
>  		return;
>  
> -	desc = sdev->vpd_pg83 + 4;
> -	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
> +	rcu_read_lock();
> +	desc = rcu_dereference(sdev->vpd_pg83) + 4;
> +	while (desc < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {

A local variable or two would help here as well.

Re: [PATCH 1/5] scsi: rescan VPD attributes

[Hannes Reinecke]

On 07/24/2015 04:38 PM, Christoph Hellwig wrote:
> On Wed, Jul 08, 2015 at 01:09:44PM +0200, Hannes Reinecke wrote:
>> +++ b/drivers/scsi/device_handler/scsi_dh_alua.c
>> @@ -264,8 +264,11 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
>>  	int device_id_size, device_id_type = 0;
>>  	struct alua_port_group *tmp_pg, *pg = NULL;
>>  
>> -	if (!sdev->vpd_pg83)
>> +	rcu_read_lock();
>> +	if (!rcu_dereference(sdev->vpd_pg83)) {
>> +		rcu_read_unlock();
>>  		return SCSI_DH_DEV_UNSUPP;
>> +	}
>>  
>>  	/*
>>  	 * Look for the correct descriptor.
>> @@ -281,8 +284,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h)
>>  	 */
>>  	memset(device_id_str, 0, 256);
>>  	device_id_size = 0;
>> -	d = sdev->vpd_pg83 + 4;
>> -	while (d < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
>> +	d = rcu_dereference(sdev->vpd_pg83) + 4;
>> +	while (d < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {
> 
> Seem like this code would benefit from a local variable in favor of the
> repeated rcu_dereference() calls.
> 
Yeah, possibly. After all, the variable isn't expected to change
under rcu_read_lock().

>> @@ -803,7 +803,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
> 
> I think this function could use a new name, e.g. scsi_read_vpd_pages?
> 
>> @@ -563,8 +563,9 @@ static void ses_match_to_enclosure(struct enclosure_device *edev,
>>  	if (!sdev->vpd_pg83_len)
>>  		return;
>>  
>> -	desc = sdev->vpd_pg83 + 4;
>> -	while (desc < sdev->vpd_pg83 + sdev->vpd_pg83_len) {
>> +	rcu_read_lock();
>> +	desc = rcu_dereference(sdev->vpd_pg83) + 4;
>> +	while (desc < rcu_dereference(sdev->vpd_pg83) + sdev->vpd_pg83_len) {
> 
> A local variable or two would help here as well.
> 
Okay.

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 4/5] scsi_dh_alua: add 'state' callback function

[Christoph Hellwig]

As per the comment on patch 3 I'd rather expose the ALUA state in the core
SCSI code.  But having this alua_state attribute in core SCSI code sounds
fine to me.

Re: [PATCH 1/5] scsi: rescan VPD attributes

[Christoph Hellwig]

On Fri, Jul 24, 2015 at 04:40:42PM +0200, Hannes Reinecke wrote:
> Yeah, possibly. After all, the variable isn't expected to change
> under rcu_read_lock().

Actually it can and will change, that's the point.  But if you use
a local variable you keep a single version of it, which won't be
freed until after rcu_read_unlock.

Re: [PATCH 4/5] scsi_dh_alua: add 'state' callback function

[Hannes Reinecke]

On 07/24/2015 04:42 PM, Christoph Hellwig wrote:
> As per the comment on patch 3 I'd rather expose the ALUA state in the core
> SCSI code.  But having this alua_state attribute in core SCSI code sounds
> fine to me.
> 
Sure, we can have an attribute 'alua_state' in the core code; after all,
ALUA is pretty much standard by now.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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 1/5] scsi: rescan VPD attributes

[Hannes Reinecke]

On 07/24/2015 04:43 PM, Christoph Hellwig wrote:
> On Fri, Jul 24, 2015 at 04:40:42PM +0200, Hannes Reinecke wrote:
>> Yeah, possibly. After all, the variable isn't expected to change
>> under rcu_read_lock().
> 
> Actually it can and will change, that's the point.  But if you use
> a local variable you keep a single version of it, which won't be
> freed until after rcu_read_unlock.
> 
Yeah, I figured this as well. Will be updating the patch.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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