linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] target_core_mod: ALUA updates
@ 2013-10-16  7:20 Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit Hannes Reinecke
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

Hi Nic,

here are some updates to TCM ALUA handling. Apart from some
minor fixes and spellchecks the main features are:
- Make supported states configurable:
  We should make the list of supported ALUA states configurable,
  as some setups would possibly like to support a small subset
  of ALUA states only.
- Asynchronous transitioning: I've switched 'transitioning'
  handling to use a workqueue, that should allow us to simulate
  asynchronous transitioning modes. IE TCM should now be capable
  of handling requests while in transitioning, and properly terminate
  these with the correct sense code.
- Include target device descriptor in VPD page 83
  For the ALUA device handler we'd need to identify the target device
  where a given target port belongs to. So include the respective
  values in the VPD page.

Hannes Reinecke (11):
  target core: rename (ex,im)plict -> (ex,im)plicit
  target_core_alua: Store supported ALUA states
  target_core_alua: Make supported states configurable
  target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED
  target_core_alua: spellcheck
  target_core_alua: Validate ALUA state transition
  target_core_alua: Allocate ALUA metadata on demand
  target_core_alua: store old and pending ALUA state
  target_core_alua: Use workqueue for ALUA transitioning
  target_core: simplify scsi_name_len calculation
  target_core_spc: Include target device descriptor in VPD page 83

 drivers/target/target_core_alua.c      | 456 ++++++++++++++++++++-------------
 drivers/target/target_core_alua.h      |  36 ++-
 drivers/target/target_core_configfs.c  |  76 +++++-
 drivers/target/target_core_device.c    |   6 +-
 drivers/target/target_core_file.c      |   2 +-
 drivers/target/target_core_pr.c        |  24 +-
 drivers/target/target_core_spc.c       |  62 ++++-
 drivers/target/target_core_transport.c |   4 +-
 drivers/target/target_core_ua.h        |   2 +-
 include/target/target_core_base.h      |  14 +-
 10 files changed, 442 insertions(+), 240 deletions(-)

-- 
1.7.12.4

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16 21:13   ` Nicholas A. Bellinger
  2013-10-16  7:20 ` [PATCH 02/11] target_core_alua: Store supported ALUA states Hannes Reinecke
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c      | 110 ++++++++++++++++-----------------
 drivers/target/target_core_alua.h      |  20 +++---
 drivers/target/target_core_configfs.c  |  26 ++++----
 drivers/target/target_core_device.c    |   6 +-
 drivers/target/target_core_file.c      |   2 +-
 drivers/target/target_core_pr.c        |  24 +++----
 drivers/target/target_core_spc.c       |   6 +-
 drivers/target/target_core_transport.c |   4 +-
 drivers/target/target_core_ua.h        |   2 +-
 include/target/target_core_base.h      |   2 +-
 10 files changed, 101 insertions(+), 101 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 4724410..8297d37 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -44,7 +44,7 @@
 static sense_reason_t core_alua_check_transition(int state, int *primary);
 static int core_alua_set_tg_pt_secondary_state(
 		struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
-		struct se_port *port, int explict, int offline);
+		struct se_port *port, int explicit, int offline);
 
 static u16 alua_lu_gps_counter;
 static u32 alua_lu_gps_count;
@@ -175,7 +175,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
 	if (ext_hdr != 0) {
 		buf[4] = 0x10;
 		/*
-		 * Set the implict transition time (in seconds) for the application
+		 * Set the implicit transition time (in seconds) for the application
 		 * client to use as a base for it's transition timeout value.
 		 *
 		 * Use the current tg_pt_gp_mem -> tg_pt_gp membership from the LUN
@@ -188,7 +188,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
 			spin_lock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
 			tg_pt_gp = tg_pt_gp_mem->tg_pt_gp;
 			if (tg_pt_gp)
-				buf[5] = tg_pt_gp->tg_pt_gp_implict_trans_secs;
+				buf[5] = tg_pt_gp->tg_pt_gp_implicit_trans_secs;
 			spin_unlock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
 		}
 	}
@@ -199,7 +199,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
 }
 
 /*
- * SET_TARGET_PORT_GROUPS for explict ALUA operation.
+ * SET_TARGET_PORT_GROUPS for explicit ALUA operation.
  *
  * See spc4r17 section 6.35
  */
@@ -232,7 +232,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	/*
-	 * Determine if explict ALUA via SET_TARGET_PORT_GROUPS is allowed
+	 * Determine if explicit ALUA via SET_TARGET_PORT_GROUPS is allowed
 	 * for the local tg_pt_gp.
 	 */
 	l_tg_pt_gp_mem = l_port->sep_alua_tg_pt_gp_mem;
@@ -251,9 +251,9 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
 	}
 	spin_unlock(&l_tg_pt_gp_mem->tg_pt_gp_mem_lock);
 
-	if (!(l_tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_EXPLICT_ALUA)) {
+	if (!(l_tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_EXPLICIT_ALUA)) {
 		pr_debug("Unable to process SET_TARGET_PORT_GROUPS"
-				" while TPGS_EXPLICT_ALUA is disabled\n");
+				" while TPGS_EXPLICIT_ALUA is disabled\n");
 		rc = TCM_UNSUPPORTED_SCSI_OPCODE;
 		goto out;
 	}
@@ -620,7 +620,7 @@ out:
 }
 
 /*
- * Check implict and explict ALUA state change request.
+ * Check implicit and explicit ALUA state change request.
  */
 static sense_reason_t
 core_alua_check_transition(int state, int *primary)
@@ -676,10 +676,10 @@ char *core_alua_dump_status(int status)
 	switch (status) {
 	case ALUA_STATUS_NONE:
 		return "None";
-	case ALUA_STATUS_ALTERED_BY_EXPLICT_STPG:
-		return "Altered by Explict STPG";
-	case ALUA_STATUS_ALTERED_BY_IMPLICT_ALUA:
-		return "Altered by Implict ALUA";
+	case ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG:
+		return "Altered by Explicit STPG";
+	case ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA:
+		return "Altered by Implicit ALUA";
 	default:
 		return "Unknown";
 	}
@@ -770,7 +770,7 @@ static int core_alua_do_transition_tg_pt(
 	struct se_node_acl *nacl,
 	unsigned char *md_buf,
 	int new_state,
-	int explict)
+	int explicit)
 {
 	struct se_dev_entry *se_deve;
 	struct se_lun_acl *lacl;
@@ -784,9 +784,9 @@ static int core_alua_do_transition_tg_pt(
 	old_state = atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
 			ALUA_ACCESS_STATE_TRANSITION);
-	tg_pt_gp->tg_pt_gp_alua_access_status = (explict) ?
-				ALUA_STATUS_ALTERED_BY_EXPLICT_STPG :
-				ALUA_STATUS_ALTERED_BY_IMPLICT_ALUA;
+	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
+				ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
+				ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
 	/*
 	 * Check for the optional ALUA primary state transition delay
 	 */
@@ -821,12 +821,12 @@ static int core_alua_do_transition_tg_pt(
 			lacl = se_deve->se_lun_acl;
 			/*
 			 * se_deve->se_lun_acl pointer may be NULL for a
-			 * entry created without explict Node+MappedLUN ACLs
+			 * entry created without explicit Node+MappedLUN ACLs
 			 */
 			if (!lacl)
 				continue;
 
-			if (explict &&
+			if (explicit &&
 			   (nacl != NULL) && (nacl == lacl->se_lun_nacl) &&
 			   (l_port != NULL) && (l_port == port))
 				continue;
@@ -866,8 +866,8 @@ static int core_alua_do_transition_tg_pt(
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state, new_state);
 
 	pr_debug("Successful %s ALUA transition TG PT Group: %s ID: %hu"
-		" from primary access state %s to %s\n", (explict) ? "explict" :
-		"implict", config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item),
+		" from primary access state %s to %s\n", (explicit) ? "explicit" :
+		"implicit", config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item),
 		tg_pt_gp->tg_pt_gp_id, core_alua_dump_state(old_state),
 		core_alua_dump_state(new_state));
 
@@ -880,7 +880,7 @@ int core_alua_do_port_transition(
 	struct se_port *l_port,
 	struct se_node_acl *l_nacl,
 	int new_state,
-	int explict)
+	int explicit)
 {
 	struct se_device *dev;
 	struct se_port *port;
@@ -917,7 +917,7 @@ int core_alua_do_port_transition(
 		 * success.
 		 */
 		core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
-					md_buf, new_state, explict);
+					md_buf, new_state, explicit);
 		atomic_dec(&lu_gp->lu_gp_ref_cnt);
 		smp_mb__after_atomic_dec();
 		kfree(md_buf);
@@ -970,7 +970,7 @@ int core_alua_do_port_transition(
 			 * success.
 			 */
 			core_alua_do_transition_tg_pt(tg_pt_gp, port,
-					nacl, md_buf, new_state, explict);
+					nacl, md_buf, new_state, explicit);
 
 			spin_lock(&dev->t10_alua.tg_pt_gps_lock);
 			atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
@@ -987,7 +987,7 @@ int core_alua_do_port_transition(
 	pr_debug("Successfully processed LU Group: %s all ALUA TG PT"
 		" Group IDs: %hu %s transition to primary state: %s\n",
 		config_item_name(&lu_gp->lu_gp_group.cg_item),
-		l_tg_pt_gp->tg_pt_gp_id, (explict) ? "explict" : "implict",
+		l_tg_pt_gp->tg_pt_gp_id, (explicit) ? "explicit" : "implicit",
 		core_alua_dump_state(new_state));
 
 	atomic_dec(&lu_gp->lu_gp_ref_cnt);
@@ -1034,7 +1034,7 @@ static int core_alua_update_tpg_secondary_metadata(
 static int core_alua_set_tg_pt_secondary_state(
 	struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
 	struct se_port *port,
-	int explict,
+	int explicit,
 	int offline)
 {
 	struct t10_alua_tg_pt_gp *tg_pt_gp;
@@ -1061,13 +1061,13 @@ static int core_alua_set_tg_pt_secondary_state(
 		atomic_set(&port->sep_tg_pt_secondary_offline, 0);
 
 	md_buf_len = tg_pt_gp->tg_pt_gp_md_buf_len;
-	port->sep_tg_pt_secondary_stat = (explict) ?
-			ALUA_STATUS_ALTERED_BY_EXPLICT_STPG :
-			ALUA_STATUS_ALTERED_BY_IMPLICT_ALUA;
+	port->sep_tg_pt_secondary_stat = (explicit) ?
+			ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
+			ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
 
 	pr_debug("Successful %s ALUA transition TG PT Group: %s ID: %hu"
-		" to secondary access state: %s\n", (explict) ? "explict" :
-		"implict", config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item),
+		" to secondary access state: %s\n", (explicit) ? "explicit" :
+		"implicit", config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item),
 		tg_pt_gp->tg_pt_gp_id, (offline) ? "OFFLINE" : "ONLINE");
 
 	spin_unlock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
@@ -1356,16 +1356,16 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
 		ALUA_ACCESS_STATE_ACTIVE_OPTMIZED);
 	/*
-	 * Enable both explict and implict ALUA support by default
+	 * Enable both explicit and implicit ALUA support by default
 	 */
 	tg_pt_gp->tg_pt_gp_alua_access_type =
-			TPGS_EXPLICT_ALUA | TPGS_IMPLICT_ALUA;
+			TPGS_EXPLICIT_ALUA | TPGS_IMPLICIT_ALUA;
 	/*
 	 * Set the default Active/NonOptimized Delay in milliseconds
 	 */
 	tg_pt_gp->tg_pt_gp_nonop_delay_msecs = ALUA_DEFAULT_NONOP_DELAY_MSECS;
 	tg_pt_gp->tg_pt_gp_trans_delay_msecs = ALUA_DEFAULT_TRANS_DELAY_MSECS;
-	tg_pt_gp->tg_pt_gp_implict_trans_secs = ALUA_DEFAULT_IMPLICT_TRANS_SECS;
+	tg_pt_gp->tg_pt_gp_implicit_trans_secs = ALUA_DEFAULT_IMPLICIT_TRANS_SECS;
 
 	if (def_group) {
 		spin_lock(&dev->t10_alua.tg_pt_gps_lock);
@@ -1465,7 +1465,7 @@ void core_alua_free_tg_pt_gp(
 	 * been called from target_core_alua_drop_tg_pt_gp().
 	 *
 	 * Here we remove *tg_pt_gp from the global list so that
-	 * no assications *OR* explict ALUA via SET_TARGET_PORT_GROUPS
+	 * no assications *OR* explicit ALUA via SET_TARGET_PORT_GROUPS
 	 * can be made while we are releasing struct t10_alua_tg_pt_gp.
 	 */
 	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
@@ -1740,13 +1740,13 @@ ssize_t core_alua_show_access_type(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
 	char *page)
 {
-	if ((tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_EXPLICT_ALUA) &&
-	    (tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_IMPLICT_ALUA))
-		return sprintf(page, "Implict and Explict\n");
-	else if (tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_IMPLICT_ALUA)
-		return sprintf(page, "Implict\n");
-	else if (tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_EXPLICT_ALUA)
-		return sprintf(page, "Explict\n");
+	if ((tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_EXPLICIT_ALUA) &&
+	    (tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_IMPLICIT_ALUA))
+		return sprintf(page, "Implicit and Explicit\n");
+	else if (tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_IMPLICIT_ALUA)
+		return sprintf(page, "Implicit\n");
+	else if (tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_EXPLICIT_ALUA)
+		return sprintf(page, "Explicit\n");
 	else
 		return sprintf(page, "None\n");
 }
@@ -1771,11 +1771,11 @@ ssize_t core_alua_store_access_type(
 	}
 	if (tmp == 3)
 		tg_pt_gp->tg_pt_gp_alua_access_type =
-			TPGS_IMPLICT_ALUA | TPGS_EXPLICT_ALUA;
+			TPGS_IMPLICIT_ALUA | TPGS_EXPLICIT_ALUA;
 	else if (tmp == 2)
-		tg_pt_gp->tg_pt_gp_alua_access_type = TPGS_EXPLICT_ALUA;
+		tg_pt_gp->tg_pt_gp_alua_access_type = TPGS_EXPLICIT_ALUA;
 	else if (tmp == 1)
-		tg_pt_gp->tg_pt_gp_alua_access_type = TPGS_IMPLICT_ALUA;
+		tg_pt_gp->tg_pt_gp_alua_access_type = TPGS_IMPLICIT_ALUA;
 	else
 		tg_pt_gp->tg_pt_gp_alua_access_type = 0;
 
@@ -1844,14 +1844,14 @@ ssize_t core_alua_store_trans_delay_msecs(
 	return count;
 }
 
-ssize_t core_alua_show_implict_trans_secs(
+ssize_t core_alua_show_implicit_trans_secs(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
 	char *page)
 {
-	return sprintf(page, "%d\n", tg_pt_gp->tg_pt_gp_implict_trans_secs);
+	return sprintf(page, "%d\n", tg_pt_gp->tg_pt_gp_implicit_trans_secs);
 }
 
-ssize_t core_alua_store_implict_trans_secs(
+ssize_t core_alua_store_implicit_trans_secs(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
 	const char *page,
 	size_t count)
@@ -1861,16 +1861,16 @@ ssize_t core_alua_store_implict_trans_secs(
 
 	ret = kstrtoul(page, 0, &tmp);
 	if (ret < 0) {
-		pr_err("Unable to extract implict_trans_secs\n");
+		pr_err("Unable to extract implicit_trans_secs\n");
 		return ret;
 	}
-	if (tmp > ALUA_MAX_IMPLICT_TRANS_SECS) {
-		pr_err("Passed implict_trans_secs: %lu, exceeds"
-			" ALUA_MAX_IMPLICT_TRANS_SECS: %d\n", tmp,
-			ALUA_MAX_IMPLICT_TRANS_SECS);
+	if (tmp > ALUA_MAX_IMPLICIT_TRANS_SECS) {
+		pr_err("Passed implicit_trans_secs: %lu, exceeds"
+			" ALUA_MAX_IMPLICIT_TRANS_SECS: %d\n", tmp,
+			ALUA_MAX_IMPLICIT_TRANS_SECS);
 		return  -EINVAL;
 	}
-	tg_pt_gp->tg_pt_gp_implict_trans_secs = (int)tmp;
+	tg_pt_gp->tg_pt_gp_implicit_trans_secs = (int)tmp;
 
 	return count;
 }
@@ -1970,8 +1970,8 @@ ssize_t core_alua_store_secondary_status(
 		return ret;
 	}
 	if ((tmp != ALUA_STATUS_NONE) &&
-	    (tmp != ALUA_STATUS_ALTERED_BY_EXPLICT_STPG) &&
-	    (tmp != ALUA_STATUS_ALTERED_BY_IMPLICT_ALUA)) {
+	    (tmp != ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG) &&
+	    (tmp != ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA)) {
 		pr_err("Illegal value for alua_tg_pt_status: %lu\n",
 				tmp);
 		return -EINVAL;
diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
index e539c3e..74cf0c0 100644
--- a/drivers/target/target_core_alua.h
+++ b/drivers/target/target_core_alua.h
@@ -7,8 +7,8 @@
  * from spc4r17 section 6.4.2 Table 135
  */
 #define TPGS_NO_ALUA				0x00
-#define TPGS_IMPLICT_ALUA			0x10
-#define TPGS_EXPLICT_ALUA			0x20
+#define TPGS_IMPLICIT_ALUA			0x10
+#define TPGS_EXPLICIT_ALUA			0x20
 
 /*
  * ASYMMETRIC ACCESS STATE field
@@ -28,8 +28,8 @@
  * from spc4r17 section 6.27 Table 246
  */
 #define ALUA_STATUS_NONE				0x00
-#define ALUA_STATUS_ALTERED_BY_EXPLICT_STPG		0x01
-#define ALUA_STATUS_ALTERED_BY_IMPLICT_ALUA		0x02
+#define ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG		0x01
+#define ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA		0x02
 
 /*
  * From spc4r17, Table D.1: ASC and ASCQ Assignement
@@ -46,17 +46,17 @@
 #define ALUA_DEFAULT_NONOP_DELAY_MSECS			100
 #define ALUA_MAX_NONOP_DELAY_MSECS			10000 /* 10 seconds */
 /*
- * Used for implict and explict ALUA transitional delay, that is disabled
+ * Used for implicit and explicit ALUA transitional delay, that is disabled
  * by default, and is intended to be used for debugging client side ALUA code.
  */
 #define ALUA_DEFAULT_TRANS_DELAY_MSECS			0
 #define ALUA_MAX_TRANS_DELAY_MSECS			30000 /* 30 seconds */
 /*
- * Used for the recommended application client implict transition timeout
+ * Used for the recommended application client implicit transition timeout
  * in seconds, returned by the REPORT_TARGET_PORT_GROUPS w/ extended header.
  */
-#define ALUA_DEFAULT_IMPLICT_TRANS_SECS			0
-#define ALUA_MAX_IMPLICT_TRANS_SECS			255
+#define ALUA_DEFAULT_IMPLICIT_TRANS_SECS			0
+#define ALUA_MAX_IMPLICIT_TRANS_SECS			255
 /*
  * Used by core_alua_update_tpg_primary_metadata() and
  * core_alua_update_tpg_secondary_metadata()
@@ -113,9 +113,9 @@ extern ssize_t core_alua_show_trans_delay_msecs(struct t10_alua_tg_pt_gp *,
 					char *);
 extern ssize_t core_alua_store_trans_delay_msecs(struct t10_alua_tg_pt_gp *,
 					const char *, size_t);
-extern ssize_t core_alua_show_implict_trans_secs(struct t10_alua_tg_pt_gp *,
+extern ssize_t core_alua_show_implicit_trans_secs(struct t10_alua_tg_pt_gp *,
 					char *);
-extern ssize_t core_alua_store_implict_trans_secs(struct t10_alua_tg_pt_gp *,
+extern ssize_t core_alua_store_implicit_trans_secs(struct t10_alua_tg_pt_gp *,
 					const char *, size_t);
 extern ssize_t core_alua_show_preferred_bit(struct t10_alua_tg_pt_gp *,
 					char *);
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index 82e81c5..d4c28a3 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2036,7 +2036,7 @@ static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_access_state(
 	int new_state, ret;
 
 	if (!tg_pt_gp->tg_pt_gp_valid_id) {
-		pr_err("Unable to do implict ALUA on non valid"
+		pr_err("Unable to do implicit ALUA on non valid"
 			" tg_pt_gp ID: %hu\n", tg_pt_gp->tg_pt_gp_valid_id);
 		return -EINVAL;
 	}
@@ -2049,9 +2049,9 @@ static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_access_state(
 	}
 	new_state = (int)tmp;
 
-	if (!(tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_IMPLICT_ALUA)) {
-		pr_err("Unable to process implict configfs ALUA"
-			" transition while TPGS_IMPLICT_ALUA is disabled\n");
+	if (!(tg_pt_gp->tg_pt_gp_alua_access_type & TPGS_IMPLICIT_ALUA)) {
+		pr_err("Unable to process implicit configfs ALUA"
+			" transition while TPGS_IMPLICIT_ALUA is disabled\n");
 		return -EINVAL;
 	}
 
@@ -2097,8 +2097,8 @@ static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_access_status(
 	new_status = (int)tmp;
 
 	if ((new_status != ALUA_STATUS_NONE) &&
-	    (new_status != ALUA_STATUS_ALTERED_BY_EXPLICT_STPG) &&
-	    (new_status != ALUA_STATUS_ALTERED_BY_IMPLICT_ALUA)) {
+	    (new_status != ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG) &&
+	    (new_status != ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA)) {
 		pr_err("Illegal ALUA access status: 0x%02x\n",
 				new_status);
 		return -EINVAL;
@@ -2210,24 +2210,24 @@ static ssize_t target_core_alua_tg_pt_gp_store_attr_trans_delay_msecs(
 SE_DEV_ALUA_TG_PT_ATTR(trans_delay_msecs, S_IRUGO | S_IWUSR);
 
 /*
- * implict_trans_secs
+ * implicit_trans_secs
  */
-static ssize_t target_core_alua_tg_pt_gp_show_attr_implict_trans_secs(
+static ssize_t target_core_alua_tg_pt_gp_show_attr_implicit_trans_secs(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
 	char *page)
 {
-	return core_alua_show_implict_trans_secs(tg_pt_gp, page);
+	return core_alua_show_implicit_trans_secs(tg_pt_gp, page);
 }
 
-static ssize_t target_core_alua_tg_pt_gp_store_attr_implict_trans_secs(
+static ssize_t target_core_alua_tg_pt_gp_store_attr_implicit_trans_secs(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
 	const char *page,
 	size_t count)
 {
-	return core_alua_store_implict_trans_secs(tg_pt_gp, page, count);
+	return core_alua_store_implicit_trans_secs(tg_pt_gp, page, count);
 }
 
-SE_DEV_ALUA_TG_PT_ATTR(implict_trans_secs, S_IRUGO | S_IWUSR);
+SE_DEV_ALUA_TG_PT_ATTR(implicit_trans_secs, S_IRUGO | S_IWUSR);
 
 /*
  * preferred
@@ -2353,7 +2353,7 @@ static struct configfs_attribute *target_core_alua_tg_pt_gp_attrs[] = {
 	&target_core_alua_tg_pt_gp_alua_write_metadata.attr,
 	&target_core_alua_tg_pt_gp_nonop_delay_msecs.attr,
 	&target_core_alua_tg_pt_gp_trans_delay_msecs.attr,
-	&target_core_alua_tg_pt_gp_implict_trans_secs.attr,
+	&target_core_alua_tg_pt_gp_implicit_trans_secs.attr,
 	&target_core_alua_tg_pt_gp_preferred.attr,
 	&target_core_alua_tg_pt_gp_tg_pt_gp_id.attr,
 	&target_core_alua_tg_pt_gp_members.attr,
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index d90dbb0..d17e10d 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -314,14 +314,14 @@ int core_enable_device_list_for_node(
 	deve = nacl->device_list[mapped_lun];
 
 	/*
-	 * Check if the call is handling demo mode -> explict LUN ACL
+	 * Check if the call is handling demo mode -> explicit LUN ACL
 	 * transition.  This transition must be for the same struct se_lun
 	 * + mapped_lun that was setup in demo mode..
 	 */
 	if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
 		if (deve->se_lun_acl != NULL) {
 			pr_err("struct se_dev_entry->se_lun_acl"
-			       " already set for demo mode -> explict"
+			       " already set for demo mode -> explicit"
 			       " LUN ACL transition\n");
 			spin_unlock_irq(&nacl->device_list_lock);
 			return -EINVAL;
@@ -329,7 +329,7 @@ int core_enable_device_list_for_node(
 		if (deve->se_lun != lun) {
 			pr_err("struct se_dev_entry->se_lun does"
 			       " match passed struct se_lun for demo mode"
-			       " -> explict LUN ACL transition\n");
+			       " -> explicit LUN ACL transition\n");
 			spin_unlock_irq(&nacl->device_list_lock);
 			return -EINVAL;
 		}
diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index b662f89..0e34cda 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -562,7 +562,7 @@ fd_execute_rw(struct se_cmd *cmd, struct scatterlist *sgl, u32 sgl_nents,
 	} else {
 		ret = fd_do_rw(cmd, sgl, sgl_nents, 1);
 		/*
-		 * Perform implict vfs_fsync_range() for fd_do_writev() ops
+		 * Perform implicit vfs_fsync_range() for fd_do_writev() ops
 		 * for SCSI WRITEs with Forced Unit Access (FUA) set.
 		 * Allow this to happen independent of WCE=0 setting.
 		 */
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index d1ae4c5..2f5d779 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -474,7 +474,7 @@ static int core_scsi3_pr_seq_non_holder(
 	 * statement.
 	 */
 	if (!ret && !other_cdb) {
-		pr_debug("Allowing explict CDB: 0x%02x for %s"
+		pr_debug("Allowing explicit CDB: 0x%02x for %s"
 			" reservation holder\n", cdb[0],
 			core_scsi3_pr_dump_type(pr_reg_type));
 
@@ -507,7 +507,7 @@ static int core_scsi3_pr_seq_non_holder(
 			 */
 
 			if (!registered_nexus) {
-				pr_debug("Allowing implict CDB: 0x%02x"
+				pr_debug("Allowing implicit CDB: 0x%02x"
 					" for %s reservation on unregistered"
 					" nexus\n", cdb[0],
 					core_scsi3_pr_dump_type(pr_reg_type));
@@ -522,7 +522,7 @@ static int core_scsi3_pr_seq_non_holder(
 			 * allow commands from registered nexuses.
 			 */
 
-			pr_debug("Allowing implict CDB: 0x%02x for %s"
+			pr_debug("Allowing implicit CDB: 0x%02x for %s"
 				" reservation\n", cdb[0],
 				core_scsi3_pr_dump_type(pr_reg_type));
 
@@ -683,7 +683,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
 					alua_port_list) {
 			/*
 			 * This pointer will be NULL for demo mode MappedLUNs
-			 * that have not been make explict via a ConfigFS
+			 * that have not been make explicit via a ConfigFS
 			 * MappedLUN group for the SCSI Initiator Node ACL.
 			 */
 			if (!deve_tmp->se_lun_acl)
@@ -1158,7 +1158,7 @@ static void core_scsi3_put_pr_reg(struct t10_pr_registration *pr_reg)
 	smp_mb__after_atomic_dec();
 }
 
-static int core_scsi3_check_implict_release(
+static int core_scsi3_check_implicit_release(
 	struct se_device *dev,
 	struct t10_pr_registration *pr_reg)
 {
@@ -1174,7 +1174,7 @@ static int core_scsi3_check_implict_release(
 	}
 	if (pr_res_holder == pr_reg) {
 		/*
-		 * Perform an implict RELEASE if the registration that
+		 * Perform an implicit RELEASE if the registration that
 		 * is being released is holding the reservation.
 		 *
 		 * From spc4r17, section 5.7.11.1:
@@ -1192,7 +1192,7 @@ static int core_scsi3_check_implict_release(
 		 * For 'All Registrants' reservation types, all existing
 		 * registrations are still processed as reservation holders
 		 * in core_scsi3_pr_seq_non_holder() after the initial
-		 * reservation holder is implictly released here.
+		 * reservation holder is implicitly released here.
 		 */
 	} else if (pr_reg->pr_reg_all_tg_pt &&
 		  (!strcmp(pr_res_holder->pr_reg_nacl->initiatorname,
@@ -2125,7 +2125,7 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
 		/*
 		 * sa_res_key=0 Unregister Reservation Key for registered I_T Nexus.
 		 */
-		pr_holder = core_scsi3_check_implict_release(
+		pr_holder = core_scsi3_check_implicit_release(
 				cmd->se_dev, pr_reg);
 		if (pr_holder < 0) {
 			ret = TCM_RESERVATION_CONFLICT;
@@ -2402,7 +2402,7 @@ static void __core_scsi3_complete_pro_release(
 	struct se_device *dev,
 	struct se_node_acl *se_nacl,
 	struct t10_pr_registration *pr_reg,
-	int explict)
+	int explicit)
 {
 	struct target_core_fabric_ops *tfo = se_nacl->se_tpg->se_tpg_tfo;
 	char i_buf[PR_REG_ISID_ID_LEN];
@@ -2416,7 +2416,7 @@ static void __core_scsi3_complete_pro_release(
 
 	pr_debug("SPC-3 PR [%s] Service Action: %s RELEASE cleared"
 		" reservation holder TYPE: %s ALL_TG_PT: %d\n",
-		tfo->get_fabric_name(), (explict) ? "explict" : "implict",
+		tfo->get_fabric_name(), (explicit) ? "explicit" : "implicit",
 		core_scsi3_pr_dump_type(pr_reg->pr_res_type),
 		(pr_reg->pr_reg_all_tg_pt) ? 1 : 0);
 	pr_debug("SPC-3 PR [%s] RELEASE Node: %s%s\n",
@@ -2692,7 +2692,7 @@ static void __core_scsi3_complete_pro_preempt(
 	memset(i_buf, 0, PR_REG_ISID_ID_LEN);
 	core_pr_dump_initiator_port(pr_reg, i_buf, PR_REG_ISID_ID_LEN);
 	/*
-	 * Do an implict RELEASE of the existing reservation.
+	 * Do an implicit RELEASE of the existing reservation.
 	 */
 	if (dev->dev_pr_res_holder)
 		__core_scsi3_complete_pro_release(dev, nacl,
@@ -2845,7 +2845,7 @@ core_scsi3_pro_preempt(struct se_cmd *cmd, int type, int scope, u64 res_key,
 				 * 5.7.11.4 Preempting, Table 52 and Figure 7.
 				 *
 				 * For a ZERO SA Reservation key, release
-				 * all other registrations and do an implict
+				 * all other registrations and do an implicit
 				 * release of active persistent reservation.
 				 *
 				 * For a non-ZERO SA Reservation key, only
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 0745395..5ff69a5 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -48,7 +48,7 @@ static void spc_fill_alua_data(struct se_port *port, unsigned char *buf)
 	buf[5]	= 0x80;
 
 	/*
-	 * Set TPGS field for explict and/or implict ALUA access type
+	 * Set TPGS field for explicit and/or implicit ALUA access type
 	 * and opteration.
 	 *
 	 * See spc4r17 section 6.4.2 Table 135
@@ -1250,7 +1250,7 @@ spc_parse_cdb(struct se_cmd *cmd, unsigned int *size)
 		*size = (cdb[3] << 8) + cdb[4];
 
 		/*
-		 * Do implict HEAD_OF_QUEUE processing for INQUIRY.
+		 * Do implicit HEAD_OF_QUEUE processing for INQUIRY.
 		 * See spc4r17 section 5.3
 		 */
 		cmd->sam_task_attr = MSG_HEAD_TAG;
@@ -1284,7 +1284,7 @@ spc_parse_cdb(struct se_cmd *cmd, unsigned int *size)
 		cmd->execute_cmd = spc_emulate_report_luns;
 		*size = (cdb[6] << 24) | (cdb[7] << 16) | (cdb[8] << 8) | cdb[9];
 		/*
-		 * Do implict HEAD_OF_QUEUE processing for REPORT_LUNS
+		 * Do implicit HEAD_OF_QUEUE processing for REPORT_LUNS
 		 * See spc4r17 section 5.3
 		 */
 		cmd->sam_task_attr = MSG_HEAD_TAG;
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 81e945e..98bb7c4 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -473,7 +473,7 @@ void transport_deregister_session(struct se_session *se_sess)
 	pr_debug("TARGET_CORE[%s]: Deregistered fabric_sess\n",
 		se_tpg->se_tpg_tfo->get_fabric_name());
 	/*
-	 * If last kref is dropping now for an explict NodeACL, awake sleeping
+	 * If last kref is dropping now for an explicit NodeACL, awake sleeping
 	 * ->acl_free_comp caller to wakeup configfs se_node_acl->acl_group
 	 * removal context.
 	 */
@@ -668,7 +668,7 @@ void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
 		cmd->transport_state |= CMD_T_FAILED;
 
 	/*
-	 * Check for case where an explict ABORT_TASK has been received
+	 * Check for case where an explicit ABORT_TASK has been received
 	 * and transport_wait_for_tasks() will be waiting for completion..
 	 */
 	if (cmd->transport_state & CMD_T_ABORTED &&
diff --git a/drivers/target/target_core_ua.h b/drivers/target/target_core_ua.h
index 0204952..be912b3 100644
--- a/drivers/target/target_core_ua.h
+++ b/drivers/target/target_core_ua.h
@@ -19,7 +19,7 @@
 #define ASCQ_2AH_RESERVATIONS_RELEASED				0x04
 #define ASCQ_2AH_REGISTRATIONS_PREEMPTED			0x05
 #define ASCQ_2AH_ASYMMETRIC_ACCESS_STATE_CHANGED		0x06
-#define ASCQ_2AH_IMPLICT_ASYMMETRIC_ACCESS_STATE_TRANSITION_FAILED 0x07
+#define ASCQ_2AH_IMPLICIT_ASYMMETRIC_ACCESS_STATE_TRANSITION_FAILED 0x07
 #define ASCQ_2AH_PRIORITY_CHANGED				0x08
 
 #define ASCQ_2CH_PREVIOUS_RESERVATION_CONFLICT_STATUS		0x09
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 5bdb8b7..1c6e54d 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -286,7 +286,7 @@ struct t10_alua_tg_pt_gp {
 	int	tg_pt_gp_alua_access_type;
 	int	tg_pt_gp_nonop_delay_msecs;
 	int	tg_pt_gp_trans_delay_msecs;
-	int	tg_pt_gp_implict_trans_secs;
+	int	tg_pt_gp_implicit_trans_secs;
 	int	tg_pt_gp_pref;
 	int	tg_pt_gp_write_metadata;
 	/* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 02/11] target_core_alua: Store supported ALUA states
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16 21:19   ` Nicholas A. Bellinger
  2013-10-16  7:20 ` [PATCH 03/11] target_core_alua: Make supported states configurable Hannes Reinecke
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

The supported ALUA states might be different for individual
devices, so store it in a separate field.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 14 ++++++++------
 drivers/target/target_core_alua.h | 11 +++++++++++
 include/target/target_core_base.h |  1 +
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 8297d37..255e83c 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -117,12 +117,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
 		/*
 		 * Set supported ASYMMETRIC ACCESS State bits
 		 */
-		buf[off] = 0x80; /* T_SUP */
-		buf[off] |= 0x40; /* O_SUP */
-		buf[off] |= 0x8; /* U_SUP */
-		buf[off] |= 0x4; /* S_SUP */
-		buf[off] |= 0x2; /* AN_SUP */
-		buf[off++] |= 0x1; /* AO_SUP */
+		buf[off++] |= tg_pt_gp->tg_pt_gp_alua_supported_states;
 		/*
 		 * TARGET PORT GROUP
 		 */
@@ -1367,6 +1362,13 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
 	tg_pt_gp->tg_pt_gp_trans_delay_msecs = ALUA_DEFAULT_TRANS_DELAY_MSECS;
 	tg_pt_gp->tg_pt_gp_implicit_trans_secs = ALUA_DEFAULT_IMPLICIT_TRANS_SECS;
 
+	/*
+	 * Enable all supported states
+	 */
+	tg_pt_gp->tg_pt_gp_alua_supported_states =
+	    ALUA_T_SUP | ALUA_O_SUP | \
+	    ALUA_U_SUP | ALUA_S_SUP | ALUA_AN_SUP | ALUA_AO_SUP;
+
 	if (def_group) {
 		spin_lock(&dev->t10_alua.tg_pt_gps_lock);
 		tg_pt_gp->tg_pt_gp_id =
diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
index 74cf0c0..e826a65 100644
--- a/drivers/target/target_core_alua.h
+++ b/drivers/target/target_core_alua.h
@@ -23,6 +23,17 @@
 #define ALUA_ACCESS_STATE_TRANSITION		0xf
 
 /*
+ * from spc4r36j section 6.37 Table 306
+ */
+#define ALUA_T_SUP		0x80
+#define ALUA_O_SUP		0x40
+#define ALUA_LBD_SUP		0x10
+#define ALUA_U_SUP		0x08
+#define ALUA_S_SUP		0x04
+#define ALUA_AN_SUP		0x02
+#define ALUA_AO_SUP		0x01
+
+/*
  * REPORT_TARGET_PORT_GROUP STATUS CODE
  *
  * from spc4r17 section 6.27 Table 246
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 1c6e54d..21f4bd5 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -282,6 +282,7 @@ struct t10_alua_lu_gp_member {
 struct t10_alua_tg_pt_gp {
 	u16	tg_pt_gp_id;
 	int	tg_pt_gp_valid_id;
+	int	tg_pt_gp_alua_supported_states;
 	int	tg_pt_gp_alua_access_status;
 	int	tg_pt_gp_alua_access_type;
 	int	tg_pt_gp_nonop_delay_msecs;
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 03/11] target_core_alua: Make supported states configurable
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 02/11] target_core_alua: Store supported ALUA states Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16 21:38   ` Nicholas A. Bellinger
  2013-10-16  7:20 ` [PATCH 04/11] target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED Hannes Reinecke
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

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

diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index d4c28a3..53e9e00 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -2131,6 +2131,55 @@ static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_access_type(
 SE_DEV_ALUA_TG_PT_ATTR(alua_access_type, S_IRUGO | S_IWUSR);
 
 /*
+ * alua_supported_states
+ */
+static ssize_t target_core_alua_tg_pt_gp_show_attr_alua_supported_states(
+	struct t10_alua_tg_pt_gp *tg_pt_gp,
+	char *page)
+{
+	return sprintf(page, "%02x\n",
+		tg_pt_gp->tg_pt_gp_alua_supported_states);
+}
+
+static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_supported_states(
+	struct t10_alua_tg_pt_gp *tg_pt_gp,
+	const char *page,
+	size_t count)
+{
+	unsigned long tmp;
+	int new_states, valid_states, ret;
+
+	if (!tg_pt_gp->tg_pt_gp_valid_id) {
+		pr_err("Unable to do set supported ALUA states on non"
+			" valid tg_pt_gp ID: %hu\n",
+			tg_pt_gp->tg_pt_gp_valid_id);
+		return -EINVAL;
+	}
+
+	ret = strict_strtoul(page, 0, &tmp);
+	if (ret < 0) {
+		pr_err("Unable to extract new supported ALUA states"
+				" from %s\n", page);
+		return -EINVAL;
+	}
+	new_states = (int)tmp;
+	valid_states = ALUA_T_SUP | ALUA_O_SUP | ALUA_LBD_SUP | \
+	    ALUA_U_SUP | ALUA_S_SUP | ALUA_AN_SUP | ALUA_AO_SUP;
+
+
+	if (new_states & ~valid_states) {
+		pr_err("Illegal supported ALUA states: 0x%02x\n",
+				new_states);
+		return -EINVAL;
+	}
+
+	tg_pt_gp->tg_pt_gp_alua_supported_states = new_states;
+	return count;
+}
+
+SE_DEV_ALUA_TG_PT_ATTR(alua_supported_states, S_IRUGO | S_IWUSR);
+
+/*
  * alua_write_metadata
  */
 static ssize_t target_core_alua_tg_pt_gp_show_attr_alua_write_metadata(
@@ -2350,6 +2399,7 @@ static struct configfs_attribute *target_core_alua_tg_pt_gp_attrs[] = {
 	&target_core_alua_tg_pt_gp_alua_access_state.attr,
 	&target_core_alua_tg_pt_gp_alua_access_status.attr,
 	&target_core_alua_tg_pt_gp_alua_access_type.attr,
+	&target_core_alua_tg_pt_gp_alua_supported_states.attr,
 	&target_core_alua_tg_pt_gp_alua_write_metadata.attr,
 	&target_core_alua_tg_pt_gp_nonop_delay_msecs.attr,
 	&target_core_alua_tg_pt_gp_trans_delay_msecs.attr,
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 04/11] target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (2 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 03/11] target_core_alua: Make supported states configurable Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 05/11] target_core_alua: spellcheck Hannes Reinecke
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

Rename ALUA_ACCESS_STATE_OPTMIZED to
ALUA_ACCESS_STATE_OPTIMIZED.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 10 +++++-----
 drivers/target/target_core_alua.h |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 255e83c..593de80 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -561,12 +561,12 @@ target_alua_state_check(struct se_cmd *cmd)
 	nonop_delay_msecs = tg_pt_gp->tg_pt_gp_nonop_delay_msecs;
 	spin_unlock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
 	/*
-	 * Process ALUA_ACCESS_STATE_ACTIVE_OPTMIZED in a separate conditional
+	 * Process ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED in a separate conditional
 	 * statement so the compiler knows explicitly to check this case first.
 	 * For the Optimized ALUA access state case, we want to process the
 	 * incoming fabric cmd ASAP..
 	 */
-	if (out_alua_state == ALUA_ACCESS_STATE_ACTIVE_OPTMIZED)
+	if (out_alua_state == ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED)
 		return 0;
 
 	switch (out_alua_state) {
@@ -621,7 +621,7 @@ static sense_reason_t
 core_alua_check_transition(int state, int *primary)
 {
 	switch (state) {
-	case ALUA_ACCESS_STATE_ACTIVE_OPTMIZED:
+	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
 	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
 	case ALUA_ACCESS_STATE_STANDBY:
 	case ALUA_ACCESS_STATE_UNAVAILABLE:
@@ -649,7 +649,7 @@ core_alua_check_transition(int state, int *primary)
 static char *core_alua_dump_state(int state)
 {
 	switch (state) {
-	case ALUA_ACCESS_STATE_ACTIVE_OPTMIZED:
+	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
 		return "Active/Optimized";
 	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
 		return "Active/NonOptimized";
@@ -1349,7 +1349,7 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
 	tg_pt_gp->tg_pt_gp_dev = dev;
 	tg_pt_gp->tg_pt_gp_md_buf_len = ALUA_MD_BUF_LEN;
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
-		ALUA_ACCESS_STATE_ACTIVE_OPTMIZED);
+		ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED);
 	/*
 	 * Enable both explicit and implicit ALUA support by default
 	 */
diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
index e826a65..88e2e83 100644
--- a/drivers/target/target_core_alua.h
+++ b/drivers/target/target_core_alua.h
@@ -15,7 +15,7 @@
  *
  * from spc4r17 section 6.27 Table 245
  */
-#define ALUA_ACCESS_STATE_ACTIVE_OPTMIZED	0x0
+#define ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED	0x0
 #define ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED	0x1
 #define ALUA_ACCESS_STATE_STANDBY		0x2
 #define ALUA_ACCESS_STATE_UNAVAILABLE		0x3
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 05/11] target_core_alua: spellcheck
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (3 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 04/11] target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 06/11] target_core_alua: Validate ALUA state transition Hannes Reinecke
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 593de80..a16115e 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -325,7 +325,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
 			spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
 		} else {
 			/*
-			 * Extact the RELATIVE TARGET PORT IDENTIFIER to identify
+			 * Extract the RELATIVE TARGET PORT IDENTIFIER to identify
 			 * the Target Port in question for the the incoming
 			 * SET_TARGET_PORT_GROUPS op.
 			 */
@@ -482,7 +482,7 @@ static inline int core_alua_state_transition(
 	u8 *alua_ascq)
 {
 	/*
-	 * Allowed CDBs for ALUA_ACCESS_STATE_TRANSITIO as defined by
+	 * Allowed CDBs for ALUA_ACCESS_STATE_TRANSITION as defined by
 	 * spc4r17 section 5.9.2.5
 	 */
 	switch (cdb[0]) {
@@ -510,9 +510,9 @@ static inline int core_alua_state_transition(
 }
 
 /*
- * return 1: Is used to signal LUN not accecsable, and check condition/not ready
+ * return 1: Is used to signal LUN not accessible, and check condition/not ready
  * return 0: Used to signal success
- * reutrn -1: Used to signal failure, and invalid cdb field
+ * return -1: Used to signal failure, and invalid cdb field
  */
 sense_reason_t
 target_alua_state_check(struct se_cmd *cmd)
@@ -797,7 +797,7 @@ static int core_alua_do_transition_tg_pt(
 		 * change, a device server shall establish a unit attention
 		 * condition for the initiator port associated with every I_T
 		 * nexus with the additional sense code set to ASYMMETRIC
-		 * ACCESS STATE CHAGED.
+		 * ACCESS STATE CHANGED.
 		 *
 		 * After an explicit target port asymmetric access state
 		 * change, a device server shall establish a unit attention
@@ -941,7 +941,7 @@ int core_alua_do_port_transition(
 				continue;
 			/*
 			 * If the target behavior port asymmetric access state
-			 * is changed for any target port group accessiable via
+			 * is changed for any target port group accessible via
 			 * a logical unit within a LU group, the target port
 			 * behavior group asymmetric access states for the same
 			 * target port group accessible via other logical units
@@ -1227,7 +1227,7 @@ void core_alua_free_lu_gp(struct t10_alua_lu_gp *lu_gp)
 		 * struct se_device is released via core_alua_free_lu_gp_mem().
 		 *
 		 * If the passed lu_gp does NOT match the default_lu_gp, assume
-		 * we want to re-assocate a given lu_gp_mem with default_lu_gp.
+		 * we want to re-associate a given lu_gp_mem with default_lu_gp.
 		 */
 		spin_lock(&lu_gp_mem->lu_gp_mem_lock);
 		if (lu_gp != default_lu_gp)
@@ -1467,7 +1467,7 @@ void core_alua_free_tg_pt_gp(
 	 * been called from target_core_alua_drop_tg_pt_gp().
 	 *
 	 * Here we remove *tg_pt_gp from the global list so that
-	 * no assications *OR* explicit ALUA via SET_TARGET_PORT_GROUPS
+	 * no associations *OR* explicit ALUA via SET_TARGET_PORT_GROUPS
 	 * can be made while we are releasing struct t10_alua_tg_pt_gp.
 	 */
 	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
@@ -1503,7 +1503,7 @@ void core_alua_free_tg_pt_gp(
 		 * core_alua_free_tg_pt_gp_mem().
 		 *
 		 * If the passed tg_pt_gp does NOT match the default_tg_pt_gp,
-		 * assume we want to re-assocate a given tg_pt_gp_mem with
+		 * assume we want to re-associate a given tg_pt_gp_mem with
 		 * default_tg_pt_gp.
 		 */
 		spin_lock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 06/11] target_core_alua: Validate ALUA state transition
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (4 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 05/11] target_core_alua: spellcheck Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16 21:40   ` Nicholas A. Bellinger
  2013-10-16  7:20 ` [PATCH 07/11] target_core_alua: Allocate ALUA metadata on demand Hannes Reinecke
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

As we now can modify the list of supported states we need to
validate the requested ALUA state when doing a state transition.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 85 ++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 29 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index a16115e..a420778 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -41,7 +41,8 @@
 #include "target_core_alua.h"
 #include "target_core_ua.h"
 
-static sense_reason_t core_alua_check_transition(int state, int *primary);
+static sense_reason_t core_alua_check_transition(int state, int valid,
+						 int *primary);
 static int core_alua_set_tg_pt_secondary_state(
 		struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
 		struct se_port *port, int explicit, int offline);
@@ -210,7 +211,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
 	unsigned char *ptr;
 	sense_reason_t rc = TCM_NO_SENSE;
 	u32 len = 4; /* Skip over RESERVED area in header */
-	int alua_access_state, primary = 0;
+	int alua_access_state, primary = 0, valid_states;
 	u16 tg_pt_id, rtpi;
 
 	if (!l_port)
@@ -252,6 +253,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
 		rc = TCM_UNSUPPORTED_SCSI_OPCODE;
 		goto out;
 	}
+	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
 
 	ptr = &buf[4]; /* Skip over RESERVED area in header */
 
@@ -263,7 +265,8 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
 		 * the state is a primary or secondary target port asymmetric
 		 * access state.
 		 */
-		rc = core_alua_check_transition(alua_access_state, &primary);
+		rc = core_alua_check_transition(alua_access_state,
+						valid_states, &primary);
 		if (rc) {
 			/*
 			 * If the SET TARGET PORT GROUPS attempts to establish
@@ -614,21 +617,57 @@ out:
 	return 0;
 }
 
+static char *core_alua_dump_state(int state)
+{
+	switch (state) {
+	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
+		return "Active/Optimized";
+	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
+		return "Active/NonOptimized";
+	case ALUA_ACCESS_STATE_STANDBY:
+		return "Standby";
+	case ALUA_ACCESS_STATE_UNAVAILABLE:
+		return "Unavailable";
+	case ALUA_ACCESS_STATE_OFFLINE:
+		return "Offline";
+	case ALUA_ACCESS_STATE_TRANSITION:
+		return "Transitioning";
+	default:
+		return "Unknown";
+	}
+
+	return NULL;
+}
+
 /*
  * Check implicit and explicit ALUA state change request.
  */
 static sense_reason_t
-core_alua_check_transition(int state, int *primary)
+core_alua_check_transition(int state, int valid, int *primary)
 {
+	/*
+	 * OPTIMIZED, NON-OPTIMIZED, STANDBY and UNAVAILABLE are
+	 * defined as primary target port asymmetric access states.
+	 */
 	switch (state) {
 	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
+		if (!(valid & ALUA_AO_SUP))
+			goto not_supported;
+		*primary = 1;
+		break;
 	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
+		if (!(valid & ALUA_AN_SUP))
+			goto not_supported;
+		*primary = 1;
+		break;
 	case ALUA_ACCESS_STATE_STANDBY:
+		if (!(valid & ALUA_S_SUP))
+			goto not_supported;
+		*primary = 1;
+		break;
 	case ALUA_ACCESS_STATE_UNAVAILABLE:
-		/*
-		 * OPTIMIZED, NON-OPTIMIZED, STANDBY and UNAVAILABLE are
-		 * defined as primary target port asymmetric access states.
-		 */
+		if (!(valid & ALUA_U_SUP))
+			goto not_supported;
 		*primary = 1;
 		break;
 	case ALUA_ACCESS_STATE_OFFLINE:
@@ -636,6 +675,8 @@ core_alua_check_transition(int state, int *primary)
 		 * OFFLINE state is defined as a secondary target port
 		 * asymmetric access state.
 		 */
+		if (!(valid & ALUA_O_SUP))
+			goto not_supported;
 		*primary = 0;
 		break;
 	default:
@@ -644,26 +685,11 @@ core_alua_check_transition(int state, int *primary)
 	}
 
 	return 0;
-}
 
-static char *core_alua_dump_state(int state)
-{
-	switch (state) {
-	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
-		return "Active/Optimized";
-	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
-		return "Active/NonOptimized";
-	case ALUA_ACCESS_STATE_STANDBY:
-		return "Standby";
-	case ALUA_ACCESS_STATE_UNAVAILABLE:
-		return "Unavailable";
-	case ALUA_ACCESS_STATE_OFFLINE:
-		return "Offline";
-	default:
-		return "Unknown";
-	}
-
-	return NULL;
+not_supported:
+	pr_err("ALUA access state %s not supported",
+	       core_alua_dump_state(state));
+	return TCM_INVALID_PARAMETER_LIST;
 }
 
 char *core_alua_dump_status(int status)
@@ -884,9 +910,10 @@ int core_alua_do_port_transition(
 	struct t10_alua_lu_gp_member *lu_gp_mem, *local_lu_gp_mem;
 	struct t10_alua_tg_pt_gp *tg_pt_gp;
 	unsigned char *md_buf;
-	int primary;
+	int primary, valid_states;
 
-	if (core_alua_check_transition(new_state, &primary) != 0)
+	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
+	if (core_alua_check_transition(new_state, valid_states, &primary) != 0)
 		return -EINVAL;
 
 	md_buf = kzalloc(l_tg_pt_gp->tg_pt_gp_md_buf_len, GFP_KERNEL);
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 07/11] target_core_alua: Allocate ALUA metadata on demand
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (5 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 06/11] target_core_alua: Validate ALUA state transition Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16 21:42   ` Nicholas A. Bellinger
  2013-10-16  7:20 ` [PATCH 08/11] target_core_alua: store old and pending ALUA state Hannes Reinecke
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

We should only allocate ALUA metadata if we're actually going
to write them.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 70 +++++++++++++++++----------------------
 drivers/target/target_core_alua.h |  3 ++
 include/target/target_core_base.h |  3 --
 3 files changed, 34 insertions(+), 42 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index a420778..b1d08bf 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -762,16 +762,22 @@ static int core_alua_write_tpg_metadata(
  */
 static int core_alua_update_tpg_primary_metadata(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
-	int primary_state,
-	unsigned char *md_buf)
+	int primary_state)
 {
+	unsigned char *md_buf;
 	struct t10_wwn *wwn = &tg_pt_gp->tg_pt_gp_dev->t10_wwn;
 	char path[ALUA_METADATA_PATH_LEN];
-	int len;
+	int len, rc;
+
+	md_buf = kzalloc(ALUA_MD_BUF_LEN, GFP_KERNEL);
+	if (!md_buf) {
+		pr_err("Unable to allocate buf for ALUA metadata\n");
+		return -ENOMEM;
+	}
 
 	memset(path, 0, ALUA_METADATA_PATH_LEN);
 
-	len = snprintf(md_buf, tg_pt_gp->tg_pt_gp_md_buf_len,
+	len = snprintf(md_buf, ALUA_MD_BUF_LEN,
 			"tg_pt_gp_id=%hu\n"
 			"alua_access_state=0x%02x\n"
 			"alua_access_status=0x%02x\n",
@@ -782,14 +788,15 @@ static int core_alua_update_tpg_primary_metadata(
 		"/var/target/alua/tpgs_%s/%s", &wwn->unit_serial[0],
 		config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item));
 
-	return core_alua_write_tpg_metadata(path, md_buf, len);
+	rc = core_alua_write_tpg_metadata(path, md_buf, len);
+	kfree(md_buf);
+	return rc;
 }
 
 static int core_alua_do_transition_tg_pt(
 	struct t10_alua_tg_pt_gp *tg_pt_gp,
 	struct se_port *l_port,
 	struct se_node_acl *nacl,
-	unsigned char *md_buf,
 	int new_state,
 	int explicit)
 {
@@ -877,8 +884,7 @@ static int core_alua_do_transition_tg_pt(
 	 */
 	if (tg_pt_gp->tg_pt_gp_write_metadata) {
 		mutex_lock(&tg_pt_gp->tg_pt_gp_md_mutex);
-		core_alua_update_tpg_primary_metadata(tg_pt_gp,
-					new_state, md_buf);
+		core_alua_update_tpg_primary_metadata(tg_pt_gp, new_state);
 		mutex_unlock(&tg_pt_gp->tg_pt_gp_md_mutex);
 	}
 	/*
@@ -909,19 +915,12 @@ int core_alua_do_port_transition(
 	struct t10_alua_lu_gp *lu_gp;
 	struct t10_alua_lu_gp_member *lu_gp_mem, *local_lu_gp_mem;
 	struct t10_alua_tg_pt_gp *tg_pt_gp;
-	unsigned char *md_buf;
 	int primary, valid_states;
 
 	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
 	if (core_alua_check_transition(new_state, valid_states, &primary) != 0)
 		return -EINVAL;
 
-	md_buf = kzalloc(l_tg_pt_gp->tg_pt_gp_md_buf_len, GFP_KERNEL);
-	if (!md_buf) {
-		pr_err("Unable to allocate buf for ALUA metadata\n");
-		return -ENOMEM;
-	}
-
 	local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
 	spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
 	lu_gp = local_lu_gp_mem->lu_gp;
@@ -939,10 +938,9 @@ int core_alua_do_port_transition(
 		 * success.
 		 */
 		core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
-					md_buf, new_state, explicit);
+					new_state, explicit);
 		atomic_dec(&lu_gp->lu_gp_ref_cnt);
 		smp_mb__after_atomic_dec();
-		kfree(md_buf);
 		return 0;
 	}
 	/*
@@ -992,7 +990,7 @@ int core_alua_do_port_transition(
 			 * success.
 			 */
 			core_alua_do_transition_tg_pt(tg_pt_gp, port,
-					nacl, md_buf, new_state, explicit);
+					nacl, new_state, explicit);
 
 			spin_lock(&dev->t10_alua.tg_pt_gps_lock);
 			atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
@@ -1014,7 +1012,6 @@ int core_alua_do_port_transition(
 
 	atomic_dec(&lu_gp->lu_gp_ref_cnt);
 	smp_mb__after_atomic_dec();
-	kfree(md_buf);
 	return 0;
 }
 
@@ -1023,13 +1020,18 @@ int core_alua_do_port_transition(
  */
 static int core_alua_update_tpg_secondary_metadata(
 	struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
-	struct se_port *port,
-	unsigned char *md_buf,
-	u32 md_buf_len)
+	struct se_port *port)
 {
+	unsigned char *md_buf;
 	struct se_portal_group *se_tpg = port->sep_tpg;
 	char path[ALUA_METADATA_PATH_LEN], wwn[ALUA_SECONDARY_METADATA_WWN_LEN];
-	int len;
+	int len, rc;
+
+	md_buf = kzalloc(ALUA_MD_BUF_LEN, GFP_KERNEL);
+	if (!md_buf) {
+		pr_err("Unable to allocate buf for ALUA metadata\n");
+		return -ENOMEM;
+	}
 
 	memset(path, 0, ALUA_METADATA_PATH_LEN);
 	memset(wwn, 0, ALUA_SECONDARY_METADATA_WWN_LEN);
@@ -1041,7 +1043,7 @@ static int core_alua_update_tpg_secondary_metadata(
 		snprintf(wwn+len, ALUA_SECONDARY_METADATA_WWN_LEN-len, "+%hu",
 				se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg));
 
-	len = snprintf(md_buf, md_buf_len, "alua_tg_pt_offline=%d\n"
+	len = snprintf(md_buf, ALUA_MD_BUF_LEN, "alua_tg_pt_offline=%d\n"
 			"alua_tg_pt_status=0x%02x\n",
 			atomic_read(&port->sep_tg_pt_secondary_offline),
 			port->sep_tg_pt_secondary_stat);
@@ -1050,7 +1052,10 @@ static int core_alua_update_tpg_secondary_metadata(
 			se_tpg->se_tpg_tfo->get_fabric_name(), wwn,
 			port->sep_lun->unpacked_lun);
 
-	return core_alua_write_tpg_metadata(path, md_buf, len);
+	rc = core_alua_write_tpg_metadata(path, md_buf, len);
+	kfree(md_buf);
+
+	return rc;
 }
 
 static int core_alua_set_tg_pt_secondary_state(
@@ -1060,8 +1065,6 @@ static int core_alua_set_tg_pt_secondary_state(
 	int offline)
 {
 	struct t10_alua_tg_pt_gp *tg_pt_gp;
-	unsigned char *md_buf;
-	u32 md_buf_len;
 	int trans_delay_msecs;
 
 	spin_lock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
@@ -1082,7 +1085,6 @@ static int core_alua_set_tg_pt_secondary_state(
 	else
 		atomic_set(&port->sep_tg_pt_secondary_offline, 0);
 
-	md_buf_len = tg_pt_gp->tg_pt_gp_md_buf_len;
 	port->sep_tg_pt_secondary_stat = (explicit) ?
 			ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
 			ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
@@ -1104,18 +1106,9 @@ static int core_alua_set_tg_pt_secondary_state(
 	 * secondary state and status
 	 */
 	if (port->sep_tg_pt_secondary_write_md) {
-		md_buf = kzalloc(md_buf_len, GFP_KERNEL);
-		if (!md_buf) {
-			pr_err("Unable to allocate md_buf for"
-				" secondary ALUA access metadata\n");
-			return -ENOMEM;
-		}
 		mutex_lock(&port->sep_tg_pt_md_mutex);
-		core_alua_update_tpg_secondary_metadata(tg_pt_gp_mem, port,
-				md_buf, md_buf_len);
+		core_alua_update_tpg_secondary_metadata(tg_pt_gp_mem, port);
 		mutex_unlock(&port->sep_tg_pt_md_mutex);
-
-		kfree(md_buf);
 	}
 
 	return 0;
@@ -1374,7 +1367,6 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
 	spin_lock_init(&tg_pt_gp->tg_pt_gp_lock);
 	atomic_set(&tg_pt_gp->tg_pt_gp_ref_cnt, 0);
 	tg_pt_gp->tg_pt_gp_dev = dev;
-	tg_pt_gp->tg_pt_gp_md_buf_len = ALUA_MD_BUF_LEN;
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
 		ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED);
 	/*
diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
index 88e2e83..1a152cd 100644
--- a/drivers/target/target_core_alua.h
+++ b/drivers/target/target_core_alua.h
@@ -78,6 +78,9 @@
  */
 #define ALUA_SECONDARY_METADATA_WWN_LEN			256
 
+/* Used by core_alua_update_tpg_(primary,secondary)_metadata */
+#define ALUA_MD_BUF_LEN					1024
+
 extern struct kmem_cache *t10_alua_lu_gp_cache;
 extern struct kmem_cache *t10_alua_lu_gp_mem_cache;
 extern struct kmem_cache *t10_alua_tg_pt_gp_cache;
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 21f4bd5..933c59d 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -290,9 +290,6 @@ struct t10_alua_tg_pt_gp {
 	int	tg_pt_gp_implicit_trans_secs;
 	int	tg_pt_gp_pref;
 	int	tg_pt_gp_write_metadata;
-	/* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
-#define ALUA_MD_BUF_LEN				1024
-	u32	tg_pt_gp_md_buf_len;
 	u32	tg_pt_gp_members;
 	atomic_t tg_pt_gp_alua_access_state;
 	atomic_t tg_pt_gp_ref_cnt;
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 08/11] target_core_alua: store old and pending ALUA state
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (6 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 07/11] target_core_alua: Allocate ALUA metadata on demand Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16  7:20 ` [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning Hannes Reinecke
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

During state transition we should be storing both the original
and the pending state.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 15 ++++++++++-----
 include/target/target_core_base.h |  6 ++++--
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index b1d08bf..33e3f23 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -804,12 +804,15 @@ static int core_alua_do_transition_tg_pt(
 	struct se_lun_acl *lacl;
 	struct se_port *port;
 	struct t10_alua_tg_pt_gp_member *mem;
-	int old_state = 0;
+
 	/*
 	 * Save the old primary ALUA access state, and set the current state
 	 * to ALUA_ACCESS_STATE_TRANSITION.
 	 */
-	old_state = atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
+	tg_pt_gp->tg_pt_gp_alua_previous_state =
+		atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
+	tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
+
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
 			ALUA_ACCESS_STATE_TRANSITION);
 	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
@@ -890,13 +893,15 @@ static int core_alua_do_transition_tg_pt(
 	/*
 	 * Set the current primary ALUA access state to the requested new state
 	 */
-	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state, new_state);
+	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
+		   tg_pt_gp->tg_pt_gp_alua_pending_state);
 
 	pr_debug("Successful %s ALUA transition TG PT Group: %s ID: %hu"
 		" from primary access state %s to %s\n", (explicit) ? "explicit" :
 		"implicit", config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item),
-		tg_pt_gp->tg_pt_gp_id, core_alua_dump_state(old_state),
-		core_alua_dump_state(new_state));
+		tg_pt_gp->tg_pt_gp_id,
+		core_alua_dump_state(tg_pt_gp->tg_pt_gp_alua_previous_state),
+		core_alua_dump_state(tg_pt_gp->tg_pt_gp_alua_pending_state));
 
 	return 0;
 }
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 933c59d..67b6ca2 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -282,8 +282,10 @@ struct t10_alua_lu_gp_member {
 struct t10_alua_tg_pt_gp {
 	u16	tg_pt_gp_id;
 	int	tg_pt_gp_valid_id;
-	int	tg_pt_gp_alua_supported_states;
-	int	tg_pt_gp_alua_access_status;
+	u8	tg_pt_gp_alua_pending_state;
+	u8	tg_pt_gp_alua_previous_state;
+	u8	tg_pt_gp_alua_supported_states;
+	u8	tg_pt_gp_alua_access_status;
 	int	tg_pt_gp_alua_access_type;
 	int	tg_pt_gp_nonop_delay_msecs;
 	int	tg_pt_gp_trans_delay_msecs;
-- 
1.7.12.4


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (7 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 08/11] target_core_alua: store old and pending ALUA state Hannes Reinecke
@ 2013-10-16  7:20 ` Hannes Reinecke
  2013-10-16 22:06   ` Nicholas A. Bellinger
  2013-10-16 22:09 ` [PATCH 00/11] target_core_mod: ALUA updates Nicholas A. Bellinger
  2013-11-06 20:54 ` Nicholas A. Bellinger
  10 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-16  7:20 UTC (permalink / raw)
  To: Nic Bellinger; +Cc: target-devel, linux-scsi, Hannes Reinecke

Use a workqueue for processing ALUA state transitions; this allows
us to process implicit delay properly.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/target/target_core_alua.c | 174 +++++++++++++++++++++++++++-----------
 include/target/target_core_base.h |   4 +
 2 files changed, 128 insertions(+), 50 deletions(-)

diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index 33e3f23..166bee6 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -761,8 +761,7 @@ static int core_alua_write_tpg_metadata(
  * Called with tg_pt_gp->tg_pt_gp_md_mutex held
  */
 static int core_alua_update_tpg_primary_metadata(
-	struct t10_alua_tg_pt_gp *tg_pt_gp,
-	int primary_state)
+	struct t10_alua_tg_pt_gp *tg_pt_gp)
 {
 	unsigned char *md_buf;
 	struct t10_wwn *wwn = &tg_pt_gp->tg_pt_gp_dev->t10_wwn;
@@ -781,7 +780,8 @@ static int core_alua_update_tpg_primary_metadata(
 			"tg_pt_gp_id=%hu\n"
 			"alua_access_state=0x%02x\n"
 			"alua_access_status=0x%02x\n",
-			tg_pt_gp->tg_pt_gp_id, primary_state,
+			tg_pt_gp->tg_pt_gp_id,
+			tg_pt_gp->tg_pt_gp_alua_pending_state,
 			tg_pt_gp->tg_pt_gp_alua_access_status);
 
 	snprintf(path, ALUA_METADATA_PATH_LEN,
@@ -793,36 +793,17 @@ static int core_alua_update_tpg_primary_metadata(
 	return rc;
 }
 
-static int core_alua_do_transition_tg_pt(
-	struct t10_alua_tg_pt_gp *tg_pt_gp,
-	struct se_port *l_port,
-	struct se_node_acl *nacl,
-	int new_state,
-	int explicit)
+static void core_alua_do_transition_tg_pt_work(struct work_struct *work)
 {
+	struct t10_alua_tg_pt_gp *tg_pt_gp = container_of(work,
+		struct t10_alua_tg_pt_gp, tg_pt_gp_transition_work.work);
+	struct se_device *dev = tg_pt_gp->tg_pt_gp_dev;
 	struct se_dev_entry *se_deve;
 	struct se_lun_acl *lacl;
 	struct se_port *port;
 	struct t10_alua_tg_pt_gp_member *mem;
-
-	/*
-	 * Save the old primary ALUA access state, and set the current state
-	 * to ALUA_ACCESS_STATE_TRANSITION.
-	 */
-	tg_pt_gp->tg_pt_gp_alua_previous_state =
-		atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
-	tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
-
-	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
-			ALUA_ACCESS_STATE_TRANSITION);
-	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
-				ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
-				ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
-	/*
-	 * Check for the optional ALUA primary state transition delay
-	 */
-	if (tg_pt_gp->tg_pt_gp_trans_delay_msecs != 0)
-		msleep_interruptible(tg_pt_gp->tg_pt_gp_trans_delay_msecs);
+	bool explicit = (tg_pt_gp->tg_pt_gp_alua_access_status ==
+			 ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG);
 
 	spin_lock(&tg_pt_gp->tg_pt_gp_lock);
 	list_for_each_entry(mem, &tg_pt_gp->tg_pt_gp_mem_list,
@@ -857,9 +838,12 @@ static int core_alua_do_transition_tg_pt(
 			if (!lacl)
 				continue;
 
-			if (explicit &&
-			   (nacl != NULL) && (nacl == lacl->se_lun_nacl) &&
-			   (l_port != NULL) && (l_port == port))
+			if ((tg_pt_gp->tg_pt_gp_alua_access_status ==
+			     ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG) &&
+			   (tg_pt_gp->tg_pt_gp_alua_nacl != NULL) &&
+			    (tg_pt_gp->tg_pt_gp_alua_nacl == lacl->se_lun_nacl) &&
+			   (tg_pt_gp->tg_pt_gp_alua_port != NULL) &&
+			    (tg_pt_gp->tg_pt_gp_alua_port == port))
 				continue;
 
 			core_scsi3_ua_allocate(lacl->se_lun_nacl,
@@ -887,7 +871,7 @@ static int core_alua_do_transition_tg_pt(
 	 */
 	if (tg_pt_gp->tg_pt_gp_write_metadata) {
 		mutex_lock(&tg_pt_gp->tg_pt_gp_md_mutex);
-		core_alua_update_tpg_primary_metadata(tg_pt_gp, new_state);
+		core_alua_update_tpg_primary_metadata(tg_pt_gp);
 		mutex_unlock(&tg_pt_gp->tg_pt_gp_md_mutex);
 	}
 	/*
@@ -902,6 +886,87 @@ static int core_alua_do_transition_tg_pt(
 		tg_pt_gp->tg_pt_gp_id,
 		core_alua_dump_state(tg_pt_gp->tg_pt_gp_alua_previous_state),
 		core_alua_dump_state(tg_pt_gp->tg_pt_gp_alua_pending_state));
+	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
+	atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
+	smp_mb__after_atomic_dec();
+	spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
+
+	if (tg_pt_gp->tg_pt_gp_transition_complete)
+		complete(tg_pt_gp->tg_pt_gp_transition_complete);
+}
+
+static int core_alua_do_transition_tg_pt(
+	struct t10_alua_tg_pt_gp *tg_pt_gp,
+	int new_state,
+	int explicit)
+{
+	struct se_device *dev = tg_pt_gp->tg_pt_gp_dev;
+	DECLARE_COMPLETION_ONSTACK(wait);
+
+	/* Nothing to be done here */
+	if (atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) == new_state)
+		return 0;
+
+	if (new_state == ALUA_ACCESS_STATE_TRANSITION)
+		return -EAGAIN;
+
+	/*
+	 * Flush any pending transitions
+	 */
+	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs &&
+	    atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) ==
+	    ALUA_ACCESS_STATE_TRANSITION) {
+		/* Just in case */
+		tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
+		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
+		flush_delayed_work(&tg_pt_gp->tg_pt_gp_transition_work);
+		wait_for_completion(&wait);
+		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
+		return 0;
+	}
+
+	/*
+	 * Save the old primary ALUA access state, and set the current state
+	 * to ALUA_ACCESS_STATE_TRANSITION.
+	 */
+	tg_pt_gp->tg_pt_gp_alua_previous_state =
+		atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
+	tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
+
+	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
+			ALUA_ACCESS_STATE_TRANSITION);
+	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
+				ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
+				ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
+
+	/*
+	 * Check for the optional ALUA primary state transition delay
+	 */
+	if (tg_pt_gp->tg_pt_gp_trans_delay_msecs != 0)
+		msleep_interruptible(tg_pt_gp->tg_pt_gp_trans_delay_msecs);
+
+	/*
+	 * Take a reference for workqueue item
+	 */
+	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
+	atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
+	smp_mb__after_atomic_inc();
+	spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
+
+	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs) {
+		unsigned long transition_tmo;
+
+		transition_tmo = tg_pt_gp->tg_pt_gp_implicit_trans_secs * HZ;
+		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
+				   &tg_pt_gp->tg_pt_gp_transition_work,
+				   transition_tmo);
+	} else {
+		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
+		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
+				   &tg_pt_gp->tg_pt_gp_transition_work, 0);
+		wait_for_completion(&wait);
+		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
+	}
 
 	return 0;
 }
@@ -915,12 +980,10 @@ int core_alua_do_port_transition(
 	int explicit)
 {
 	struct se_device *dev;
-	struct se_port *port;
-	struct se_node_acl *nacl;
 	struct t10_alua_lu_gp *lu_gp;
 	struct t10_alua_lu_gp_member *lu_gp_mem, *local_lu_gp_mem;
 	struct t10_alua_tg_pt_gp *tg_pt_gp;
-	int primary, valid_states;
+	int primary, valid_states, rc = 0;
 
 	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
 	if (core_alua_check_transition(new_state, valid_states, &primary) != 0)
@@ -942,11 +1005,13 @@ int core_alua_do_port_transition(
 		 * core_alua_do_transition_tg_pt() will always return
 		 * success.
 		 */
-		core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
-					new_state, explicit);
+		l_tg_pt_gp->tg_pt_gp_alua_port = l_port;
+		l_tg_pt_gp->tg_pt_gp_alua_nacl = l_nacl;
+		rc = core_alua_do_transition_tg_pt(l_tg_pt_gp,
+						   new_state, explicit);
 		atomic_dec(&lu_gp->lu_gp_ref_cnt);
 		smp_mb__after_atomic_dec();
-		return 0;
+		return rc;
 	}
 	/*
 	 * For all other LU groups aside from 'default_lu_gp', walk all of
@@ -981,11 +1046,11 @@ int core_alua_do_port_transition(
 				continue;
 
 			if (l_tg_pt_gp == tg_pt_gp) {
-				port = l_port;
-				nacl = l_nacl;
+				tg_pt_gp->tg_pt_gp_alua_port = l_port;
+				tg_pt_gp->tg_pt_gp_alua_nacl = l_nacl;
 			} else {
-				port = NULL;
-				nacl = NULL;
+				tg_pt_gp->tg_pt_gp_alua_port = NULL;
+				tg_pt_gp->tg_pt_gp_alua_nacl = NULL;
 			}
 			atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
 			smp_mb__after_atomic_inc();
@@ -994,12 +1059,14 @@ int core_alua_do_port_transition(
 			 * core_alua_do_transition_tg_pt() will always return
 			 * success.
 			 */
-			core_alua_do_transition_tg_pt(tg_pt_gp, port,
-					nacl, new_state, explicit);
+			rc = core_alua_do_transition_tg_pt(tg_pt_gp,
+					new_state, explicit);
 
 			spin_lock(&dev->t10_alua.tg_pt_gps_lock);
 			atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
 			smp_mb__after_atomic_dec();
+			if (rc)
+				break;
 		}
 		spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
 
@@ -1009,15 +1076,18 @@ int core_alua_do_port_transition(
 	}
 	spin_unlock(&lu_gp->lu_gp_lock);
 
-	pr_debug("Successfully processed LU Group: %s all ALUA TG PT"
-		" Group IDs: %hu %s transition to primary state: %s\n",
-		config_item_name(&lu_gp->lu_gp_group.cg_item),
-		l_tg_pt_gp->tg_pt_gp_id, (explicit) ? "explicit" : "implicit",
-		core_alua_dump_state(new_state));
+	if (!rc) {
+		pr_debug("Successfully processed LU Group: %s all ALUA TG PT"
+			 " Group IDs: %hu %s transition to primary state: %s\n",
+			 config_item_name(&lu_gp->lu_gp_group.cg_item),
+			 l_tg_pt_gp->tg_pt_gp_id,
+			 (explicit) ? "explicit" : "implicit",
+			 core_alua_dump_state(new_state));
+	}
 
 	atomic_dec(&lu_gp->lu_gp_ref_cnt);
 	smp_mb__after_atomic_dec();
-	return 0;
+	return rc;
 }
 
 /*
@@ -1371,6 +1441,8 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
 	mutex_init(&tg_pt_gp->tg_pt_gp_md_mutex);
 	spin_lock_init(&tg_pt_gp->tg_pt_gp_lock);
 	atomic_set(&tg_pt_gp->tg_pt_gp_ref_cnt, 0);
+	INIT_DELAYED_WORK(&tg_pt_gp->tg_pt_gp_transition_work,
+			  core_alua_do_transition_tg_pt_work);
 	tg_pt_gp->tg_pt_gp_dev = dev;
 	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
 		ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED);
@@ -1499,6 +1571,8 @@ void core_alua_free_tg_pt_gp(
 	dev->t10_alua.alua_tg_pt_gps_counter--;
 	spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
 
+	flush_delayed_work(&tg_pt_gp->tg_pt_gp_transition_work);
+
 	/*
 	 * Allow a struct t10_alua_tg_pt_gp_member * referenced by
 	 * core_alua_get_tg_pt_gp_by_name() in
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 67b6ca2..b02bb61 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -301,6 +301,10 @@ struct t10_alua_tg_pt_gp {
 	struct config_group tg_pt_gp_group;
 	struct list_head tg_pt_gp_list;
 	struct list_head tg_pt_gp_mem_list;
+	struct se_port *tg_pt_gp_alua_port;
+	struct se_node_acl *tg_pt_gp_alua_nacl;
+	struct delayed_work tg_pt_gp_transition_work;
+	struct completion *tg_pt_gp_transition_complete;
 };
 
 struct t10_alua_tg_pt_gp_member {
-- 
1.7.12.4

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit
  2013-10-16  7:20 ` [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit Hannes Reinecke
@ 2013-10-16 21:13   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 21:13 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/target/target_core_alua.c      | 110 ++++++++++++++++-----------------
>  drivers/target/target_core_alua.h      |  20 +++---
>  drivers/target/target_core_configfs.c  |  26 ++++----
>  drivers/target/target_core_device.c    |   6 +-
>  drivers/target/target_core_file.c      |   2 +-
>  drivers/target/target_core_pr.c        |  24 +++----
>  drivers/target/target_core_spc.c       |   6 +-
>  drivers/target/target_core_transport.c |   4 +-
>  drivers/target/target_core_ua.h        |   2 +-
>  include/target/target_core_base.h      |   2 +-
>  10 files changed, 101 insertions(+), 101 deletions(-)
> 

Applied to target-pending/for-next.

Thanks Hannes!

--nab


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/11] target_core_alua: Store supported ALUA states
  2013-10-16  7:20 ` [PATCH 02/11] target_core_alua: Store supported ALUA states Hannes Reinecke
@ 2013-10-16 21:19   ` Nicholas A. Bellinger
  2013-10-17  5:48     ` Hannes Reinecke
  0 siblings, 1 reply; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 21:19 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> The supported ALUA states might be different for individual
> devices, so store it in a separate field.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/target/target_core_alua.c | 14 ++++++++------
>  drivers/target/target_core_alua.h | 11 +++++++++++
>  include/target/target_core_base.h |  1 +
>  3 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> index 8297d37..255e83c 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -117,12 +117,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
>  		/*
>  		 * Set supported ASYMMETRIC ACCESS State bits
>  		 */
> -		buf[off] = 0x80; /* T_SUP */
> -		buf[off] |= 0x40; /* O_SUP */
> -		buf[off] |= 0x8; /* U_SUP */
> -		buf[off] |= 0x4; /* S_SUP */
> -		buf[off] |= 0x2; /* AN_SUP */
> -		buf[off++] |= 0x1; /* AO_SUP */
> +		buf[off++] |= tg_pt_gp->tg_pt_gp_alua_supported_states;
>  		/*
>  		 * TARGET PORT GROUP
>  		 */
> @@ -1367,6 +1362,13 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
>  	tg_pt_gp->tg_pt_gp_trans_delay_msecs = ALUA_DEFAULT_TRANS_DELAY_MSECS;
>  	tg_pt_gp->tg_pt_gp_implicit_trans_secs = ALUA_DEFAULT_IMPLICIT_TRANS_SECS;
>  
> +	/*
> +	 * Enable all supported states
> +	 */
> +	tg_pt_gp->tg_pt_gp_alua_supported_states =
> +	    ALUA_T_SUP | ALUA_O_SUP | \
> +	    ALUA_U_SUP | ALUA_S_SUP | ALUA_AN_SUP | ALUA_AO_SUP;
> +
>  	if (def_group) {
>  		spin_lock(&dev->t10_alua.tg_pt_gps_lock);
>  		tg_pt_gp->tg_pt_gp_id =
> diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
> index 74cf0c0..e826a65 100644
> --- a/drivers/target/target_core_alua.h
> +++ b/drivers/target/target_core_alua.h
> @@ -23,6 +23,17 @@
>  #define ALUA_ACCESS_STATE_TRANSITION		0xf
>  
>  /*
> + * from spc4r36j section 6.37 Table 306
> + */
> +#define ALUA_T_SUP		0x80
> +#define ALUA_O_SUP		0x40
> +#define ALUA_LBD_SUP		0x10
> +#define ALUA_U_SUP		0x08
> +#define ALUA_S_SUP		0x04
> +#define ALUA_AN_SUP		0x02
> +#define ALUA_AO_SUP		0x01
> +
> +/*

How about making these the supported bits, TPGS mode, and ALUA access
state definitions common between target_core_alua.c and
scsi_dh_alua.c..?

--nab


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 03/11] target_core_alua: Make supported states configurable
  2013-10-16  7:20 ` [PATCH 03/11] target_core_alua: Make supported states configurable Hannes Reinecke
@ 2013-10-16 21:38   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 21:38 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/target/target_core_configfs.c | 50 +++++++++++++++++++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index d4c28a3..53e9e00 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -2131,6 +2131,55 @@ static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_access_type(
>  SE_DEV_ALUA_TG_PT_ATTR(alua_access_type, S_IRUGO | S_IWUSR);
>  
>  /*
> + * alua_supported_states
> + */
> +static ssize_t target_core_alua_tg_pt_gp_show_attr_alua_supported_states(
> +	struct t10_alua_tg_pt_gp *tg_pt_gp,
> +	char *page)
> +{
> +	return sprintf(page, "%02x\n",
> +		tg_pt_gp->tg_pt_gp_alua_supported_states);
> +}
> +
> +static ssize_t target_core_alua_tg_pt_gp_store_attr_alua_supported_states(
> +	struct t10_alua_tg_pt_gp *tg_pt_gp,
> +	const char *page,
> +	size_t count)
> +{
> +	unsigned long tmp;
> +	int new_states, valid_states, ret;
> +
> +	if (!tg_pt_gp->tg_pt_gp_valid_id) {
> +		pr_err("Unable to do set supported ALUA states on non"
> +			" valid tg_pt_gp ID: %hu\n",
> +			tg_pt_gp->tg_pt_gp_valid_id);
> +		return -EINVAL;
> +	}
> +
> +	ret = strict_strtoul(page, 0, &tmp);
> +	if (ret < 0) {
> +		pr_err("Unable to extract new supported ALUA states"
> +				" from %s\n", page);
> +		return -EINVAL;
> +	}
> +	new_states = (int)tmp;
> +	valid_states = ALUA_T_SUP | ALUA_O_SUP | ALUA_LBD_SUP | \
> +	    ALUA_U_SUP | ALUA_S_SUP | ALUA_AN_SUP | ALUA_AO_SUP;
> +
> +
> +	if (new_states & ~valid_states) {
> +		pr_err("Illegal supported ALUA states: 0x%02x\n",
> +				new_states);
> +		return -EINVAL;
> +	}
> +
> +	tg_pt_gp->tg_pt_gp_alua_supported_states = new_states;
> +	return count;
> +}
> +
> +SE_DEV_ALUA_TG_PT_ATTR(alua_supported_states, S_IRUGO | S_IWUSR);
> +
> +/*
>   * alua_write_metadata
>   */
>  static ssize_t target_core_alua_tg_pt_gp_show_attr_alua_write_metadata(
> @@ -2350,6 +2399,7 @@ static struct configfs_attribute *target_core_alua_tg_pt_gp_attrs[] = {
>  	&target_core_alua_tg_pt_gp_alua_access_state.attr,
>  	&target_core_alua_tg_pt_gp_alua_access_status.attr,
>  	&target_core_alua_tg_pt_gp_alua_access_type.attr,
> +	&target_core_alua_tg_pt_gp_alua_supported_states.attr,
>  	&target_core_alua_tg_pt_gp_alua_write_metadata.attr,
>  	&target_core_alua_tg_pt_gp_nonop_delay_msecs.attr,
>  	&target_core_alua_tg_pt_gp_trans_delay_msecs.attr,

I'm thinking this might be better served by individual attributes
representing the seven supported ALUA states, instead of a single
attribute representing them all..

That would certainly make it easier for userspace to manipulate..

--nab

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 06/11] target_core_alua: Validate ALUA state transition
  2013-10-16  7:20 ` [PATCH 06/11] target_core_alua: Validate ALUA state transition Hannes Reinecke
@ 2013-10-16 21:40   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 21:40 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> As we now can modify the list of supported states we need to
> validate the requested ALUA state when doing a state transition.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---

Looks good.

--nab

>  drivers/target/target_core_alua.c | 85 ++++++++++++++++++++++++++-------------
>  1 file changed, 56 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> index a16115e..a420778 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -41,7 +41,8 @@
>  #include "target_core_alua.h"
>  #include "target_core_ua.h"
>  
> -static sense_reason_t core_alua_check_transition(int state, int *primary);
> +static sense_reason_t core_alua_check_transition(int state, int valid,
> +						 int *primary);
>  static int core_alua_set_tg_pt_secondary_state(
>  		struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
>  		struct se_port *port, int explicit, int offline);
> @@ -210,7 +211,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
>  	unsigned char *ptr;
>  	sense_reason_t rc = TCM_NO_SENSE;
>  	u32 len = 4; /* Skip over RESERVED area in header */
> -	int alua_access_state, primary = 0;
> +	int alua_access_state, primary = 0, valid_states;
>  	u16 tg_pt_id, rtpi;
>  
>  	if (!l_port)
> @@ -252,6 +253,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
>  		rc = TCM_UNSUPPORTED_SCSI_OPCODE;
>  		goto out;
>  	}
> +	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
>  
>  	ptr = &buf[4]; /* Skip over RESERVED area in header */
>  
> @@ -263,7 +265,8 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
>  		 * the state is a primary or secondary target port asymmetric
>  		 * access state.
>  		 */
> -		rc = core_alua_check_transition(alua_access_state, &primary);
> +		rc = core_alua_check_transition(alua_access_state,
> +						valid_states, &primary);
>  		if (rc) {
>  			/*
>  			 * If the SET TARGET PORT GROUPS attempts to establish
> @@ -614,21 +617,57 @@ out:
>  	return 0;
>  }
>  
> +static char *core_alua_dump_state(int state)
> +{
> +	switch (state) {
> +	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
> +		return "Active/Optimized";
> +	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
> +		return "Active/NonOptimized";
> +	case ALUA_ACCESS_STATE_STANDBY:
> +		return "Standby";
> +	case ALUA_ACCESS_STATE_UNAVAILABLE:
> +		return "Unavailable";
> +	case ALUA_ACCESS_STATE_OFFLINE:
> +		return "Offline";
> +	case ALUA_ACCESS_STATE_TRANSITION:
> +		return "Transitioning";
> +	default:
> +		return "Unknown";
> +	}
> +
> +	return NULL;
> +}
> +
>  /*
>   * Check implicit and explicit ALUA state change request.
>   */
>  static sense_reason_t
> -core_alua_check_transition(int state, int *primary)
> +core_alua_check_transition(int state, int valid, int *primary)
>  {
> +	/*
> +	 * OPTIMIZED, NON-OPTIMIZED, STANDBY and UNAVAILABLE are
> +	 * defined as primary target port asymmetric access states.
> +	 */
>  	switch (state) {
>  	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
> +		if (!(valid & ALUA_AO_SUP))
> +			goto not_supported;
> +		*primary = 1;
> +		break;
>  	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
> +		if (!(valid & ALUA_AN_SUP))
> +			goto not_supported;
> +		*primary = 1;
> +		break;
>  	case ALUA_ACCESS_STATE_STANDBY:
> +		if (!(valid & ALUA_S_SUP))
> +			goto not_supported;
> +		*primary = 1;
> +		break;
>  	case ALUA_ACCESS_STATE_UNAVAILABLE:
> -		/*
> -		 * OPTIMIZED, NON-OPTIMIZED, STANDBY and UNAVAILABLE are
> -		 * defined as primary target port asymmetric access states.
> -		 */
> +		if (!(valid & ALUA_U_SUP))
> +			goto not_supported;
>  		*primary = 1;
>  		break;
>  	case ALUA_ACCESS_STATE_OFFLINE:
> @@ -636,6 +675,8 @@ core_alua_check_transition(int state, int *primary)
>  		 * OFFLINE state is defined as a secondary target port
>  		 * asymmetric access state.
>  		 */
> +		if (!(valid & ALUA_O_SUP))
> +			goto not_supported;
>  		*primary = 0;
>  		break;
>  	default:
> @@ -644,26 +685,11 @@ core_alua_check_transition(int state, int *primary)
>  	}
>  
>  	return 0;
> -}
>  
> -static char *core_alua_dump_state(int state)
> -{
> -	switch (state) {
> -	case ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED:
> -		return "Active/Optimized";
> -	case ALUA_ACCESS_STATE_ACTIVE_NON_OPTIMIZED:
> -		return "Active/NonOptimized";
> -	case ALUA_ACCESS_STATE_STANDBY:
> -		return "Standby";
> -	case ALUA_ACCESS_STATE_UNAVAILABLE:
> -		return "Unavailable";
> -	case ALUA_ACCESS_STATE_OFFLINE:
> -		return "Offline";
> -	default:
> -		return "Unknown";
> -	}
> -
> -	return NULL;
> +not_supported:
> +	pr_err("ALUA access state %s not supported",
> +	       core_alua_dump_state(state));
> +	return TCM_INVALID_PARAMETER_LIST;
>  }
>  
>  char *core_alua_dump_status(int status)
> @@ -884,9 +910,10 @@ int core_alua_do_port_transition(
>  	struct t10_alua_lu_gp_member *lu_gp_mem, *local_lu_gp_mem;
>  	struct t10_alua_tg_pt_gp *tg_pt_gp;
>  	unsigned char *md_buf;
> -	int primary;
> +	int primary, valid_states;
>  
> -	if (core_alua_check_transition(new_state, &primary) != 0)
> +	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
> +	if (core_alua_check_transition(new_state, valid_states, &primary) != 0)
>  		return -EINVAL;
>  
>  	md_buf = kzalloc(l_tg_pt_gp->tg_pt_gp_md_buf_len, GFP_KERNEL);

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 07/11] target_core_alua: Allocate ALUA metadata on demand
  2013-10-16  7:20 ` [PATCH 07/11] target_core_alua: Allocate ALUA metadata on demand Hannes Reinecke
@ 2013-10-16 21:42   ` Nicholas A. Bellinger
  0 siblings, 0 replies; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 21:42 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> We should only allocate ALUA metadata if we're actually going
> to write them.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/target/target_core_alua.c | 70 +++++++++++++++++----------------------
>  drivers/target/target_core_alua.h |  3 ++
>  include/target/target_core_base.h |  3 --
>  3 files changed, 34 insertions(+), 42 deletions(-)
> 

Looks reasonable to me..

--nab

> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
> index a420778..b1d08bf 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -762,16 +762,22 @@ static int core_alua_write_tpg_metadata(
>   */
>  static int core_alua_update_tpg_primary_metadata(
>  	struct t10_alua_tg_pt_gp *tg_pt_gp,
> -	int primary_state,
> -	unsigned char *md_buf)
> +	int primary_state)
>  {
> +	unsigned char *md_buf;
>  	struct t10_wwn *wwn = &tg_pt_gp->tg_pt_gp_dev->t10_wwn;
>  	char path[ALUA_METADATA_PATH_LEN];
> -	int len;
> +	int len, rc;
> +
> +	md_buf = kzalloc(ALUA_MD_BUF_LEN, GFP_KERNEL);
> +	if (!md_buf) {
> +		pr_err("Unable to allocate buf for ALUA metadata\n");
> +		return -ENOMEM;
> +	}
>  
>  	memset(path, 0, ALUA_METADATA_PATH_LEN);
>  
> -	len = snprintf(md_buf, tg_pt_gp->tg_pt_gp_md_buf_len,
> +	len = snprintf(md_buf, ALUA_MD_BUF_LEN,
>  			"tg_pt_gp_id=%hu\n"
>  			"alua_access_state=0x%02x\n"
>  			"alua_access_status=0x%02x\n",
> @@ -782,14 +788,15 @@ static int core_alua_update_tpg_primary_metadata(
>  		"/var/target/alua/tpgs_%s/%s", &wwn->unit_serial[0],
>  		config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item));
>  
> -	return core_alua_write_tpg_metadata(path, md_buf, len);
> +	rc = core_alua_write_tpg_metadata(path, md_buf, len);
> +	kfree(md_buf);
> +	return rc;
>  }
>  
>  static int core_alua_do_transition_tg_pt(
>  	struct t10_alua_tg_pt_gp *tg_pt_gp,
>  	struct se_port *l_port,
>  	struct se_node_acl *nacl,
> -	unsigned char *md_buf,
>  	int new_state,
>  	int explicit)
>  {
> @@ -877,8 +884,7 @@ static int core_alua_do_transition_tg_pt(
>  	 */
>  	if (tg_pt_gp->tg_pt_gp_write_metadata) {
>  		mutex_lock(&tg_pt_gp->tg_pt_gp_md_mutex);
> -		core_alua_update_tpg_primary_metadata(tg_pt_gp,
> -					new_state, md_buf);
> +		core_alua_update_tpg_primary_metadata(tg_pt_gp, new_state);
>  		mutex_unlock(&tg_pt_gp->tg_pt_gp_md_mutex);
>  	}
>  	/*
> @@ -909,19 +915,12 @@ int core_alua_do_port_transition(
>  	struct t10_alua_lu_gp *lu_gp;
>  	struct t10_alua_lu_gp_member *lu_gp_mem, *local_lu_gp_mem;
>  	struct t10_alua_tg_pt_gp *tg_pt_gp;
> -	unsigned char *md_buf;
>  	int primary, valid_states;
>  
>  	valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
>  	if (core_alua_check_transition(new_state, valid_states, &primary) != 0)
>  		return -EINVAL;
>  
> -	md_buf = kzalloc(l_tg_pt_gp->tg_pt_gp_md_buf_len, GFP_KERNEL);
> -	if (!md_buf) {
> -		pr_err("Unable to allocate buf for ALUA metadata\n");
> -		return -ENOMEM;
> -	}
> -
>  	local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
>  	spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
>  	lu_gp = local_lu_gp_mem->lu_gp;
> @@ -939,10 +938,9 @@ int core_alua_do_port_transition(
>  		 * success.
>  		 */
>  		core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
> -					md_buf, new_state, explicit);
> +					new_state, explicit);
>  		atomic_dec(&lu_gp->lu_gp_ref_cnt);
>  		smp_mb__after_atomic_dec();
> -		kfree(md_buf);
>  		return 0;
>  	}
>  	/*
> @@ -992,7 +990,7 @@ int core_alua_do_port_transition(
>  			 * success.
>  			 */
>  			core_alua_do_transition_tg_pt(tg_pt_gp, port,
> -					nacl, md_buf, new_state, explicit);
> +					nacl, new_state, explicit);
>  
>  			spin_lock(&dev->t10_alua.tg_pt_gps_lock);
>  			atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
> @@ -1014,7 +1012,6 @@ int core_alua_do_port_transition(
>  
>  	atomic_dec(&lu_gp->lu_gp_ref_cnt);
>  	smp_mb__after_atomic_dec();
> -	kfree(md_buf);
>  	return 0;
>  }
>  
> @@ -1023,13 +1020,18 @@ int core_alua_do_port_transition(
>   */
>  static int core_alua_update_tpg_secondary_metadata(
>  	struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
> -	struct se_port *port,
> -	unsigned char *md_buf,
> -	u32 md_buf_len)
> +	struct se_port *port)
>  {
> +	unsigned char *md_buf;
>  	struct se_portal_group *se_tpg = port->sep_tpg;
>  	char path[ALUA_METADATA_PATH_LEN], wwn[ALUA_SECONDARY_METADATA_WWN_LEN];
> -	int len;
> +	int len, rc;
> +
> +	md_buf = kzalloc(ALUA_MD_BUF_LEN, GFP_KERNEL);
> +	if (!md_buf) {
> +		pr_err("Unable to allocate buf for ALUA metadata\n");
> +		return -ENOMEM;
> +	}
>  
>  	memset(path, 0, ALUA_METADATA_PATH_LEN);
>  	memset(wwn, 0, ALUA_SECONDARY_METADATA_WWN_LEN);
> @@ -1041,7 +1043,7 @@ static int core_alua_update_tpg_secondary_metadata(
>  		snprintf(wwn+len, ALUA_SECONDARY_METADATA_WWN_LEN-len, "+%hu",
>  				se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg));
>  
> -	len = snprintf(md_buf, md_buf_len, "alua_tg_pt_offline=%d\n"
> +	len = snprintf(md_buf, ALUA_MD_BUF_LEN, "alua_tg_pt_offline=%d\n"
>  			"alua_tg_pt_status=0x%02x\n",
>  			atomic_read(&port->sep_tg_pt_secondary_offline),
>  			port->sep_tg_pt_secondary_stat);
> @@ -1050,7 +1052,10 @@ static int core_alua_update_tpg_secondary_metadata(
>  			se_tpg->se_tpg_tfo->get_fabric_name(), wwn,
>  			port->sep_lun->unpacked_lun);
>  
> -	return core_alua_write_tpg_metadata(path, md_buf, len);
> +	rc = core_alua_write_tpg_metadata(path, md_buf, len);
> +	kfree(md_buf);
> +
> +	return rc;
>  }
>  
>  static int core_alua_set_tg_pt_secondary_state(
> @@ -1060,8 +1065,6 @@ static int core_alua_set_tg_pt_secondary_state(
>  	int offline)
>  {
>  	struct t10_alua_tg_pt_gp *tg_pt_gp;
> -	unsigned char *md_buf;
> -	u32 md_buf_len;
>  	int trans_delay_msecs;
>  
>  	spin_lock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
> @@ -1082,7 +1085,6 @@ static int core_alua_set_tg_pt_secondary_state(
>  	else
>  		atomic_set(&port->sep_tg_pt_secondary_offline, 0);
>  
> -	md_buf_len = tg_pt_gp->tg_pt_gp_md_buf_len;
>  	port->sep_tg_pt_secondary_stat = (explicit) ?
>  			ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
>  			ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
> @@ -1104,18 +1106,9 @@ static int core_alua_set_tg_pt_secondary_state(
>  	 * secondary state and status
>  	 */
>  	if (port->sep_tg_pt_secondary_write_md) {
> -		md_buf = kzalloc(md_buf_len, GFP_KERNEL);
> -		if (!md_buf) {
> -			pr_err("Unable to allocate md_buf for"
> -				" secondary ALUA access metadata\n");
> -			return -ENOMEM;
> -		}
>  		mutex_lock(&port->sep_tg_pt_md_mutex);
> -		core_alua_update_tpg_secondary_metadata(tg_pt_gp_mem, port,
> -				md_buf, md_buf_len);
> +		core_alua_update_tpg_secondary_metadata(tg_pt_gp_mem, port);
>  		mutex_unlock(&port->sep_tg_pt_md_mutex);
> -
> -		kfree(md_buf);
>  	}
>  
>  	return 0;
> @@ -1374,7 +1367,6 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
>  	spin_lock_init(&tg_pt_gp->tg_pt_gp_lock);
>  	atomic_set(&tg_pt_gp->tg_pt_gp_ref_cnt, 0);
>  	tg_pt_gp->tg_pt_gp_dev = dev;
> -	tg_pt_gp->tg_pt_gp_md_buf_len = ALUA_MD_BUF_LEN;
>  	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
>  		ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED);
>  	/*
> diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
> index 88e2e83..1a152cd 100644
> --- a/drivers/target/target_core_alua.h
> +++ b/drivers/target/target_core_alua.h
> @@ -78,6 +78,9 @@
>   */
>  #define ALUA_SECONDARY_METADATA_WWN_LEN			256
>  
> +/* Used by core_alua_update_tpg_(primary,secondary)_metadata */
> +#define ALUA_MD_BUF_LEN					1024
> +
>  extern struct kmem_cache *t10_alua_lu_gp_cache;
>  extern struct kmem_cache *t10_alua_lu_gp_mem_cache;
>  extern struct kmem_cache *t10_alua_tg_pt_gp_cache;
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index 21f4bd5..933c59d 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -290,9 +290,6 @@ struct t10_alua_tg_pt_gp {
>  	int	tg_pt_gp_implicit_trans_secs;
>  	int	tg_pt_gp_pref;
>  	int	tg_pt_gp_write_metadata;
> -	/* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
> -#define ALUA_MD_BUF_LEN				1024
> -	u32	tg_pt_gp_md_buf_len;
>  	u32	tg_pt_gp_members;
>  	atomic_t tg_pt_gp_alua_access_state;
>  	atomic_t tg_pt_gp_ref_cnt;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning
  2013-10-16  7:20 ` [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning Hannes Reinecke
@ 2013-10-16 22:06   ` Nicholas A. Bellinger
  2013-10-17  6:52     ` Hannes Reinecke
  0 siblings, 1 reply; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 22:06 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> Use a workqueue for processing ALUA state transitions; this allows
> us to process implicit delay properly.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>  drivers/target/target_core_alua.c | 174 +++++++++++++++++++++++++++-----------
>  include/target/target_core_base.h |   4 +
>  2 files changed, 128 insertions(+), 50 deletions(-)
> 

<SNIP>

> +static int core_alua_do_transition_tg_pt(
> +	struct t10_alua_tg_pt_gp *tg_pt_gp,
> +	int new_state,
> +	int explicit)
> +{
> +	struct se_device *dev = tg_pt_gp->tg_pt_gp_dev;
> +	DECLARE_COMPLETION_ONSTACK(wait);
> +
> +	/* Nothing to be done here */
> +	if (atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) == new_state)
> +		return 0;
> +
> +	if (new_state == ALUA_ACCESS_STATE_TRANSITION)
> +		return -EAGAIN;
> +
> +	/*
> +	 * Flush any pending transitions
> +	 */
> +	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs &&
> +	    atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) ==
> +	    ALUA_ACCESS_STATE_TRANSITION) {
> +		/* Just in case */
> +		tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
> +		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
> +		flush_delayed_work(&tg_pt_gp->tg_pt_gp_transition_work);
> +		wait_for_completion(&wait);
> +		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
> +		return 0;
> +	}
> +
> +	/*
> +	 * Save the old primary ALUA access state, and set the current state
> +	 * to ALUA_ACCESS_STATE_TRANSITION.
> +	 */
> +	tg_pt_gp->tg_pt_gp_alua_previous_state =
> +		atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
> +	tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
> +
> +	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
> +			ALUA_ACCESS_STATE_TRANSITION);
> +	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
> +				ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
> +				ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
> +
> +	/*
> +	 * Check for the optional ALUA primary state transition delay
> +	 */
> +	if (tg_pt_gp->tg_pt_gp_trans_delay_msecs != 0)
> +		msleep_interruptible(tg_pt_gp->tg_pt_gp_trans_delay_msecs);
> +
> +	/*
> +	 * Take a reference for workqueue item
> +	 */
> +	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
> +	atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
> +	smp_mb__after_atomic_inc();
> +	spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
> +
> +	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs) {
> +		unsigned long transition_tmo;
> +
> +		transition_tmo = tg_pt_gp->tg_pt_gp_implicit_trans_secs * HZ;
> +		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
> +				   &tg_pt_gp->tg_pt_gp_transition_work,
> +				   transition_tmo);
> +	} else {
> +		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
> +		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
> +				   &tg_pt_gp->tg_pt_gp_transition_work, 0);
> +		wait_for_completion(&wait);
> +		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
> +	}

Mmm, the trans_delay_msecs delay seems a bit out of place now..

How about dropping it's usage with msleep_interruptible() above, and
instead combining it with the delay passed into queue_delayed_work()..?

--nab



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] target_core_mod: ALUA updates
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (8 preceding siblings ...)
  2013-10-16  7:20 ` [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning Hannes Reinecke
@ 2013-10-16 22:09 ` Nicholas A. Bellinger
  2013-11-06 20:54 ` Nicholas A. Bellinger
  10 siblings, 0 replies; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-16 22:09 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> Hi Nic,
> 
> here are some updates to TCM ALUA handling. Apart from some
> minor fixes and spellchecks the main features are:
> - Make supported states configurable:
>   We should make the list of supported ALUA states configurable,
>   as some setups would possibly like to support a small subset
>   of ALUA states only.
> - Asynchronous transitioning: I've switched 'transitioning'
>   handling to use a workqueue, that should allow us to simulate
>   asynchronous transitioning modes. IE TCM should now be capable
>   of handling requests while in transitioning, and properly terminate
>   these with the correct sense code.
> - Include target device descriptor in VPD page 83
>   For the ALUA device handler we'd need to identify the target device
>   where a given target port belongs to. So include the respective
>   values in the VPD page.
> 
> Hannes Reinecke (11):
>   target core: rename (ex,im)plict -> (ex,im)plicit
>   target_core_alua: Store supported ALUA states
>   target_core_alua: Make supported states configurable
>   target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED
>   target_core_alua: spellcheck
>   target_core_alua: Validate ALUA state transition
>   target_core_alua: Allocate ALUA metadata on demand
>   target_core_alua: store old and pending ALUA state
>   target_core_alua: Use workqueue for ALUA transitioning
>   target_core: simplify scsi_name_len calculation
>   target_core_spc: Include target device descriptor in VPD page 83
> 

Hey Hannes,

It looks like Patches #10 && #11 did not make it to the list.

Care to address the minor comments on the others and repost a -v2 with
the missing ones..?

--nab

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 02/11] target_core_alua: Store supported ALUA states
  2013-10-16 21:19   ` Nicholas A. Bellinger
@ 2013-10-17  5:48     ` Hannes Reinecke
  0 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-17  5:48 UTC (permalink / raw)
  To: Nicholas A. Bellinger; +Cc: Nic Bellinger, target-devel, linux-scsi

On 10/16/2013 11:19 PM, Nicholas A. Bellinger wrote:
> On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
>> The supported ALUA states might be different for individual
>> devices, so store it in a separate field.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>  drivers/target/target_core_alua.c | 14 ++++++++------
>>  drivers/target/target_core_alua.h | 11 +++++++++++
>>  include/target/target_core_base.h |  1 +
>>  3 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
>> index 8297d37..255e83c 100644
>> --- a/drivers/target/target_core_alua.c
>> +++ b/drivers/target/target_core_alua.c
>> @@ -117,12 +117,7 @@ target_emulate_report_target_port_groups(struct se_cmd *cmd)
>>  		/*
>>  		 * Set supported ASYMMETRIC ACCESS State bits
>>  		 */
>> -		buf[off] = 0x80; /* T_SUP */
>> -		buf[off] |= 0x40; /* O_SUP */
>> -		buf[off] |= 0x8; /* U_SUP */
>> -		buf[off] |= 0x4; /* S_SUP */
>> -		buf[off] |= 0x2; /* AN_SUP */
>> -		buf[off++] |= 0x1; /* AO_SUP */
>> +		buf[off++] |= tg_pt_gp->tg_pt_gp_alua_supported_states;
>>  		/*
>>  		 * TARGET PORT GROUP
>>  		 */
>> @@ -1367,6 +1362,13 @@ struct t10_alua_tg_pt_gp *core_alua_allocate_tg_pt_gp(struct se_device *dev,
>>  	tg_pt_gp->tg_pt_gp_trans_delay_msecs = ALUA_DEFAULT_TRANS_DELAY_MSECS;
>>  	tg_pt_gp->tg_pt_gp_implicit_trans_secs = ALUA_DEFAULT_IMPLICIT_TRANS_SECS;
>>  
>> +	/*
>> +	 * Enable all supported states
>> +	 */
>> +	tg_pt_gp->tg_pt_gp_alua_supported_states =
>> +	    ALUA_T_SUP | ALUA_O_SUP | \
>> +	    ALUA_U_SUP | ALUA_S_SUP | ALUA_AN_SUP | ALUA_AO_SUP;
>> +
>>  	if (def_group) {
>>  		spin_lock(&dev->t10_alua.tg_pt_gps_lock);
>>  		tg_pt_gp->tg_pt_gp_id =
>> diff --git a/drivers/target/target_core_alua.h b/drivers/target/target_core_alua.h
>> index 74cf0c0..e826a65 100644
>> --- a/drivers/target/target_core_alua.h
>> +++ b/drivers/target/target_core_alua.h
>> @@ -23,6 +23,17 @@
>>  #define ALUA_ACCESS_STATE_TRANSITION		0xf
>>  
>>  /*
>> + * from spc4r36j section 6.37 Table 306
>> + */
>> +#define ALUA_T_SUP		0x80
>> +#define ALUA_O_SUP		0x40
>> +#define ALUA_LBD_SUP		0x10
>> +#define ALUA_U_SUP		0x08
>> +#define ALUA_S_SUP		0x04
>> +#define ALUA_AN_SUP		0x02
>> +#define ALUA_AO_SUP		0x01
>> +
>> +/*
> 
> How about making these the supported bits, TPGS mode, and ALUA access
> state definitions common between target_core_alua.c and
> scsi_dh_alua.c..?
> 
Sure. Good idea.

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)

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning
  2013-10-16 22:06   ` Nicholas A. Bellinger
@ 2013-10-17  6:52     ` Hannes Reinecke
  2013-10-18 19:04       ` Nicholas A. Bellinger
  0 siblings, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2013-10-17  6:52 UTC (permalink / raw)
  To: Nicholas A. Bellinger; +Cc: Nic Bellinger, target-devel, linux-scsi

On 10/17/2013 12:06 AM, Nicholas A. Bellinger wrote:
> On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
>> Use a workqueue for processing ALUA state transitions; this allows
>> us to process implicit delay properly.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>  drivers/target/target_core_alua.c | 174 +++++++++++++++++++++++++++-----------
>>  include/target/target_core_base.h |   4 +
>>  2 files changed, 128 insertions(+), 50 deletions(-)
>>
> 
> <SNIP>
> 
>> +static int core_alua_do_transition_tg_pt(
>> +	struct t10_alua_tg_pt_gp *tg_pt_gp,
>> +	int new_state,
>> +	int explicit)
>> +{
>> +	struct se_device *dev = tg_pt_gp->tg_pt_gp_dev;
>> +	DECLARE_COMPLETION_ONSTACK(wait);
>> +
>> +	/* Nothing to be done here */
>> +	if (atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) == new_state)
>> +		return 0;
>> +
>> +	if (new_state == ALUA_ACCESS_STATE_TRANSITION)
>> +		return -EAGAIN;
>> +
>> +	/*
>> +	 * Flush any pending transitions
>> +	 */
>> +	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs &&
>> +	    atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) ==
>> +	    ALUA_ACCESS_STATE_TRANSITION) {
>> +		/* Just in case */
>> +		tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
>> +		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
>> +		flush_delayed_work(&tg_pt_gp->tg_pt_gp_transition_work);
>> +		wait_for_completion(&wait);
>> +		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
>> +		return 0;
>> +	}
>> +
>> +	/*
>> +	 * Save the old primary ALUA access state, and set the current state
>> +	 * to ALUA_ACCESS_STATE_TRANSITION.
>> +	 */
>> +	tg_pt_gp->tg_pt_gp_alua_previous_state =
>> +		atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
>> +	tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
>> +
>> +	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
>> +			ALUA_ACCESS_STATE_TRANSITION);
>> +	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
>> +				ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
>> +				ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
>> +
>> +	/*
>> +	 * Check for the optional ALUA primary state transition delay
>> +	 */
>> +	if (tg_pt_gp->tg_pt_gp_trans_delay_msecs != 0)
>> +		msleep_interruptible(tg_pt_gp->tg_pt_gp_trans_delay_msecs);
>> +
>> +	/*
>> +	 * Take a reference for workqueue item
>> +	 */
>> +	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
>> +	atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
>> +	smp_mb__after_atomic_inc();
>> +	spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
>> +
>> +	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs) {
>> +		unsigned long transition_tmo;
>> +
>> +		transition_tmo = tg_pt_gp->tg_pt_gp_implicit_trans_secs * HZ;
>> +		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
>> +				   &tg_pt_gp->tg_pt_gp_transition_work,
>> +				   transition_tmo);
>> +	} else {
>> +		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
>> +		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
>> +				   &tg_pt_gp->tg_pt_gp_transition_work, 0);
>> +		wait_for_completion(&wait);
>> +		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
>> +	}
> 
> Mmm, the trans_delay_msecs delay seems a bit out of place now..
> 
> How about dropping it's usage with msleep_interruptible() above, and
> instead combining it with the delay passed into queue_delayed_work()..?
> 
Yeah, we could.
Actually I was thinking of opening up the implicit transition time
to be a general transitioning time, to be used for both implicit and
explicit.
Reasoning here is that one could kick off a userland tool once
'transitioning' mode has been requested.
That tool would then go ahead and do whatever is required to switch
paths around etc, and could write the final ALUA state into configfs.
That would then terminate the workqueue and the system would
continue with the new state.
If the userland tool fails to execute in time the system would
revert to the requested state as it does now.

The only problem with that approach is that it's currently
impossible to have an atomic implicit transition for several tpgs.

You can to an atomic _explicit_ transition, as SET TARGET PORT
GROUPS _can_ carry information about all target port groups.
(Mind you, scsi_dh_alua doesn't use it that way, it only sends
information for the active/optimized target port group).
But you cannot achieve a similar operation via configfs; there you
can only set one tpg at a time, and each of this will potentially
trigger a move to transitioning.

While this is not a violation of the spec, it is certainly
confusing. I'd rather have a way to set the new state for _all_ tpgs
at once and only then kick off the transitioning mechanism.

Any ideas how this could be achieved?

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning
  2013-10-17  6:52     ` Hannes Reinecke
@ 2013-10-18 19:04       ` Nicholas A. Bellinger
  0 siblings, 0 replies; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-10-18 19:04 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Thu, 2013-10-17 at 08:52 +0200, Hannes Reinecke wrote:
> On 10/17/2013 12:06 AM, Nicholas A. Bellinger wrote:
> > On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> >> Use a workqueue for processing ALUA state transitions; this allows
> >> us to process implicit delay properly.
> >>
> >> Signed-off-by: Hannes Reinecke <hare@suse.de>
> >> ---
> >>  drivers/target/target_core_alua.c | 174 +++++++++++++++++++++++++++-----------
> >>  include/target/target_core_base.h |   4 +
> >>  2 files changed, 128 insertions(+), 50 deletions(-)
> >>
> > 
> > <SNIP>
> > 
> >> +static int core_alua_do_transition_tg_pt(
> >> +	struct t10_alua_tg_pt_gp *tg_pt_gp,
> >> +	int new_state,
> >> +	int explicit)
> >> +{
> >> +	struct se_device *dev = tg_pt_gp->tg_pt_gp_dev;
> >> +	DECLARE_COMPLETION_ONSTACK(wait);
> >> +
> >> +	/* Nothing to be done here */
> >> +	if (atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) == new_state)
> >> +		return 0;
> >> +
> >> +	if (new_state == ALUA_ACCESS_STATE_TRANSITION)
> >> +		return -EAGAIN;
> >> +
> >> +	/*
> >> +	 * Flush any pending transitions
> >> +	 */
> >> +	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs &&
> >> +	    atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state) ==
> >> +	    ALUA_ACCESS_STATE_TRANSITION) {
> >> +		/* Just in case */
> >> +		tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
> >> +		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
> >> +		flush_delayed_work(&tg_pt_gp->tg_pt_gp_transition_work);
> >> +		wait_for_completion(&wait);
> >> +		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
> >> +		return 0;
> >> +	}
> >> +
> >> +	/*
> >> +	 * Save the old primary ALUA access state, and set the current state
> >> +	 * to ALUA_ACCESS_STATE_TRANSITION.
> >> +	 */
> >> +	tg_pt_gp->tg_pt_gp_alua_previous_state =
> >> +		atomic_read(&tg_pt_gp->tg_pt_gp_alua_access_state);
> >> +	tg_pt_gp->tg_pt_gp_alua_pending_state = new_state;
> >> +
> >> +	atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
> >> +			ALUA_ACCESS_STATE_TRANSITION);
> >> +	tg_pt_gp->tg_pt_gp_alua_access_status = (explicit) ?
> >> +				ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
> >> +				ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
> >> +
> >> +	/*
> >> +	 * Check for the optional ALUA primary state transition delay
> >> +	 */
> >> +	if (tg_pt_gp->tg_pt_gp_trans_delay_msecs != 0)
> >> +		msleep_interruptible(tg_pt_gp->tg_pt_gp_trans_delay_msecs);
> >> +
> >> +	/*
> >> +	 * Take a reference for workqueue item
> >> +	 */
> >> +	spin_lock(&dev->t10_alua.tg_pt_gps_lock);
> >> +	atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
> >> +	smp_mb__after_atomic_inc();
> >> +	spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
> >> +
> >> +	if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs) {
> >> +		unsigned long transition_tmo;
> >> +
> >> +		transition_tmo = tg_pt_gp->tg_pt_gp_implicit_trans_secs * HZ;
> >> +		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
> >> +				   &tg_pt_gp->tg_pt_gp_transition_work,
> >> +				   transition_tmo);
> >> +	} else {
> >> +		tg_pt_gp->tg_pt_gp_transition_complete = &wait;
> >> +		queue_delayed_work(tg_pt_gp->tg_pt_gp_dev->tmr_wq,
> >> +				   &tg_pt_gp->tg_pt_gp_transition_work, 0);
> >> +		wait_for_completion(&wait);
> >> +		tg_pt_gp->tg_pt_gp_transition_complete = NULL;
> >> +	}
> > 
> > Mmm, the trans_delay_msecs delay seems a bit out of place now..
> > 
> > How about dropping it's usage with msleep_interruptible() above, and
> > instead combining it with the delay passed into queue_delayed_work()..?
> > 
> Yeah, we could.
> Actually I was thinking of opening up the implicit transition time
> to be a general transitioning time, to be used for both implicit and
> explicit.

Sounds reasonable to me..

> Reasoning here is that one could kick off a userland tool once
> 'transitioning' mode has been requested.
> That tool would then go ahead and do whatever is required to switch
> paths around etc, and could write the final ALUA state into configfs.
> That would then terminate the workqueue and the system would
> continue with the new state.
> If the userland tool fails to execute in time the system would
> revert to the requested state as it does now.

Also sounds reasonable.  This timeout, along with the explicit +
implicit timeout probably needs to be limited to a value less than the
SCSI abort timeout on the host side..

> 
> The only problem with that approach is that it's currently
> impossible to have an atomic implicit transition for several tpgs.
> 
> You can to an atomic _explicit_ transition, as SET TARGET PORT
> GROUPS _can_ carry information about all target port groups.
> (Mind you, scsi_dh_alua doesn't use it that way, it only sends
> information for the active/optimized target port group).
> But you cannot achieve a similar operation via configfs; there you
> can only set one tpg at a time, and each of this will potentially
> trigger a move to transitioning.
> 
> While this is not a violation of the spec, it is certainly
> confusing. I'd rather have a way to set the new state for _all_ tpgs
> at once and only then kick off the transitioning mechanism.
> 
> Any ideas how this could be achieved?
> 

Two ideas..

The first would be to add a new attribute at 

   /sys/kernel/config/target/core/$HBA/$DEV/alua/do_transition

that triggers the implicit transitions for each 
../$HBA/$DEV/alua/$TG_PT_GP that has had a queue_transition (or
something to that effect) bit set..

The second is similar, but would involve putting the do_transition
attribute into ../$HBA/$DEV/attrib/ instead of the ../alua directory, in
order to avoid some minor breakage with lio-utils that incorrectly
assumes everything in ../alua/ is a configfs directory.

--nab

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] target_core_mod: ALUA updates
  2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
                   ` (9 preceding siblings ...)
  2013-10-16 22:09 ` [PATCH 00/11] target_core_mod: ALUA updates Nicholas A. Bellinger
@ 2013-11-06 20:54 ` Nicholas A. Bellinger
  2013-11-12 21:49   ` Nicholas A. Bellinger
  10 siblings, 1 reply; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-11-06 20:54 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

Hello Hannes,

Any updates on this series..?

I'd really like to get the -v2 of this included for v3.13, along with
the initial Referrals bits.

--nab

On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> Hi Nic,
> 
> here are some updates to TCM ALUA handling. Apart from some
> minor fixes and spellchecks the main features are:
> - Make supported states configurable:
>   We should make the list of supported ALUA states configurable,
>   as some setups would possibly like to support a small subset
>   of ALUA states only.
> - Asynchronous transitioning: I've switched 'transitioning'
>   handling to use a workqueue, that should allow us to simulate
>   asynchronous transitioning modes. IE TCM should now be capable
>   of handling requests while in transitioning, and properly terminate
>   these with the correct sense code.
> - Include target device descriptor in VPD page 83
>   For the ALUA device handler we'd need to identify the target device
>   where a given target port belongs to. So include the respective
>   values in the VPD page.
> 
> Hannes Reinecke (11):
>   target core: rename (ex,im)plict -> (ex,im)plicit
>   target_core_alua: Store supported ALUA states
>   target_core_alua: Make supported states configurable
>   target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED
>   target_core_alua: spellcheck
>   target_core_alua: Validate ALUA state transition
>   target_core_alua: Allocate ALUA metadata on demand
>   target_core_alua: store old and pending ALUA state
>   target_core_alua: Use workqueue for ALUA transitioning
>   target_core: simplify scsi_name_len calculation
>   target_core_spc: Include target device descriptor in VPD page 83
> 
>  drivers/target/target_core_alua.c      | 456 ++++++++++++++++++++-------------
>  drivers/target/target_core_alua.h      |  36 ++-
>  drivers/target/target_core_configfs.c  |  76 +++++-
>  drivers/target/target_core_device.c    |   6 +-
>  drivers/target/target_core_file.c      |   2 +-
>  drivers/target/target_core_pr.c        |  24 +-
>  drivers/target/target_core_spc.c       |  62 ++++-
>  drivers/target/target_core_transport.c |   4 +-
>  drivers/target/target_core_ua.h        |   2 +-
>  include/target/target_core_base.h      |  14 +-
>  10 files changed, 442 insertions(+), 240 deletions(-)
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] target_core_mod: ALUA updates
  2013-11-06 20:54 ` Nicholas A. Bellinger
@ 2013-11-12 21:49   ` Nicholas A. Bellinger
  2013-11-13  6:46     ` Hannes Reinecke
  0 siblings, 1 reply; 23+ messages in thread
From: Nicholas A. Bellinger @ 2013-11-12 21:49 UTC (permalink / raw)
  To: Hannes Reinecke; +Cc: Nic Bellinger, target-devel, linux-scsi

On Wed, 2013-11-06 at 12:54 -0800, Nicholas A. Bellinger wrote:
> Hello Hannes,
> 
> Any updates on this series..?
> 
> I'd really like to get the -v2 of this included for v3.13, along with
> the initial Referrals bits.
> 

Any update Hannes..?

I'm fine with merging this series without the common ALUA definition
breakout with scsi_dh_alua for v3.13..

Care to respin a -v2 with the handful of minor comments in the original
posting..?

--nab

> 
> On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> > Hi Nic,
> > 
> > here are some updates to TCM ALUA handling. Apart from some
> > minor fixes and spellchecks the main features are:
> > - Make supported states configurable:
> >   We should make the list of supported ALUA states configurable,
> >   as some setups would possibly like to support a small subset
> >   of ALUA states only.
> > - Asynchronous transitioning: I've switched 'transitioning'
> >   handling to use a workqueue, that should allow us to simulate
> >   asynchronous transitioning modes. IE TCM should now be capable
> >   of handling requests while in transitioning, and properly terminate
> >   these with the correct sense code.
> > - Include target device descriptor in VPD page 83
> >   For the ALUA device handler we'd need to identify the target device
> >   where a given target port belongs to. So include the respective
> >   values in the VPD page.
> > 
> > Hannes Reinecke (11):
> >   target core: rename (ex,im)plict -> (ex,im)plicit
> >   target_core_alua: Store supported ALUA states
> >   target_core_alua: Make supported states configurable
> >   target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED
> >   target_core_alua: spellcheck
> >   target_core_alua: Validate ALUA state transition
> >   target_core_alua: Allocate ALUA metadata on demand
> >   target_core_alua: store old and pending ALUA state
> >   target_core_alua: Use workqueue for ALUA transitioning
> >   target_core: simplify scsi_name_len calculation
> >   target_core_spc: Include target device descriptor in VPD page 83
> > 
> >  drivers/target/target_core_alua.c      | 456 ++++++++++++++++++++-------------
> >  drivers/target/target_core_alua.h      |  36 ++-
> >  drivers/target/target_core_configfs.c  |  76 +++++-
> >  drivers/target/target_core_device.c    |   6 +-
> >  drivers/target/target_core_file.c      |   2 +-
> >  drivers/target/target_core_pr.c        |  24 +-
> >  drivers/target/target_core_spc.c       |  62 ++++-
> >  drivers/target/target_core_transport.c |   4 +-
> >  drivers/target/target_core_ua.h        |   2 +-
> >  include/target/target_core_base.h      |  14 +-
> >  10 files changed, 442 insertions(+), 240 deletions(-)
> > 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 00/11] target_core_mod: ALUA updates
  2013-11-12 21:49   ` Nicholas A. Bellinger
@ 2013-11-13  6:46     ` Hannes Reinecke
  0 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2013-11-13  6:46 UTC (permalink / raw)
  To: Nicholas A. Bellinger; +Cc: Nic Bellinger, target-devel, linux-scsi

On 11/12/2013 10:49 PM, Nicholas A. Bellinger wrote:
> On Wed, 2013-11-06 at 12:54 -0800, Nicholas A. Bellinger wrote:
>> Hello Hannes,
>>
>> Any updates on this series..?
>>
>> I'd really like to get the -v2 of this included for v3.13, along with
>> the initial Referrals bits.
>>
> 
> Any update Hannes..?
> 
> I'm fine with merging this series without the common ALUA definition
> breakout with scsi_dh_alua for v3.13..
> 
> Care to respin a -v2 with the handful of minor comments in the original
> posting..?
> 
Gnaa. I knew I've forgotten something.

But yeah, I'll be posting a new series.

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)

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2013-11-13  6:46 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-16  7:20 [PATCH 00/11] target_core_mod: ALUA updates Hannes Reinecke
2013-10-16  7:20 ` [PATCH 01/11] target core: rename (ex,im)plict -> (ex,im)plicit Hannes Reinecke
2013-10-16 21:13   ` Nicholas A. Bellinger
2013-10-16  7:20 ` [PATCH 02/11] target_core_alua: Store supported ALUA states Hannes Reinecke
2013-10-16 21:19   ` Nicholas A. Bellinger
2013-10-17  5:48     ` Hannes Reinecke
2013-10-16  7:20 ` [PATCH 03/11] target_core_alua: Make supported states configurable Hannes Reinecke
2013-10-16 21:38   ` Nicholas A. Bellinger
2013-10-16  7:20 ` [PATCH 04/11] target_core_alua: Rename ALUA_ACCESS_STATE_OPTIMIZED Hannes Reinecke
2013-10-16  7:20 ` [PATCH 05/11] target_core_alua: spellcheck Hannes Reinecke
2013-10-16  7:20 ` [PATCH 06/11] target_core_alua: Validate ALUA state transition Hannes Reinecke
2013-10-16 21:40   ` Nicholas A. Bellinger
2013-10-16  7:20 ` [PATCH 07/11] target_core_alua: Allocate ALUA metadata on demand Hannes Reinecke
2013-10-16 21:42   ` Nicholas A. Bellinger
2013-10-16  7:20 ` [PATCH 08/11] target_core_alua: store old and pending ALUA state Hannes Reinecke
2013-10-16  7:20 ` [PATCH 09/11] target_core_alua: Use workqueue for ALUA transitioning Hannes Reinecke
2013-10-16 22:06   ` Nicholas A. Bellinger
2013-10-17  6:52     ` Hannes Reinecke
2013-10-18 19:04       ` Nicholas A. Bellinger
2013-10-16 22:09 ` [PATCH 00/11] target_core_mod: ALUA updates Nicholas A. Bellinger
2013-11-06 20:54 ` Nicholas A. Bellinger
2013-11-12 21:49   ` Nicholas A. Bellinger
2013-11-13  6:46     ` Hannes Reinecke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).