[PATCHv2 RESENDv2 0/9] target: Save memory on unused se_dev_entrys and se_luns

[PATCHv2 RESENDv2 0/9] target: Save memory on unused se_dev_entrys and se_luns

[Andy Grover]

Hi nab, hch, and all,

This patchset reduces the amount of memory for se_dev_entry and se_lun
arrays by waiting to allocate array members until they are
created. This patch saves up to 261KB per TPG, and up to 65KB per
ACL. It also fixes a number of locking bugs around these data
structures.

Andy Grover (9):
  target: Add locking to some accesses to nacl.device_list
  target: Don't unlock/relock tpg_lun_lock in loop in add_node_to_devs
  target: Allocate se_dev_entrys in device list only when used
  target: core_tpg_post_dellun can return void
  target: Change core_dev_del_lun to take a se_lun instead of
    unpacked_lun
  target: Rename core_tpg_post_dellun to remove_lun
  target: Allocate se_luns only when used
  target: Remove core_tpg_release_virtual_lun0 function
  target: Refactor core_enable_device_list_for_node

 drivers/target/sbp/sbp_target.c              |   6 +-
 drivers/target/target_core_device.c          | 311 +++++++++++++--------------
 drivers/target/target_core_fabric_configfs.c |  35 +--
 drivers/target/target_core_internal.h        |   9 +-
 drivers/target/target_core_pr.c              |  40 +++-
 drivers/target/target_core_spc.c             |   2 +-
 drivers/target/target_core_tpg.c             | 189 ++++------------
 drivers/target/target_core_ua.c              |   3 +
 include/target/target_core_base.h            |  17 +-
 9 files changed, 246 insertions(+), 366 deletions(-)

-- 
1.9.3

[PATCHv2 RESENDv2 1/9] target: Add locking to some accesses to nacl.device_list

[Andy Grover]

Make sure all accesses of deve elements in device_list are protected. This
will become critically important once device_list entries are potentially
NULL.

Reported-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_device.c          |  7 ++---
 drivers/target/target_core_fabric_configfs.c | 10 +++++--
 drivers/target/target_core_pr.c              | 40 +++++++++++++++++++++-------
 drivers/target/target_core_ua.c              |  3 +++
 4 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 26416c1..9789a11 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -361,11 +361,12 @@ int core_enable_device_list_for_node(
 
 	deve->creation_time = get_jiffies_64();
 	deve->attach_count++;
-	spin_unlock_irq(&nacl->device_list_lock);
 
-	spin_lock_bh(&port->sep_alua_lock);
+	spin_lock(&port->sep_alua_lock);
 	list_add_tail(&deve->alua_port_list, &port->sep_alua_list);
-	spin_unlock_bh(&port->sep_alua_lock);
+	spin_unlock(&port->sep_alua_lock);
+
+	spin_unlock_irq(&nacl->device_list_lock);
 
 	return 0;
 }
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 7de9f04..804e7f0 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -141,13 +141,19 @@ static int target_fabric_mappedlun_unlink(
 	struct se_lun_acl *lacl = container_of(to_config_group(lun_acl_ci),
 			struct se_lun_acl, se_lun_group);
 	struct se_node_acl *nacl = lacl->se_lun_nacl;
-	struct se_dev_entry *deve = nacl->device_list[lacl->mapped_lun];
+	struct se_dev_entry *deve;
 	struct se_portal_group *se_tpg;
+
+	spin_lock_irq(&nacl->device_list_lock);
+	deve = nacl->device_list[lacl->mapped_lun];
 	/*
 	 * Determine if the underlying MappedLUN has already been released..
 	 */
-	if (!deve->se_lun)
+	if (!deve->se_lun) {
+		spin_unlock_irq(&nacl->device_list_lock);
 		return 0;
+	}
+	spin_unlock_irq(&nacl->device_list_lock);
 
 	lun = container_of(to_config_group(lun_ci), struct se_lun, lun_group);
 	se_tpg = lun->lun_sep->sep_tpg;
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 3013287..552eedf 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -311,8 +311,13 @@ static int core_scsi3_pr_seq_non_holder(
 	int we = 0; /* Write Exclusive */
 	int legacy = 0; /* Act like a legacy device and return
 			 * RESERVATION CONFLICT on some CDBs */
+	int def_pr_registered;
 
+	spin_lock_irq(&se_sess->se_node_acl->device_list_lock);
 	se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
+	def_pr_registered = se_deve->def_pr_registered;
+	spin_unlock_irq(&se_sess->se_node_acl->device_list_lock);
+
 	/*
 	 * Determine if the registration should be ignored due to
 	 * non-matching ISIDs in target_scsi3_pr_reservation_check().
@@ -329,7 +334,7 @@ static int core_scsi3_pr_seq_non_holder(
 		 * Some commands are only allowed for the persistent reservation
 		 * holder.
 		 */
-		if ((se_deve->def_pr_registered) && !(ignore_reg))
+		if ((def_pr_registered) && !(ignore_reg))
 			registered_nexus = 1;
 		break;
 	case PR_TYPE_WRITE_EXCLUSIVE_REGONLY:
@@ -339,7 +344,7 @@ static int core_scsi3_pr_seq_non_holder(
 		 * Some commands are only allowed for registered I_T Nexuses.
 		 */
 		reg_only = 1;
-		if ((se_deve->def_pr_registered) && !(ignore_reg))
+		if ((def_pr_registered) && !(ignore_reg))
 			registered_nexus = 1;
 		break;
 	case PR_TYPE_WRITE_EXCLUSIVE_ALLREG:
@@ -349,7 +354,7 @@ static int core_scsi3_pr_seq_non_holder(
 		 * Each registered I_T Nexus is a reservation holder.
 		 */
 		all_reg = 1;
-		if ((se_deve->def_pr_registered) && !(ignore_reg))
+		if ((def_pr_registered) && !(ignore_reg))
 			registered_nexus = 1;
 		break;
 	default:
@@ -947,13 +952,21 @@ int core_scsi3_check_aptpl_registration(
 	struct se_lun_acl *lun_acl)
 {
 	struct se_node_acl *nacl = lun_acl->se_lun_nacl;
-	struct se_dev_entry *deve = nacl->device_list[lun_acl->mapped_lun];
+	struct se_dev_entry *deve;
+	int ret;
 
 	if (dev->dev_reservation_flags & DRF_SPC2_RESERVATIONS)
 		return 0;
 
-	return __core_scsi3_check_aptpl_registration(dev, tpg, lun,
-				lun->unpacked_lun, nacl, deve);
+	spin_lock_irq(&nacl->device_list_lock);
+	deve = nacl->device_list[lun_acl->mapped_lun];
+
+	ret =  __core_scsi3_check_aptpl_registration(dev, tpg, lun,
+			lun->unpacked_lun, nacl, deve);
+
+	spin_unlock_irq(&nacl->device_list_lock);
+
+	return ret;
 }
 
 static void __core_scsi3_dump_registration(
@@ -1451,7 +1464,6 @@ core_scsi3_decode_spec_i_port(
 
 	memset(dest_iport, 0, 64);
 
-	local_se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
 	/*
 	 * Allocate a struct pr_transport_id_holder and setup the
 	 * local_node_acl and local_se_deve pointers and add to
@@ -1466,11 +1478,18 @@ core_scsi3_decode_spec_i_port(
 	INIT_LIST_HEAD(&tidh_new->dest_list);
 	tidh_new->dest_tpg = tpg;
 	tidh_new->dest_node_acl = se_sess->se_node_acl;
+
+	spin_lock_irq(&se_sess->se_node_acl->device_list_lock);
+	local_se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
+
 	tidh_new->dest_se_deve = local_se_deve;
 
 	local_pr_reg = __core_scsi3_alloc_registration(cmd->se_dev,
 				se_sess->se_node_acl, local_se_deve, l_isid,
 				sa_res_key, all_tg_pt, aptpl);
+
+	spin_unlock_irq(&se_sess->se_node_acl->device_list_lock);
+
 	if (!local_pr_reg) {
 		kfree(tidh_new);
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
@@ -2002,7 +2021,6 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
 {
 	struct se_session *se_sess = cmd->se_sess;
 	struct se_device *dev = cmd->se_dev;
-	struct se_dev_entry *se_deve;
 	struct se_lun *se_lun = cmd->se_lun;
 	struct se_portal_group *se_tpg;
 	struct t10_pr_registration *pr_reg, *pr_reg_p, *pr_reg_tmp;
@@ -2016,7 +2034,6 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 	se_tpg = se_sess->se_tpg;
-	se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
 
 	if (se_tpg->se_tpg_tfo->sess_get_initiator_sid) {
 		memset(&isid_buf[0], 0, PR_REG_ISID_LEN);
@@ -2041,19 +2058,24 @@ core_scsi3_emulate_pro_register(struct se_cmd *cmd, u64 res_key, u64 sa_res_key,
 			return 0;
 
 		if (!spec_i_pt) {
+			struct se_dev_entry *se_deve;
 			/*
 			 * Perform the Service Action REGISTER on the Initiator
 			 * Port Endpoint that the PRO was received from on the
 			 * Logical Unit of the SCSI device server.
 			 */
+			spin_lock_irq(&se_sess->se_node_acl->device_list_lock);
+			se_deve = se_sess->se_node_acl->device_list[cmd->orig_fe_lun];
 			if (core_scsi3_alloc_registration(cmd->se_dev,
 					se_sess->se_node_acl, se_deve, isid_ptr,
 					sa_res_key, all_tg_pt, aptpl,
 					register_type, 0)) {
 				pr_err("Unable to allocate"
 					" struct t10_pr_registration\n");
+				spin_unlock_irq(&se_sess->se_node_acl->device_list_lock);
 				return TCM_INVALID_PARAMETER_LIST;
 			}
+			spin_unlock_irq(&se_sess->se_node_acl->device_list_lock);
 		} else {
 			/*
 			 * Register both the Initiator port that received
diff --git a/drivers/target/target_core_ua.c b/drivers/target/target_core_ua.c
index 505519b..c63abad 100644
--- a/drivers/target/target_core_ua.c
+++ b/drivers/target/target_core_ua.c
@@ -51,9 +51,12 @@ target_scsi3_ua_check(struct se_cmd *cmd)
 	if (!nacl)
 		return 0;
 
+	spin_lock_irq(&nacl->device_list_lock);
 	deve = nacl->device_list[cmd->orig_fe_lun];
 	if (!atomic_read(&deve->ua_count))
 		return 0;
+	spin_unlock_irq(&nacl->device_list_lock);
+
 	/*
 	 * From sam4r14, section 5.14 Unit attention condition:
 	 *
-- 
1.9.3

[PATCHv2 RESENDv2 2/9] target: Don't unlock/relock tpg_lun_lock in loop in add_node_to_devs

[Andy Grover]

It's not safe to unlock/relock in core_tpg_add_node_to_devs. Remove this.

As a consequence, core_enable_device_list_for_node needs to be called with
a lock held & irqs off. Add a comment mentioning this. Change
spin_lock_irqs to spin_locks. Change error handling to goto-style.

Also change the other place enable_device_list_for_node is called,
add_initiator_node_lun_acl. Hold tpg_lun_lock across all uses of lun.
Change error handling to release lock from error paths. Remove
core_dev_get_lun.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_device.c | 78 ++++++++++++++-----------------------
 drivers/target/target_core_tpg.c    |  3 --
 2 files changed, 29 insertions(+), 52 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 9789a11..b15219e 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -295,7 +295,7 @@ void core_update_device_list_access(
 
 /*      core_enable_device_list_for_node():
  *
- *
+ * Called with tpg_lun_lock held & irqs off
  */
 int core_enable_device_list_for_node(
 	struct se_lun *lun,
@@ -307,8 +307,9 @@ int core_enable_device_list_for_node(
 {
 	struct se_port *port = lun->lun_sep;
 	struct se_dev_entry *deve;
+	int ret = 0;
 
-	spin_lock_irq(&nacl->device_list_lock);
+	spin_lock(&nacl->device_list_lock);
 
 	deve = nacl->device_list[mapped_lun];
 
@@ -322,15 +323,15 @@ int core_enable_device_list_for_node(
 			pr_err("struct se_dev_entry->se_lun_acl"
 			       " already set for demo mode -> explicit"
 			       " LUN ACL transition\n");
-			spin_unlock_irq(&nacl->device_list_lock);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 		if (deve->se_lun != lun) {
 			pr_err("struct se_dev_entry->se_lun does"
 			       " match passed struct se_lun for demo mode"
 			       " -> explicit LUN ACL transition\n");
-			spin_unlock_irq(&nacl->device_list_lock);
-			return -EINVAL;
+			ret = -EINVAL;
+			goto out;
 		}
 		deve->se_lun_acl = lun_acl;
 
@@ -342,8 +343,7 @@ int core_enable_device_list_for_node(
 			deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
 		}
 
-		spin_unlock_irq(&nacl->device_list_lock);
-		return 0;
+		goto out;
 	}
 
 	deve->se_lun = lun;
@@ -366,9 +366,10 @@ int core_enable_device_list_for_node(
 	list_add_tail(&deve->alua_port_list, &port->sep_alua_list);
 	spin_unlock(&port->sep_alua_lock);
 
-	spin_unlock_irq(&nacl->device_list_lock);
+out:
+	spin_unlock(&nacl->device_list_lock);
 
-	return 0;
+	return ret;
 }
 
 /*      core_disable_device_list_for_node():
@@ -1299,39 +1300,6 @@ struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_l
 	return lun;
 }
 
-/*      core_dev_get_lun():
- *
- *
- */
-static struct se_lun *core_dev_get_lun(struct se_portal_group *tpg, u32 unpacked_lun)
-{
-	struct se_lun *lun;
-
-	spin_lock(&tpg->tpg_lun_lock);
-	if (unpacked_lun > (TRANSPORT_MAX_LUNS_PER_TPG-1)) {
-		pr_err("%s LUN: %u exceeds TRANSPORT_MAX_LUNS_PER"
-			"_TPG-1: %u for Target Portal Group: %hu\n",
-			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
-			TRANSPORT_MAX_LUNS_PER_TPG-1,
-			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		spin_unlock(&tpg->tpg_lun_lock);
-		return NULL;
-	}
-	lun = tpg->tpg_lun_list[unpacked_lun];
-
-	if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE) {
-		pr_err("%s Logical Unit Number: %u is not active on"
-			" Target Portal Group: %hu, ignoring request.\n",
-			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
-			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		spin_unlock(&tpg->tpg_lun_lock);
-		return NULL;
-	}
-	spin_unlock(&tpg->tpg_lun_lock);
-
-	return lun;
-}
-
 struct se_lun_acl *core_dev_init_initiator_node_lun_acl(
 	struct se_portal_group *tpg,
 	struct se_node_acl *nacl,
@@ -1370,19 +1338,24 @@ int core_dev_add_initiator_node_lun_acl(
 {
 	struct se_lun *lun;
 	struct se_node_acl *nacl;
+	int ret = 0;
 
-	lun = core_dev_get_lun(tpg, unpacked_lun);
+	spin_lock(&tpg->tpg_lun_lock);
+	lun = tpg->tpg_lun_list[unpacked_lun];
 	if (!lun) {
 		pr_err("%s Logical Unit Number: %u is not active on"
 			" Target Portal Group: %hu, ignoring request.\n",
 			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
 			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
 
 	nacl = lacl->se_lun_nacl;
-	if (!nacl)
-		return -EINVAL;
+	if (!nacl) {
+		ret = -EINVAL;
+		goto out;
+	}
 
 	if ((lun->lun_access & TRANSPORT_LUNFLAGS_READ_ONLY) &&
 	    (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE))
@@ -1392,7 +1365,10 @@ int core_dev_add_initiator_node_lun_acl(
 
 	if (core_enable_device_list_for_node(lun, lacl, lacl->mapped_lun,
 			lun_access, nacl, tpg) < 0)
-		return -EINVAL;
+	{
+		ret = -EINVAL;
+		goto out;
+	}
 
 	spin_lock(&lun->lun_acl_lock);
 	list_add_tail(&lacl->lacl_list, &lun->lun_acl_list);
@@ -1410,7 +1386,11 @@ int core_dev_add_initiator_node_lun_acl(
 	 * pre-registrations that need to be enabled for this LUN ACL..
 	 */
 	core_scsi3_check_aptpl_registration(lun->lun_se_dev, tpg, lun, lacl);
-	return 0;
+
+out:
+	spin_unlock(&tpg->tpg_lun_lock);
+
+	return ret;
 }
 
 /*      core_dev_del_initiator_node_lun_acl():
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index c036595..88fddcf 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -137,8 +137,6 @@ void core_tpg_add_node_to_devs(
 		if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
 			continue;
 
-		spin_unlock(&tpg->tpg_lun_lock);
-
 		dev = lun->lun_se_dev;
 		/*
 		 * By default in LIO-Target $FABRIC_MOD,
@@ -166,7 +164,6 @@ void core_tpg_add_node_to_devs(
 
 		core_enable_device_list_for_node(lun, NULL, lun->unpacked_lun,
 				lun_access, acl, tpg);
-		spin_lock(&tpg->tpg_lun_lock);
 	}
 	spin_unlock(&tpg->tpg_lun_lock);
 }
-- 
1.9.3

Re: [PATCHv2 RESENDv2 2/9] target: Don't unlock/relock tpg_lun_lock in loop in add_node_to_devs

[Christoph Hellwig]

> - *
> + * Called with tpg_lun_lock held & irqs off
>   */

Instead of writing this down the code should assert the pre-conditions, e.g.

	assert_spin_locked(&tpg->tpg_lun_lock);
	WARN_ON_ONCE(!irqs_disabled());

>  	if (core_enable_device_list_for_node(lun, lacl, lacl->mapped_lun,
>  			lun_access, nacl, tpg) < 0)
> -		return -EINVAL;
> +	{

Wrong brace placement.

Also I can't see that anything ensures disabled irqs here.

[PATCHv2 RESENDv2 3/9] target: Allocate se_dev_entrys in device list only when used

[Andy Grover]

Remove TRANSPORT_LUNFLAGS_INITIATOR_ACCESS and just look at if a
non-NULL value is in the node_acl->device_list for the given lun. Since
usually nowhere near all se_dev_entrys are used, this nets a sizable
reduction in memory use.

In core_disable_device_list_for_node, move all calls to functions
referencing deve inside the spinlock, since it's not safe to access deve
outside it. Skip reinitializing, since the deve is actually being freed.

Do not allocate device_list array separately, but include it in
se_node_acl.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_device.c          | 94 +++++++++++++++-------------
 drivers/target/target_core_fabric_configfs.c |  4 +-
 drivers/target/target_core_spc.c             |  2 +-
 drivers/target/target_core_tpg.c             | 41 +-----------
 include/target/target_core_base.h            |  7 +--
 5 files changed, 59 insertions(+), 89 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index b15219e..c5ed8f8 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -67,7 +67,7 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
 
 	spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
 	se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
-	if (se_cmd->se_deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
+	if (se_cmd->se_deve) {
 		struct se_dev_entry *deve = se_cmd->se_deve;
 
 		deve->total_cmds++;
@@ -143,7 +143,6 @@ EXPORT_SYMBOL(transport_lookup_cmd_lun);
 
 int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
 {
-	struct se_dev_entry *deve;
 	struct se_lun *se_lun = NULL;
 	struct se_session *se_sess = se_cmd->se_sess;
 	struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
@@ -154,9 +153,9 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u32 unpacked_lun)
 
 	spin_lock_irqsave(&se_sess->se_node_acl->device_list_lock, flags);
 	se_cmd->se_deve = se_sess->se_node_acl->device_list[unpacked_lun];
-	deve = se_cmd->se_deve;
+	if (se_cmd->se_deve) {
+		struct se_dev_entry *deve = se_cmd->se_deve;
 
-	if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS) {
 		se_tmr->tmr_lun = deve->se_lun;
 		se_cmd->se_lun = deve->se_lun;
 		se_lun = deve->se_lun;
@@ -204,7 +203,7 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		deve = nacl->device_list[i];
 
-		if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
+		if (!deve)
 			continue;
 
 		lun = deve->se_lun;
@@ -243,14 +242,11 @@ int core_free_device_list_for_node(
 	struct se_lun *lun;
 	u32 i;
 
-	if (!nacl->device_list)
-		return 0;
-
 	spin_lock_irq(&nacl->device_list_lock);
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		deve = nacl->device_list[i];
 
-		if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
+		if (!deve)
 			continue;
 
 		if (!deve->se_lun) {
@@ -268,9 +264,6 @@ int core_free_device_list_for_node(
 	}
 	spin_unlock_irq(&nacl->device_list_lock);
 
-	array_free(nacl->device_list, TRANSPORT_MAX_LUNS_PER_TPG);
-	nacl->device_list = NULL;
-
 	return 0;
 }
 
@@ -283,12 +276,14 @@ void core_update_device_list_access(
 
 	spin_lock_irq(&nacl->device_list_lock);
 	deve = nacl->device_list[mapped_lun];
-	if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) {
-		deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY;
-		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
-	} else {
-		deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_WRITE;
-		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
+	if (deve) {
+		if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) {
+			deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY;
+			deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
+		} else {
+			deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_WRITE;
+			deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
+		}
 	}
 	spin_unlock_irq(&nacl->device_list_lock);
 }
@@ -318,7 +313,7 @@ int core_enable_device_list_for_node(
 	 * 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) {
 		if (deve->se_lun_acl != NULL) {
 			pr_err("struct se_dev_entry->se_lun_acl"
 			       " already set for demo mode -> explicit"
@@ -346,18 +341,27 @@ int core_enable_device_list_for_node(
 		goto out;
 	}
 
+	deve = kzalloc(sizeof(*deve), GFP_ATOMIC);
+	if (!deve) {
+		spin_unlock_irq(&nacl->device_list_lock);
+		return -ENOMEM;
+	}
+
+	nacl->device_list[mapped_lun] = deve;
+
+	atomic_set(&deve->ua_count, 0);
+	atomic_set(&deve->pr_ref_count, 0);
+	spin_lock_init(&deve->ua_lock);
+	INIT_LIST_HEAD(&deve->alua_port_list);
+	INIT_LIST_HEAD(&deve->ua_list);
 	deve->se_lun = lun;
 	deve->se_lun_acl = lun_acl;
 	deve->mapped_lun = mapped_lun;
-	deve->lun_flags |= TRANSPORT_LUNFLAGS_INITIATOR_ACCESS;
 
-	if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) {
-		deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY;
+	if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE)
 		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
-	} else {
-		deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_WRITE;
+	else
 		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
-	}
 
 	deve->creation_time = get_jiffies_64();
 	deve->attach_count++;
@@ -385,7 +389,23 @@ int core_disable_device_list_for_node(
 	struct se_portal_group *tpg)
 {
 	struct se_port *port = lun->lun_sep;
-	struct se_dev_entry *deve = nacl->device_list[mapped_lun];
+	struct se_dev_entry *deve;
+
+	core_scsi3_free_pr_reg_from_nacl(lun->lun_se_dev, nacl);
+
+	spin_lock_irq(&nacl->device_list_lock);
+	deve = nacl->device_list[mapped_lun];
+	if (!deve) {
+		spin_unlock_irq(&nacl->device_list_lock);
+		return 0;
+	}
+
+	/*
+	 * Wait for any in process SPEC_I_PT=1 or REGISTER_AND_MOVE
+	 * PR operation to complete.
+	 */
+	while (atomic_read(&deve->pr_ref_count) != 0)
+		cpu_relax();
 
 	/*
 	 * If the MappedLUN entry is being disabled, the entry in
@@ -400,29 +420,19 @@ int core_disable_device_list_for_node(
 	 * NodeACL context specific PR metadata for demo-mode
 	 * MappedLUN *deve will be released below..
 	 */
-	spin_lock_bh(&port->sep_alua_lock);
+	spin_lock(&port->sep_alua_lock);
 	list_del(&deve->alua_port_list);
-	spin_unlock_bh(&port->sep_alua_lock);
-	/*
-	 * Wait for any in process SPEC_I_PT=1 or REGISTER_AND_MOVE
-	 * PR operation to complete.
-	 */
-	while (atomic_read(&deve->pr_ref_count) != 0)
-		cpu_relax();
+	spin_unlock(&port->sep_alua_lock);
 
-	spin_lock_irq(&nacl->device_list_lock);
 	/*
 	 * Disable struct se_dev_entry LUN ACL mapping
 	 */
 	core_scsi3_ua_release_all(deve);
-	deve->se_lun = NULL;
-	deve->se_lun_acl = NULL;
-	deve->lun_flags = 0;
-	deve->creation_time = 0;
-	deve->attach_count--;
+	nacl->device_list[mapped_lun] = NULL;
+	kfree(deve);
+
 	spin_unlock_irq(&nacl->device_list_lock);
 
-	core_scsi3_free_pr_reg_from_nacl(lun->lun_se_dev, nacl);
 	return 0;
 }
 
@@ -443,7 +453,7 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg)
 		spin_lock_irq(&nacl->device_list_lock);
 		for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 			deve = nacl->device_list[i];
-			if (lun != deve->se_lun)
+			if (!deve || lun != deve->se_lun)
 				continue;
 			spin_unlock_irq(&nacl->device_list_lock);
 
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 804e7f0..0955c948 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -113,7 +113,7 @@ static int target_fabric_mappedlun_link(
 	 */
 	spin_lock_irq(&lacl->se_lun_nacl->device_list_lock);
 	deve = lacl->se_lun_nacl->device_list[lacl->mapped_lun];
-	if (deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS)
+	if (deve)
 		lun_access = deve->lun_flags;
 	else
 		lun_access =
@@ -149,7 +149,7 @@ static int target_fabric_mappedlun_unlink(
 	/*
 	 * Determine if the underlying MappedLUN has already been released..
 	 */
-	if (!deve->se_lun) {
+	if (!deve || !deve->se_lun) {
 		spin_unlock_irq(&nacl->device_list_lock);
 		return 0;
 	}
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index 6cd7222..cb47ffe 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -1250,7 +1250,7 @@ sense_reason_t spc_emulate_report_luns(struct se_cmd *cmd)
 	spin_lock_irq(&sess->se_node_acl->device_list_lock);
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		deve = sess->se_node_acl->device_list[i];
-		if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
+		if (!deve)
 			continue;
 		/*
 		 * We determine the correct LUN LIST LENGTH even once we
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 88fddcf..82a25e1 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -62,7 +62,7 @@ static void core_clear_initiator_node_from_tpg(
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		deve = nacl->device_list[i];
 
-		if (!(deve->lun_flags & TRANSPORT_LUNFLAGS_INITIATOR_ACCESS))
+		if (!deve)
 			continue;
 
 		if (!deve->se_lun) {
@@ -214,35 +214,6 @@ static void *array_zalloc(int n, size_t size, gfp_t flags)
 	return a;
 }
 
-/*      core_create_device_list_for_node():
- *
- *
- */
-static int core_create_device_list_for_node(struct se_node_acl *nacl)
-{
-	struct se_dev_entry *deve;
-	int i;
-
-	nacl->device_list = array_zalloc(TRANSPORT_MAX_LUNS_PER_TPG,
-			sizeof(struct se_dev_entry), GFP_KERNEL);
-	if (!nacl->device_list) {
-		pr_err("Unable to allocate memory for"
-			" struct se_node_acl->device_list\n");
-		return -ENOMEM;
-	}
-	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
-		deve = nacl->device_list[i];
-
-		atomic_set(&deve->ua_count, 0);
-		atomic_set(&deve->pr_ref_count, 0);
-		spin_lock_init(&deve->ua_lock);
-		INIT_LIST_HEAD(&deve->alua_port_list);
-		INIT_LIST_HEAD(&deve->ua_list);
-	}
-
-	return 0;
-}
-
 /*	core_tpg_check_initiator_node_acl()
  *
  *
@@ -279,11 +250,6 @@ struct se_node_acl *core_tpg_check_initiator_node_acl(
 
 	tpg->se_tpg_tfo->set_default_node_attributes(acl);
 
-	if (core_create_device_list_for_node(acl) < 0) {
-		tpg->se_tpg_tfo->tpg_release_fabric_acl(tpg, acl);
-		return NULL;
-	}
-
 	if (core_set_queue_depth_for_node(tpg, acl) < 0) {
 		core_free_device_list_for_node(acl, tpg);
 		tpg->se_tpg_tfo->tpg_release_fabric_acl(tpg, acl);
@@ -405,11 +371,6 @@ struct se_node_acl *core_tpg_add_initiator_node_acl(
 
 	tpg->se_tpg_tfo->set_default_node_attributes(acl);
 
-	if (core_create_device_list_for_node(acl) < 0) {
-		tpg->se_tpg_tfo->tpg_release_fabric_acl(tpg, acl);
-		return ERR_PTR(-ENOMEM);
-	}
-
 	if (core_set_queue_depth_for_node(tpg, acl) < 0) {
 		core_free_device_list_for_node(acl, tpg);
 		tpg->se_tpg_tfo->tpg_release_fabric_acl(tpg, acl);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 9ec9864..f3761da 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -174,9 +174,8 @@ enum se_cmd_flags_table {
 /* struct se_dev_entry->lun_flags and struct se_lun->lun_access */
 enum transport_lunflags_table {
 	TRANSPORT_LUNFLAGS_NO_ACCESS		= 0x00,
-	TRANSPORT_LUNFLAGS_INITIATOR_ACCESS	= 0x01,
-	TRANSPORT_LUNFLAGS_READ_ONLY		= 0x02,
-	TRANSPORT_LUNFLAGS_READ_WRITE		= 0x04,
+	TRANSPORT_LUNFLAGS_READ_ONLY		= 0x01,
+	TRANSPORT_LUNFLAGS_READ_WRITE		= 0x02,
 };
 
 /*
@@ -589,7 +588,7 @@ struct se_node_acl {
 	char			acl_tag[MAX_ACL_TAG_SIZE];
 	/* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */
 	atomic_t		acl_pr_ref_count;
-	struct se_dev_entry	**device_list;
+	struct se_dev_entry	*device_list[TRANSPORT_MAX_LUNS_PER_TPG];
 	struct se_session	*nacl_sess;
 	struct se_portal_group *se_tpg;
 	spinlock_t		device_list_lock;
-- 
1.9.3

Re: [PATCHv2 RESENDv2 3/9] target: Allocate se_dev_entrys in device list only when used

[Christoph Hellwig]

> +	/*
> +	 * Wait for any in process SPEC_I_PT=1 or REGISTER_AND_MOVE
> +	 * PR operation to complete.
> +	 */
> +	while (atomic_read(&deve->pr_ref_count) != 0)
> +		cpu_relax();

A busy loop under an irq disabling spinlock is fairly nasty.  Can we just
remove the pointer to the entry from the device list array, then unlock
and then have the busy loop?

>  enum transport_lunflags_table {
>  	TRANSPORT_LUNFLAGS_NO_ACCESS		= 0x00,
> -	TRANSPORT_LUNFLAGS_INITIATOR_ACCESS	= 0x01,
> -	TRANSPORT_LUNFLAGS_READ_ONLY		= 0x02,
> -	TRANSPORT_LUNFLAGS_READ_WRITE		= 0x04,
> +	TRANSPORT_LUNFLAGS_READ_ONLY		= 0x01,
> +	TRANSPORT_LUNFLAGS_READ_WRITE		= 0x02,

Not something for this patch, but having the read only and read/write
flags is silly.  Having one of them should be enough, either a rdonly
or a writeable one would do it.

[PATCHv2 RESENDv2 4/9] target: core_tpg_post_dellun can return void

[Andy Grover]

Nothing in it can raise an error.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_internal.h | 2 +-
 drivers/target/target_core_tpg.c      | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index de9cab7..463fddc 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -83,7 +83,7 @@ struct se_lun *core_tpg_alloc_lun(struct se_portal_group *, u32);
 int	core_tpg_add_lun(struct se_portal_group *, struct se_lun *,
 		u32, struct se_device *);
 struct se_lun *core_tpg_pre_dellun(struct se_portal_group *, u32 unpacked_lun);
-int	core_tpg_post_dellun(struct se_portal_group *, struct se_lun *);
+void core_tpg_post_dellun(struct se_portal_group *, struct se_lun *);
 
 /* target_core_transport.c */
 extern struct kmem_cache *se_tmr_req_cache;
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 82a25e1..321268d 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -825,18 +825,15 @@ struct se_lun *core_tpg_pre_dellun(
 	return lun;
 }
 
-int core_tpg_post_dellun(
+void core_tpg_post_dellun(
 	struct se_portal_group *tpg,
 	struct se_lun *lun)
 {
 	core_clear_lun_from_tpg(lun, tpg);
 	transport_clear_lun_ref(lun);
-
 	core_dev_unexport(lun->lun_se_dev, tpg, lun);
 
 	spin_lock(&tpg->tpg_lun_lock);
 	lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
 	spin_unlock(&tpg->tpg_lun_lock);
-
-	return 0;
 }
-- 
1.9.3

Re: [PATCHv2 RESENDv2 4/9] target: core_tpg_post_dellun can return void

[Christoph Hellwig]

On Mon, Jun 23, 2014 at 04:55:33PM -0700, Andy Grover wrote:
> Nothing in it can raise an error.
> 
> Signed-off-by: Andy Grover <agrover@redhat.com>

Looks good,

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

[PATCHv2 RESENDv2 5/9] target: Change core_dev_del_lun to take a se_lun instead of unpacked_lun

[Andy Grover]

Remove core_tpg_pre_dellun entirely, since we don't need to get/check
a pointer we already have.

Nothing else can return an error, so core_dev_del_lun can return void.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_device.c          | 18 +++++-----------
 drivers/target/target_core_fabric_configfs.c |  2 +-
 drivers/target/target_core_internal.h        |  3 +--
 drivers/target/target_core_tpg.c             | 32 +---------------------------
 4 files changed, 8 insertions(+), 47 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index c5ed8f8..c90d118 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1261,24 +1261,16 @@ struct se_lun *core_dev_add_lun(
  *
  *
  */
-int core_dev_del_lun(
+void core_dev_del_lun(
 	struct se_portal_group *tpg,
-	u32 unpacked_lun)
+	struct se_lun *lun)
 {
-	struct se_lun *lun;
-
-	lun = core_tpg_pre_dellun(tpg, unpacked_lun);
-	if (IS_ERR(lun))
-		return PTR_ERR(lun);
-
-	core_tpg_post_dellun(tpg, lun);
-
-	pr_debug("%s_TPG[%u]_LUN[%u] - Deactivated %s Logical Unit from"
+	pr_debug("%s_TPG[%u]_LUN[%u] - Deactivating %s Logical Unit from"
 		" device object\n", tpg->se_tpg_tfo->get_fabric_name(),
-		tpg->se_tpg_tfo->tpg_get_tag(tpg), unpacked_lun,
+		tpg->se_tpg_tfo->tpg_get_tag(tpg), lun->unpacked_lun,
 		tpg->se_tpg_tfo->get_fabric_name());
 
-	return 0;
+	core_tpg_post_dellun(tpg, lun);
 }
 
 struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_lun)
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 0955c948..811b2f4 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -827,7 +827,7 @@ static int target_fabric_port_unlink(
 		tf->tf_ops.fabric_pre_unlink(se_tpg, lun);
 	}
 
-	core_dev_del_lun(se_tpg, lun->unpacked_lun);
+	core_dev_del_lun(se_tpg, lun);
 	return 0;
 }
 
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 463fddc..22c4261 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -46,7 +46,7 @@ int	se_dev_set_fabric_max_sectors(struct se_device *, u32);
 int	se_dev_set_optimal_sectors(struct se_device *, u32);
 int	se_dev_set_block_size(struct se_device *, u32);
 struct se_lun *core_dev_add_lun(struct se_portal_group *, struct se_device *, u32);
-int	core_dev_del_lun(struct se_portal_group *, u32);
+void	core_dev_del_lun(struct se_portal_group *, struct se_lun *);
 struct se_lun *core_get_lun_from_tpg(struct se_portal_group *, u32);
 struct se_lun_acl *core_dev_init_initiator_node_lun_acl(struct se_portal_group *,
 		struct se_node_acl *, u32, int *);
@@ -82,7 +82,6 @@ void	core_tpg_wait_for_nacl_pr_ref(struct se_node_acl *);
 struct se_lun *core_tpg_alloc_lun(struct se_portal_group *, u32);
 int	core_tpg_add_lun(struct se_portal_group *, struct se_lun *,
 		u32, struct se_device *);
-struct se_lun *core_tpg_pre_dellun(struct se_portal_group *, u32 unpacked_lun);
 void core_tpg_post_dellun(struct se_portal_group *, struct se_lun *);
 
 /* target_core_transport.c */
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 321268d..b50d667 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -298,7 +298,7 @@ void core_tpg_clear_object_luns(struct se_portal_group *tpg)
 			continue;
 
 		spin_unlock(&tpg->tpg_lun_lock);
-		core_dev_del_lun(tpg, lun->unpacked_lun);
+		core_dev_del_lun(tpg, lun);
 		spin_lock(&tpg->tpg_lun_lock);
 	}
 	spin_unlock(&tpg->tpg_lun_lock);
@@ -795,36 +795,6 @@ int core_tpg_add_lun(
 	return 0;
 }
 
-struct se_lun *core_tpg_pre_dellun(
-	struct se_portal_group *tpg,
-	u32 unpacked_lun)
-{
-	struct se_lun *lun;
-
-	if (unpacked_lun > (TRANSPORT_MAX_LUNS_PER_TPG-1)) {
-		pr_err("%s LUN: %u exceeds TRANSPORT_MAX_LUNS_PER_TPG"
-			"-1: %u for Target Portal Group: %u\n",
-			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
-			TRANSPORT_MAX_LUNS_PER_TPG-1,
-			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		return ERR_PTR(-EOVERFLOW);
-	}
-
-	spin_lock(&tpg->tpg_lun_lock);
-	lun = tpg->tpg_lun_list[unpacked_lun];
-	if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE) {
-		pr_err("%s Logical Unit Number: %u is not active on"
-			" Target Portal Group: %u, ignoring request.\n",
-			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
-			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		spin_unlock(&tpg->tpg_lun_lock);
-		return ERR_PTR(-ENODEV);
-	}
-	spin_unlock(&tpg->tpg_lun_lock);
-
-	return lun;
-}
-
 void core_tpg_post_dellun(
 	struct se_portal_group *tpg,
 	struct se_lun *lun)
-- 
1.9.3

Re: [PATCHv2 RESENDv2 5/9] target: Change core_dev_del_lun to take a se_lun instead of unpacked_lun

[Christoph Hellwig]

On Mon, Jun 23, 2014 at 04:55:34PM -0700, Andy Grover wrote:
> Remove core_tpg_pre_dellun entirely, since we don't need to get/check
> a pointer we already have.
> 
> Nothing else can return an error, so core_dev_del_lun can return void.
> 
> Signed-off-by: Andy Grover <agrover@redhat.com>

Looks good,

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

[PATCHv2 RESENDv2 6/9] target: Rename core_tpg_post_dellun to remove_lun

[Andy Grover]

A clearer name, now that pre_dellun is gone.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_device.c   | 2 +-
 drivers/target/target_core_internal.h | 2 +-
 drivers/target/target_core_tpg.c      | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index c90d118..f2bf417 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1270,7 +1270,7 @@ void core_dev_del_lun(
 		tpg->se_tpg_tfo->tpg_get_tag(tpg), lun->unpacked_lun,
 		tpg->se_tpg_tfo->get_fabric_name());
 
-	core_tpg_post_dellun(tpg, lun);
+	core_tpg_remove_lun(tpg, lun);
 }
 
 struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_lun)
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 22c4261..42ef4ab 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -82,7 +82,7 @@ void	core_tpg_wait_for_nacl_pr_ref(struct se_node_acl *);
 struct se_lun *core_tpg_alloc_lun(struct se_portal_group *, u32);
 int	core_tpg_add_lun(struct se_portal_group *, struct se_lun *,
 		u32, struct se_device *);
-void core_tpg_post_dellun(struct se_portal_group *, struct se_lun *);
+void core_tpg_remove_lun(struct se_portal_group *, struct se_lun *);
 
 /* target_core_transport.c */
 extern struct kmem_cache *se_tmr_req_cache;
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index b50d667..5d2c774 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -625,7 +625,7 @@ static void core_tpg_release_virtual_lun0(struct se_portal_group *se_tpg)
 {
 	struct se_lun *lun = &se_tpg->tpg_virt_lun0;
 
-	core_tpg_post_dellun(se_tpg, lun);
+	core_tpg_remove_lun(se_tpg, lun);
 }
 
 int core_tpg_register(
@@ -795,7 +795,7 @@ int core_tpg_add_lun(
 	return 0;
 }
 
-void core_tpg_post_dellun(
+void core_tpg_remove_lun(
 	struct se_portal_group *tpg,
 	struct se_lun *lun)
 {
-- 
1.9.3

Re: [PATCHv2 RESENDv2 6/9] target: Rename core_tpg_post_dellun to remove_lun

[Christoph Hellwig]

On Mon, Jun 23, 2014 at 04:55:35PM -0700, Andy Grover wrote:
> A clearer name, now that pre_dellun is gone.
> 
> Signed-off-by: Andy Grover <agrover@redhat.com>

Looks good, but might be worth merging into the previous patch.

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

[PATCHv2 RESENDv2 7/9] target: Allocate se_luns only when used

[Andy Grover]

Instead of allocating the tpg_lun_list and each member of the list when
the tpg is created, make the list part of the parent struct, and use an
element being non-NULL to indicate an active LUN. This will save memory
if less than all the se_luns are actually configured.

Now that things are actually getting freed, split out core_tpg_free_lun
from core_tpg_remove_lun, because we don't want to free() the statically-
allocated virtual_lun0.

Remove array_free and array_zalloc.

Remove core_get_lun_from_tpg.

Change core_dev_add_lun to take a se_lun and return int

Do not allocate tpg_lun_list, put it directly in se_portal_group.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/sbp/sbp_target.c              |   6 +-
 drivers/target/target_core_device.c          |  43 ++----------
 drivers/target/target_core_fabric_configfs.c |  21 +++---
 drivers/target/target_core_internal.h        |   4 +-
 drivers/target/target_core_tpg.c             | 101 +++++++++------------------
 include/target/target_core_base.h            |  10 +--
 6 files changed, 54 insertions(+), 131 deletions(-)

diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index e7e9372..62b9d47 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -187,7 +187,7 @@ static struct se_lun *sbp_get_lun_from_tpg(struct sbp_tpg *tpg, int lun)
 	spin_lock(&se_tpg->tpg_lun_lock);
 	se_lun = se_tpg->tpg_lun_list[lun];
 
-	if (se_lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
+	if (!se_lun)
 		se_lun = ERR_PTR(-ENODEV);
 
 	spin_unlock(&se_tpg->tpg_lun_lock);
@@ -1947,7 +1947,7 @@ static int sbp_count_se_tpg_luns(struct se_portal_group *tpg)
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		struct se_lun *se_lun = tpg->tpg_lun_list[i];
 
-		if (se_lun->lun_status == TRANSPORT_LUN_STATUS_FREE)
+		if (!se_lun)
 			continue;
 
 		count++;
@@ -2027,7 +2027,7 @@ static int sbp_update_unit_directory(struct sbp_tport *tport)
 		struct se_device *dev;
 		int type;
 
-		if (se_lun->lun_status == TRANSPORT_LUN_STATUS_FREE)
+		if (!se_lun)
 			continue;
 
 		spin_unlock(&tport->tpg->se_tpg.tpg_lun_lock);
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index f2bf417..28ab38f 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1214,22 +1214,17 @@ int se_dev_set_block_size(struct se_device *dev, u32 block_size)
 	return 0;
 }
 
-struct se_lun *core_dev_add_lun(
+int core_dev_add_lun(
 	struct se_portal_group *tpg,
 	struct se_device *dev,
-	u32 unpacked_lun)
+	struct se_lun *lun)
 {
-	struct se_lun *lun;
 	int rc;
 
-	lun = core_tpg_alloc_lun(tpg, unpacked_lun);
-	if (IS_ERR(lun))
-		return lun;
-
 	rc = core_tpg_add_lun(tpg, lun,
 				TRANSPORT_LUNFLAGS_READ_WRITE, dev);
 	if (rc < 0)
-		return ERR_PTR(rc);
+		return rc;
 
 	pr_debug("%s_TPG[%u]_LUN[%u] - Activated %s Logical Unit from"
 		" CORE HBA: %u\n", tpg->se_tpg_tfo->get_fabric_name(),
@@ -1254,7 +1249,7 @@ struct se_lun *core_dev_add_lun(
 		spin_unlock_irq(&tpg->acl_node_lock);
 	}
 
-	return lun;
+	return rc;
 }
 
 /*      core_dev_del_lun():
@@ -1271,35 +1266,7 @@ void core_dev_del_lun(
 		tpg->se_tpg_tfo->get_fabric_name());
 
 	core_tpg_remove_lun(tpg, lun);
-}
-
-struct se_lun *core_get_lun_from_tpg(struct se_portal_group *tpg, u32 unpacked_lun)
-{
-	struct se_lun *lun;
-
-	spin_lock(&tpg->tpg_lun_lock);
-	if (unpacked_lun > (TRANSPORT_MAX_LUNS_PER_TPG-1)) {
-		pr_err("%s LUN: %u exceeds TRANSPORT_MAX_LUNS"
-			"_PER_TPG-1: %u for Target Portal Group: %hu\n",
-			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
-			TRANSPORT_MAX_LUNS_PER_TPG-1,
-			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		spin_unlock(&tpg->tpg_lun_lock);
-		return NULL;
-	}
-	lun = tpg->tpg_lun_list[unpacked_lun];
-
-	if (lun->lun_status != TRANSPORT_LUN_STATUS_FREE) {
-		pr_err("%s Logical Unit Number: %u is not free on"
-			" Target Portal Group: %hu, ignoring request.\n",
-			tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
-			tpg->se_tpg_tfo->tpg_get_tag(tpg));
-		spin_unlock(&tpg->tpg_lun_lock);
-		return NULL;
-	}
-	spin_unlock(&tpg->tpg_lun_lock);
-
-	return lun;
+	core_tpg_free_lun(tpg, lun);
 }
 
 struct se_lun_acl *core_dev_init_initiator_node_lun_acl(
diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
index 811b2f4..95c1cd5 100644
--- a/drivers/target/target_core_fabric_configfs.c
+++ b/drivers/target/target_core_fabric_configfs.c
@@ -760,7 +760,6 @@ static int target_fabric_port_link(
 	struct config_item *tpg_ci;
 	struct se_lun *lun = container_of(to_config_group(lun_ci),
 				struct se_lun, lun_group);
-	struct se_lun *lun_p;
 	struct se_portal_group *se_tpg;
 	struct se_device *dev =
 		container_of(to_config_group(se_dev_ci), struct se_device, dev_group);
@@ -788,11 +787,10 @@ static int target_fabric_port_link(
 		return -EEXIST;
 	}
 
-	lun_p = core_dev_add_lun(se_tpg, dev, lun->unpacked_lun);
-	if (IS_ERR(lun_p)) {
+	ret = core_dev_add_lun(se_tpg, dev, lun);
+	if (ret < 0) {
 		pr_err("core_dev_add_lun() failed\n");
-		ret = PTR_ERR(lun_p);
-		goto out;
+		return ret;
 	}
 
 	if (tf->tf_ops.fabric_post_link) {
@@ -805,8 +803,6 @@ static int target_fabric_port_link(
 	}
 
 	return 0;
-out:
-	return ret;
 }
 
 static int target_fabric_port_unlink(
@@ -892,16 +888,17 @@ static struct config_group *target_fabric_make_lun(
 	if (unpacked_lun > UINT_MAX)
 		return ERR_PTR(-EINVAL);
 
-	lun = core_get_lun_from_tpg(se_tpg, unpacked_lun);
-	if (!lun)
-		return ERR_PTR(-EINVAL);
+	lun = core_tpg_alloc_lun(se_tpg, unpacked_lun);
+	if (IS_ERR(lun))
+		return ERR_CAST(lun);
 
 	lun_cg = &lun->lun_group;
 	lun_cg->default_groups = kmalloc(sizeof(struct config_group *) * 2,
 				GFP_KERNEL);
 	if (!lun_cg->default_groups) {
 		pr_err("Unable to allocate lun_cg->default_groups\n");
-		return ERR_PTR(-ENOMEM);
+		errno = -ENOMEM;
+		goto out;
 	}
 
 	config_group_init_type_name(&lun->lun_group, name,
@@ -923,6 +920,8 @@ static struct config_group *target_fabric_make_lun(
 
 	return &lun->lun_group;
 out:
+	if (lun)
+		core_tpg_free_lun(se_tpg, lun);
 	if (lun_cg)
 		kfree(lun_cg->default_groups);
 	return ERR_PTR(errno);
diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
index 42ef4ab..4f3e6d5 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -45,9 +45,8 @@ int	se_dev_set_max_sectors(struct se_device *, u32);
 int	se_dev_set_fabric_max_sectors(struct se_device *, u32);
 int	se_dev_set_optimal_sectors(struct se_device *, u32);
 int	se_dev_set_block_size(struct se_device *, u32);
-struct se_lun *core_dev_add_lun(struct se_portal_group *, struct se_device *, u32);
+int	core_dev_add_lun(struct se_portal_group *, struct se_device *, struct se_lun *);
 void	core_dev_del_lun(struct se_portal_group *, struct se_lun *);
-struct se_lun *core_get_lun_from_tpg(struct se_portal_group *, u32);
 struct se_lun_acl *core_dev_init_initiator_node_lun_acl(struct se_portal_group *,
 		struct se_node_acl *, u32, int *);
 int	core_dev_add_initiator_node_lun_acl(struct se_portal_group *,
@@ -83,6 +82,7 @@ struct se_lun *core_tpg_alloc_lun(struct se_portal_group *, u32);
 int	core_tpg_add_lun(struct se_portal_group *, struct se_lun *,
 		u32, struct se_device *);
 void core_tpg_remove_lun(struct se_portal_group *, struct se_lun *);
+void core_tpg_free_lun(struct se_portal_group *, struct se_lun *);
 
 /* target_core_transport.c */
 extern struct kmem_cache *se_tmr_req_cache;
diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index 5d2c774..a63a3d7 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -134,7 +134,7 @@ void core_tpg_add_node_to_devs(
 	spin_lock(&tpg->tpg_lun_lock);
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		lun = tpg->tpg_lun_list[i];
-		if (lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE)
+		if (!lun)
 			continue;
 
 		dev = lun->lun_se_dev;
@@ -186,34 +186,6 @@ static int core_set_queue_depth_for_node(
 	return 0;
 }
 
-void array_free(void *array, int n)
-{
-	void **a = array;
-	int i;
-
-	for (i = 0; i < n; i++)
-		kfree(a[i]);
-	kfree(a);
-}
-
-static void *array_zalloc(int n, size_t size, gfp_t flags)
-{
-	void **a;
-	int i;
-
-	a = kzalloc(n * sizeof(void*), flags);
-	if (!a)
-		return NULL;
-	for (i = 0; i < n; i++) {
-		a[i] = kzalloc(size, flags);
-		if (!a[i]) {
-			array_free(a, n);
-			return NULL;
-		}
-	}
-	return a;
-}
-
 /*	core_tpg_check_initiator_node_acl()
  *
  *
@@ -293,8 +265,7 @@ void core_tpg_clear_object_luns(struct se_portal_group *tpg)
 	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
 		lun = tpg->tpg_lun_list[i];
 
-		if ((lun->lun_status != TRANSPORT_LUN_STATUS_ACTIVE) ||
-		    (lun->lun_se_dev == NULL))
+		if ((!lun) || (lun->lun_se_dev == NULL))
 			continue;
 
 		spin_unlock(&tpg->tpg_lun_lock);
@@ -606,7 +577,6 @@ static int core_tpg_setup_virtual_lun0(struct se_portal_group *se_tpg)
 	int ret;
 
 	lun->unpacked_lun = 0;
-	lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
 	atomic_set(&lun->lun_acl_count, 0);
 	init_completion(&lun->lun_shutdown_comp);
 	INIT_LIST_HEAD(&lun->lun_acl_list);
@@ -635,30 +605,6 @@ int core_tpg_register(
 	void *tpg_fabric_ptr,
 	int se_tpg_type)
 {
-	struct se_lun *lun;
-	u32 i;
-
-	se_tpg->tpg_lun_list = array_zalloc(TRANSPORT_MAX_LUNS_PER_TPG,
-			sizeof(struct se_lun), GFP_KERNEL);
-	if (!se_tpg->tpg_lun_list) {
-		pr_err("Unable to allocate struct se_portal_group->"
-				"tpg_lun_list\n");
-		return -ENOMEM;
-	}
-
-	for (i = 0; i < TRANSPORT_MAX_LUNS_PER_TPG; i++) {
-		lun = se_tpg->tpg_lun_list[i];
-		lun->unpacked_lun = i;
-		lun->lun_link_magic = SE_LUN_LINK_MAGIC;
-		lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
-		atomic_set(&lun->lun_acl_count, 0);
-		init_completion(&lun->lun_shutdown_comp);
-		INIT_LIST_HEAD(&lun->lun_acl_list);
-		spin_lock_init(&lun->lun_acl_lock);
-		spin_lock_init(&lun->lun_sep_lock);
-		init_completion(&lun->lun_ref_comp);
-	}
-
 	se_tpg->se_tpg_type = se_tpg_type;
 	se_tpg->se_tpg_fabric_ptr = tpg_fabric_ptr;
 	se_tpg->se_tpg_tfo = tfo;
@@ -671,13 +617,9 @@ int core_tpg_register(
 	spin_lock_init(&se_tpg->session_lock);
 	spin_lock_init(&se_tpg->tpg_lun_lock);
 
-	if (se_tpg->se_tpg_type == TRANSPORT_TPG_TYPE_NORMAL) {
-		if (core_tpg_setup_virtual_lun0(se_tpg) < 0) {
-			array_free(se_tpg->tpg_lun_list,
-				   TRANSPORT_MAX_LUNS_PER_TPG);
+	if (se_tpg->se_tpg_type == TRANSPORT_TPG_TYPE_NORMAL)
+		if (core_tpg_setup_virtual_lun0(se_tpg) < 0)
 			return -ENOMEM;
-		}
-	}
 
 	spin_lock_bh(&tpg_lock);
 	list_add_tail(&se_tpg->se_tpg_node, &tpg_list);
@@ -734,7 +676,7 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
 		core_tpg_release_virtual_lun0(se_tpg);
 
 	se_tpg->se_tpg_fabric_ptr = NULL;
-	array_free(se_tpg->tpg_lun_list, TRANSPORT_MAX_LUNS_PER_TPG);
+
 	return 0;
 }
 EXPORT_SYMBOL(core_tpg_deregister);
@@ -743,7 +685,7 @@ struct se_lun *core_tpg_alloc_lun(
 	struct se_portal_group *tpg,
 	u32 unpacked_lun)
 {
-	struct se_lun *lun;
+	struct se_lun *lun, *new_lun;
 
 	if (unpacked_lun > (TRANSPORT_MAX_LUNS_PER_TPG-1)) {
 		pr_err("%s LUN: %u exceeds TRANSPORT_MAX_LUNS_PER_TPG"
@@ -754,9 +696,14 @@ struct se_lun *core_tpg_alloc_lun(
 		return ERR_PTR(-EOVERFLOW);
 	}
 
+	new_lun = kzalloc(sizeof(*lun), GFP_KERNEL);
+	if (!new_lun)
+		return ERR_PTR(-ENOMEM);
+
 	spin_lock(&tpg->tpg_lun_lock);
 	lun = tpg->tpg_lun_list[unpacked_lun];
-	if (lun->lun_status == TRANSPORT_LUN_STATUS_ACTIVE) {
+	if (lun) {
+		kfree(new_lun);
 		pr_err("TPG Logical Unit Number: %u is already active"
 			" on %s Target Portal Group: %u, ignoring request.\n",
 			unpacked_lun, tpg->se_tpg_tfo->get_fabric_name(),
@@ -764,9 +711,21 @@ struct se_lun *core_tpg_alloc_lun(
 		spin_unlock(&tpg->tpg_lun_lock);
 		return ERR_PTR(-EINVAL);
 	}
+
+	new_lun->unpacked_lun = unpacked_lun;
+	new_lun->lun_link_magic = SE_LUN_LINK_MAGIC;
+	atomic_set(&new_lun->lun_acl_count, 0);
+	init_completion(&new_lun->lun_shutdown_comp);
+	INIT_LIST_HEAD(&new_lun->lun_acl_list);
+	spin_lock_init(&new_lun->lun_acl_lock);
+	spin_lock_init(&new_lun->lun_sep_lock);
+	init_completion(&new_lun->lun_ref_comp);
+
+	tpg->tpg_lun_list[unpacked_lun] = new_lun;
+
 	spin_unlock(&tpg->tpg_lun_lock);
 
-	return lun;
+	return new_lun;
 }
 
 int core_tpg_add_lun(
@@ -789,7 +748,6 @@ int core_tpg_add_lun(
 
 	spin_lock(&tpg->tpg_lun_lock);
 	lun->lun_access = lun_access;
-	lun->lun_status = TRANSPORT_LUN_STATUS_ACTIVE;
 	spin_unlock(&tpg->tpg_lun_lock);
 
 	return 0;
@@ -802,8 +760,15 @@ void core_tpg_remove_lun(
 	core_clear_lun_from_tpg(lun, tpg);
 	transport_clear_lun_ref(lun);
 	core_dev_unexport(lun->lun_se_dev, tpg, lun);
+}
 
+void core_tpg_free_lun(
+	struct se_portal_group *tpg,
+	struct se_lun *lun)
+{
 	spin_lock(&tpg->tpg_lun_lock);
-	lun->lun_status = TRANSPORT_LUN_STATUS_FREE;
+	tpg->tpg_lun_list[lun->unpacked_lun] = NULL;
 	spin_unlock(&tpg->tpg_lun_lock);
+
+	kfree(lun);
 }
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index f3761da..e7b3d57 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -125,12 +125,6 @@ enum hba_flags_table {
 	HBA_FLAGS_PSCSI_MODE	= 0x02,
 };
 
-/* struct se_lun->lun_status */
-enum transport_lun_status_table {
-	TRANSPORT_LUN_STATUS_FREE = 0,
-	TRANSPORT_LUN_STATUS_ACTIVE = 1,
-};
-
 /* struct se_portal_group->se_tpg_type */
 enum transport_tpg_type_table {
 	TRANSPORT_TPG_TYPE_NORMAL = 0,
@@ -708,8 +702,6 @@ struct se_port_stat_grps {
 struct se_lun {
 #define SE_LUN_LINK_MAGIC			0xffff7771
 	u32			lun_link_magic;
-	/* See transport_lun_status_table */
-	enum transport_lun_status_table lun_status;
 	u32			lun_access;
 	u32			lun_flags;
 	u32			unpacked_lun;
@@ -878,7 +870,7 @@ struct se_portal_group {
 	struct list_head	se_tpg_node;
 	/* linked list for initiator ACL list */
 	struct list_head	acl_node_list;
-	struct se_lun		**tpg_lun_list;
+	struct se_lun		*tpg_lun_list[TRANSPORT_MAX_LUNS_PER_TPG];
 	struct se_lun		tpg_virt_lun0;
 	/* List of TCM sessions associated wth this TPG */
 	struct list_head	tpg_sess_list;
-- 
1.9.3

Re: [PATCHv2 RESENDv2 7/9] target: Allocate se_luns only when used

[Christoph Hellwig]

Looks good,

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

[PATCHv2 RESENDv2 8/9] target: Remove core_tpg_release_virtual_lun0 function

[Andy Grover]

Simple and just called from one place.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_tpg.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
index a63a3d7..bc0299a 100644
--- a/drivers/target/target_core_tpg.c
+++ b/drivers/target/target_core_tpg.c
@@ -591,13 +591,6 @@ static int core_tpg_setup_virtual_lun0(struct se_portal_group *se_tpg)
 	return 0;
 }
 
-static void core_tpg_release_virtual_lun0(struct se_portal_group *se_tpg)
-{
-	struct se_lun *lun = &se_tpg->tpg_virt_lun0;
-
-	core_tpg_remove_lun(se_tpg, lun);
-}
-
 int core_tpg_register(
 	struct target_core_fabric_ops *tfo,
 	struct se_wwn *se_wwn,
@@ -673,7 +666,7 @@ int core_tpg_deregister(struct se_portal_group *se_tpg)
 	spin_unlock_irq(&se_tpg->acl_node_lock);
 
 	if (se_tpg->se_tpg_type == TRANSPORT_TPG_TYPE_NORMAL)
-		core_tpg_release_virtual_lun0(se_tpg);
+		core_tpg_remove_lun(se_tpg, &se_tpg->tpg_virt_lun0);
 
 	se_tpg->se_tpg_fabric_ptr = NULL;
 
-- 
1.9.3

Re: [PATCHv2 RESENDv2 8/9] target: Remove core_tpg_release_virtual_lun0 function

[Christoph Hellwig]

On Mon, Jun 23, 2014 at 04:55:37PM -0700, Andy Grover wrote:
> Simple and just called from one place.
> 
> Signed-off-by: Andy Grover <agrover@redhat.com>

Looks good,

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

[PATCHv2 RESENDv2 9/9] target: Refactor core_enable_device_list_for_node

[Andy Grover]

Create helper functions to alloc a deve and to transition it from
demo mode to explicit.

Signed-off-by: Andy Grover <agrover@redhat.com>
---
 drivers/target/target_core_device.c | 121 ++++++++++++++++++++++--------------
 1 file changed, 74 insertions(+), 47 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 28ab38f..dc510d5 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -288,6 +288,73 @@ void core_update_device_list_access(
 	spin_unlock_irq(&nacl->device_list_lock);
 }
 
+static struct se_dev_entry *core_alloc_se_dev_entry(
+	struct se_lun *lun,
+	struct se_lun_acl *lun_acl,
+	u32 mapped_lun,
+	u32 lun_access,
+	struct se_node_acl *nacl)
+{
+	struct se_dev_entry *deve;
+
+	/* Holding locks, can't sleep */
+	deve = kzalloc(sizeof(*deve), GFP_ATOMIC);
+	if (!deve)
+		return ERR_PTR(-ENOMEM);
+
+	atomic_set(&deve->ua_count, 0);
+	atomic_set(&deve->pr_ref_count, 0);
+	spin_lock_init(&deve->ua_lock);
+	INIT_LIST_HEAD(&deve->alua_port_list);
+	INIT_LIST_HEAD(&deve->ua_list);
+	deve->se_lun = lun;
+	deve->se_lun_acl = lun_acl;
+	deve->mapped_lun = mapped_lun;
+
+	if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE)
+		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
+	else
+		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
+
+	deve->creation_time = get_jiffies_64();
+
+	nacl->device_list[mapped_lun] = deve;
+
+	return deve;
+}
+
+static int core_transition_deve_to_explicit(
+	struct se_lun *lun,
+	struct se_lun_acl *lun_acl,
+	struct se_dev_entry *deve,
+	u32 lun_access)
+{
+	if (deve->se_lun_acl != NULL) {
+		pr_err("struct se_dev_entry->se_lun_acl"
+		       " already set for demo mode -> explicit"
+		       " LUN ACL transition\n");
+		return -EINVAL;
+	}
+	if (deve->se_lun != lun) {
+		pr_err("struct se_dev_entry->se_lun does"
+		       " match passed struct se_lun for demo mode"
+		       " -> explicit LUN ACL transition\n");
+		return -EINVAL;
+	}
+
+	deve->se_lun_acl = lun_acl;
+
+	if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) {
+		deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY;
+		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
+	} else {
+		deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_WRITE;
+		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
+	}
+
+	return 0;
+}
+
 /*      core_enable_device_list_for_node():
  *
  * Called with tpg_lun_lock held & irqs off
@@ -314,62 +381,22 @@ int core_enable_device_list_for_node(
 	 * + mapped_lun that was setup in demo mode..
 	 */
 	if (deve) {
-		if (deve->se_lun_acl != NULL) {
-			pr_err("struct se_dev_entry->se_lun_acl"
-			       " already set for demo mode -> explicit"
-			       " LUN ACL transition\n");
-			ret = -EINVAL;
-			goto out;
-		}
-		if (deve->se_lun != lun) {
-			pr_err("struct se_dev_entry->se_lun does"
-			       " match passed struct se_lun for demo mode"
-			       " -> explicit LUN ACL transition\n");
-			ret = -EINVAL;
-			goto out;
-		}
-		deve->se_lun_acl = lun_acl;
-
-		if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE) {
-			deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_ONLY;
-			deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
-		} else {
-			deve->lun_flags &= ~TRANSPORT_LUNFLAGS_READ_WRITE;
-			deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
-		}
-
+		ret = core_transition_deve_to_explicit(lun, lun_acl, deve,
+						       lun_access);
 		goto out;
 	}
 
-	deve = kzalloc(sizeof(*deve), GFP_ATOMIC);
-	if (!deve) {
-		spin_unlock_irq(&nacl->device_list_lock);
-		return -ENOMEM;
+	deve = core_alloc_se_dev_entry(lun, lun_acl, mapped_lun, lun_access, nacl);
+	if (IS_ERR(deve)) {
+		ret = PTR_ERR(deve);
+		goto out;
 	}
 
-	nacl->device_list[mapped_lun] = deve;
-
-	atomic_set(&deve->ua_count, 0);
-	atomic_set(&deve->pr_ref_count, 0);
-	spin_lock_init(&deve->ua_lock);
-	INIT_LIST_HEAD(&deve->alua_port_list);
-	INIT_LIST_HEAD(&deve->ua_list);
-	deve->se_lun = lun;
-	deve->se_lun_acl = lun_acl;
-	deve->mapped_lun = mapped_lun;
-
-	if (lun_access & TRANSPORT_LUNFLAGS_READ_WRITE)
-		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_WRITE;
-	else
-		deve->lun_flags |= TRANSPORT_LUNFLAGS_READ_ONLY;
-
-	deve->creation_time = get_jiffies_64();
-	deve->attach_count++;
-
 	spin_lock(&port->sep_alua_lock);
 	list_add_tail(&deve->alua_port_list, &port->sep_alua_list);
 	spin_unlock(&port->sep_alua_lock);
 
+	deve->attach_count++;
 out:
 	spin_unlock(&nacl->device_list_lock);
 
-- 
1.9.3