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@xxxxxxxxxx>
---
drivers/target/target_core_device.c | 81 ++++++++++++++-----------------------
drivers/target/target_core_tpg.c | 3 --
2 files changed, 31 insertions(+), 53 deletions(-)
diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 10130ea..5d4a8b3 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -295,7 +295,6 @@ void core_update_device_list_access(
/* core_enable_device_list_for_node():
*
- *
*/
int core_enable_device_list_for_node(
struct se_lun *lun,
@@ -307,8 +306,12 @@ 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);
+ assert_spin_locked(&tpg->tpg_lun_lock);
+ WARN_ON_ONCE(!irqs_disabled());
+
+ spin_lock(&nacl->device_list_lock);
deve = nacl->device_list[mapped_lun];
@@ -322,15 +325,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 +345,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 +368,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 +1302,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 +1340,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_irq(&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))
@@ -1391,8 +1366,10 @@ int core_dev_add_initiator_node_lun_acl(
lacl->se_lun = lun;
if (core_enable_device_list_for_node(lun, lacl, lacl->mapped_lun,
- lun_access, nacl, tpg) < 0)
- return -EINVAL;
+ lun_access, nacl, tpg) < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
spin_lock(&lun->lun_acl_lock);
list_add_tail(&lacl->lacl_list, &lun->lun_acl_list);
@@ -1410,7 +1387,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_irq(&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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
