[PATCH 0/6] SCSI 'access_state' attribute

[PATCH 0/6] SCSI 'access_state' attribute

[Hannes Reinecke]

Hi all,

here's the patchset to add an 'access_state' and 'preferred_path'
attribute. It will display the access state of a path if a
hardware handler is attached.
The access_state is given in terms of SCSI ALUA, and
the vendor-specific access state (eg for rdac or alua)
are mapped onto the ALUA values.
Additionally the 'is_visible' callback is updated to
only display the attributes if they are supported.

Hannes Reinecke (6):
  scsi: Add 'access_state' and 'preferred_path' attribute
  scsi_dh_alua: use common definitions for ALUA state
  scsi_dh_alua: update 'access_state' field
  scsi_dh_rdac: update 'access_state' field
  scsi_dh_emc: update 'access_state' field
  scsi_sysfs: call 'device_add' after attaching device handler

 drivers/scsi/device_handler/scsi_dh_alua.c | 106 +++++++++++++++++++----------
 drivers/scsi/device_handler/scsi_dh_emc.c  |   7 +-
 drivers/scsi/device_handler/scsi_dh_rdac.c |  38 +++++++++--
 drivers/scsi/scsi_sysfs.c                  |  88 ++++++++++++++++++++++--
 include/scsi/scsi_device.h                 |   1 +
 include/scsi/scsi_proto.h                  |  12 ++++
 6 files changed, 202 insertions(+), 50 deletions(-)

-- 
2.6.2

[PATCH 1/6] scsi: Add 'access_state' and 'preferred_path' attribute

[Hannes Reinecke]

Add an 'access_state' field to struct scsi_device
and display them in sysfs as 'access_state' and
'preferred_path' attribute.

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

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index d8b275b..b6a1af8 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -81,6 +81,33 @@ const char *scsi_host_state_name(enum scsi_host_state state)
 	return name;
 }
 
+static const struct {
+	unsigned char	value;
+	char		*name;
+} sdev_access_states[] = {
+	{ SCSI_ACCESS_STATE_OPTIMAL, "active/optimized" },
+	{ SCSI_ACCESS_STATE_ACTIVE, "active/non-optimized" },
+	{ SCSI_ACCESS_STATE_STANDBY, "standby" },
+	{ SCSI_ACCESS_STATE_UNAVAILABLE, "unavailable" },
+	{ SCSI_ACCESS_STATE_LBA, "lba-dependent" },
+	{ SCSI_ACCESS_STATE_OFFLINE, "offline" },
+	{ SCSI_ACCESS_STATE_TRANSITIONING, "transitioning" },
+};
+
+const char *scsi_access_state_name(unsigned char 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;
@@ -973,6 +1000,43 @@ 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);
+	unsigned char access_state;
+	const char *access_state_name;
+
+	if (!sdev->handler)
+		return -EINVAL;
+
+	access_state = (sdev->access_state & SCSI_ACCESS_STATE_MASK);
+	access_state_name = scsi_access_state_name(access_state);
+
+	return snprintf(buf, 32, "%s\n",
+			access_state_name ? access_state_name : "unknown");
+}
+static DEVICE_ATTR(access_state, S_IRUGO, sdev_show_access_state, NULL);
+
+static ssize_t
+sdev_show_preferred_path(struct device *dev,
+			 struct device_attribute *attr,
+			 char *buf)
+{
+	struct scsi_device *sdev = to_scsi_device(dev);
+
+	if (!sdev->handler)
+		return -EINVAL;
+
+	if (sdev->access_state & SCSI_ACCESS_STATE_PREFERRED)
+		return snprintf(buf,2, "1\n");
+	else
+		return snprintf(buf, 2, "0\n");
+}
+static DEVICE_ATTR(preferred_path, S_IRUGO, sdev_show_preferred_path, NULL);
 #endif
 
 static ssize_t
@@ -1020,6 +1084,14 @@ static umode_t scsi_sdev_attr_is_visible(struct kobject *kobj,
 	    !sdev->host->hostt->change_queue_depth)
 		return 0;
 
+#ifdef CONFIG_SCSI_DH
+	if (attr == &dev_attr_access_state.attr &&
+	    !sdev->handler)
+		return 0;
+	if (attr == &dev_attr_preferred_path.attr &&
+	    !sdev->handler)
+		return 0;
+#endif
 	return attr->mode;
 }
 
@@ -1069,6 +1141,8 @@ static struct attribute *scsi_sdev_attrs[] = {
 	&dev_attr_wwid.attr,
 #ifdef CONFIG_SCSI_DH
 	&dev_attr_dh_state.attr,
+	&dev_attr_access_state.attr,
+	&dev_attr_preferred_path.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 4af2b24..c067019 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -201,6 +201,7 @@ struct scsi_device {
 	struct scsi_device_handler *handler;
 	void			*handler_data;
 
+	unsigned char		access_state;
 	enum scsi_device_state sdev_state;
 	unsigned long		sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long))));
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index a9fbf1b..c2ae21c 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -277,5 +277,17 @@ struct scsi_lun {
 	__u8 scsi_lun[8];
 };
 
+/* SPC asymmetric access states */
+#define SCSI_ACCESS_STATE_OPTIMAL     0x00
+#define SCSI_ACCESS_STATE_ACTIVE      0x01
+#define SCSI_ACCESS_STATE_STANDBY     0x02
+#define SCSI_ACCESS_STATE_UNAVAILABLE 0x03
+#define SCSI_ACCESS_STATE_LBA         0x04
+#define SCSI_ACCESS_STATE_OFFLINE     0x0e
+#define SCSI_ACCESS_STATE_TRANSITIONING 0x0f
+
+/* Values for REPORT TARGET GROUP STATES */
+#define SCSI_ACCESS_STATE_MASK        0x0f
+#define SCSI_ACCESS_STATE_PREFERRED   0x80
 
 #endif /* _SCSI_PROTO_H_ */
-- 
2.6.2

[PATCH 2/6] scsi_dh_alua: use common definitions for ALUA state

[Hannes Reinecke]

scsi_proto.h now contains definitions for the ALUA state,
so we don't have to carry them in the device handler.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/device_handler/scsi_dh_alua.c | 58 +++++++++++++-----------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 9a7657e..19f6539 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -31,14 +31,6 @@
 #define ALUA_DH_NAME "alua"
 #define ALUA_DH_VER "2.0"
 
-#define TPGS_STATE_OPTIMIZED		0x0
-#define TPGS_STATE_NONOPTIMIZED		0x1
-#define TPGS_STATE_STANDBY		0x2
-#define TPGS_STATE_UNAVAILABLE		0x3
-#define TPGS_STATE_LBA_DEPENDENT	0x4
-#define TPGS_STATE_OFFLINE		0xe
-#define TPGS_STATE_TRANSITIONING	0xf
-
 #define TPGS_SUPPORT_NONE		0x00
 #define TPGS_SUPPORT_OPTIMIZED		0x01
 #define TPGS_SUPPORT_NONOPTIMIZED	0x02
@@ -180,7 +172,7 @@ static int submit_stpg(struct scsi_device *sdev, int group_id,
 
 	/* Prepare the data buffer */
 	memset(stpg_data, 0, stpg_len);
-	stpg_data[4] = TPGS_STATE_OPTIMIZED & 0x0f;
+	stpg_data[4] = SCSI_ACCESS_STATE_OPTIMAL;
 	put_unaligned_be16(group_id, &stpg_data[6]);
 
 	/* Prepare the command. */
@@ -248,7 +240,7 @@ struct alua_port_group *alua_alloc_pg(struct scsi_device *sdev,
 	}
 	pg->group_id = group_id;
 	pg->tpgs = tpgs;
-	pg->state = TPGS_STATE_OPTIMIZED;
+	pg->state = SCSI_ACCESS_STATE_OPTIMAL;
 	if (optimize_stpg)
 		pg->flags |= ALUA_OPTIMIZE_STPG;
 	kref_init(&pg->kref);
@@ -378,22 +370,22 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
 	return SCSI_DH_OK;
 }
 
-static char print_alua_state(int state)
+static char print_alua_state(unsigned char state)
 {
 	switch (state) {
-	case TPGS_STATE_OPTIMIZED:
+	case SCSI_ACCESS_STATE_OPTIMAL:
 		return 'A';
-	case TPGS_STATE_NONOPTIMIZED:
+	case SCSI_ACCESS_STATE_ACTIVE:
 		return 'N';
-	case TPGS_STATE_STANDBY:
+	case SCSI_ACCESS_STATE_STANDBY:
 		return 'S';
-	case TPGS_STATE_UNAVAILABLE:
+	case SCSI_ACCESS_STATE_UNAVAILABLE:
 		return 'U';
-	case TPGS_STATE_LBA_DEPENDENT:
+	case SCSI_ACCESS_STATE_LBA:
 		return 'L';
-	case TPGS_STATE_OFFLINE:
+	case SCSI_ACCESS_STATE_OFFLINE:
 		return 'O';
-	case TPGS_STATE_TRANSITIONING:
+	case SCSI_ACCESS_STATE_TRANSITIONING:
 		return 'T';
 	default:
 		return 'X';
@@ -647,7 +639,7 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		    valid_states&TPGS_SUPPORT_OPTIMIZED?'A':'a');
 
 	switch (pg->state) {
-	case TPGS_STATE_TRANSITIONING:
+	case SCSI_ACCESS_STATE_TRANSITIONING:
 		if (time_before(jiffies, pg->expiry)) {
 			/* State transition, retry */
 			pg->interval = 2;
@@ -655,11 +647,11 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		} else {
 			/* Transitioning time exceeded, set port to standby */
 			err = SCSI_DH_IO;
-			pg->state = TPGS_STATE_STANDBY;
+			pg->state = SCSI_ACCESS_STATE_STANDBY;
 			pg->expiry = 0;
 		}
 		break;
-	case TPGS_STATE_OFFLINE:
+	case SCSI_ACCESS_STATE_OFFLINE:
 		/* Path unusable */
 		err = SCSI_DH_DEV_OFFLINED;
 		pg->expiry = 0;
@@ -693,20 +685,20 @@ static unsigned alua_stpg(struct scsi_device *sdev, struct alua_port_group *pg)
 		return SCSI_DH_RETRY;
 	}
 	switch (pg->state) {
-	case TPGS_STATE_OPTIMIZED:
+	case SCSI_ACCESS_STATE_OPTIMAL:
 		return SCSI_DH_OK;
-	case TPGS_STATE_NONOPTIMIZED:
+	case SCSI_ACCESS_STATE_ACTIVE:
 		if ((pg->flags & ALUA_OPTIMIZE_STPG) &&
 		    !pg->pref &&
 		    (pg->tpgs & TPGS_MODE_IMPLICIT))
 			return SCSI_DH_OK;
 		break;
-	case TPGS_STATE_STANDBY:
-	case TPGS_STATE_UNAVAILABLE:
+	case SCSI_ACCESS_STATE_STANDBY:
+	case SCSI_ACCESS_STATE_UNAVAILABLE:
 		break;
-	case TPGS_STATE_OFFLINE:
+	case SCSI_ACCESS_STATE_OFFLINE:
 		return SCSI_DH_IO;
-	case TPGS_STATE_TRANSITIONING:
+	case SCSI_ACCESS_STATE_TRANSITIONING:
 		break;
 	default:
 		sdev_printk(KERN_INFO, sdev,
@@ -760,7 +752,7 @@ static void alua_rtpg_work(struct work_struct *work)
 
 		pg->flags &= ~ALUA_PG_RUN_RTPG;
 		spin_unlock_irqrestore(&pg->lock, flags);
-		if (state == TPGS_STATE_TRANSITIONING) {
+		if (state == SCSI_ACCESS_STATE_TRANSITIONING) {
 			if (alua_tur(sdev) == SCSI_DH_RETRY) {
 				spin_lock_irqsave(&pg->lock, flags);
 				pg->flags &= ~ALUA_PG_RUNNING;
@@ -1006,7 +998,7 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
 {
 	struct alua_dh_data *h = sdev->handler_data;
 	struct alua_port_group __rcu *pg;
-	int state = TPGS_STATE_OPTIMIZED;
+	unsigned char state = SCSI_ACCESS_STATE_OPTIMAL;
 	int ret = BLKPREP_OK;
 
 	rcu_read_lock();
@@ -1014,11 +1006,11 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
 	if (pg)
 		state = pg->state;
 	rcu_read_unlock();
-	if (state == TPGS_STATE_TRANSITIONING)
+	if (state == SCSI_ACCESS_STATE_TRANSITIONING)
 		ret = BLKPREP_DEFER;
-	else if (state != TPGS_STATE_OPTIMIZED &&
-		 state != TPGS_STATE_NONOPTIMIZED &&
-		 state != TPGS_STATE_LBA_DEPENDENT) {
+	else if (state != SCSI_ACCESS_STATE_OPTIMAL &&
+		 state != SCSI_ACCESS_STATE_ACTIVE &&
+		 state != SCSI_ACCESS_STATE_LBA) {
 		ret = BLKPREP_KILL;
 		req->cmd_flags |= REQ_QUIET;
 	}
-- 
2.6.2

[PATCH 3/6] scsi_dh_alua: update 'access_state' field

[Hannes Reinecke]

Track attached SCSI devices and update the 'access_state' field
whenever an ALUA state change has been detected.

Reviewed-by: Bart Van Assche <bart.vanassche@sandisk.com>
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 | 48 ++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 19f6539..5bcdf8d 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -24,6 +24,7 @@
 #include <linux/module.h>
 #include <asm/unaligned.h>
 #include <scsi/scsi.h>
+#include <scsi/scsi_proto.h>
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_dh.h>
@@ -75,6 +76,7 @@ struct alua_port_group {
 	struct kref		kref;
 	struct rcu_head		rcu;
 	struct list_head	node;
+	struct list_head	dh_list;
 	unsigned char		device_id_str[256];
 	int			device_id_len;
 	int			group_id;
@@ -92,6 +94,7 @@ struct alua_port_group {
 };
 
 struct alua_dh_data {
+	struct list_head	node;
 	struct alua_port_group	*pg;
 	int			group_id;
 	spinlock_t		pg_lock;
@@ -247,6 +250,7 @@ struct alua_port_group *alua_alloc_pg(struct scsi_device *sdev,
 	INIT_DELAYED_WORK(&pg->rtpg_work, alua_rtpg_work);
 	INIT_LIST_HEAD(&pg->rtpg_list);
 	INIT_LIST_HEAD(&pg->node);
+	INIT_LIST_HEAD(&pg->dh_list);
 	spin_lock_init(&pg->lock);
 
 	spin_lock(&port_group_lock);
@@ -328,6 +332,8 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
 {
 	int rel_port = -1, group_id;
 	struct alua_port_group *pg, *old_pg = NULL;
+	bool pg_updated;
+	unsigned long flags;
 
 	group_id = scsi_vpd_tpg_id(sdev, &rel_port);
 	if (group_id < 0) {
@@ -357,10 +363,22 @@ static int alua_check_vpd(struct scsi_device *sdev, struct alua_dh_data *h,
 	old_pg = h->pg;
 	if (old_pg != pg) {
 		/* port group has changed. Update to new port group */
+		if (h->pg) {
+			spin_lock_irqsave(&old_pg->lock, flags);
+			list_del_rcu(&h->node);
+			spin_unlock_irqrestore(&old_pg->lock, flags);
+		}
 		rcu_assign_pointer(h->pg, pg);
+		pg_updated = true;
 	}
+
+	spin_lock_irqsave(&pg->lock, flags);
 	if (sdev->synchronous_alua)
 		pg->flags |= ALUA_SYNC_STPG;
+	if (pg_updated)
+		list_add_rcu(&h->node, &pg->dh_list);
+	spin_unlock_irqrestore(&pg->lock, flags);
+
 	alua_rtpg_queue(h->pg, sdev, NULL, true);
 	spin_unlock(&h->pg_lock);
 
@@ -613,8 +631,18 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 			if (spin_trylock_irqsave(&tmp_pg->lock, flags)) {
 				if ((tmp_pg == pg) ||
 				    !(tmp_pg->flags & ALUA_PG_RUNNING)) {
+					struct alua_dh_data *h;
+
 					tmp_pg->state = desc[0] & 0x0f;
 					tmp_pg->pref = desc[0] >> 7;
+					rcu_read_lock();
+					list_for_each_entry_rcu(h,
+						&tmp_pg->dh_list, node) {
+						/* h->sdev should always be valid */
+						BUG_ON(!h->sdev);
+						h->sdev->access_state = desc[0];
+					}
+					rcu_read_unlock();
 				}
 				if (tmp_pg == pg)
 					valid_states = desc[1];
@@ -645,10 +673,22 @@ static int alua_rtpg(struct scsi_device *sdev, struct alua_port_group *pg)
 			pg->interval = 2;
 			err = SCSI_DH_RETRY;
 		} else {
+			struct alua_dh_data *h;
+
 			/* Transitioning time exceeded, set port to standby */
 			err = SCSI_DH_IO;
 			pg->state = SCSI_ACCESS_STATE_STANDBY;
 			pg->expiry = 0;
+			rcu_read_lock();
+			list_for_each_entry_rcu(h, &pg->dh_list, node) {
+				BUG_ON(!h->sdev);
+				h->sdev->access_state =
+					(pg->state & SCSI_ACCESS_STATE_MASK);
+				if (pg->pref)
+					h->sdev->access_state |=
+						SCSI_ACCESS_STATE_PREFERRED;
+			}
+			rcu_read_unlock();
 		}
 		break;
 	case SCSI_ACCESS_STATE_OFFLINE:
@@ -1041,6 +1081,7 @@ static int alua_bus_attach(struct scsi_device *sdev)
 	rcu_assign_pointer(h->pg, NULL);
 	h->init_error = SCSI_DH_OK;
 	h->sdev = sdev;
+	INIT_LIST_HEAD(&h->node);
 
 	mutex_init(&h->init_mutex);
 	err = alua_initialize(sdev, h);
@@ -1070,9 +1111,12 @@ static void alua_bus_detach(struct scsi_device *sdev)
 	rcu_assign_pointer(h->pg, NULL);
 	h->sdev = NULL;
 	spin_unlock(&h->pg_lock);
-	if (pg)
+	if (pg) {
+		spin_lock(&pg->lock);
+		list_del_rcu(&h->node);
+		spin_unlock(&pg->lock);
 		kref_put(&pg->kref, release_port_group);
-
+	}
 	sdev->handler_data = NULL;
 	kfree(h);
 }
-- 
2.6.2

[PATCH 4/6] scsi_dh_rdac: update 'access_state' field

[Hannes Reinecke]

Track attached SCSI devices and update the 'access_state'
whenever the path state of the device changes.

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

diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index 93880ed..06fbd0b 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -165,6 +165,7 @@ struct rdac_controller {
 	struct work_struct	ms_work;
 	struct scsi_device	*ms_sdev;
 	struct list_head	ms_head;
+	struct list_head	dh_list;
 };
 
 struct c2_inquiry {
@@ -181,7 +182,9 @@ struct c2_inquiry {
 };
 
 struct rdac_dh_data {
+	struct list_head	node;
 	struct rdac_controller	*ctlr;
+	struct scsi_device	*sdev;
 #define UNINITIALIZED_LUN	(1 << 8)
 	unsigned		lun;
 
@@ -392,6 +395,7 @@ static struct rdac_controller *get_controller(int index, char *array_name,
 	INIT_WORK(&ctlr->ms_work, send_mode_select);
 	INIT_LIST_HEAD(&ctlr->ms_head);
 	list_add(&ctlr->node, &ctlr_list);
+	INIT_LIST_HEAD(&ctlr->dh_list);
 
 	return ctlr;
 }
@@ -455,7 +459,8 @@ static int get_lun_info(struct scsi_device *sdev, struct rdac_dh_data *h,
 
 static int check_ownership(struct scsi_device *sdev, struct rdac_dh_data *h)
 {
-	int err;
+	int err, access_state;
+	struct rdac_dh_data *tmp;
 	struct c9_inquiry *inqp;
 
 	h->state = RDAC_STATE_ACTIVE;
@@ -471,19 +476,31 @@ static int check_ownership(struct scsi_device *sdev, struct rdac_dh_data *h)
 			h->mode = RDAC_MODE; /* LUN in RDAC mode */
 
 		/* Update ownership */
-		if (inqp->avte_cvp & 0x1)
+		if (inqp->avte_cvp & 0x1) {
 			h->lun_state = RDAC_LUN_OWNED;
-		else {
+			access_state = SCSI_ACCESS_STATE_OPTIMAL;
+		} else {
 			h->lun_state = RDAC_LUN_UNOWNED;
-			if (h->mode == RDAC_MODE)
+			if (h->mode == RDAC_MODE) {
 				h->state = RDAC_STATE_PASSIVE;
+				access_state = SCSI_ACCESS_STATE_STANDBY;
+			} else
+				access_state = SCSI_ACCESS_STATE_ACTIVE;
 		}
 
 		/* Update path prio*/
-		if (inqp->path_prio & 0x1)
+		if (inqp->path_prio & 0x1) {
 			h->preferred = RDAC_PREFERRED;
-		else
+			access_state |= SCSI_ACCESS_STATE_PREFERRED;
+		} else
 			h->preferred = RDAC_NON_PREFERRED;
+		rcu_read_lock();
+		list_for_each_entry_rcu(tmp, &h->ctlr->dh_list, node) {
+			/* h->sdev should always be valid */
+			BUG_ON(!tmp->sdev);
+			tmp->sdev->access_state = access_state;
+		}
+		rcu_read_unlock();
 	}
 
 	return err;
@@ -508,6 +525,10 @@ static int initialize_controller(struct scsi_device *sdev,
 		h->ctlr = get_controller(index, array_name, array_id, sdev);
 		if (!h->ctlr)
 			err = SCSI_DH_RES_TEMP_UNAVAIL;
+		else {
+			list_add_rcu(&h->node, &h->ctlr->dh_list);
+			h->sdev = sdev;
+		}
 		spin_unlock(&list_lock);
 	}
 	return err;
@@ -829,8 +850,11 @@ static void rdac_bus_detach( struct scsi_device *sdev )
 		flush_workqueue(kmpath_rdacd);
 
 	spin_lock(&list_lock);
-	if (h->ctlr)
+	if (h->ctlr) {
+		list_del_rcu(&h->node);
+		h->sdev = NULL;
 		kref_put(&h->ctlr->kref, release_controller);
+	}
 	spin_unlock(&list_lock);
 	sdev->handler_data = NULL;
 	kfree(h);
-- 
2.6.2

[PATCH 5/6] scsi_dh_emc: update 'access_state' field

[Hannes Reinecke]

Update the 'access_state' field of the SCSI device whenever
the path state changes.

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

diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index e6fb97c..375d818 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -199,7 +199,12 @@ static int parse_sp_info_reply(struct scsi_device *sdev,
 	csdev->lun_state = csdev->buffer[4];
 	csdev->current_sp = csdev->buffer[8];
 	csdev->port = csdev->buffer[7];
-
+	if (csdev->lun_state == CLARIION_LUN_OWNED)
+		sdev->access_state = SCSI_ACCESS_STATE_OPTIMAL;
+	else
+		sdev->access_state = SCSI_ACCESS_STATE_STANDBY;
+	if (csdev->default_sp == csdev->current_sp)
+		sdev->access_state |= SCSI_ACCESS_STATE_PREFERRED;
 out:
 	return err;
 }
-- 
2.6.2

[PATCH 6/6] scsi_sysfs: call 'device_add' after attaching device handler

[Hannes Reinecke]

'device_add' will be evaluating the 'is_visible' callback
when creating the sysfs attributes. As by this time the
device handler has not been attached the 'access_state'
attribute will never be visible.

This patch moves the code around so that the device handler
is present by the time 'is_visible' is evaluated to
correctly display the 'access_state' attribute.

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

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index b6a1af8..4e7f944 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1226,13 +1226,6 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 
 	scsi_autopm_get_device(sdev);
 
-	error = device_add(&sdev->sdev_gendev);
-	if (error) {
-		sdev_printk(KERN_INFO, sdev,
-				"failed to add device: %d\n", error);
-		return error;
-	}
-
 	error = scsi_dh_add_device(sdev);
 	if (error)
 		/*
@@ -1241,6 +1234,13 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 		sdev_printk(KERN_INFO, sdev,
 				"failed to add device handler: %d\n", error);
 
+	error = device_add(&sdev->sdev_gendev);
+	if (error) {
+		sdev_printk(KERN_INFO, sdev,
+				"failed to add device: %d\n", error);
+		return error;
+	}
+
 	device_enable_async_suspend(&sdev->sdev_dev);
 	error = device_add(&sdev->sdev_dev);
 	if (error) {
-- 
2.6.2

RE: [PATCH 1/6] scsi: Add 'access_state' and 'preferred_path' attribute

[Seymour, Shane M]

in https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt

it says:

- show() must not use snprintf() when formatting the value to be
  returned to user space. If you can guarantee that an overflow
  will never happen you can use sprintf() otherwise you must use
  scnprintf().

> -----Original Message-----
> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi-
> owner@vger.kernel.org] On Behalf Of Hannes Reinecke
> Sent: Tuesday, March 01, 2016 3:56 PM
> To: Martin K. Petersen
> Cc: Christoph Hellwig; Ewan Milne; Bart van Assche; James Bottomley; linux-
> scsi@vger.kernel.org; Hannes Reinecke
> Subject: [PATCH 1/6] scsi: Add 'access_state' and 'preferred_path' attribute
> 
> Add an 'access_state' field to struct scsi_device and display them in sysfs as
> 'access_state' and 'preferred_path' attribute.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/scsi/scsi_sysfs.c  | 74
> ++++++++++++++++++++++++++++++++++++++++++++++
>  include/scsi/scsi_device.h |  1 +
>  include/scsi/scsi_proto.h  | 12 ++++++++
>  3 files changed, 87 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index
> d8b275b..b6a1af8 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -81,6 +81,33 @@ const char *scsi_host_state_name(enum
> scsi_host_state state)
>  	return name;
>  }
> 
> +static const struct {
> +	unsigned char	value;
> +	char		*name;
> +} sdev_access_states[] = {
> +	{ SCSI_ACCESS_STATE_OPTIMAL, "active/optimized" },
> +	{ SCSI_ACCESS_STATE_ACTIVE, "active/non-optimized" },
> +	{ SCSI_ACCESS_STATE_STANDBY, "standby" },
> +	{ SCSI_ACCESS_STATE_UNAVAILABLE, "unavailable" },
> +	{ SCSI_ACCESS_STATE_LBA, "lba-dependent" },
> +	{ SCSI_ACCESS_STATE_OFFLINE, "offline" },
> +	{ SCSI_ACCESS_STATE_TRANSITIONING, "transitioning" }, };
> +
> +const char *scsi_access_state_name(unsigned char 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;
> @@ -973,6 +1000,43 @@ 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);
> +	unsigned char access_state;
> +	const char *access_state_name;
> +
> +	if (!sdev->handler)
> +		return -EINVAL;
> +
> +	access_state = (sdev->access_state & SCSI_ACCESS_STATE_MASK);
> +	access_state_name = scsi_access_state_name(access_state);
> +
> +	return snprintf(buf, 32, "%s\n",
> +			access_state_name ? access_state_name :
> "unknown"); } static
> +DEVICE_ATTR(access_state, S_IRUGO, sdev_show_access_state, NULL);
> +
> +static ssize_t
> +sdev_show_preferred_path(struct device *dev,
> +			 struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct scsi_device *sdev = to_scsi_device(dev);
> +
> +	if (!sdev->handler)
> +		return -EINVAL;
> +
> +	if (sdev->access_state & SCSI_ACCESS_STATE_PREFERRED)
> +		return snprintf(buf,2, "1\n");
> +	else
> +		return snprintf(buf, 2, "0\n");
> +}
> +static DEVICE_ATTR(preferred_path, S_IRUGO,
> sdev_show_preferred_path,
> +NULL);
>  #endif
> 
>  static ssize_t
> @@ -1020,6 +1084,14 @@ static umode_t scsi_sdev_attr_is_visible(struct
> kobject *kobj,
>  	    !sdev->host->hostt->change_queue_depth)
>  		return 0;
> 
> +#ifdef CONFIG_SCSI_DH
> +	if (attr == &dev_attr_access_state.attr &&
> +	    !sdev->handler)
> +		return 0;
> +	if (attr == &dev_attr_preferred_path.attr &&
> +	    !sdev->handler)
> +		return 0;
> +#endif
>  	return attr->mode;
>  }
> 
> @@ -1069,6 +1141,8 @@ static struct attribute *scsi_sdev_attrs[] = {
>  	&dev_attr_wwid.attr,
>  #ifdef CONFIG_SCSI_DH
>  	&dev_attr_dh_state.attr,
> +	&dev_attr_access_state.attr,
> +	&dev_attr_preferred_path.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
> 4af2b24..c067019 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -201,6 +201,7 @@ struct scsi_device {
>  	struct scsi_device_handler *handler;
>  	void			*handler_data;
> 
> +	unsigned char		access_state;
>  	enum scsi_device_state sdev_state;
>  	unsigned long		sdev_data[0];
>  } __attribute__((aligned(sizeof(unsigned long)))); diff --git
> a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h index
> a9fbf1b..c2ae21c 100644
> --- a/include/scsi/scsi_proto.h
> +++ b/include/scsi/scsi_proto.h
> @@ -277,5 +277,17 @@ struct scsi_lun {
>  	__u8 scsi_lun[8];
>  };
> 
> +/* SPC asymmetric access states */
> +#define SCSI_ACCESS_STATE_OPTIMAL     0x00
> +#define SCSI_ACCESS_STATE_ACTIVE      0x01
> +#define SCSI_ACCESS_STATE_STANDBY     0x02
> +#define SCSI_ACCESS_STATE_UNAVAILABLE 0x03
> +#define SCSI_ACCESS_STATE_LBA         0x04
> +#define SCSI_ACCESS_STATE_OFFLINE     0x0e
> +#define SCSI_ACCESS_STATE_TRANSITIONING 0x0f
> +
> +/* Values for REPORT TARGET GROUP STATES */
> +#define SCSI_ACCESS_STATE_MASK        0x0f
> +#define SCSI_ACCESS_STATE_PREFERRED   0x80
> 
>  #endif /* _SCSI_PROTO_H_ */
> --
> 2.6.2
> 
> --
> 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/6] scsi: Add 'access_state' and 'preferred_path' attribute

[Hannes Reinecke]

On 03/01/2016 03:08 PM, Seymour, Shane M wrote:
> in https://www.kernel.org/doc/Documentation/filesystems/sysfs.txt
> 
> it says:
> 
> - show() must not use snprintf() when formatting the value to be
>   returned to user space. If you can guarantee that an overflow
>   will never happen you can use sprintf() otherwise you must use
>   scnprintf().
> 
So? I _do_ know the length of the strings I'm going to write into the
buffer, so there cannot possibly be an overflow.
Unless I've made an error during calculation of the string length, of
course. But both functions can only return fixed length strings, so I'm
safe 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/6] scsi: Add 'access_state' and 'preferred_path' attribute

[Christoph Hellwig]

On Tue, Mar 01, 2016 at 05:55:38AM +0100, Hannes Reinecke wrote:
> Add an 'access_state' field to struct scsi_device
> and display them in sysfs as 'access_state' and
> 'preferred_path' attribute.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Looks fine,

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

(I don't care about the *printf holy war for constant strings.  In all
 truth sysfs should switch to the seq_file API ASAP and end that misery..)

Re: [PATCH 6/6] scsi_sysfs: call 'device_add' after attaching device handler

[Christoph Hellwig]

Looks fine,

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

Re: [PATCH 4/6] scsi_dh_rdac: update 'access_state' field

[Christoph Hellwig]

On Tue, Mar 01, 2016 at 05:55:41AM +0100, Hannes Reinecke wrote:
> Track attached SCSI devices and update the 'access_state'
> whenever the path state of the device changes.

Looks fine,

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

Re: [PATCH 5/6] scsi_dh_emc: update 'access_state' field

[Christoph Hellwig]

On Tue, Mar 01, 2016 at 05:55:42AM +0100, Hannes Reinecke wrote:
> Update the 'access_state' field of the SCSI device whenever
> the path state changes.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Looks fine,

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

Re: [PATCH 2/6] scsi_dh_alua: use common definitions for ALUA state

[Bart Van Assche]

On 02/29/2016 08:55 PM, Hannes Reinecke wrote:
> scsi_proto.h now contains definitions for the ALUA state,
> so we don't have to carry them in the device handler.
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Reviewed-by: Bart van Assche <bart.vanassche@sandisk.com>

Re: [PATCH 6/6] scsi_sysfs: call 'device_add' after attaching device handler

[Bart Van Assche]

On 02/29/2016 08:55 PM, Hannes Reinecke wrote:
> @@ -1241,6 +1234,13 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
>   		sdev_printk(KERN_INFO, sdev,
>   				"failed to add device handler: %d\n", error);
>
> +	error = device_add(&sdev->sdev_gendev);
> +	if (error) {
> +		sdev_printk(KERN_INFO, sdev,
> +				"failed to add device: %d\n", error);
> +		return error;
> +	}
> +
>   	device_enable_async_suspend(&sdev->sdev_dev);
>   	error = device_add(&sdev->sdev_dev);
>   	if (error) {

Hello Hannes,

I think that scsi_dh_remove_device() should be called before returning 
if device_add() fails.

Bart.

Re: [PATCH 1/6] scsi: Add 'access_state' and 'preferred_path' attribute

[Bart Van Assche]

On 03/01/2016 05:06 AM, Christoph Hellwig wrote:
> (I don't care about the *printf holy war for constant strings.  In all
>   truth sysfs should switch to the seq_file API ASAP and end that misery..)

Agreed that it's not that important whether snprintf() or sprintf() is 
used. But sprintf() has an advantage, namely that it's not needed to 
specify the output buffer size. Which means fewer magic constants and 
hence easier to read code ...

Bart.