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

[PATCHv2 RESEND 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, and includes some related
simplification patches too. This is a rework of a few patches I submitted
in December in a larger series, but keeping it simple by sticking with
fixed-size arrays for device_list and tpg_lun_list for now.

Testing: created and removed luns and mapped luns, worked ok.

Changes in resend:
  Rebase to latest target-pending/for-next
Changes in v2:
  Added patches 1-2, which ensure locking around the arrays
  Added patch 9, refactors enable_device_list_for_node
  Patch 7/6 folded into series

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

[PATCHv2 RESEND 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 65001e1..54f5b14 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.8.5.3

[PATCHv2 RESEND 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 54f5b14..6d91e4d 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():
@@ -1295,39 +1296,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,
@@ -1366,19 +1334,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))
@@ -1388,7 +1361,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);
@@ -1406,7 +1382,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.8.5.3

[PATCHv2 RESEND 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 6d91e4d..8a37471 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 3bebc71..f6f4606 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -1239,7 +1239,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 ec3e3a3..515a72e 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,
 };
 
 /*
@@ -585,7 +584,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.8.5.3

[PATCHv2 RESEND 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.8.5.3

[PATCHv2 RESEND 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 8a37471..4e3de34 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1257,24 +1257,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.8.5.3

[PATCHv2 RESEND 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 4e3de34..7224689 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1266,7 +1266,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.8.5.3

[PATCHv2 RESEND 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 ad04ea9..56b520a 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 7224689..0bd09c8 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1210,22 +1210,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(),
@@ -1250,7 +1245,7 @@ struct se_lun *core_dev_add_lun(
 		spin_unlock_irq(&tpg->acl_node_lock);
 	}
 
-	return lun;
+	return rc;
 }
 
 /*      core_dev_del_lun():
@@ -1267,35 +1262,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 515a72e..0e147d6 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,
@@ -703,8 +697,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;
@@ -873,7 +865,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.8.5.3

[PATCHv2 RESEND 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.8.5.3

[PATCHv2 RESEND 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 0bd09c8..c15b7aa 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.8.5.3

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

[Andy Grover]

On 03/24/2014 04:43 PM, Andy Grover wrote:
> 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, and includes some related
> simplification patches too. This is a rework of a few patches I submitted
> in December in a larger series, but keeping it simple by sticking with
> fixed-size arrays for device_list and tpg_lun_list for now.
>
> Testing: created and removed luns and mapped luns, worked ok.
>
> Changes in resend:
>    Rebase to latest target-pending/for-next
> Changes in v2:
>    Added patches 1-2, which ensure locking around the arrays
>    Added patch 9, refactors enable_device_list_for_node
>    Patch 7/6 folded into series

Does this set look ok, or any other changes needed? Just haven't heard 
anything.

Thanks -- Andy

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

[Christoph Hellwig]

On Wed, Apr 23, 2014 at 03:36:27PM -0700, Andy Grover wrote:
> Does this set look ok, or any other changes needed? Just haven't heard 
> anything.

Hi Andy,

the series looks reasonable to me, at least for the points I previously
comment on.

Cheers,
	Christoph

p.s. it helps to cc me if you want a quick reply.

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

[Andy Grover]

On 04/24/2014 10:34 PM, Christoph Hellwig wrote:
> On Wed, Apr 23, 2014 at 03:36:27PM -0700, Andy Grover wrote:
>> Does this set look ok, or any other changes needed? Just haven't heard
>> anything.
>
> the series looks reasonable to me, at least for the points I previously
> comment on.
>
> Cheers,
> 	Christoph
>
> p.s. it helps to cc me if you want a quick reply.

Cool, thanks. Will do in the future.

Nick, thoughts?

Regards -- Andy

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

[Nicholas A. Bellinger]

On Thu, 2014-05-01 at 10:53 -0700, Andy Grover wrote:
> On 04/24/2014 10:34 PM, Christoph Hellwig wrote:
> > On Wed, Apr 23, 2014 at 03:36:27PM -0700, Andy Grover wrote:
> >> Does this set look ok, or any other changes needed? Just haven't heard
> >> anything.
> >
> > the series looks reasonable to me, at least for the points I previously
> > comment on.
> >
> > Cheers,
> > 	Christoph
> >
> > p.s. it helps to cc me if you want a quick reply.
> 
> Cool, thanks. Will do in the future.
> 
> Nick, thoughts?
> 

So now that target code (plus Sagi's patches) is finally working
correctly with active I/O shutdown with iser + friends, I'm not exactly
crazy about introducing a bunch of changes that potentially break long
standing assumptions about how se_lun + se_dev_entry pointers are
accessed.

That said, I'd much rather see proper percpu refcounting for
se_dev_entry introduced ahead of these types of memory space related
optimizations.

--nab

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

[Andy Grover]

On 05/01/2014 02:21 PM, Nicholas A. Bellinger wrote:

> So now that target code (plus Sagi's patches) is finally working
> correctly with active I/O shutdown with iser + friends, I'm not exactly
> crazy about introducing a bunch of changes that potentially break long
> standing assumptions about how se_lun + se_dev_entry pointers are
> accessed.

This saves a huge amount of wasted memory per nodeacl and per tpg. It's 
a net reduction of 120 lines of code.

If you wanted to push the iser work to Linus by itself, maybe this goes 
into your tree immediately after that? That would give it plenty of time 
in your tree before it went upstream.

> That said, I'd much rather see proper percpu refcounting for
> se_dev_entry introduced ahead of these types of memory space related
> optimizations.

These patches are not refcount-related, so they should be orthogonal to 
any refcount improvements.

I think the set from March 24 should still apply, but I'm happy to 
re-send or post a repo if that would be helpful to you.

Regards -- Andy